Repository navigation
Reassert GHCR auth re-runs the whole fenced transaction (incl. a kustomize-controller restart) when it has nothing to change #3039
Description
🤖 Generated by the Agentic Engineer
Evidence
🔑 Reassert Flux and consumer GHCR pull credential is the largest step in the prod deploy. Measured across recent successful merge_group deploys of ci.yaml:
| Deploy | 🚀 Deploy to Production |
Reassert step |
|---|---|---|
| 2026-08-09 19:22Z (run 31331452300) | 36.5 min | 1974 s (90%) |
| 2026-08-08 15:21Z (run 31264201837) | 4.6 min | 62 s (22%) |
| 2026-08-08 21:28Z (run 31279357884) | 4.8 min | 65 s (23%) |
Typical deploy is 4.4–5.7 min. Full step breakdown of the 4.6 min run:
67 s 🔑 Stage Flux and consumer GHCR pull credential
62 s 🔑 Reassert Flux and consumer GHCR pull credential
46 s 🔁 Trigger Flux reconciliation
38 s 🛡️ Suspend autoscaling for gated Cilium rollout
13 s 🔍 Verify the published evidence before promoting
2 s 🔎 Verify Flux GHCR pull credential after publish <-- --check-only
0 s 🔄 Update cluster (Talos machine config) <-- gate-suppressed
Stage + Reassert are ~129 s of a ~276 s deploy — 47% — and both run the same scripts/refresh-flux-ghcr-auth.sh with no arguments.
The 36.5 min run is a separate, non-recurring cause and not a defect. With the Cilium gate released, that deploy performed the rolling Talos v1.13.7 upgrade that two weeks of gate-suppressed machine-config sync had queued up — exactly what #3035 predicted. The reassert step absorbed the serial per-node reboot wait (~4–5 min × 6 nodes). That serialization is correct and must not be parallelized. This issue is about the 62 s steady-state floor, not that run.
Root cause
In the steady state the reassert has nothing to do — its own log says secret/variables-base patched (no change) and secret/ksail-registry-credentials patched (no change). It pays the cost anyway, because the full (non---check-only) path unconditionally runs the fenced transaction:
pause_flux_policy_parent
pause_flux_policy_handoff # -> restart_flux_kustomize_controller_for_handoff (line 3483)
stage_fanout_before_talos
resume_flux_policy_handoff
resume_flux_policy_parent
pause_flux_policy_handoff restarts the kustomize-controller Deployment and waits for rollout status (2 min timeout). That fence is correct — Flux documents that suspension does not stop an already-started execution — but it means every prod deploy restarts the GitOps engine twice, once in Stage and once in Reassert, plus the per-namespace fan-out existence checks and lease acquisition each time.
Why the obvious fix is wrong
"Run --check-only first, escalate on failure" does not work, and should not be attempted: --check-only exits at line 2649, immediately after verify_ghcr_pull_credential, which only proves the SOPS credential can pull the required GHCR packages from the runner. It never inspects the Kubernetes fan-out or Talos node state. A partial ksail cluster update — precisely what the reassert exists to repair — leaves the SOPS credential perfectly valid, so the check would pass and the drift would never be repaired. That would delete the safety net while looking like an optimization.
Expected
A read-only cluster-state verification that runs before any fence, and escalates to the full transaction only on drift. Unlike --check-only it must cover what the reassert actually repairs: the fan-out consumer secrets (verify_consumer_secret, line 309) and the node-level GHCR_PULL_VERIFIED_REVISION_ANNOTATION / ..._IMAGE_ANNOTATION values. Read-only, so it needs no policy pause and no controller restart.
Reassert then becomes: verify → if clean, exit; if drifted, run the existing fenced transaction unchanged.
Acceptance criteria
- A no-drift reassert completes without pausing Flux policy, without restarting
kustomize-controller, and without acquiring the sync lease. - A reassert that detects fan-out or node-annotation drift still runs the current fenced transaction unchanged.
- A test proves the escalation fires for a partially-applied Talos machine config — the case
--check-onlycannot see — so this cannot regress into the rejected design above. - Deploy-step timing recorded before/after in the PR body.
Expected saving ~60 s per deploy (~22%) plus one fewer GitOps-engine restart per deploy. Rough size: M. The 3751-line bash script is also well past the "bash first, Go when it grows" threshold; that migration is out of scope here.
Metadata
Metadata
Assignees
Labels
Type
Fields
Projects
- Status
Activity
coderabbitai commentedon Aug 9, 2026
🔗 Related PRs
#2625 - fix(deploy): synchronize production GHCR pull auth [merged]
#2633 - fix(deploy): verify incoming image on Talos nodes [merged]
#2635 - fix(deploy): reboot stale nodes so containerd adopts refreshed GHCR auth [merged]
#2793 - feat(cilium): activate homogeneous device selection [merged]
#2930 - feat(ci): verify published evidence before promoting the prod digest [merged]
📝 Issue Planner
Check the box below or use the
@coderabbitai plancommand to generate an implementation plan and prompts that you can use with your favorite coding assistant.🧪 Issue enrichment is currently in open beta.
You can configure auto-planning by selecting labels in the issue_enrichment configuration.
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:💬 Have feedback or questions? Drop into our discord!
devantler commentedon Aug 9, 2026
Re-scoped: make the fence LAZY, do not probe around it
#3041 attempted the shape this issue described — a read-only probe before the fence — and it was closed after three P1 findings, all valid. The premise was wrong in a way worth recording so the next attempt does not repeat it.
Why a pre-probe cannot work. The transaction is not a checklist, it is a convergence loop:
stage_fanout_before_talosre-proves every consumer and requires a whole node pass with no mutations before root cutover, because a node can appear mid-roll. Three existing tests pin that. Any probe cheap enough to be worth taking is a single point-in-time read, and therefore weaker than the thing it skips. Two further gaps followed from the same error: the probe checked both ends of the SOPS →variables-base→seed-ghcr→ OpenBao → ExternalSecret →ghcr-authchain and neither middle (so a post-raft-restore stale seed reads clean), and it read no Flux Kustomization at all (so a retry after a failed fence-resume could exit "converged" while Flux stayed suspended).The corrected design. Keep the transaction and its convergence loop untouched. Move the fence from before the first read to before the first write:
pause_flux_policy_parent/pause_flux_policy_handoff— and thekustomize-controllerrestart they carry — are acquired lazily, at the first operation that actually mutates cluster state.EXITtrap already resumes only fences that were acquired (flux_policy_*_acquired), so the unacquired case needs no new cleanup path.This is a larger change than the probe, but it is the one that is actually safe, and it keeps the saving: the fence is the cost, and runs that write nothing stop paying it.
Reusable from the closed branch (
claude/ghcr-auth-reassert-fast-path-3039): the two-run fixture pattern —runHelperthenrunHelperPreservingClusterState, which is the production stage/reassert shape and is what makes "the second run must not restart the controller" expressible — plus theflux-controller-restart-countassertion, and the consumer-secret fake falling back to the persistedvariables-basevalue so an already-converged cluster can be represented at all.Priority note. Measured saving is ~60 s of a deploy that currently takes ~52 min. #3042 is the one that matters for deploy time; this is worth doing after it, when the reassert is ~60 s of a ~5 min deploy.
devantler commentedon Aug 9, 2026
Correction to the note above: the end-of-run branch sweep reaped
claude/ghcr-auth-reassert-fast-path-3039, since a closed PR whose head SHA still matches is a delete candidate. That was the right call by the sweep, but it makes my "reusable from the branch" pointer wrong.The reusable material is on #3041 itself — a closed PR keeps its commits and diff, so
Files changedthere still carries the two-run fixture pattern, theflux-controller-restart-countassertion, and the consumer-secret fake fallback. Nothing is lost; the pointer just changes from the branch to the PR.devantler commentedon Sep 23, 2026
Fresh production evidence (2026-09-23): the existing unconditional GHCR-auth fence restarted kustomize-controller at 07:01-07:03 UTC; the deployment annotation was subsequently patched again at 07:08 UTC by kubectl-patch. During the first rollout, aws/Kustomization/aws emitted a HealthCheckFailed warning at 07:03:06 UTC because its status PATCH was canceled with the terminating controller context. It recovered to Ready=True and Healthy=True on the unchanged AWS OCI digest at 07:07:12 UTC; both controller replicas became Ready with no restarts. This is the recurring noisy outcome of the redundant controller handoff described here, not AWS workload drift. Preserve the fence on a real write, but make it lazy as this issue specifies; do not suppress the warning.
devantler commentedon Sep 23, 2026
The new draft #4080 pins the RED no-drift behavior and two safety counterexamples (partial Talos node config and stale OpenBao seed after a raft restore). The no-drift second run currently restarts kustomize-controller (count 1→2), increments the Lease resourceVersion (23→37), and writes policy/Secrets/force-sync annotations.
Implementation constraint discovered while tracing the full loop:
stage_image_verification_webhook_budget,patch_variables_base,force_sync_resource, andpatch_root_secretall write unconditionally. Moving the current fence calls beneath these functions would leave real writes unfenced; skippingforce_sync_resourcebecause Kubernetes Secrets look current repeats the P1 stale-OpenBao failure on closed #3041. The live PushSecret status exposes refresh time and syncedResourceVersion, but not a proof of the present remote OpenBao value after a raft restore. Its source/consumer status is therefore insufficient to authorize a no-write path.The safe design must keep the existing convergence loop and either obtain a narrowly scoped, read-only proof of the OpenBao seed before omitting force-sync, or retain the force-sync and its write fence. Also check for residual Flux policy fences and recovery journals before any no-lease exit. I will not suppress the production warning or weaken these checks to make the counters disappear. Draft #4080 remains RED while this is resolved.