Skip reconciliation when using sailoperator.io/ignore annotation

controllers/istiorevision/istiorevision_controller.go

@@ -290,39 +290,45 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Named("istiorevision").
 
 		// namespaced resources
-		Watches(&corev1.ConfigMap{}, ownedResourceHandler).
-		Watches(&appsv1.Deployment{}, ownedResourceHandler). // we don't ignore the status here because we use it to calculate the IstioRevision status
+		Watches(&corev1.ConfigMap{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		// We don't ignore the status for Deployments because we use it to calculate the IstioRevision status
+		Watches(&appsv1.Deployment{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
 		// +lint-watches:ignore: Endpoints (older versions of istiod chart create Endpoints for remote installs, but this controller watches EndpointSlices)
 		// +lint-watches:ignore: EndpointSlice (istiod chart creates Endpoints for remote installs, but this controller watches EndpointSlices)
-		Watches(&discoveryv1.EndpointSlice{}, endpointSliceHandler).
-		Watches(&corev1.Service{}, ownedResourceHandler, builder.WithPredicates(ignoreStatusChange())).
+		Watches(&discoveryv1.EndpointSlice{}, endpointSliceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&corev1.Service{}, ownedResourceHandler,
+			builder.WithPredicates(ignoreStatusChange(), predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// +lint-watches:ignore: NetworkPolicy (FIXME: NetworkPolicy has not yet been added upstream, but is WIP)
-		Watches(&networkingv1.NetworkPolicy{}, ownedResourceHandler, builder.WithPredicates(ignoreStatusChange())).
+		Watches(&networkingv1.NetworkPolicy{}, ownedResourceHandler,
+			builder.WithPredicates(ignoreStatusChange(), predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// We use predicate.IgnoreUpdate() so that we skip the reconciliation when a pull secret is added to the ServiceAccount.
 		// This is necessary so that we don't remove the newly-added secret.
 		// TODO: this is a temporary hack until we implement the correct solution on the Helm-render side
 		Watches(&corev1.ServiceAccount{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdate())).
-		Watches(&rbacv1.Role{}, ownedResourceHandler).
-		Watches(&rbacv1.RoleBinding{}, ownedResourceHandler).
-		Watches(&policyv1.PodDisruptionBudget{}, ownedResourceHandler, builder.WithPredicates(ignoreStatusChange())).
-		Watches(&autoscalingv2.HorizontalPodAutoscaler{}, ownedResourceHandler, builder.WithPredicates(ignoreStatusChange())).
+		Watches(&rbacv1.Role{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&rbacv1.RoleBinding{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&policyv1.PodDisruptionBudget{}, ownedResourceHandler,
+			builder.WithPredicates(ignoreStatusChange(), predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&autoscalingv2.HorizontalPodAutoscaler{}, ownedResourceHandler,
+			builder.WithPredicates(ignoreStatusChange(), predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// +lint-watches:ignore: Namespace (not found in charts, but must be watched to reconcile IstioRevision when its namespace is created)
-		Watches(&corev1.Namespace{}, nsHandler, builder.WithPredicates(ignoreStatusChange())).
+		Watches(&corev1.Namespace{}, nsHandler, builder.WithPredicates(ignoreStatusChange()), builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// +lint-watches:ignore: Pod (not found in charts, but must be watched to reconcile IstioRevision when a pod references it)
-		Watches(&corev1.Pod{}, podHandler, builder.WithPredicates(ignoreStatusChange())).
+		Watches(&corev1.Pod{}, podHandler, builder.WithPredicates(ignoreStatusChange(), predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// +lint-watches:ignore: IstioRevisionTag (not found in charts, but must be watched to reconcile IstioRevision when a pod references it)
 		Watches(&v1.IstioRevisionTag{}, revisionTagHandler).
 
 		// cluster-scoped resources
-		Watches(&rbacv1.ClusterRole{}, ownedResourceHandler).
-		Watches(&rbacv1.ClusterRoleBinding{}, ownedResourceHandler).
-		Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler).
-		Watches(&admissionv1.ValidatingWebhookConfiguration{}, ownedResourceHandler, builder.WithPredicates(validatingWebhookConfigPredicate())).
+		Watches(&rbacv1.ClusterRole{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&rbacv1.ClusterRoleBinding{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
+		Watches(&admissionv1.ValidatingWebhookConfiguration{}, ownedResourceHandler,
+			builder.WithPredicates(validatingWebhookConfigPredicate(), predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// +lint-watches:ignore: IstioCNI (not found in charts, but this controller needs to watch it to update the IstioRevision status)
 		Watches(&v1.IstioCNI{}, istioCniHandler).

pkg/predicate/predicate.go

@@ -25,3 +25,17 @@ func IgnoreUpdate() predicate.Funcs {
 		UpdateFunc: func(e event.UpdateEvent) bool { return false },
 	}
 }
+
+// IgnoreUpdateWhenAnnotation returns a predicate that ignores update events
+// when the sailoperator.io/ignore annotation is set to "true" on the resource.
+func IgnoreUpdateWhenAnnotation() predicate.Funcs {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			if e.ObjectOld == nil || e.ObjectNew == nil {
+				return false
+			}
+
+			return e.ObjectNew.GetAnnotations()["sailoperator.io/ignore"] != "true"
+		},
+	}
+}

pkg/predicate/predicate_test.go

@@ -18,6 +18,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 )
 
@@ -28,3 +30,57 @@ func TestIgnoreUpdate(t *testing.T) {
 	assert.Equal(t, true, predicate.Delete(event.DeleteEvent{}))
 	assert.Equal(t, true, predicate.Generic(event.GenericEvent{}))
 }
+
+func TestIgnoreUpdateWhenAnnotation(t *testing.T) {
+	predicate := IgnoreUpdateWhenAnnotation()
+	// Object does not contain sailoperator.io/ignore annotation
+	// so reconciliation should be done and both objects should be equal
+	assert.Equal(t, true, predicate.Update(event.UpdateEvent{
+		ObjectOld: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{},
+		},
+		ObjectNew: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{},
+			Data: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}))
+	// Object has sailoperator.io/ignore annotation set with wrong value
+	// so reconciliation should be done and both objects should be equal
+	assert.Equal(t, true, predicate.Update(event.UpdateEvent{
+		ObjectOld: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{},
+		},
+		ObjectNew: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					"sailoperator.io/ignore": "wrongvalue",
+				},
+			},
+			Data: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}))
+	// Object has sailoperator.io/ignore annotation set to "true"
+	// so reconciliation should be skipped
+	assert.Equal(t, false, predicate.Update(event.UpdateEvent{
+		ObjectOld: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{},
+		},
+		ObjectNew: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					"sailoperator.io/ignore": "true",
+				},
+			},
+			Data: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}))
+	assert.Equal(t, true, predicate.Create(event.CreateEvent{}))
+	assert.Equal(t, true, predicate.Delete(event.DeleteEvent{}))
+	assert.Equal(t, true, predicate.Generic(event.GenericEvent{}))
+}

tests/integration/api/istiorevision_test.go

@@ -561,6 +561,32 @@ var _ = Describe("IstioRevision resource", Label("istiorevision"), Ordered, func
 			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(sa), sa)).To(Succeed())
 			Expect(sa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: "other-pull-secret"}))
 		})
+
+		It("skips reconcile when sailoperator.io/ignore annotation is set to true on a resource", func() {
+			waitForInFlightReconcileToFinish()
+
+			webhook := &admissionv1.MutatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "istio-sidecar-injector-" + revName + "-" + istioNamespace,
+				},
+			}
+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(webhook), webhook)).To(Succeed())
+
+			GinkgoWriter.Println("webhook:", webhook)
+
+			expectNoReconciliation(istioRevisionController, func() {
+				By("adding sailoperator.io/ignore annotation to ConfigMap")
+				webhook.Annotations = map[string]string{
+					"sailoperator.io/ignore": "true",
+				}
+				webhook.Labels["app"] = "sidecar-injector-test"
+				Expect(k8sClient.Update(ctx, webhook)).To(Succeed())
+			})
+
+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(webhook), webhook)).To(Succeed())
+			Expect(webhook.Annotations["sailoperator.io/ignore"]).To(Equal("true"))
+			Expect(webhook.Labels["app"]).To(Equal("sidecar-injector-test"))
+		})
 	})
 
 	DescribeTableSubtree("reconciling when revision is in use",