diff --git a/controllers/istiorevision/istiorevision_controller.go b/controllers/istiorevision/istiorevision_controller.go index 6d4dd4907..2026ed13b 100644 --- a/controllers/istiorevision/istiorevision_controller.go +++ b/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). diff --git a/pkg/predicate/predicate.go b/pkg/predicate/predicate.go index fc4f7466c..14c2c0c65 100644 --- a/pkg/predicate/predicate.go +++ b/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" + }, + } +} diff --git a/pkg/predicate/predicate_test.go b/pkg/predicate/predicate_test.go index 08dca50cb..8a876e107 100644 --- a/pkg/predicate/predicate_test.go +++ b/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{})) +} diff --git a/tests/integration/api/istiorevision_test.go b/tests/integration/api/istiorevision_test.go index f13fbdd6e..e1cd55d04 100644 --- a/tests/integration/api/istiorevision_test.go +++ b/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",