Skip to content

[BUG] TaintAdopted events are emitted repeatedly for taints the controller applied itself #383

Description

@tejassinghbhati

What happened?

The controller keeps emitting TaintAdopted events for taints it applied itself. Any node that is still unready and already carries the rule's taint gets a fresh TaintAdopted event, and an Adopting pre-existing taint log line, on every single reconcile.

NodeReconciler gets its rules from the in-memory cache. getApplicableRulesForNode returns a deep copy of the cached rule, and that copy carries whatever status the rule had the last time RuleReconciler ran. Node evaluations persisted since then are not in it. Since the rule spec is immutable, GenerationChangedPredicate means RuleReconciler rarely fires again, so that snapshot can stay stale for the life of the process.

evaluateRuleForNode decides adoption from that status:

isFirstEvaluation := r.getPreviousNodeEvaluation(rule, node.Name) == nil

For a node the snapshot never contained, this is always true, so the adoption branch fires every time:

case !shouldRemoveTaint && currentlyHasTaint:
    if isFirstEvaluation {
        log.Info("Adopting pre-existing taint", ...)
        r.EventRecorder.Eventf(node, nil, corev1.EventTypeNormal, "TaintAdopted", "AdoptTaint", "%s", message)
    }

TaintAdopted is supposed to tell an operator that a taint was already on the node and the rule has taken ownership of it, which is what #134 and #158 added it for. Firing it repeatedly for the controller's own taints makes it useless as a signal, since you cannot tell a real adoption from noise. It also adds pointless writes to the events API for every unready node on every condition change.

Note this is not the same as the rule status being wrong. The persisted status.nodeEvaluations is correct, it is only the cached copy the evaluation reads from that is behind.

Steps to Reproduce

  1. Create a continuous rule with taint readiness.k8s.io/example:NoSchedule and a condition the node does not satisfy.
  2. Let the controller apply the taint.
  3. Leave the node unready and let it reconcile a few more times, for example by touching an unrelated condition or label.
  4. kubectl get events --field-selector involvedObject.name=<node> shows a TaintAdopted event for each pass, even though the controller applied that taint in step 2.

An envtest spec shows it directly. After one reconcile applies the taint, three more reconciles produce three adoption events:

INFO  Adopting pre-existing taint  {"node": "adopt-events-node", "rule": "adopt-events-rule", ...}
INFO  Adopting pre-existing taint  {"node": "adopt-events-node", "rule": "adopt-events-rule", ...}
INFO  Adopting pre-existing taint  {"node": "adopt-events-node", "rule": "adopt-events-rule", ...}
[FAILED] the rule already owns this taint, so no further TaintAdopted events should be emitted

Expected Behavior

TaintAdopted should be emitted once, and only when the taint was on the node before the rule evaluated it. A taint the controller applied itself is not an adoption.

Controller Version / Image Tag

main (commit d74dabd)

Kubernetes Version

Not version specific. Reproduced on envtest with Kubernetes 1.36.2 binaries.

Additional Environment Details

I checked the open issues and PRs before filing. #362 is also about events but the opposite gap, no Warning events on failures, so it does not overlap. Of the open PRs that touch this area, #294, #315, #343 and #209 all read getPreviousNodeEvaluation or isFirstEvaluation but none refresh the cached status, so none of them fix this. #294 would inherit the same staleness for the new metric it adds.

I have a fix and tests ready and will open a PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions