[BUG]: taint-blocked node held by sequencing flaps between waiting and blocked every reconcile

[ayuskauskas]

Skyhook Version

operator/v0.19.0 (also reproduced on main at 4755b301)

Installation method

Helm

Kubernetes Version

n/a — reproduced in envtest

Component

Operator (controller-manager)

Describe the bug

A node whose taints are not tolerated by a NodeWright, and which is simultaneously held at waiting by cross-NodeWright sequencing, oscillates between waiting and blocked on every reconcile and never converges.

Two writers disagree within a single pass. RunSkyhookPackages calls IntrospectSkyhook (skyhook_controller.go:1321) and then SelectNodes (1326):

1. IntrospectNode forces StatusWaiting for any non-complete node that IsNodeReadyForSkyhook rejects (cluster_state_v2.go:1320-1324).
2. selectNodesWithCompartments then forces StatusBlocked because the node's taints are not tolerable.

Both SetStatus calls differ from the in-memory annotation, so node.updated latches even though the net persisted value is unchanged. SaveNodesAndSkyhook issues a Node PATCH and the reconcile requeues at 2s, and the next pass repeats identically — a no-op write plus a status update every 2 seconds, per affected node, indefinitely.

The ignore path had the same defect and was fixed in #586 by making the ignore label authoritative in IntrospectNode:

// Ignore overrides sequencing waits so a settled blocked node does not
// flip to Waiting and back to Blocked on every selection pass.
if !node.IsComplete() && CheckNodeIgnoreLabel(node) {
    node.SetStatus(v1alpha1.StatusBlocked)
    return node.Changed()
}

Taints need the equivalent guard. The awkward part is that IntrospectNode has no tolerations available: they are assembled in SelectNodes from Spec.AdditionalTolerations plus the reconciler's runtimeRequiredTolerations, so either those need plumbing through, or the scan in selectNodesWithCompartments needs to learn not to override a sequencing-held waiting.

Scope note. This is pre-existing, not introduced by #586, but #586 widens it. Before that PR, SetStatus(StatusBlocked) only ran for nodes GetNodesForNextBatch() actually returned, so the churn required the compartment to have batch capacity for the node. #586 moves the call into a scan over every node in every compartment, so the capacity precondition disappears. Measured with an identical probe on both commits:

scenario	base 4755b301	#586 head a9662b0c
batch has room for the node	waiting -> blocked (churns)	waiting -> blocked (churns)
batch soaked up by healthy nodes	waiting -> waiting (stable)	waiting -> blocked (churns)

Steps to reproduce

In operator/internal/controller, build two NodeWrights over one node:

1. first with priority: 1, second with priority: 2, both selecting the node, each with one package and interruptionBudget.count: 1.
2. Give the node a taint that neither NodeWright tolerates, e.g. special=yes:NoSchedule.
3. Leave first incomplete on the node, so IsNodeReadyForSkyhook returns false for second.
4. For second, run IntrospectSkyhook(second, allSkyhooks, logger) followed by NewNodePicker(logger, nil).SelectNodes(second), three times, printing the node's status after each call.

Observed, on every pass:

pass1: introspect=waiting  select=blocked
pass2: introspect=waiting  select=blocked
pass3: introspect=waiting  select=blocked

Expected: the node settles on one status and stops producing writes.

To see the widening specifically, add enough healthy filler nodes ahead of the tainted one to consume the batch. On 4755b301 the tainted node then stays waiting on every pass; on a9662b0c it still flips to blocked.

Additional context

The user-visible symptom beyond the write traffic is that the NodeWright carries a TaintNotTolerable condition naming the node while .status.nodeStatus for that node alternates, so tooling reading per-node status sees it flapping.

Raised as follow-up to the review on #586; that PR is otherwise good and the churn is not its doing.

[efegokdemir]

/assign