ROX-30138: Add tests for WithSelector

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

pkg/reconciler/reconciler_test.go

@@ -21,7 +21,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"strconv"
+	"sync"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -1492,45 +1494,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 +1508,158 @@ var _ = Describe("Reconciler", func() {
 		})
 	})
 
+	_ = Describe("WithSelector", 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
+		)
+
+		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"}
+
+			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()}
+
+			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() {
+				ensureDeleteCR(ctx, mgr, labeledObjKey, labeledObj)
+			})
+
+			By("ensuring the unlabeled CR is deleted", func() {
+				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("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("verifying that both CRs were reconciled after setting label to the unlabeled CR", func() {
+				Eventually(func() []string {
+					mu.Lock()
+					defer mu.Unlock()
+					return reconciledCRs
+				}, "10s", "100ms").Should(ContainElements(labeledObjKey.Name, anotherObjKey.Name))
+			})
+		})
+
+		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)
+			})
+
+			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 {
+					mu.Lock()
+					defer mu.Unlock()
+					return reconciledCRs
+				}, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name))
+
+				Consistently(func() []string {
+					mu.Lock()
+					defer mu.Unlock()
+					return anotherReconciledCRs
+				}, "2s", "100ms").Should(BeEmpty())
+			})
+
+			By("creating a CR with matching labels for the second manager", func() {
+				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() {
+				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))
+			})
+		})
+	})
+
 	_ = Describe("Test custom controller setup", func() {
 		var (
 			mgr                   manager.Manager
@@ -1743,3 +1858,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 setupManagerWithSelectorAndPostHook(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
+}