Skip to content

Fix: gate resource convergence on TLS issuance terminal state - #65

Merged
ashishmax31 merged 1 commit into
mainfrom
fix-converge-tls-gate
Aug 6, 2026
Merged

Fix: gate resource convergence on TLS issuance terminal state#65
ashishmax31 merged 1 commit into
mainfrom
fix-converge-tls-gate

Conversation

@ashishmax31

Copy link
Copy Markdown
Contributor

Symptom (seen hub-side)

The hub observed a live release with empty public URLs — it bit the preview-comment flow, which reads Status.ExternalAddress off a converged resource.

Root cause: ordering

svcReconciler already computes a TLS tri-state (certIssuanceStageIssuing / Ready / Unavailable) and uses it to decide what to publish:

  • Readyhttps:// address
  • Unavailable (cert-manager failure, or 2-minute grace period elapsed) → http:// fallback
  • Issuingpublish nothing, so a user is never handed an https URL serving Traefik's self-signed default

But that tri-state gated only URL population. deriveSummaryStatus sets the summary StackResourceConverged condition by mirroring StackResourceWorkloadConverged whenever no verdict was filed — so a resource whose pods had rolled out flipped Converged=True while ExternalAddress was still empty.

StackResourceConverged is the right thing to gate: internal/controller/stack/aggregate.go counts children by that condition, and its resourcesConverged result is what stamps Stack.Status.LastConverged (revision + releaseID) — which is exactly what the hub's pkg/worker/release/converge.go and models.release_live_status read to call a release live.

The gate

One call in svcReconciler.reconcile: the Issuing stage now files the existing retriable not-ready verdict, using the stage's own reason/message (CertificateIssuing). deriveNotReady then writes Converged=False. No new timing logic, no duplicated state.

It cannot wedge

The gate reuses certGracePeriod (2 minutes), measured from TLSConfigured's LastTransitionTime. Once it elapses classifyCertificate returns Unavailable — http:// fallback, no verdict, convergence proceeds. The pre-existing deferred requeue wakes the reconciler at expiry, so a silently stalled certificate (which produces no Certificate watch event) still unblocks on time rather than waiting for an unrelated event.

Only TLS-on-a-public-port resources are affected. No exposed port, no TLS on the port, or an unresolvable ClusterIssuer all yield a stage that is not Issuing, so they converge exactly as before.

Tests

internal/controller/stackresource/svc_reconciler_test.go, new Describe driving the full reconcile pass against a mocked client, each case seeded with WorkloadConverged=True so the gate is what decides:

  • pending cert inside grace → not converged, reason CertificateIssuing, no address, deferred requeue bounded by the grace period
  • cert issued → converged, https:// address
  • FailedIssuanceAttempts > 0 → converged, http:// address
  • grace period expired → converged, http:// address
  • no public/TLS ingress → unchanged (covered by the existing orchestration specs asserting no verdicts are filed)

Verification

  • go build ./..., go vet ./... clean
  • make test-unit: only pre-existing pkg/portcheck failures, which self-describe as environmental (they need a dial to 192.0.2.1 to hang; this network answers ICMP unreachable). Untouched by this change.
  • The new issuing-stage spec was confirmed to fail with the reportNotReady line removed and pass with it — it reproduces the bug.
  • make lint not run: the pinned golangci-lint v1.54.2 fails to typecheck this Go version repo-wide (pre-existing, unrelated to this diff).

A TLS port publishes no external address while its certificate is still
issuing, but nothing held Converged back — the summary mirrored the
workload's own convergence. The hub saw a live release with empty public
URLs.

The svc reconciler now files a not-ready verdict for the issuing stage,
so the same tri-state that gates URL population gates convergence. The
grace period still bounds it: a certificate that never resolves falls
back to http:// and converges.
@ashishmax31
ashishmax31 merged commit 0f565d7 into main Aug 6, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant