Skip to content

Commit 81c36cb

Browse files
kurlovporridge
authored andcommitted
Fix reconciling with label selector for multiple reconcilers.
1 parent 6261f25 commit 81c36cb

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

pkg/reconciler/reconciler.go

+6
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
ctrl "sigs.k8s.io/controller-runtime"
4242
"sigs.k8s.io/controller-runtime/pkg/client"
4343
"sigs.k8s.io/controller-runtime/pkg/controller"
44+
"sigs.k8s.io/controller-runtime/pkg/event"
4445
"sigs.k8s.io/controller-runtime/pkg/handler"
4546
"sigs.k8s.io/controller-runtime/pkg/predicate"
4647
"sigs.k8s.io/controller-runtime/pkg/source"
@@ -571,6 +572,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
571572
return ctrl.Result{}, err
572573
}
573574

575+
if r.selectorPredicate != nil && !r.selectorPredicate.Generic(event.GenericEvent{Object: obj}) {
576+
log.V(1).Info("Label selector does not match, skipping reconcile")
577+
return ctrl.Result{}, nil
578+
}
579+
574580
// The finalizer must be present on the CR before we can do anything. Otherwise, if the reconciliation fails,
575581
// there might be resources created by the chart that will not be garbage-collected
576582
// (cluster-scoped resources or resources in other namespaces, which are not bound by an owner reference).

pkg/reconciler/reconciler_test.go

+39
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ var _ = Describe("Reconciler", func() {
486486
Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue())
487487

488488
obj = testutil.BuildTestCR(gvk)
489+
obj.SetLabels(map[string]string{"foo": "bar"})
489490
objKey = types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}
490491
req = reconcile.Request{NamespacedName: objKey}
491492
})
@@ -518,6 +519,8 @@ var _ = Describe("Reconciler", func() {
518519
cancel()
519520
})
520521

522+
selector := metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}
523+
521524
// After migration to Ginkgo v2 this can be rewritten using e.g. DescribeTable.
522525
parameterizedReconcilerTests := func(opts reconcilerTestSuiteOpts) {
523526
BeforeEach(func() {
@@ -535,6 +538,7 @@ var _ = Describe("Reconciler", func() {
535538
WithInstallAnnotations(annotation.InstallDescription{}),
536539
WithUpgradeAnnotations(annotation.UpgradeDescription{}),
537540
WithUninstallAnnotations(annotation.UninstallDescription{}),
541+
WithSelector(selector),
538542
WithOverrideValues(map[string]string{
539543
"image.repository": "custom-nginx",
540544
}),
@@ -549,6 +553,7 @@ var _ = Describe("Reconciler", func() {
549553
WithInstallAnnotations(annotation.InstallDescription{}),
550554
WithUpgradeAnnotations(annotation.UpgradeDescription{}),
551555
WithUninstallAnnotations(annotation.UninstallDescription{}),
556+
WithSelector(selector),
552557
WithOverrideValues(map[string]string{
553558
"image.repository": "custom-nginx",
554559
}),
@@ -1425,6 +1430,40 @@ var _ = Describe("Reconciler", func() {
14251430
})
14261431
})
14271432
})
1433+
When("label selector succeeds", func() {
1434+
It("reconciles only matching label", func() {
1435+
By("setting an invalid action client getter to assert different reconcile results", func() {
1436+
r.actionClientGetter = helmclient.ActionClientGetterFunc(func(context.Context, client.Object) (helmclient.ActionInterface, error) {
1437+
fakeClient := helmfake.NewActionClient()
1438+
return &fakeClient, nil
1439+
})
1440+
})
1441+
1442+
By("setting not matching label to the CR", func() {
1443+
Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed())
1444+
obj.SetLabels(map[string]string{"foo": "baz"})
1445+
Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed())
1446+
})
1447+
1448+
By("reconciling is skipped, action client was not called and no error returned", func() {
1449+
res, err := r.Reconcile(ctx, req)
1450+
Expect(res).To(Equal(reconcile.Result{}))
1451+
Expect(err).ToNot(HaveOccurred())
1452+
})
1453+
1454+
By("setting matching label to the CR", func() {
1455+
Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed())
1456+
obj.SetLabels(map[string]string{"foo": "bar"})
1457+
Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed())
1458+
})
1459+
1460+
By("reconciling is not skipped and error returned because of broken action client", func() {
1461+
res, err := r.Reconcile(ctx, req)
1462+
Expect(res).To(Equal(reconcile.Result{}))
1463+
Expect(err).To(MatchError("get not implemented"))
1464+
})
1465+
})
1466+
})
14281467
})
14291468
})
14301469
})

0 commit comments

Comments
 (0)