✨ feat: introduce NodeReadinessEvaluation (NRE) CRD and controller

[Karthik-K-N]

Description

Introduces the NodeReadinessEvaluation (NRE) custom resource and its dedicated reconciler. NRE provides a per-node, read-only mirror of the evaluated state of every applicable NodeReadinessRule, giving operators a single object to kubectl get nre <node-name> to see the full readiness picture of any node without having to cross-reference multiple rule statuses.

Discussion document: https://docs.google.com/document/d/1DOP1G6i__nQN8qbhlSvdswkUquMeSKQY4LU554AbcgM/edit?usp=sharing

Feature Flag

The controller is opt-in via: --enable-node-readiness-evaluation

Related Issue

Type of Change

Testing

Checklist

• make test passes
• make lint passes

Does this PR introduce a user-facing change?

Doc #(issue)

[netlify]

✅ Deploy Preview for node-readiness-controller canceled.

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

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Karthik-K-N
Once this PR has been reviewed and has the lgtm label, please assign sergeykanzhelev 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

[ajaysundark]

@Karthik-K-N IIUC, this would trigger two parallel reconciliations for node updates (NodeReconciler and NREReconciler) and one is handling the output of the other. I wonder if a separate controller for NRE is a better fitting pattern for this or should be bridged into NodeReconciler's watch?

[Karthik-K-N]

yes I thought about that and I missed to point out why I chose this way, here are my thoughts

1. Initially tried combining both into the NodeReconciler, but then when I checked with best practices of writing controller, if we do so we will be merging two different operations into one, Node controller adding/removing taints,NRE just storing the result, One failure should not cause requeue for other and I think NodeReconcile should be as quick as possible as it affects the workloads.
2. Followed the existing pattern of having separate controllers like NRR, Node and NRE
3. Even if there is race and NRE reconciles first and later the Node, Since the NRE also watches for condition, taint and label, if something updated by the Node Rec then eventually it triggers the NRE. We will be eventually consistent.

These are the thoughts and I lean towards keeping them separate, but let me know what do you think.

[DsThakurRawat]

i tested this branch and found two behaviours worth knowing about, plus one interaction with #315. all three are reproduced with runnable tests, happy to share them.

first, a rule-creation window that never heals. the NRE fan-out fires on the rule Create event (GenerationChangedPredicate only filters updates), but the shared ruleCache is only populated on the rule controller's second reconcile pass, because the first one adds the finalizer and returns RequeueAfter one second. so the NRE reconciles for every matching node run against a cache that cannot contain the new rule yet. for rules whose evaluation then changes a node, the taint write re-triggers the node watch and everything heals. but for a rule whose conditions are already satisfied everywhere, nothing mutates any node, the rule's status patch doesn't bump generation, heartbeats don't pass the node predicate (it compares only condition type to status), and informer resyncs are filtered as no-ops. the NRE just permanently misses the rule until some unrelated node change. deletion is fine, reconcileDelete empties the cache before dropping the finalizer. i think this is the concrete version of the question ajaysundark raised above: the reconciler's correctness depends on another controller's queue having run first. listing rules from the informer inside Reconcile instead of reading the private cache would remove the ordering dependency entirely.

second, this branch merges cleanly with #315, and the two disagree once combined. evaluateRuleForNode there honours conditionPolicy anyOf, while buildRuleEvaluation here ANDs every condition unconditionally. an anyOf rule with one of two conditions satisfied ends up enforced as satisfied (no taint) while the NRE for the same node reports it Unmatched. a shared evaluation helper honouring GetConditionPolicy() in both paths would keep them from drifting.

small one: nothing currently produces RuleStatusError, so the Errors count, State Pending, and the Evaluated False condition are unreachable in this iteration. fine if that's intentional groundwork, might deserve a TODO.

[ajaysundark]

@Karthik-K-N is this ready for review?

[Karthik-K-N]

@Karthik-K-N is this ready for review?

yes , its ready for review

[kubernetes-prow]

@Karthik-K-N: The following test 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-lint	7c0ecc4	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.

[DsThakurRawat]

The rule-creation window I flagged here on Aug 11 has a second victim now, and this one is worse because it's the reporting surface. Walked the sequence on your head 7c0ecc42: a rule Create passes GenerationChangedPredicate into this controller too, ruleToNodeRequests enqueues the matching nodes immediately, but the NRE reconciler reads getApplicableRulesForNode, which serves only the private ruleCache, and the cache isn't populated until the rule controller's second reconcile, because the first one just adds the finalizer and returns RequeueAfter one second (nodereadinessrule_controller.go:118-125, cache write at :139). So for a rule whose conditions are already satisfied on every matching node and whose taint is absent everywhere, the NRE usually reconciles against the empty cache and writes state: Ready with zero rule entries. Nothing corrects it after that: evaluation mutates no node, so no node event fires, and resync updates die on your equality predicates in the For(). Until something else happens to touch that node, the NRE keeps telling any reader (#327's history story, dashboards) that no rule applies to it while one actively does.

The Aug 11 fix suggestion covers both consumers, and it got cheaper to justify: have this controller read rules through the informer instead of the private cache, and the ordering dependency disappears rather than getting patched around.

Separately, I reproduced prow's lint failure locally so it can be named precisely: gocritic ifElseChain on the four-way taint transition at nodereadinessevaluation_controller.go:290, and unparam's unused ctx in the test's sharedSetup at :91. Both trivial.

What I checked that holds: rule deletions do reach this controller (in controller-runtime v0.24.1 a nil DeleteFunc defaults to passing, so GenerationChangedPredicate only filters updates), and your D1 shows the prune works once a reconcile runs; the CEL immutability on nodeSelector makes mapping by selector safe across rule updates; node deletion is covered by the ownerReference; and the single-writer separation is real, this reconciler writes nothing but its own status. One RBAC nit: the marker asks delete on nodereadinessevaluations but nothing in the code ever calls it, GC does that work.