fix(control-plane): keep provisioning gateways out of Degraded while pods start

[rh-amarin]

Summary

Supersedes #261 (credit to @RunItBack1127 for the original diagnosis and approach). #261 no longer applies cleanly: #276 moved the health loop from DeploymentReadiness to the rollout-aware ObserveGatewayRollout. This PR redoes the fix on the current code.

The bug

A newly created gateway could briefly show Degraded while it was still being provisioned, then flip to Running: Provisioning → Degraded → Running.

ObserveGatewayRollout only reports rollingOut=true until the Deployment's updated replica exists. After that, while the pod is still Pending, pulling its image, or starting, it reports rollingOut=false, ready=false. The health loop's plain !ready branch then stamps Degraded without checking whether the gateway ever reached Running. This happens well inside the provisioning path's own 2-minute readiness window (WaitForGatewayReady). It also contradicts the spec, which says the phase SHALL be Provisioning while the Deployment is applied but not yet Ready.

Reproduction on main (939a60e, kind in a Lima VM)

The node was cordoned for 75s once the certgen hook pod was bound, so the gateway pod stayed Pending:

+0s   created gateway                  phase=Provisioning
+5s   Deployment created, pod Pending  phase=Provisioning  (updated=1 available=0)
+33s  health tick                      phase=Degraded  status='0/1 updated replicas available'
+77s  node uncordoned, pod starts      phase=Degraded
+81s  pod Ready                        phase=Running   status='Healthy'

INFO gateway health: 3JzuBKaUbw8uDzQaZGxZi92QSQz Provisioning -> Degraded (0/1 updated replicas available)

This is easy to miss in the standard e2e run: with the image already cached, the pod becomes Ready about 9s after the gateway is created, which is shorter than one 30s health tick. Slow image pulls or scheduling hit it on real clusters.

The fix

• Add a deployment-readiness grace window to the health loop, the same way the existing route-readiness window works (evaluateDeploymentReadiness, markDeploymentNotReady, clearDeploymentTimer).
• While the gateway is Provisioning and its Deployment is not Ready (with no rollout in progress), the gateway stays Provisioning and status gets the observed reason.
• If the Deployment is still not Ready when the window elapses, the gateway moves to Degraded with status deployment not ready after <window>: <reason>.
• The window is set by GATEWAY_DEPLOYMENT_READY_TIMEOUT (default 10m). It normally never fires, because the provisioning path sets Degraded itself after its own 2-minute window. It covers the case where that provisioning pass never finishes, e.g. the control plane restarts mid-provision.
• Running and Degraded gateways that lose readiness still go to Degraded immediately. Steady-state behaviour doesn't change.
• Spec: added the scenario "Gateway pod still starting during provisioning" to openshell-gateway-health.spec.md.

Test plan

• go test ./internal/reconciler/ passes
• New TestReconcileGatewayHealth_ProvisioningPodNotYetAvailableStaysProvisioning fails on main with the same symptom as the VM run (phase updated to Degraded (status "0/1 updated replicas available")) and passes with this change
• Unit tests for the window: within the window, after it elapses, Running/Degraded degrade immediately, and parsing of the env override
• make check
• Rerun the Lima reproduction on this branch and expect no Degraded stamp

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: d28c0cda-f682-4fb8-b0e3-d5438d8e7cec

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[hypershell-delivery]

Amber review

Status: Complete

View the submitted review.

[hypershell-delivery]

Verdict

This is a well-scoped, correct fix: it adds a deployment-readiness grace window that keeps a first-time provisioning gateway at Provisioning while its pod is pending/pulling/starting, and only escalates to Degraded immediately for gateways that had already reached Running. The change mirrors the pre-existing route-readiness window precisely, ships a spec scenario, and is covered by focused tests for the within-window, beyond-window, immediate-degrade, and env-override paths.

What I verified

• State-machine correctness (health.go reconcileGatewayHealth -> evaluateDeploymentReadiness): the plain !ready branch now delegates to evaluateDeploymentReadiness, which extends the grace window only when currentPhase == Provisioning; Running (lost readiness) and Degraded gateways degrade immediately with no window. This matches the PR intent and the new spec scenario.
• Timer lifecycle parity: deploymentNotReadySince is created (markDeploymentNotReady), read, and cleared (clearDeploymentTimer) using the same pattern as the pre-existing routeNotReadySince map. It is cleared on the route-eval branch, the default (Running) branch, on window-elapse, and for non-provisioning phases. No new class of unbounded map growth beyond what the route timer already carries.
• Config parsing (deploymentReadyTimeout()): validates GATEWAY_DEPLOYMENT_READY_TIMEOUT, rejects non-positive/unparseable values, logs a WARN with %q (no injection vector; env is operator-controlled), and falls back to the 10m default. Config stays separate from code.
• Error handling / conventions: no panic(), no secrets in logs or reason strings, os/log/fmt already imported, conventional commit message, spec updated alongside the behavior change.
• Test Diff Scrutiny: the only edits to pre-existing tests are additive struct-literal fields in the newHealthRec / newSettledHealthRec helpers (deploymentReadyTimeout plus the new map). No existing assertion was flipped from accept->reject or optional->required; every behavioral test is net-new. No hidden contract change.

I could not run go build/go test (no Go toolchain in this environment), so the compile/test confirmation relies on static review plus the PR's stated make check / go test ./internal/reconciler/ results.

Cross-PR coordination

An open pull request (#261) proposes a competing fix for the same "Provisioning -> Degraded -> Running" flap and edits the identical three files (health.go, health_test.go, openshell-gateway-health.spec.md), also introducing a ~10m readiness-timeout mechanism. This PR's description explicitly states it supersedes #261 (rebased onto the ObserveGatewayRollout health path). These are duplicate solutions to one bug: maintainers must decide which one to take and close the other, rather than merge both. That is the one cross-PR item requiring a decision.

Previous concerns

• Prior Minor - !ready && rollingOut branch does not clear the deployment-not-ready timer: Still present. The case !ready && rollingOut arm (health.go ~L365-L373) still returns early without calling clearDeploymentTimer, so a start time stamped while provisioning persists across a subsequent roll. This remains the intended backstop semantics (the window measures total provisioning-not-ready time, and the 10m default is generous), so it is non-blocking - noted again only so the choice stays explicit.

There were no other prior Amber findings and no open inline threads requiring a reply.

---

Findings Summary (ordered by severity, highest first):

1. [Minor] !ready && rollingOut branch does not clear the deployment-not-ready timer; retained start time is likely intended backstop behavior but worth an explicit note - Reconciliation (health.go ~L365)

Convention Checklist:

Convention	Result
No panic() in production code	Pass
Errors wrapped / status reason preserved	Pass
No secrets in logs or responses	Pass
Input validated (env value parsed/validated)	Pass
Reconcile pattern (not create-or-skip)	Pass
Config separate from code (env override)	Pass
Status updated on error/degrade paths	Pass
Proper context propagation	Pass
Test Diff Scrutiny (no flipped assertions)	Pass
Spec updated for behavior change	Pass
Conventional commit message	Pass