fix(cruisecontrol): sequence CC rolls around scaling operations (fixes #301)

[amuraru]

Fixes #301.

Replaces the "flip capacity.json out of the pod-template hash" approach (#303): that flip drops the CC roll that loads a new broker's capacity into CruiseControl (CC reads capacity.json only at startup and estimates unknown brokers via a -1 default koperator doesn't generate), silently breaking upscale. This PR keeps the roll and sequences it — no dependency on capacity estimation or a -1 default.

The race

A broker add/remove regenerates capacity.json, which is hashed into the CC pod-template annotation and rolls the CC Deployment. That roll (KafkaClusterReconciler) and the add_broker/remove_broker op (CruiseControlOperationReconciler) are driven by two uncoordinated controllers, so an op can race the roll: the fresh CC pod loses its in-memory task and resets its metric-sampling window, stalling scaling. The op can even be created before the roll is triggered — KafkaClusterReconciler marks a new broker GracefulUpscaleRequired (which makes the task controller create add_broker) before, and independently of, the CC reconciler that regenerates capacity.json and patches the Deployment.

Removal is chicken-and-egg: marking a broker GracefulDownscaleRequired needs a CC-ready cc.BrokersWithState(), but in the pre-downscale window capacity.json is regenerated with a fallback entry for the departing broker, which rolls CC and keeps it un-ready — so the downscale is never marked and no remove_broker op is created. This stalled the KRaft removal e2e on master.

Fix — sequence the roll, keep exact per-broker capacity

1. Don't roll CC during removal (load-bearing). isBrokerRemovalPending (broker in status, dropped from spec) broadens the existing "reuse the deployed capacity.json" guard so CC isn't rolled in the pre-downscale window → stays ready → downscale marked → remove_broker created. mergeCapacityConfig reuses the deployed config but additively merges newly-joined brokers (user-provided capacity if the CR defines one, else generated), keeping departing entries verbatim — so a mixed add+remove still writes the new broker's capacity, and a pure removal returns the deployed config byte-for-byte (no spurious roll). While a downscale is actively running (a remove_broker task is in flight) the deployed config is reused verbatim with no merge — appending a concurrently-added broker would roll CC and kill that in-flight removal; the added broker's capacity is written by the next full regeneration once the downscale completes (its add_broker is deferred until then by the roll gate).
2. Gate CC ops on a Deployment that's safe to submit to (requeueIfCCDeploymentNotRolledOut). Every op except stop_execution is deferred while the CC Deployment is mid-rollout — a restart wipes the in-flight task, and a roll can be triggered by any hashed pod-template change (capacity.json, cruisecontrol.properties, clusterConfigs.json, log4j.properties), not just broker scaling. Because a capacity roll can also land just after an op is submitted, every gated op additionally defers while the pod-template capacity hash lags the current ConfigMap capacity.json hash; add_broker also requires that capacity.json to already contain the target broker id(s) — closing both the "op created before the roll is triggered" and "op submitted mid-roll" windows, so an add always runs against a CC that has loaded the new broker's exact capacity. Every check fails open when the Deployment/ConfigMap is absent or capacity isn't koperator-managed (never deadlocks); a wedged rollout (ProgressDeadlineExceeded) is logged at error level and backs off its requeue.

Asymmetric, so no deadlock: removal reuses capacity.json (no roll); an add still rolls CC and the op waits for that roll to settle. Correct for homogeneous and heterogeneous clusters (no estimation / -1 dependency).

Tests

• Unit: TestIsBrokerRemovalPending; TestGenerateCapacityConfigReuseAndMerge (verbatim reuse on pure removal, merge on add incl. a user-provided new broker, and an unparseable deployed config reused verbatim); TestCapacityConfigContainsBrokers; TestCapacityConfigHash; TestIsDeploymentRolling; TestDeploymentRolloutTimedOut; and TestRequeueIfCCDeploymentNotRolledOut, which drives the gate directly with fake Deployment/ConfigMap states (stop_execution never gated, rebalance/remove_disks deferred only while rolling, hash mismatch and add_broker-with-absent-capacity requeue, fail-open, execute). The full controllers/tests envtest suite passes.
• e2e testKRaftBrokerScaling: upscale 3→4 (add broker 103) then downscale 4→3, asserting exactly one add_broker then one remove_broker op, exact broker/controller ids (not just counts), capacity.json gaining then dropping broker 103, the CC pod-template capacity hash carrying that capacity.json, and controllers untouched. Samples omit a -1 universal-default capacityConfig so per-broker generation is actually exercised; the quiescent-CC gate counts queued and retry-pending ops as in-flight.

Verified locally: go build ./..., go vet -tags e2e ./..., golangci-lint (no new issues), unit tests, and the controllers/tests envtest suite pass.

🤖 Generated with Claude Code

[amuraru]

Discarding the CC roll-sequencing changes (#301 / 82088cc). After further investigation, the stall we were chasing was caused by a broker's only Service
being torn down while its removal was pending (fixed minimally in b22ab71), plus an e2e test that applied a full manifest instead of patching spec.Brokers.

The elaborate machinery in 82088cc — the CC-op roll gate, mergeCapacityConfig, and the IsDownscaleRunning retuning — targets a different mechanism (a
capacity.json change rolling the CC Deployment onto an in-flight op). That race is real but self-healing: a lost task surfaces as CompletedWithError, and
with the default ErrorPolicy: retry koperator automatically re-fires the operation after a short backoff. So the patch buys us avoidance of a bounded delay
and cleaner logs — not correctness — at the cost of ~2000 lines, a scaling-only capacity-merge path, and a gate that can defer operations forever if it
regresses. That's a poor trade for an optimization, so we're dropping it.

[amuraru]

Replaced by a much shorter fix: #318