fix: clear stale FailedNodes entry when a node recovers

[bhuvan-somisetty]

Description

This is a follow-up to #332, extracted per @ajaysundark's request to review #209 and evaluate the recordFailure/stale FailedNodes patch separately from the ruleCache data-race fix.

processNodeAgainstAllRules (the NodeReconciler path in node_controller.go) records a NodeFailure via recordNodeFailure when evaluateRuleForNode errors, but never clears it when the node later evaluates successfully. Since rule here is a deep-copied snapshot carried over from ruleCache (per #332), a stale failure entry from a prior reconcile keeps getting copied forward into rule.Status.FailedNodes on every subsequent successful evaluation, so a recovered node is stuck listed as failed indefinitely.

nodereadinessrule_controller.go's processAllNodesForRule (the RuleReconciler path) already clears FailedNodes inline on the success branch, so it isn't affected — only the NodeReconciler path needed the fix.

This adds a clearNodeFailure helper (matching the existing recordNodeFailure pattern) and calls it on the success branch of processNodeAgainstAllRules.

Related

Follow-up to #332, evaluated from #209.

Type of Change

/kind bug

Testing

• go build ./...
• go vet ./...
• go test ./internal/controller/...
• golangci-lint run ./internal/controller/...
• Added TestClearNodeFailure and TestRecordThenClearNodeFailure covering the helper directly.

Checklist

• make test passes
• make lint passes

Does this PR introduce a user-facing change?

Fixed a bug where a node that recovered from an evaluation error remained listed in a NodeReadinessRule's status.failedNodes indefinitely.

[netlify]

✅ Deploy Preview for node-readiness-controller canceled.

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

[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 haircommander 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.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

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.

[bhuvan-somisetty]

/assign @ajaysundark

[ajaysundark]

/ok-to-test

[bhuvan-somisetty]

Thanks for the ok-to-test, @ajaysundark! All the Prow jobs (test, e2e, verify-all) are green now. Whenever you get a chance to take a look, let me know if you'd like anything changed.

[ajaysundark]

/cc @vitorfloriano @Priyankasaggu11929 do either of you have time for this scenario review?

[Priyankasaggu11929]

/cc @vitorfloriano @Priyankasaggu11929 do either of you have time for this scenario review?

I'll take a look at it, and post some comments by tomorrow EOD.

[Priyankasaggu11929]

or wait, I see PR #209 landed first and

@bhuvan-somisetty - you have already dropped some review comments around it here #332 (comment)

IIUC (from just PR titles and from the code changes as well), both these PRs are supposed to be fixing the same problem.

Let's try to get #209 merged.

[bhuvan-somisetty]

Makes sense, #209 already covers this exact fix along with the data race one. Closing this in favor of that. Thanks for checking!

[ajaysundark]

Thanks for your PR. Let's track the fix in the current open PR #381