operator: batch checkpoint rebaselining is budgeted by cardinality, not identity — phantom batches and lost failures on membership churn

[ayuskauskas]

Summary

RebaselineBatchCheckpoints in the operator is both triggered and budgeted by ΔMatched — a scalar cardinality delta. A count cannot express identity churn, so three distinct cases still produce phantom batch progress or lose a failure. All three share one root cause and one fix.

This is not a regression. Before #589 there was no rebaseline mechanism at all and every arrival of an already-complete node was counted as batch progress. #589 is a strict improvement that happens not to cover these three cases. Filing separately so #589 can land on its narrower scope.

Symptoms

1. Net-zero identity churn bypasses rebaselining entirely.
evaluateCompletedBatches gates the call on previous.Matched != len(compartment.GetNodes()). If two pending nodes leave and two already-complete nodes join in the same reconcile, the count is unchanged, the branch is never entered, and EvaluateCurrentBatch sees deltaCompleted = +2 — a two-node successful batch for nodes that were never dispatched.

2. Net delta under-budgets gross churn.
remaining is |membershipDelta|. With 5 joins and 3 leaves, Matched moves by +2, so remaining = 2 against 5 actual joiners. Up to 3 carried-in completions escape absorption and land as progress.

3. The decreasing branch never consults previousNodeStatus, losing failures.
For membershipDelta < 0, RebaselineBatchCheckpoints absorbs purely on counts. If a compartment has checkpoint FailedNodes=1 (node-a Erroring), node-a departs, and node-b newly errors in the same reconcile, currentFailed stays 1, the (!increasing && delta >= 0) guard returns early, and EvaluateCurrentBatch computes deltaFailed = 0.

Symptom 3 is the one to weight heaviest: node-b's failure is never attributed to any batch, so ConsecutiveFailures never increments and failureThreshold / SafetyLimit can fail to trip. A rollout can keep marching through a failing fleet. On a cluster with an autoscaler or rolling node replacement, compartment membership changes routinely over the life of a long rollout, so this is not a rare path.

Root cause

Membership churn is being reconstructed from cardinality when what the accounting actually needs is identity: which specific nodes reached a terminal state under this reconcile's observation, versus which arrived already terminal.

Suggested direction

#589 introduces previousNodeStatus (a snapshot of Status.NodeStatus taken in IntrospectSkyhook before the IntrospectNode loop mutates it), which already carries the identity information needed. Once it exists, Matched and membershipDelta may not be needed for this purpose at all — the baseline can be set directly by identity:

CompletedNodes = previouslyCompleted   // nodes present now that were already terminal before this reconcile
FailedNodes    = previouslyFailed

run unconditionally rather than gated on a count change. EvaluateCurrentBatch's delta then becomes "terminal transitions observed this reconcile among current members," which resolves all three symptoms uniformly, and the existing negative-delta clamp in EvaluateCurrentBatch already covers pure departures.

This needs to carry forward the guard landing in #589 for the cases where previousNodeStatus is lossy: CleanupRemovedNodes prunes entries for nodes that leave the skyhook's node set, and UpdateSkyhookPauseStatus / IntrospectNode overwrite every entry with paused / disabled. A missing or placeholder entry has to be treated as "already terminal," not as "newly progressed."

Worth checking whether this can be covered by envtest in internal/controller and internal/wrapper without a chainsaw run — I believe it can, since it is all status arithmetic.

Notes

• Do not fold this into fix(operator): persist corrected batch checkpoints #589. That PR has been through three review rounds already and its current scope is correct.
• Any fix here should come with table entries for all three symptoms above; none of them is currently pinned by a test.

[efegokdemir]

/assign