Repository navigation
[BUG] Handle node deletion, label selector unmatching, and status drift in NodeReconciler #338
Description
What happened?
When a node is deleted from the cluster, NodeReconciler.Reconcile gets a NotFound error from r.Get and silently swallows it via client.IgnoreNotFound(err). The deleted node is never removed from rule.Status.NodeEvaluations, AppliedNodes, or FailedNodes on any rule, and the node_readiness_nodes_by_state metric keeps counting it forever.
Separately, RuleReconciler.cleanupDeletedNodes only filters NodeEvaluations — it misses AppliedNodes and FailedNodes, so those two fields also go stale.
Two related points originally filed here are split out per this review:
AppliedNodesreal-time sync — deferred until Split rule.status.NodeEvaluationResults into a separate CRD NodeReadinessEvaluation #89 lands (per-node status is moving out ofrule.statusentirely).- Orphaned taints on selector unmatch — moved to [BUG] Orphaned readiness taint remains when a node's labels no longer match a rule's selector #342 (needs its own design discussion).
Expected behavior
- Deleting a node removes it from
NodeEvaluations,AppliedNodes, andFailedNodeson every rule, and updates the metric. cleanupDeletedNodescleans up all three fields, not justNodeEvaluations.
Repro
- Create a
NodeReadinessRulematching worker nodes. - Confirm a node shows up in
rule.Status.NodeEvaluations/AppliedNodes. kubectl delete node worker-1.worker-1is still in the rule status and in/metrics.
Affected components
internal/controller/node_controller.go(Reconcile)internal/controller/nodereadinessrule_controller.go(cleanupDeletedNodes)
Controller version / K8s version
main (v0.4.1+) / v1.30.0–v1.31.0
Metadata
Metadata
Assignees
Labels
No labels
Activity
[-]fix(controller): handle node deletion, label selector unmatching, and status drift in NodeReconciler[/-][+][BUG] Handle node deletion, label selector unmatching, and status drift in NodeReconciler[/+]bhuvan-somisetty commentedon Aug 2, 2026
/assign
fix: handle node deletion, label selector unmatching, and status drif…
fix(controller): handle node deletion, label selector unmatching, and…
fix(controller): handle node deletion and selector unmatch
ajaysundark commentedon Aug 3, 2026
We are looking to drop the per-node fields in rule.status soon so improving
rule.Status.*Nodesis lower priority.This was as designed originally. Because after the label is deleted, NRC cannot find the 'applicable rules' for this node any more:
node-readiness-controller/internal/controller/node_controller.go
Line 118 in 43bff46
To solve this case, what is needed is to detect when a node "used to match" a rule but no longer matches it.
controller relying on its status (for pre-existing match) to decide on the action is fragile and cannot be a reliable signal for it to act on.
The alternative is for NodeReconciler to evaluate 'all rules' for all node updates, instead of just the matching rules, so if a node is not matched by label, but it had a managed taint, it will send patch to remove. But it is expensive especially with the status updates.
If a user has a manual action to delete a matching label (possibly via a script), it feels like the taint removal can also be patched in the same script. So in reality, this doesnt feel a high-impact bug, besides NRC supports a dry-run feature to capture misconfiguration scenarios to avoid this kind of manual errors.
Can we create a separate issue for this instead of clubbing it all together in a single issue?
bhuvan-somisetty commentedon Aug 3, 2026
Makes sense on all counts. Split the selector-unmatch part into #342 for its own design discussion, and dropped the AppliedNodes real-time sync from here too since #89 is moving that away anyway. #338/#339 are now just node deletion cleanup + making cleanupDeletedNodes cover AppliedNodes/FailedNodes, not just NodeEvaluations. Updated the PR to match.
ajaysundark commentedon Aug 9, 2026
Hi @bhuvan-somisetty can you rather clean up the description to be concise on what this is about? The verbose descriptions for a rather straightforward issue is overwhelming for the readers.
AI tooling could be reducing maintainer toil, not add to them. When working with such tools, consider taking effort to 'process' the information, understand the issue and work with the community manually. In open-source, we like to work with humans in slack and issues! :)
ajaysundark commentedon Aug 9, 2026
cc @Priyankasaggu11929 we should consider adding a note for AI assisted contribution guidelines in our project.
Priyankasaggu11929 commentedon Aug 10, 2026
I'll send a PR adding some discreet instructions.
I am seeing the pattern all over the project now - AI generated PR/issues with AI generated description, continuing with AI generated comments. 😓 (another example - #376 (comment))
Not helpful at all, to maintainers reviewing these changes.
bhuvan-somisetty commentedon Aug 10, 2026
Fair point, and taken to heart — trimmed the description above down to just what's needed to reproduce and fix it. Will keep future issues/PRs shorter and lean on discussion in comments (or slack) instead of front-loading everything into the description.