Skip to content

Commit de5b043

Browse files
committed
Update the controller predicate logic
Signed-off-by: chiragkyal <ckyal@redhat.com>
1 parent 88f97b2 commit de5b043

2 files changed

Lines changed: 77 additions & 3 deletions

File tree

pkg/controller/trustmanager/controller.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
ctrl "sigs.k8s.io/controller-runtime"
1717
"sigs.k8s.io/controller-runtime/pkg/builder"
1818
"sigs.k8s.io/controller-runtime/pkg/client"
19+
"sigs.k8s.io/controller-runtime/pkg/event"
1920
"sigs.k8s.io/controller-runtime/pkg/handler"
2021
"sigs.k8s.io/controller-runtime/pkg/predicate"
2122
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -95,13 +96,30 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
9596
return []reconcile.Request{}
9697
}
9798

98-
// predicate function to ignore events for objects not managed by controller.
99-
controllerManagedResources := predicate.NewPredicateFuncs(func(object client.Object) bool {
99+
isManagedResource := func(object client.Object) bool {
100100
labels := object.GetLabels()
101101
matches := labels != nil && labels[common.ManagedResourceLabelKey] == RequestEnqueueLabelValue
102102
r.log.V(4).Info("predicate evaluation", "object", fmt.Sprintf("%T", object), "name", object.GetName(), "namespace", object.GetNamespace(), "labels", labels, "matches", matches)
103103
return matches
104-
})
104+
}
105+
106+
// Predicate to filter events for resources managed by this controller.
107+
// On updates, checks both old and new objects so that events where the
108+
// managed label is removed still trigger reconciliation.
109+
controllerManagedResources := predicate.Funcs{
110+
CreateFunc: func(e event.CreateEvent) bool {
111+
return isManagedResource(e.Object)
112+
},
113+
UpdateFunc: func(e event.UpdateEvent) bool {
114+
return isManagedResource(e.ObjectOld) || isManagedResource(e.ObjectNew)
115+
},
116+
DeleteFunc: func(e event.DeleteEvent) bool {
117+
return isManagedResource(e.Object)
118+
},
119+
GenericFunc: func(e event.GenericEvent) bool {
120+
return isManagedResource(e.Object)
121+
},
122+
}
105123

106124
controllerManagedResourcePredicates := builder.WithPredicates(controllerManagedResources)
107125

test/e2e/trustmanager_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,62 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func()
371371
})
372372
})
373373

374+
// -------------------------------------------------------------------------
375+
// Managed label removal reconciliation
376+
// -------------------------------------------------------------------------
377+
378+
Context("managed label removal reconciliation", func() {
379+
It("should restore the managed label when removed externally from resources", func() {
380+
createTrustManager(newTrustManagerCR())
381+
382+
// The "app" label is the managed resource label used by the predicate
383+
// to filter watch events. Removing it tests that the predicate checks
384+
// both old and new objects on updates, so the event is not silently dropped.
385+
386+
By("removing managed label from ServiceAccount")
387+
sa, err := clientset.CoreV1().ServiceAccounts(trustManagerNamespace).Get(ctx, trustManagerServiceAccountName, metav1.GetOptions{})
388+
Expect(err).ShouldNot(HaveOccurred())
389+
delete(sa.Labels, "app")
390+
_, err = clientset.CoreV1().ServiceAccounts(trustManagerNamespace).Update(ctx, sa, metav1.UpdateOptions{})
391+
Expect(err).ShouldNot(HaveOccurred())
392+
393+
By("verifying controller restores managed label on ServiceAccount")
394+
Eventually(func(g Gomega) {
395+
sa, err := clientset.CoreV1().ServiceAccounts(trustManagerNamespace).Get(ctx, trustManagerServiceAccountName, metav1.GetOptions{})
396+
g.Expect(err).ShouldNot(HaveOccurred())
397+
g.Expect(sa.Labels).Should(HaveKeyWithValue("app", trustManagerCommonName))
398+
}, lowTimeout, fastPollInterval).Should(Succeed())
399+
400+
By("removing managed label from Deployment")
401+
dep, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{})
402+
Expect(err).ShouldNot(HaveOccurred())
403+
delete(dep.Labels, "app")
404+
_, err = clientset.AppsV1().Deployments(trustManagerNamespace).Update(ctx, dep, metav1.UpdateOptions{})
405+
Expect(err).ShouldNot(HaveOccurred())
406+
407+
By("verifying controller restores managed label on Deployment")
408+
Eventually(func(g Gomega) {
409+
dep, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{})
410+
g.Expect(err).ShouldNot(HaveOccurred())
411+
g.Expect(dep.Labels).Should(HaveKeyWithValue("app", trustManagerCommonName))
412+
}, lowTimeout, fastPollInterval).Should(Succeed())
413+
414+
By("removing managed label from ClusterRole")
415+
cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, trustManagerClusterRoleName, metav1.GetOptions{})
416+
Expect(err).ShouldNot(HaveOccurred())
417+
delete(cr.Labels, "app")
418+
_, err = clientset.RbacV1().ClusterRoles().Update(ctx, cr, metav1.UpdateOptions{})
419+
Expect(err).ShouldNot(HaveOccurred())
420+
421+
By("verifying controller restores managed label on ClusterRole")
422+
Eventually(func(g Gomega) {
423+
cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, trustManagerClusterRoleName, metav1.GetOptions{})
424+
g.Expect(err).ShouldNot(HaveOccurred())
425+
g.Expect(cr.Labels).Should(HaveKeyWithValue("app", trustManagerCommonName))
426+
}, lowTimeout, fastPollInterval).Should(Succeed())
427+
})
428+
})
429+
374430
// -------------------------------------------------------------------------
375431
// Deployment configuration
376432
// -------------------------------------------------------------------------

0 commit comments

Comments
 (0)