From 24bd217a6f12a057de1e1fcab5a0ec5c3b473d34 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Mon, 20 Oct 2025 13:55:08 +0200 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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()) })