fix(controller): handle node deletion, label selector unmatching, and status drift in NodeReconciler

[bhuvan-somisetty]

Description

This PR resolves issue #338 by fixing systemic node lifecycle state management, orphaned taint cleanup, and status synchronization bugs across NodeReconciler and RuleReconciler:

1. Node Deletion Cleanup: When a node is deleted from the cluster, NodeReconciler.Reconcile handles the apierrors.IsNotFound(err) condition via handleNodeDeletion(ctx, nodeName). It purges the deleted node from rule.Status.NodeEvaluations, rule.Status.AppliedNodes, and rule.Status.FailedNodes across all cached rules and updates Prometheus node_readiness_nodes_by_state gauge metrics.
2. Comprehensive cleanupDeletedNodes: Updated RuleReconciler.cleanupDeletedNodes to filter AppliedNodes and FailedNodes alongside NodeEvaluations.
3. Orphaned Taint Removal on Selector Unmatch: In processNodeAgainstAllRules, when a node's labels change such that it no longer matches a rule's spec.NodeSelector, the controller checks if the node holds rule.Spec.Taint or has status entries. If so, it invokes removeTaintBySpec to remove the orphaned taint and strips the node from rule status.
4. Real-time AppliedNodes Synchronization: Updated NodeReconciler status patch logic to append matching nodes to rule.Status.AppliedNodes upon successful evaluation or remove them if evaluation fails/unmatches.

Related Issue

Fixes #338

Type of Change

/kind bug

Testing

• Unit and integration tests added in internal/controller/node_controller_test.go and internal/controller/nodereadinessrule_controller_test.go covering node deletion handling, label selector unmatch taint removal, and AppliedNodes status synchronization.
• Verified test suite passes locally.

Checklist

• make test passes
• make lint passes

Does this PR introduce a user-facing change?

Fix state inconsistency, orphaned readiness taints, and status drift in NodeReconciler on node deletion and label selector changes.

Signed-off-by: bhuvan-somisetty somisettybhuvan5@gmail.com

[netlify]

✅ Deploy Preview for node-readiness-controller canceled.

Name	Link
🔨 Latest commit	c3bb1da
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a739eb6d929440008258b7f

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bhuvan-somisetty
Once this PR has been reviewed and has the lgtm label, please assign tallclair for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kubernetes-prow]

Hi @bhuvan-somisetty. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kubernetes-prow]

@bhuvan-somisetty: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[bhuvan-somisetty]

/retest

[kubernetes-prow]

@bhuvan-somisetty: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[bhuvan-somisetty]

Hi @ajaysundark @vitorfloriano @Priyankasaggu11929,

Could you please take a look at this PR when you get a chance?

This PR resolves issue #338 by fixing node deletion cleanup, label selector unmatching taint removal, and real-time \AppliedNodes\ status synchronization across \NodeReconciler\ and \RuleReconciler.

Thanks!

[Priyankasaggu11929]

Thanks for the PR @bhuvan-somisetty!

I'll spend some time understanding the reported issue (because I've a feeling that atleast Orphaned Taints on Node Label Selector Unmatch is done by design so far.)

will drop some comments by EOD tomorrow.

[Priyankasaggu11929]

/ok-to-test

[ajaysundark]

we are looking to move away from per-node status updates in NRC #89, this could wait until the per-node api is landed? also it is a good idea to send small PRs with reviewable context than to push multiple orthogonal changes into a single PR.

xref -- #338 (comment)

cc @Priyankasaggu11929 @Karthik-K-N

[bhuvan-somisetty]

Good call, dropped the AppliedNodes real-time sync piece since it's per-node status work that #89 is going to replace anyway. Also pulled the selector-unmatch taint cleanup out into its own issue (#342) per your comment on #338, so this PR is now scoped to just node deletion handling + making cleanupDeletedNodes cover AppliedNodes/FailedNodes too, not just NodeEvaluations. Pushed the update.

[bhuvan-somisetty]

Rebased onto main to resolve the conflict in node_controller.go and pushed. All checks (test, test-e2e, verify-all) are passing now, ready for review.

[kubernetes-prow]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kubernetes-prow]

@bhuvan-somisetty: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-node-readiness-controller-scale-test-binary-1000-continuous	c3bb1da	link	true	/test pull-node-readiness-controller-scale-test-binary-1000-continuous
pull-node-readiness-controller-lint	c3bb1da	link	true	/test pull-node-readiness-controller-lint

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.