fix: trigger reconcile on rule deletion-timestamp updates

The RuleReconciler watch used a bare GenerationChangedPredicate. When a
rule with the cleanup-taints finalizer is deleted, the API server only
sets DeletionTimestamp; it does not bump Generation. The predicate
filtered the update and reconcileDelete never ran, leaving the rule in
Terminating and the managed taints orphaned on nodes.

Replace the predicate with one that also passes Update events where
DeletionTimestamp transitions from unset to set, and add unit coverage
so a future regression to a bare GenerationChangedPredicate fails the
build instead of silently breaking deletion.

Author: sahitya-chandra

internal/controller/nodereadinessrule_controller.go

@@ -37,6 +37,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	readinessv1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1"
@@ -78,12 +79,28 @@ func NewRuleReadinessController(mgr ctrl.Manager, clientset kubernetes.Interface
 	}
 }
 
+// rulePredicate also passes the DeletionTimestamp-set transition: the API
+// server does not bump Generation when it sets DeletionTimestamp, so a bare
+// GenerationChangedPredicate would drop deletion events for rules with a finalizer
+var rulePredicate = predicate.Funcs{
+	UpdateFunc: func(e event.UpdateEvent) bool {
+		if e.ObjectOld == nil || e.ObjectNew == nil {
+			return false
+		}
+		if e.ObjectNew.GetGeneration() != e.ObjectOld.GetGeneration() {
+			return true
+		}
+		return e.ObjectOld.GetDeletionTimestamp().IsZero() &&
+			!e.ObjectNew.GetDeletionTimestamp().IsZero()
+	},
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *RuleReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("nodereadiness-controller").
 		WithOptions(controller.Options{MaxConcurrentReconciles: 1}).
-		For(&readinessv1alpha1.NodeReadinessRule{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
+		For(&readinessv1alpha1.NodeReadinessRule{}, builder.WithPredicates(rulePredicate)).
 		Complete(r)
 }
 

internal/controller/nodereadinessrule_controller_test.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	nodereadinessiov1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1"
@@ -1380,4 +1381,46 @@ var _ = Describe("NodeReadinessRule Controller", func() {
 			}, time.Second*5).Should(BeTrue(), "All AppliedNodes should have corresponding NodeEvaluations")
 		})
 	})
+
+	Context("rulePredicate", func() {
+		newRule := func(generation int64, deletionTimestamp *metav1.Time) *nodereadinessiov1alpha1.NodeReadinessRule {
+			return &nodereadinessiov1alpha1.NodeReadinessRule{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "predicate-rule",
+					Generation:        generation,
+					DeletionTimestamp: deletionTimestamp,
+				},
+			}
+		}
+
+		It("passes updates that bump generation", func() {
+			Expect(rulePredicate.Update(event.UpdateEvent{
+				ObjectOld: newRule(1, nil),
+				ObjectNew: newRule(2, nil),
+			})).To(BeTrue())
+		})
+
+		It("passes updates that set the deletion timestamp without bumping generation", func() {
+			now := metav1.Now()
+			Expect(rulePredicate.Update(event.UpdateEvent{
+				ObjectOld: newRule(1, nil),
+				ObjectNew: newRule(1, &now),
+			})).To(BeTrue(), "deletion-marker updates must trigger reconcile so the finalizer can be removed")
+		})
+
+		It("ignores status-only updates", func() {
+			Expect(rulePredicate.Update(event.UpdateEvent{
+				ObjectOld: newRule(1, nil),
+				ObjectNew: newRule(1, nil),
+			})).To(BeFalse())
+		})
+
+		It("ignores updates while the rule is already terminating", func() {
+			now := metav1.Now()
+			Expect(rulePredicate.Update(event.UpdateEvent{
+				ObjectOld: newRule(1, &now),
+				ObjectNew: newRule(1, &now),
+			})).To(BeFalse())
+		})
+	})
 })