From 9f43c1995fee9174d809a638e0c8d813f05aaff3 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Thu, 27 Mar 2025 03:03:36 +0100 Subject: [PATCH 1/3] Pass label selector predicate to the Secret Watch and handler --- pkg/reconciler/reconciler.go | 39 +++++++++++++----- pkg/reconciler/reconciler_test.go | 67 +++++++++++++++++++++++++------ 2 files changed, 84 insertions(+), 22 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 9f3c47c0..97d6a039 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -35,6 +35,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/record" @@ -74,7 +75,7 @@ type Reconciler struct { log logr.Logger gvk *schema.GroupVersionKind chrt *chart.Chart - selectorPredicate predicate.Predicate + labelSelector metav1.LabelSelector overrideValues map[string]string skipDependentWatches bool maxConcurrentReconciles int @@ -523,15 +524,11 @@ func WithValueMapper(m values.Mapper) Option { } } -// WithSelector is an Option that configures the reconciler to creates a -// predicate that is used to filter resources based on the specified selector +// WithSelector is an Option that configures label selector for the reconciler. +// The label selector is used to filter watch resources. func WithSelector(s metav1.LabelSelector) Option { return func(r *Reconciler) error { - p, err := predicate.LabelSelectorPredicate(s) - if err != nil { - return err - } - r.selectorPredicate = p + r.labelSelector = s return nil } } @@ -1028,8 +1025,18 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err obj.SetGroupVersionKind(*r.gvk) var preds []predicate.Predicate - if r.selectorPredicate != nil { - preds = append(preds, r.selectorPredicate) + var secretPreds []predicate.TypedPredicate[*corev1.Secret] + if r.labelSelector.MatchLabels != nil { + selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector) + if err != nil { + return err + } + secretPred, err := createSecretPredicate(&r.labelSelector) + if err != nil { + return err + } + preds = append(preds, selectorPredicate) + secretPreds = append(secretPreds, secretPred) } if err := c.Watch( @@ -1060,6 +1067,7 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err obj, handler.OnlyControllerOwner(), ), + secretPreds..., ), ); err != nil { return err @@ -1086,3 +1094,14 @@ func ensureDeployedRelease(u *updater.Updater, rel *release.Release) { updater.EnsureDeployedRelease(rel), ) } + +func createSecretPredicate(labelSelector *metav1.LabelSelector) (predicate.TypedPredicate[*corev1.Secret], error) { + selector, err := metav1.LabelSelectorAsSelector(labelSelector) + if err != nil { + return nil, err + } + tp := predicate.NewTypedPredicateFuncs(func(s *corev1.Secret) bool { + return selector.Matches(labels.Set(s.GetLabels())) + }) + return tp, nil +} diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 86d00123..b80be616 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -53,6 +53,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" "sigs.k8s.io/yaml" @@ -449,18 +450,21 @@ var _ = Describe("Reconciler", func() { selector := metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}} Expect(WithSelector(selector)(r)).To(Succeed()) - Expect(r.selectorPredicate).NotTo(BeNil()) - - Expect(r.selectorPredicate.Create(event.CreateEvent{Object: objLabeled})).To(BeTrue()) - Expect(r.selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objLabeled})).To(BeTrue()) - Expect(r.selectorPredicate.Delete(event.DeleteEvent{Object: objLabeled})).To(BeTrue()) - Expect(r.selectorPredicate.Generic(event.GenericEvent{Object: objLabeled})).To(BeTrue()) - - Expect(r.selectorPredicate.Create(event.CreateEvent{Object: objUnlabeled})).To(BeFalse()) - Expect(r.selectorPredicate.Update(event.UpdateEvent{ObjectOld: objLabeled, ObjectNew: objUnlabeled})).To(BeFalse()) - Expect(r.selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objUnlabeled})).To(BeFalse()) - Expect(r.selectorPredicate.Delete(event.DeleteEvent{Object: objUnlabeled})).To(BeFalse()) - Expect(r.selectorPredicate.Generic(event.GenericEvent{Object: objUnlabeled})).To(BeFalse()) + Expect(r.labelSelector).NotTo(BeNil()) + + selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector) + Expect(err).ToNot(HaveOccurred()) + + Expect(selectorPredicate.Create(event.CreateEvent{Object: objLabeled})).To(BeTrue()) + Expect(selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objLabeled})).To(BeTrue()) + Expect(selectorPredicate.Delete(event.DeleteEvent{Object: objLabeled})).To(BeTrue()) + Expect(selectorPredicate.Generic(event.GenericEvent{Object: objLabeled})).To(BeTrue()) + + Expect(selectorPredicate.Create(event.CreateEvent{Object: objUnlabeled})).To(BeFalse()) + Expect(selectorPredicate.Update(event.UpdateEvent{ObjectOld: objLabeled, ObjectNew: objUnlabeled})).To(BeFalse()) + Expect(selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objUnlabeled})).To(BeFalse()) + Expect(selectorPredicate.Delete(event.DeleteEvent{Object: objUnlabeled})).To(BeFalse()) + Expect(selectorPredicate.Generic(event.GenericEvent{Object: objUnlabeled})).To(BeFalse()) }) }) }) @@ -1488,6 +1492,45 @@ var _ = Describe("Reconciler", func() { }) }) }) + When("label selector set", func() { + It("reconcile only matching CR", func() { + By("adding selector to the reconciler", func() { + selectorFoo := metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}} + Expect(WithSelector(selectorFoo)(r)).To(Succeed()) + }) + + By("adding not matching label to the CR", func() { + Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed()) + obj.SetLabels(map[string]string{"app": "bar"}) + Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) + }) + + By("reconciling skiped and no actions for the release", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).ToNot(HaveOccurred()) + }) + + By("verifying the release has not changed", func() { + rel, err := ac.Get(obj.GetName()) + Expect(err).ToNot(HaveOccurred()) + Expect(rel).NotTo(BeNil()) + Expect(*rel).To(Equal(*currentRelease)) + }) + + By("adding matching label to the CR", func() { + Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed()) + obj.SetLabels(map[string]string{"app": "foo"}) + Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) + }) + + By("successfully reconciling with correct labels", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) }) }) }) From 58509ecf36c7c5ffb759d3de0b5359daf729d69a Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Thu, 22 May 2025 09:47:24 +0200 Subject: [PATCH 2/3] Simplify watcher predicate setup --- pkg/reconciler/reconciler.go | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 97d6a039..95026533 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -35,7 +35,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/record" @@ -1025,18 +1024,13 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err obj.SetGroupVersionKind(*r.gvk) var preds []predicate.Predicate - var secretPreds []predicate.TypedPredicate[*corev1.Secret] + if r.labelSelector.MatchLabels != nil { selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector) if err != nil { return err } - secretPred, err := createSecretPredicate(&r.labelSelector) - if err != nil { - return err - } preds = append(preds, selectorPredicate) - secretPreds = append(secretPreds, secretPred) } if err := c.Watch( @@ -1060,14 +1054,14 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err if err := c.Watch( source.Kind( mgr.GetCache(), - secret, - handler.TypedEnqueueRequestForOwner[*corev1.Secret]( + client.Object(secret), + handler.TypedEnqueueRequestForOwner[client.Object]( mgr.GetScheme(), mgr.GetRESTMapper(), obj, handler.OnlyControllerOwner(), ), - secretPreds..., + preds..., ), ); err != nil { return err @@ -1094,14 +1088,3 @@ func ensureDeployedRelease(u *updater.Updater, rel *release.Release) { updater.EnsureDeployedRelease(rel), ) } - -func createSecretPredicate(labelSelector *metav1.LabelSelector) (predicate.TypedPredicate[*corev1.Secret], error) { - selector, err := metav1.LabelSelectorAsSelector(labelSelector) - if err != nil { - return nil, err - } - tp := predicate.NewTypedPredicateFuncs(func(s *corev1.Secret) bool { - return selector.Matches(labels.Set(s.GetLabels())) - }) - return tp, nil -} From c3f3347dc7705a9039caf817246f5f83f28be9bc Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Mon, 16 Jun 2025 11:10:54 +0200 Subject: [PATCH 3/3] Fix typo --- pkg/reconciler/reconciler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index b80be616..03de115f 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1505,7 +1505,7 @@ var _ = Describe("Reconciler", func() { Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) }) - By("reconciling skiped and no actions for the release", func() { + By("reconciling skipped and no actions for the release", func() { res, err := r.Reconcile(ctx, req) Expect(res).To(Equal(reconcile.Result{})) Expect(err).ToNot(HaveOccurred())