From b13799108cdd4f302dc5402266862dc8ec5e6f99 Mon Sep 17 00:00:00 2001 From: Gergo Huszty Date: Thu, 22 May 2025 16:48:23 +0200 Subject: [PATCH 1/2] skip succeeded pods Signed-off-by: Gergo Huszty --- controllers/istiorevision/istiorevision_controller.go | 3 +++ controllers/istiorevisiontag/istiorevisiontag_controller.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/controllers/istiorevision/istiorevision_controller.go b/controllers/istiorevision/istiorevision_controller.go index 3ca53a095..b96872be9 100644 --- a/controllers/istiorevision/istiorevision_controller.go +++ b/controllers/istiorevision/istiorevision_controller.go @@ -477,6 +477,9 @@ func (r *Reconciler) isRevisionReferenced(ctx context.Context, rev *v1.IstioRevi return false, fmt.Errorf("failed to list pods: %w", err) } for _, pod := range podList.Items { + if pod.Status.Phase == corev1.PodSucceeded { + continue + } if ns, found := nsMap[pod.Namespace]; found && podReferencesRevision(pod, ns, rev) { log.V(2).Info("Revision is referenced by Pod", "Pod", client.ObjectKeyFromObject(&pod)) return true, nil diff --git a/controllers/istiorevisiontag/istiorevisiontag_controller.go b/controllers/istiorevisiontag/istiorevisiontag_controller.go index 175b2ee77..bd81ee8f8 100644 --- a/controllers/istiorevisiontag/istiorevisiontag_controller.go +++ b/controllers/istiorevisiontag/istiorevisiontag_controller.go @@ -375,6 +375,9 @@ func (r *Reconciler) isRevisionTagReferencedByWorkloads(ctx context.Context, tag return false, fmt.Errorf("failed to list pods: %w", err) } for _, pod := range podList.Items { + if pod.Status.Phase == corev1.PodSucceeded { + continue + } if ns, found := nsMap[pod.Namespace]; found && podReferencesRevisionTag(pod, tag, ns) { log.V(2).Info("RevisionTag is referenced by Pod", "Pod", client.ObjectKeyFromObject(&pod)) return true, nil From ec4728d7afa893d03436659eb4fc9551ddb0335a Mon Sep 17 00:00:00 2001 From: Gergo Huszty Date: Thu, 22 May 2025 17:06:44 +0200 Subject: [PATCH 2/2] ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty --- .../istiorevision/istiorevision_controller_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/controllers/istiorevision/istiorevision_controller_test.go b/controllers/istiorevision/istiorevision_controller_test.go index 81ca0c6f3..e6b12b6c0 100644 --- a/controllers/istiorevision/istiorevision_controller_test.go +++ b/controllers/istiorevision/istiorevision_controller_test.go @@ -537,6 +537,7 @@ func TestDetermineInUseCondition(t *testing.T) { podLabels map[string]string podAnnotations map[string]string nsLabels map[string]string + podPhase corev1.PodPhase enableAllNamespaces bool interceptors interceptor.Funcs matchesRevision string @@ -555,6 +556,13 @@ func TestDetermineInUseCondition(t *testing.T) { matchesRevision: "default", }, + // pod succeeded + { + podAnnotations: map[string]string{"istio.io/rev": "default"}, + podPhase: corev1.PodSucceeded, + matchesRevision: "", + }, + // namespace labels only { nsLabels: map[string]string{"istio-injection": "enabled"}, @@ -687,6 +695,9 @@ func TestDetermineInUseCondition(t *testing.T) { nameBuilder.WriteString(k + ":" + v + ",") } } + if len(tc.podPhase) > 0 { + nameBuilder.WriteString("Phase:" + string(tc.podPhase) + ",") + } name := strings.TrimSuffix(nameBuilder.String(), ",") t.Run(name, func(t *testing.T) { @@ -723,6 +734,9 @@ func TestDetermineInUseCondition(t *testing.T) { Labels: tc.podLabels, Annotations: tc.podAnnotations, }, + Status: corev1.PodStatus{ + Phase: tc.podPhase, + }, } cl := fake.NewClientBuilder().