From 24bd217a6f12a057de1e1fcab5a0ec5c3b473d34 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Mon, 20 Oct 2025 13:55:08 +0200 Subject: [PATCH 01/10] Add tests for WithSelector --- pkg/reconciler/reconciler_test.go | 177 +++++++++++++++++++++++------- 1 file changed, 138 insertions(+), 39 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 03de115f..1ac14b38 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -22,6 +22,8 @@ import ( "errors" "fmt" "strconv" + "strings" + "sync" "time" . "github.com/onsi/ginkgo/v2" @@ -49,6 +51,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/config" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -1492,45 +1495,6 @@ 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 skipped 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()) - }) - }) - }) }) }) }) @@ -1545,6 +1509,141 @@ var _ = Describe("Reconciler", func() { }) }) + _ = Describe("WithSelector integration test", func() { + var ( + mgr manager.Manager + ctx context.Context + cancel context.CancelFunc + reconciledCRs []string + reconciledCRsMutex sync.Mutex + labeledObj *unstructured.Unstructured + unlabeledObj *unstructured.Unstructured + labeledObjKey types.NamespacedName + unlabeledObjKey types.NamespacedName + ) + + BeforeEach(func() { + reconciledCRs = []string{} + mgr = getManagerOrFail() + matchingLabels := map[string]string{"app": "foo"} + + r, err := New( + WithGroupVersionKind(gvk), + WithChart(chrt), + WithSelector(metav1.LabelSelector{MatchLabels: matchingLabels}), + ) + Expect(err).ToNot(HaveOccurred()) + + originalReconciler := r + wrappedReconciler := &Reconciler{} + *wrappedReconciler = *originalReconciler + wrappedReconciler.client = mgr.GetClient() + + // Override Reconcile to track reconciliations + reconciler := reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + reconciledCRsMutex.Lock() + reconciledCRs = append(reconciledCRs, req.NamespacedName.String()) + reconciledCRsMutex.Unlock() + return wrappedReconciler.Reconcile(ctx, req) + }) + + controllerName := fmt.Sprintf("%v-controller", strings.ToLower(gvk.Kind)) + Expect(wrappedReconciler.addDefaults(mgr, controllerName)).To(Succeed()) + wrappedReconciler.setupScheme(mgr) + + c, err := controller.New(controllerName, mgr, controller.Options{ + Reconciler: reconciler, + MaxConcurrentReconciles: 1, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(wrappedReconciler.setupWatches(mgr, c)).To(Succeed()) + + labeledObj = testutil.BuildTestCR(gvk) + labeledObj.SetName("labeled-cr") + labeledObj.SetLabels(matchingLabels) + labeledObjKey = types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()} + + unlabeledObj = testutil.BuildTestCR(gvk) + unlabeledObj.SetName("unlabeled-cr") + unlabeledObjKey = types.NamespacedName{Namespace: unlabeledObj.GetNamespace(), Name: unlabeledObj.GetName()} + + ctx, cancel = context.WithCancel(context.Background()) + go func() { + Expect(mgr.Start(ctx)).To(Succeed()) + }() + Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue()) + }) + + AfterEach(func() { + By("ensuring the labeled CR is deleted", func() { + err := mgr.GetAPIReader().Get(ctx, labeledObjKey, labeledObj) + if !apierrors.IsNotFound(err) { + Expect(err).ToNot(HaveOccurred()) + labeledObj.SetFinalizers([]string{}) + Expect(mgr.GetClient().Update(ctx, labeledObj)).To(Succeed()) + Expect(mgr.GetClient().Delete(ctx, labeledObj)).To(Succeed()) + } + }) + + By("ensuring the unlabeled CR is deleted", func() { + err := mgr.GetAPIReader().Get(ctx, unlabeledObjKey, unlabeledObj) + if !apierrors.IsNotFound(err) { + Expect(err).ToNot(HaveOccurred()) + unlabeledObj.SetFinalizers([]string{}) + Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) + Expect(mgr.GetClient().Delete(ctx, unlabeledObj)).To(Succeed()) + } + }) + + cancel() + }) + + It("should only reconcile CRs matching the label selector", func() { + By("creating a CR with matching labels", func() { + Expect(mgr.GetClient().Create(ctx, labeledObj)).To(Succeed()) + }) + + By("creating a CR without matching labels", func() { + Expect(mgr.GetClient().Create(ctx, unlabeledObj)).To(Succeed()) + }) + + By("waiting for reconciliations to complete", func() { + Eventually(func() []string { + reconciledCRsMutex.Lock() + defer reconciledCRsMutex.Unlock() + return reconciledCRs + }, "5s", "100ms").Should(ContainElement(labeledObjKey.String())) + }) + + By("verifying only the labeled CR was reconciled", func() { + reconciledCRsMutex.Lock() + defer reconciledCRsMutex.Unlock() + Expect(reconciledCRs).To(ContainElement(labeledObjKey.String())) + Expect(reconciledCRs).NotTo(ContainElement(unlabeledObjKey.String())) + }) + + By("updating the unlabeled CR to have matching labels", func() { + Expect(mgr.GetClient().Get(ctx, unlabeledObjKey, unlabeledObj)).To(Succeed()) + unlabeledObj.SetLabels(map[string]string{"app": "foo"}) + Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) + }) + + By("waiting for the previously unlabeled CR to be reconciled", func() { + Eventually(func() []string { + reconciledCRsMutex.Lock() + defer reconciledCRsMutex.Unlock() + return reconciledCRs + }, "5s", "100ms").Should(ContainElement(unlabeledObjKey.String())) + }) + + By("verifying the previously unlabeled CR was reconciled after label change", func() { + reconciledCRsMutex.Lock() + defer reconciledCRsMutex.Unlock() + Expect(reconciledCRs).To(ContainElement(unlabeledObjKey.String())) + }) + }) + }) + _ = Describe("Test custom controller setup", func() { var ( mgr manager.Manager From 25d253a51459b087d0921b3d62fb0b034c911d21 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Thu, 23 Oct 2025 10:11:51 +0200 Subject: [PATCH 02/10] Fix lint --- pkg/reconciler/reconciler_test.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 1ac14b38..9128618c 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1534,29 +1534,24 @@ var _ = Describe("Reconciler", func() { ) Expect(err).ToNot(HaveOccurred()) - originalReconciler := r - wrappedReconciler := &Reconciler{} - *wrappedReconciler = *originalReconciler - wrappedReconciler.client = mgr.GetClient() - - // Override Reconcile to track reconciliations + // Wrap the Reconcile method to track reconciliations reconciler := reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { reconciledCRsMutex.Lock() reconciledCRs = append(reconciledCRs, req.NamespacedName.String()) reconciledCRsMutex.Unlock() - return wrappedReconciler.Reconcile(ctx, req) + return r.Reconcile(ctx, req) }) controllerName := fmt.Sprintf("%v-controller", strings.ToLower(gvk.Kind)) - Expect(wrappedReconciler.addDefaults(mgr, controllerName)).To(Succeed()) - wrappedReconciler.setupScheme(mgr) + Expect(r.addDefaults(mgr, controllerName)).To(Succeed()) + r.setupScheme(mgr) c, err := controller.New(controllerName, mgr, controller.Options{ Reconciler: reconciler, MaxConcurrentReconciles: 1, }) Expect(err).ToNot(HaveOccurred()) - Expect(wrappedReconciler.setupWatches(mgr, c)).To(Succeed()) + Expect(r.setupWatches(mgr, c)).To(Succeed()) labeledObj = testutil.BuildTestCR(gvk) labeledObj.SetName("labeled-cr") From d486710e441e5b0a99b2e2b0a9ede861340a8aca Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Mon, 3 Nov 2025 10:56:48 +0100 Subject: [PATCH 03/10] Small fix --- pkg/reconciler/reconciler_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 9128618c..de90bd69 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1509,7 +1509,7 @@ var _ = Describe("Reconciler", func() { }) }) - _ = Describe("WithSelector integration test", func() { + _ = Describe("WithSelector test", func() { var ( mgr manager.Manager ctx context.Context @@ -1534,7 +1534,6 @@ var _ = Describe("Reconciler", func() { ) Expect(err).ToNot(HaveOccurred()) - // Wrap the Reconcile method to track reconciliations reconciler := reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { reconciledCRsMutex.Lock() reconciledCRs = append(reconciledCRs, req.NamespacedName.String()) From 23cd0b63713444f6bb73cb7175dad07dfbc7aff5 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Tue, 4 Nov 2025 15:19:18 +0100 Subject: [PATCH 04/10] Apply suggestions from code review Co-authored-by: Marcin Owsiany --- 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 de90bd69..13a4c5e1 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1509,7 +1509,7 @@ var _ = Describe("Reconciler", func() { }) }) - _ = Describe("WithSelector test", func() { + _ = Describe("WithSelector", func() { var ( mgr manager.Manager ctx context.Context From 0a43969ecbc07e37bd27f86477772a1e59206d78 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Tue, 4 Nov 2025 15:22:33 +0100 Subject: [PATCH 05/10] Fix indentation --- pkg/reconciler/reconciler_test.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 13a4c5e1..89777784 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1515,6 +1515,7 @@ var _ = Describe("Reconciler", func() { ctx context.Context cancel context.CancelFunc reconciledCRs []string + matchingLabels map[string]string reconciledCRsMutex sync.Mutex labeledObj *unstructured.Unstructured unlabeledObj *unstructured.Unstructured @@ -1525,7 +1526,7 @@ var _ = Describe("Reconciler", func() { BeforeEach(func() { reconciledCRs = []string{} mgr = getManagerOrFail() - matchingLabels := map[string]string{"app": "foo"} + matchingLabels = map[string]string{"app": "foo"} r, err := New( WithGroupVersionKind(gvk), @@ -1571,22 +1572,24 @@ var _ = Describe("Reconciler", func() { AfterEach(func() { By("ensuring the labeled CR is deleted", func() { err := mgr.GetAPIReader().Get(ctx, labeledObjKey, labeledObj) - if !apierrors.IsNotFound(err) { - Expect(err).ToNot(HaveOccurred()) - labeledObj.SetFinalizers([]string{}) - Expect(mgr.GetClient().Update(ctx, labeledObj)).To(Succeed()) - Expect(mgr.GetClient().Delete(ctx, labeledObj)).To(Succeed()) + if apierrors.IsNotFound(err) { + return } + Expect(err).ToNot(HaveOccurred()) + labeledObj.SetFinalizers([]string{}) + Expect(mgr.GetClient().Update(ctx, labeledObj)).To(Succeed()) + Expect(mgr.GetClient().Delete(ctx, labeledObj)).To(Succeed()) }) By("ensuring the unlabeled CR is deleted", func() { err := mgr.GetAPIReader().Get(ctx, unlabeledObjKey, unlabeledObj) - if !apierrors.IsNotFound(err) { - Expect(err).ToNot(HaveOccurred()) - unlabeledObj.SetFinalizers([]string{}) - Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) - Expect(mgr.GetClient().Delete(ctx, unlabeledObj)).To(Succeed()) + if apierrors.IsNotFound(err) { + return } + Expect(err).ToNot(HaveOccurred()) + unlabeledObj.SetFinalizers([]string{}) + Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) + Expect(mgr.GetClient().Delete(ctx, unlabeledObj)).To(Succeed()) }) cancel() @@ -1618,7 +1621,7 @@ var _ = Describe("Reconciler", func() { By("updating the unlabeled CR to have matching labels", func() { Expect(mgr.GetClient().Get(ctx, unlabeledObjKey, unlabeledObj)).To(Succeed()) - unlabeledObj.SetLabels(map[string]string{"app": "foo"}) + unlabeledObj.SetLabels(matchingLabels) Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) }) From 072866ae2535b95387dd70c83e6cf9a9b27fa36d Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Thu, 27 Nov 2025 06:51:59 +0100 Subject: [PATCH 06/10] Use hooks and add another test with two managers --- pkg/reconciler/reconciler_test.go | 210 ++++++++++++++++++------------ 1 file changed, 125 insertions(+), 85 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 89777784..9acd3be4 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -22,7 +22,6 @@ import ( "errors" "fmt" "strconv" - "strings" "sync" "time" @@ -51,7 +50,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/config" - "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -1511,132 +1509,146 @@ var _ = Describe("Reconciler", func() { _ = Describe("WithSelector", func() { var ( - mgr manager.Manager - ctx context.Context - cancel context.CancelFunc - reconciledCRs []string - matchingLabels map[string]string - reconciledCRsMutex sync.Mutex - labeledObj *unstructured.Unstructured - unlabeledObj *unstructured.Unstructured - labeledObjKey types.NamespacedName - unlabeledObjKey types.NamespacedName + ctx context.Context + cancel context.CancelFunc + mgr manager.Manager + reconciledCRs []string + anotherReconciledCRs []string + matchingLabels map[string]string + anotherMatchingLabels map[string]string + labeledObj *unstructured.Unstructured + anotherObj *unstructured.Unstructured + labeledObjKey types.NamespacedName + anotherObjKey types.NamespacedName + mu sync.Mutex ) BeforeEach(func() { - reconciledCRs = []string{} - mgr = getManagerOrFail() - matchingLabels = map[string]string{"app": "foo"} - - r, err := New( - WithGroupVersionKind(gvk), - WithChart(chrt), - WithSelector(metav1.LabelSelector{MatchLabels: matchingLabels}), - ) - Expect(err).ToNot(HaveOccurred()) + ctx, cancel = context.WithCancel(context.Background()) - reconciler := reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { - reconciledCRsMutex.Lock() - reconciledCRs = append(reconciledCRs, req.NamespacedName.String()) - reconciledCRsMutex.Unlock() - return r.Reconcile(ctx, req) - }) + mu.Lock() + reconciledCRs = nil + anotherReconciledCRs = nil + mu.Unlock() - controllerName := fmt.Sprintf("%v-controller", strings.ToLower(gvk.Kind)) - Expect(r.addDefaults(mgr, controllerName)).To(Succeed()) - r.setupScheme(mgr) + matchingLabels = map[string]string{"app": "foo"} + anotherMatchingLabels = map[string]string{"app": "bar"} - c, err := controller.New(controllerName, mgr, controller.Options{ - Reconciler: reconciler, - MaxConcurrentReconciles: 1, + trackingHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, rel release.Release, log logr.Logger) error { + mu.Lock() + defer mu.Unlock() + reconciledCRs = append(reconciledCRs, obj.GetName()) + return nil }) - Expect(err).ToNot(HaveOccurred()) - Expect(r.setupWatches(mgr, c)).To(Succeed()) + mgr = setupManagerWithPostHook(ctx, trackingHook, matchingLabels) labeledObj = testutil.BuildTestCR(gvk) labeledObj.SetName("labeled-cr") labeledObj.SetLabels(matchingLabels) labeledObjKey = types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()} - unlabeledObj = testutil.BuildTestCR(gvk) - unlabeledObj.SetName("unlabeled-cr") - unlabeledObjKey = types.NamespacedName{Namespace: unlabeledObj.GetNamespace(), Name: unlabeledObj.GetName()} - - ctx, cancel = context.WithCancel(context.Background()) - go func() { - Expect(mgr.Start(ctx)).To(Succeed()) - }() - Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue()) + anotherObj = testutil.BuildTestCR(gvk) + anotherObj.SetName("another-cr") + anotherObjKey = types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()} }) AfterEach(func() { By("ensuring the labeled CR is deleted", func() { - err := mgr.GetAPIReader().Get(ctx, labeledObjKey, labeledObj) - if apierrors.IsNotFound(err) { - return - } - Expect(err).ToNot(HaveOccurred()) - labeledObj.SetFinalizers([]string{}) - Expect(mgr.GetClient().Update(ctx, labeledObj)).To(Succeed()) - Expect(mgr.GetClient().Delete(ctx, labeledObj)).To(Succeed()) + ensureDeleteCR(ctx, mgr, labeledObjKey, labeledObj) }) By("ensuring the unlabeled CR is deleted", func() { - err := mgr.GetAPIReader().Get(ctx, unlabeledObjKey, unlabeledObj) - if apierrors.IsNotFound(err) { - return - } - Expect(err).ToNot(HaveOccurred()) - unlabeledObj.SetFinalizers([]string{}) - Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) - Expect(mgr.GetClient().Delete(ctx, unlabeledObj)).To(Succeed()) + ensureDeleteCR(ctx, mgr, anotherObjKey, anotherObj) }) - cancel() }) It("should only reconcile CRs matching the label selector", func() { + By("creating a CR without matching labels", func() { + Expect(mgr.GetClient().Create(ctx, anotherObj)).To(Succeed()) + }) + + By("verifying that the labeled reconciler does not reconcile CR without labels", func() { + Consistently(func() []string { + mu.Lock() + defer mu.Unlock() + return reconciledCRs + }, "2s", "100ms").Should(BeEmpty()) + }) + By("creating a CR with matching labels", func() { Expect(mgr.GetClient().Create(ctx, labeledObj)).To(Succeed()) }) - By("creating a CR without matching labels", func() { - Expect(mgr.GetClient().Create(ctx, unlabeledObj)).To(Succeed()) + By("verifying only the labeled CR was reconciled", func() { + Eventually(func() []string { + mu.Lock() + defer mu.Unlock() + return reconciledCRs + }).Should(HaveExactElements(labeledObjKey.Name)) + }) + + By("updating the unlabeled CR to have matching labels", func() { + Expect(mgr.GetClient().Get(ctx, anotherObjKey, anotherObj)).To(Succeed()) + anotherObj.SetLabels(matchingLabels) + Expect(mgr.GetClient().Update(ctx, anotherObj)).To(Succeed()) }) - By("waiting for reconciliations to complete", func() { + By("verifying that both CRs were reconciled after setting label to the unlabeled CR", func() { Eventually(func() []string { - reconciledCRsMutex.Lock() - defer reconciledCRsMutex.Unlock() + mu.Lock() + defer mu.Unlock() return reconciledCRs - }, "5s", "100ms").Should(ContainElement(labeledObjKey.String())) + }, "10s", "100ms").Should(ContainElements(labeledObjKey.Name, anotherObjKey.Name)) }) + }) - By("verifying only the labeled CR was reconciled", func() { - reconciledCRsMutex.Lock() - defer reconciledCRsMutex.Unlock() - Expect(reconciledCRs).To(ContainElement(labeledObjKey.String())) - Expect(reconciledCRs).NotTo(ContainElement(unlabeledObjKey.String())) + It("should reconcile CRs independently when using two managers with different label selectors", func() { + By("creating another manager with a different label selector", func() { + postHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, rel release.Release, log logr.Logger) error { + mu.Lock() + defer mu.Unlock() + anotherReconciledCRs = append(anotherReconciledCRs, obj.GetName()) + return nil + }) + _ = setupManagerWithPostHook(ctx, postHook, anotherMatchingLabels) }) - By("updating the unlabeled CR to have matching labels", func() { - Expect(mgr.GetClient().Get(ctx, unlabeledObjKey, unlabeledObj)).To(Succeed()) - unlabeledObj.SetLabels(matchingLabels) - Expect(mgr.GetClient().Update(ctx, unlabeledObj)).To(Succeed()) + By("creating a CR with matching labels for the first manager", func() { + Expect(mgr.GetClient().Create(ctx, labeledObj)).To(Succeed()) }) - By("waiting for the previously unlabeled CR to be reconciled", func() { + By("verifying that only the first manager reconciled the CR", func() { Eventually(func() []string { - reconciledCRsMutex.Lock() - defer reconciledCRsMutex.Unlock() + mu.Lock() + defer mu.Unlock() return reconciledCRs - }, "5s", "100ms").Should(ContainElement(unlabeledObjKey.String())) + }, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name)) + + Consistently(func() []string { + mu.Lock() + defer mu.Unlock() + return anotherReconciledCRs + }, "2s", "100ms").Should(BeEmpty()) }) - By("verifying the previously unlabeled CR was reconciled after label change", func() { - reconciledCRsMutex.Lock() - defer reconciledCRsMutex.Unlock() - Expect(reconciledCRs).To(ContainElement(unlabeledObjKey.String())) + By("creating a CR with matching labels for the second manager", func() { + anotherObj.SetLabels(anotherMatchingLabels) + Expect(mgr.GetClient().Create(ctx, anotherObj)).To(Succeed()) + }) + + By("verifying that both managers reconcile only matching labels CRs", func() { + Eventually(func() []string { + mu.Lock() + defer mu.Unlock() + return reconciledCRs + }, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name)) + + Eventually(func() []string { + mu.Lock() + defer mu.Unlock() + return anotherReconciledCRs + }, "10s", "100ms").Should(HaveExactElements(anotherObjKey.Name)) }) }) }) @@ -1839,3 +1851,31 @@ func verifyEvent(ctx context.Context, cl client.Reader, obj metav1.Object, event Reason: %q Message: %q`, eventType, reason, message)) } + +func ensureDeleteCR(ctx context.Context, mgr manager.Manager, crKey types.NamespacedName, cr *unstructured.Unstructured) { + err := mgr.GetAPIReader().Get(ctx, crKey, cr) + if apierrors.IsNotFound(err) { + return + } + Expect(err).ToNot(HaveOccurred()) + cr.SetFinalizers([]string{}) + Expect(mgr.GetClient().Update(ctx, cr)).To(Succeed()) + Expect(mgr.GetClient().Delete(ctx, cr)).To(Succeed()) +} + +func setupManagerWithPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) manager.Manager { + mgr := getManagerOrFail() + r, err := New( + WithGroupVersionKind(gvk), + WithChart(chrt), + WithSelector(metav1.LabelSelector{MatchLabels: matchingLabels}), + WithPostHook(postHook), + ) + Expect(err).ToNot(HaveOccurred()) + Expect(r.SetupWithManager(mgr)).To(Succeed()) + go func() { + Expect(mgr.Start(ctx)).To(Succeed()) + }() + Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue()) + return mgr +} From 25d481013c94523b77de007f903bc9d29f06dfc0 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Thu, 27 Nov 2025 07:00:47 +0100 Subject: [PATCH 07/10] Fix linting --- pkg/reconciler/reconciler_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 9acd3be4..0a64fe72 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1534,7 +1534,7 @@ var _ = Describe("Reconciler", func() { matchingLabels = map[string]string{"app": "foo"} anotherMatchingLabels = map[string]string{"app": "bar"} - trackingHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, rel release.Release, log logr.Logger) error { + trackingHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { mu.Lock() defer mu.Unlock() reconciledCRs = append(reconciledCRs, obj.GetName()) @@ -1605,7 +1605,7 @@ var _ = Describe("Reconciler", func() { It("should reconcile CRs independently when using two managers with different label selectors", func() { By("creating another manager with a different label selector", func() { - postHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, rel release.Release, log logr.Logger) error { + postHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { mu.Lock() defer mu.Unlock() anotherReconciledCRs = append(anotherReconciledCRs, obj.GetName()) From ae66a8557079c8025ace897a5c815f66ed10f280 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Thu, 27 Nov 2025 08:41:10 +0100 Subject: [PATCH 08/10] Improve assertion --- pkg/reconciler/reconciler.go | 2 +- pkg/reconciler/reconciler_test.go | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index fc6632eb..3f9a5a58 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -1025,7 +1025,7 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err var preds []predicate.Predicate - if r.labelSelector.Size() > 0 { + if len(r.labelSelector.MatchLabels) > 0 || len(r.labelSelector.MatchExpressions) > 0 { selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector) if err != nil { return err diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 0a64fe72..8a97f038 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -21,6 +21,7 @@ import ( "context" "errors" "fmt" + "slices" "strconv" "sync" "time" @@ -1537,10 +1538,12 @@ var _ = Describe("Reconciler", func() { trackingHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { mu.Lock() defer mu.Unlock() - reconciledCRs = append(reconciledCRs, obj.GetName()) + if !slices.Contains(reconciledCRs, obj.GetName()) { + reconciledCRs = append(reconciledCRs, obj.GetName()) + } return nil }) - mgr = setupManagerWithPostHook(ctx, trackingHook, matchingLabels) + mgr = setupManagerWithSelectorAndPostHook(ctx, trackingHook, matchingLabels) labeledObj = testutil.BuildTestCR(gvk) labeledObj.SetName("labeled-cr") @@ -1608,10 +1611,12 @@ var _ = Describe("Reconciler", func() { postHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { mu.Lock() defer mu.Unlock() - anotherReconciledCRs = append(anotherReconciledCRs, obj.GetName()) + if !slices.Contains(anotherReconciledCRs, obj.GetName()) { + anotherReconciledCRs = append(anotherReconciledCRs, obj.GetName()) + } return nil }) - _ = setupManagerWithPostHook(ctx, postHook, anotherMatchingLabels) + _ = setupManagerWithSelectorAndPostHook(ctx, postHook, anotherMatchingLabels) }) By("creating a CR with matching labels for the first manager", func() { @@ -1633,8 +1638,10 @@ var _ = Describe("Reconciler", func() { }) By("creating a CR with matching labels for the second manager", func() { - anotherObj.SetLabels(anotherMatchingLabels) Expect(mgr.GetClient().Create(ctx, anotherObj)).To(Succeed()) + Expect(mgr.GetClient().Get(ctx, anotherObjKey, anotherObj)).To(Succeed()) + anotherObj.SetLabels(anotherMatchingLabels) + Expect(mgr.GetClient().Update(ctx, anotherObj)).To(Succeed()) }) By("verifying that both managers reconcile only matching labels CRs", func() { @@ -1863,7 +1870,7 @@ func ensureDeleteCR(ctx context.Context, mgr manager.Manager, crKey types.Namesp Expect(mgr.GetClient().Delete(ctx, cr)).To(Succeed()) } -func setupManagerWithPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) manager.Manager { +func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) manager.Manager { mgr := getManagerOrFail() r, err := New( WithGroupVersionKind(gvk), From bd631b623a0883a2122f3da2e2887dd5ca15df65 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Mon, 1 Dec 2025 10:34:26 +0100 Subject: [PATCH 09/10] Add managers cleanup and change Eventually asserting for mutex objects --- pkg/reconciler/reconciler_test.go | 190 +++++++++++++++++------------- 1 file changed, 108 insertions(+), 82 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 8a97f038..870157b4 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1512,71 +1512,84 @@ var _ = Describe("Reconciler", func() { var ( ctx context.Context cancel context.CancelFunc - mgr manager.Manager - reconciledCRs []string - anotherReconciledCRs []string matchingLabels map[string]string anotherMatchingLabels map[string]string - labeledObj *unstructured.Unstructured - anotherObj *unstructured.Unstructured - labeledObjKey types.NamespacedName - anotherObjKey types.NamespacedName mu sync.Mutex + doneChans []chan struct{} ) BeforeEach(func() { - ctx, cancel = context.WithCancel(context.Background()) - - mu.Lock() - reconciledCRs = nil - anotherReconciledCRs = nil - mu.Unlock() - matchingLabels = map[string]string{"app": "foo"} anotherMatchingLabels = map[string]string{"app": "bar"} + doneChans = nil + ctx, cancel = context.WithCancel(context.Background()) - trackingHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { - mu.Lock() - defer mu.Unlock() - if !slices.Contains(reconciledCRs, obj.GetName()) { - reconciledCRs = append(reconciledCRs, obj.GetName()) - } - return nil - }) - mgr = setupManagerWithSelectorAndPostHook(ctx, trackingHook, matchingLabels) - - labeledObj = testutil.BuildTestCR(gvk) - labeledObj.SetName("labeled-cr") - labeledObj.SetLabels(matchingLabels) - labeledObjKey = types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()} + cleanupMgr := getManagerOrFail() + cleanupClient := cleanupMgr.GetClient() + crList := &unstructured.UnstructuredList{} + crList.SetGroupVersionKind(gvk) + Expect(cleanupClient.List(ctx, crList)).To(Succeed()) + + for i := range crList.Items { + cr := &crList.Items[i] + // Remove all finalizers to allow CR deletion + cr.SetFinalizers([]string{}) + Expect(cleanupClient.Update(ctx, cr)).To(Succeed()) + Expect(cleanupClient.Delete(ctx, cr)).To(Succeed()) + } - anotherObj = testutil.BuildTestCR(gvk) - anotherObj.SetName("another-cr") - anotherObjKey = types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()} + // Wait for all CRs to be deleted + Eventually(func() bool { + crList := &unstructured.UnstructuredList{} + crList.SetGroupVersionKind(gvk) + if err := cleanupClient.List(ctx, crList); err != nil { + return false + } + return len(crList.Items) == 0 + }, "10s", "100ms").Should(BeTrue()) }) AfterEach(func() { - By("ensuring the labeled CR is deleted", func() { - ensureDeleteCR(ctx, mgr, labeledObjKey, labeledObj) - }) - - By("ensuring the unlabeled CR is deleted", func() { - ensureDeleteCR(ctx, mgr, anotherObjKey, anotherObj) - }) - cancel() + // Cancel the context to stop all managers + if cancel != nil { + cancel() + } + // Wait for all managers to shut down completely + for _, done := range doneChans { + select { + case <-done: + // Manager has shut down + case <-time.After(5 * time.Second): + // Timeout waiting for manager shutdown + } + } }) It("should only reconcile CRs matching the label selector", func() { + labeledObj := testutil.BuildTestCR(gvk) + labeledObj.SetName("labeled-cr-test1") + labeledObj.SetLabels(matchingLabels) + labeledObjKey := types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()} + + anotherObj := testutil.BuildTestCR(gvk) + anotherObj.SetName("another-cr-test1") + anotherObjKey := types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()} + + var reconciledCRs []string + postHook := makePostHook(&mu, &reconciledCRs) + mgr, done := setupManagerWithSelectorAndPostHook(ctx, postHook, matchingLabels) + doneChans = append(doneChans, done) + By("creating a CR without matching labels", func() { Expect(mgr.GetClient().Create(ctx, anotherObj)).To(Succeed()) }) By("verifying that the labeled reconciler does not reconcile CR without labels", func() { - Consistently(func() []string { + Consistently(func() bool { mu.Lock() defer mu.Unlock() - return reconciledCRs - }, "2s", "100ms").Should(BeEmpty()) + return len(reconciledCRs) == 0 + }, "2s", "100ms").Should(BeTrue()) }) By("creating a CR with matching labels", func() { @@ -1584,11 +1597,11 @@ var _ = Describe("Reconciler", func() { }) By("verifying only the labeled CR was reconciled", func() { - Eventually(func() []string { + Eventually(func() bool { mu.Lock() defer mu.Unlock() - return reconciledCRs - }).Should(HaveExactElements(labeledObjKey.Name)) + return len(reconciledCRs) == 1 && reconciledCRs[0] == labeledObjKey.Name + }).Should(BeTrue()) }) By("updating the unlabeled CR to have matching labels", func() { @@ -1598,43 +1611,53 @@ var _ = Describe("Reconciler", func() { }) By("verifying that both CRs were reconciled after setting label to the unlabeled CR", func() { - Eventually(func() []string { + Eventually(func() bool { mu.Lock() defer mu.Unlock() - return reconciledCRs - }, "10s", "100ms").Should(ContainElements(labeledObjKey.Name, anotherObjKey.Name)) + return len(reconciledCRs) == 2 && + slices.Contains(reconciledCRs, labeledObjKey.Name) && + slices.Contains(reconciledCRs, anotherObjKey.Name) + }, "10s", "100ms").Should(BeTrue()) }) }) It("should reconcile CRs independently when using two managers with different label selectors", func() { - By("creating another manager with a different label selector", func() { - postHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { - mu.Lock() - defer mu.Unlock() - if !slices.Contains(anotherReconciledCRs, obj.GetName()) { - anotherReconciledCRs = append(anotherReconciledCRs, obj.GetName()) - } - return nil - }) - _ = setupManagerWithSelectorAndPostHook(ctx, postHook, anotherMatchingLabels) - }) + labeledObj := testutil.BuildTestCR(gvk) + labeledObj.SetName("labeled-cr-test2") + labeledObj.SetLabels(matchingLabels) + labeledObjKey := types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()} + + anotherObj := testutil.BuildTestCR(gvk) + anotherObj.SetName("another-cr-test2") + anotherObjKey := types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()} + + var reconciledCRs []string + var anotherReconciledCRs []string + + postHook := makePostHook(&mu, &reconciledCRs) + mgr, done := setupManagerWithSelectorAndPostHook(ctx, postHook, matchingLabels) + doneChans = append(doneChans, done) + + postHook2 := makePostHook(&mu, &anotherReconciledCRs) + _, done2 := setupManagerWithSelectorAndPostHook(ctx, postHook2, anotherMatchingLabels) + doneChans = append(doneChans, done2) By("creating a CR with matching labels for the first manager", func() { Expect(mgr.GetClient().Create(ctx, labeledObj)).To(Succeed()) }) By("verifying that only the first manager reconciled the CR", func() { - Eventually(func() []string { + Eventually(func() bool { mu.Lock() defer mu.Unlock() - return reconciledCRs - }, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name)) + return len(reconciledCRs) == 1 && reconciledCRs[0] == labeledObjKey.Name + }, "10s", "100ms").Should(BeTrue()) - Consistently(func() []string { + Consistently(func() bool { mu.Lock() defer mu.Unlock() - return anotherReconciledCRs - }, "2s", "100ms").Should(BeEmpty()) + return len(anotherReconciledCRs) == 0 + }, "2s", "100ms").Should(BeTrue()) }) By("creating a CR with matching labels for the second manager", func() { @@ -1645,17 +1668,17 @@ var _ = Describe("Reconciler", func() { }) By("verifying that both managers reconcile only matching labels CRs", func() { - Eventually(func() []string { + Eventually(func() bool { mu.Lock() defer mu.Unlock() - return reconciledCRs - }, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name)) + return len(reconciledCRs) == 1 && reconciledCRs[0] == labeledObjKey.Name + }, "10s", "100ms").Should(BeTrue()) - Eventually(func() []string { + Eventually(func() bool { mu.Lock() defer mu.Unlock() - return anotherReconciledCRs - }, "10s", "100ms").Should(HaveExactElements(anotherObjKey.Name)) + return len(anotherReconciledCRs) == 1 && anotherReconciledCRs[0] == anotherObjKey.Name + }, "10s", "100ms").Should(BeTrue()) }) }) }) @@ -1859,18 +1882,18 @@ func verifyEvent(ctx context.Context, cl client.Reader, obj metav1.Object, event Message: %q`, eventType, reason, message)) } -func ensureDeleteCR(ctx context.Context, mgr manager.Manager, crKey types.NamespacedName, cr *unstructured.Unstructured) { - err := mgr.GetAPIReader().Get(ctx, crKey, cr) - if apierrors.IsNotFound(err) { - return - } - Expect(err).ToNot(HaveOccurred()) - cr.SetFinalizers([]string{}) - Expect(mgr.GetClient().Update(ctx, cr)).To(Succeed()) - Expect(mgr.GetClient().Delete(ctx, cr)).To(Succeed()) +func makePostHook(mu *sync.Mutex, reconciledCRs *[]string) hook.PostHook { + return hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error { + mu.Lock() + defer mu.Unlock() + if !slices.Contains(*reconciledCRs, obj.GetName()) { + *reconciledCRs = append(*reconciledCRs, obj.GetName()) + } + return nil + }) } -func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) manager.Manager { +func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) (manager.Manager, chan struct{}) { mgr := getManagerOrFail() r, err := New( WithGroupVersionKind(gvk), @@ -1880,9 +1903,12 @@ func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.Post ) Expect(err).ToNot(HaveOccurred()) Expect(r.SetupWithManager(mgr)).To(Succeed()) + + done := make(chan struct{}) go func() { + defer close(done) Expect(mgr.Start(ctx)).To(Succeed()) }() Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue()) - return mgr + return mgr, done } From 91492137d21decee0d8461749e8cc3c662e704d1 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Mon, 1 Dec 2025 10:34:50 +0100 Subject: [PATCH 10/10] return protobuf Size() method for label setup check --- pkg/reconciler/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 3f9a5a58..fc6632eb 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -1025,7 +1025,7 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err var preds []predicate.Predicate - if len(r.labelSelector.MatchLabels) > 0 || len(r.labelSelector.MatchExpressions) > 0 { + if r.labelSelector.Size() > 0 { selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector) if err != nil { return err