fix(ci): retry GKE UAT bringup on transient node-pool timeout - #2066
Conversation
The GKE actuator hardcodes a 30m node-pool create timeout, but a slow GPU pool can take ~35m; Terraform fails the apply even though GKE finishes and reports the pool RUNNING moments later (run 30965268896). On daytime-up / skip_delete runs that false failure strands a healthy cluster and its whole VPC network group, leaking the GPU node and contributing to NETWORKS quota exhaustion. Wrap Bringup Infra in a bounded 2-attempt retry: terraform apply is idempotent, so a second attempt refreshes state, waits out the still-completing (or already-healthy) pool, and converges. Still fails the step if both attempts fail, so a genuinely-stuck provision surfaces and (on nightly) teardown runs. The durable fix is an upstream node-pool timeout bump 30m->60m (mirroring the AKS actuator's mchmarny/cluster#31), tracked in NVIDIA#2065. Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe GCP Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/uat-gcp.yaml:
- Around line 471-486: Increase the workflow job timeout setting associated with
the documented job budget to at least 380 minutes, accounting for the
two-attempt Bringup Infra loop and preserving sufficient time for the always-run
Destroy Cluster teardown. Keep the retry behavior in the Bringup Infra step
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2fd5a1fc-64e0-4846-9c5c-052457905c27
📒 Files selected for processing (1)
.github/workflows/uat-gcp.yaml
| brought_up=false | ||
| for attempt in 1 2; do | ||
| echo "Bringup attempt ${attempt}..." | ||
| if /usr/bin/docker run \ | ||
| -e CONFIG_CONTENT="$(base64 < "$CLUSTER_CONFIG")" \ | ||
| -e AUTO_APPROVE=true \ | ||
| -e KEY_CONTENT="$(base64 < ${{ steps.auth.outputs.credentials_file_path }})" \ | ||
| ${{ env.GKE_ACTUATOR_IMAGE }} apply; then | ||
| brought_up=true | ||
| break | ||
| fi | ||
| if [ "${attempt}" -lt 2 ]; then | ||
| echo "Bringup attempt ${attempt} failed; retrying in 60s (a transient node-pool state often self-heals, and apply is idempotent)..." | ||
| sleep 60 | ||
| fi | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/uat-gcp.yaml | head -100Repository: NVIDIA/aicr
Length of output: 5524
🏁 Script executed:
sed -n '70,90p' .github/workflows/uat-gcp.yamlRepository: NVIDIA/aicr
Length of output: 1289
🏁 Script executed:
sed -n '471,486p' .github/workflows/uat-gcp.yamlRepository: NVIDIA/aicr
Length of output: 856
🏁 Script executed:
# Check for always() blocks and teardown steps
rg "always\(\)" .github/workflows/uat-gcp.yaml -A 5Repository: NVIDIA/aicr
Length of output: 3091
🏁 Script executed:
# Look at the full job structure to find timeout and all steps
ast-grep outline .github/workflows/uat-gcp.yamlRepository: NVIDIA/aicr
Length of output: 194
🏁 Script executed:
# Find the step name and context for the bringup loop
sed -n '450,490p' .github/workflows/uat-gcp.yamlRepository: NVIDIA/aicr
Length of output: 2426
🏁 Script executed:
# Look for any step with "Bringup" or similar in its name
rg -i "bringup|name:" .github/workflows/uat-gcp.yaml | head -30Repository: NVIDIA/aicr
Length of output: 1589
🏁 Script executed:
# Get a broader view around the provisioning section (lines 400-500)
sed -n '400,500p' .github/workflows/uat-gcp.yamlRepository: NVIDIA/aicr
Length of output: 5159
Increase job timeout to account for the two-attempt Bringup Infra retry.
The Bringup Infra step (line 452, steps.infra) runs the GKE actuator apply in a loop with up to two attempts separated by a 60-second sleep. Each attempt enforces a 30-minute node-pool create timeout. In the worst case, both attempts execute: ~30 minutes + 60 seconds + ~30 minutes = ~60 minutes for the Bringup Infra step alone.
The job budget comment (lines 72–80) documents an existing ~268-minute allocation (~30 minutes for GKE provisioning + other setup + UAT phases + debug collection + ~40 minutes teardown). Adding a ~60-minute Bringup retry path yields approximately 328 minutes, which exceeds the current 280-minute timeout.
When a job timeout expires, pending always() steps do not run. The Destroy Cluster teardown runs under always() conditions and requires ~40 minutes. If the timeout fires before teardown completes, the cluster and its VPC network group leak, stranding the GPU node and consuming NETWORKS quota.
Either increase the timeout to guarantee teardown headroom (suggested: 380 minutes minimum), or constrain the Bringup retry loop to one final attempt while preserving idempotency elsewhere.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/uat-gcp.yaml around lines 471 - 486, Increase the workflow
job timeout setting associated with the documented job budget to at least 380
minutes, accounting for the two-attempt Bringup Infra loop and preserving
sufficient time for the always-run Destroy Cluster teardown. Keep the retry
behavior in the Bringup Infra step unchanged.
|
Two follow-up defects on main from this PR. The budget comment is stale and the timeout cap is insufficient against the now-pinned v0.5.17 actuator. v0.5.17 widens the Terraform node-pool create/update timeout from 30m to 60m, but the retry comment still describes the old 30m cap and refers to a "pending upstream timeout bump" that has already landed. On the fast-recovery path (first apply observed at ~49m, second apply quick), the retry pushes the total to approximately 285m — already past the unchanged 280m cap. If the second apply approaches the new 60m timeout, the total reaches ~353m, well past the proposed 315m. The budget comment and The retry also has a false-green path on a retained ERROR node pool. Terraform provider Google v7.35 records the pool ID before creation and retains it when the pool reaches ERROR state; a subsequent apply produces a no-change plan and exits 0 while the pool is broken. |
|
uat-gcp.yaml:457–470 (retry comment) · uat-gcp.yaml:72–82 (budget comment) · uat-gcp.yaml:472–491 (retry loop) Stale retry comment (lines 457–470). The comment describes the old v0.5.16 actuator behavior: "30m node-pool create timeout" and "durable fix is an upstream node-pool timeout bump 30m→60m ... tracked in #2065." Both are stale. Main now pins v0.5.17, which already widens the Terraform node-pool create/update timeout to 60m. The "pending upstream fix" has landed. Stale budget (lines 72–82). The budget comment sizes the job for a single ~30m bringup attempt. With v0.5.17's 60m timeouts, the fast-recovery retry path reaches ~285m (first apply observed at ~49m, plus sleep, plus quick second apply, plus full UAT), exceeding the 280m cap. A worst-case second apply approaching 60m pushes the total to ~353m. The teardown MUST fit in this budget — a job-level timeout cancels pending False-green on retained ERROR pool (lines 472–491). |
#2066 added a second bringup attempt to uat-gcp without raising the job timeout, which was budgeted for one. The step carries no step-level timeout, and the failure it retries is the actuator's own hardcoded 30m node-pool timeout — so the first attempt burns the full 30m before failing, plus a 60s backoff. That is ~31m on top of the ~268m the comment budgets, i.e. ~299m against a 280m cap. The consequence is not a slow run. As the same comment notes, a job-level timeout cancels pending always() steps, so tripping the cap skips Destroy Cluster and leaks the GPU node and its VPC network group — precisely what #2066 was written to prevent. It only bites when the first attempt burns its full timeout AND the retry succeeds AND the run is otherwise near budget, but that is the exact path a retry is supposed to rescue. Raise the cap to 320, leaving ~21m over the worst case, and record the arithmetic so the next change to either number has the reasoning in front of it. The comment also warns against reducing the cap on the strength of #2065: raising the actuator's node-pool timeout 30m->60m makes a single attempt cost up to ~30m more, so the base budget rises to ~298 even if the retry never fires. That change trades a frequent retry for a longer first attempt; it does not free budget. uat-aws and uat-azure are unaffected: their bringup runs a single apply, and their retry loops are short API-call retries the existing headroom already absorbs. Signed-off-by: Brian Lockwood <lockwobr@gmail.com>
Summary
Wrap the GCP UAT Bringup Infra step in a bounded 2-attempt retry so a transient node-pool provisioning timeout no longer fails the run (and strands a healthy cluster).
Motivation / Context
The GKE actuator (
ghcr.io/mchmarny/cluster/gke) hardcodes a 30-minutecreatetimeout ongoogle_container_node_pool(terraform/compute.tf). A slow GPU pool can take ~35 min: Terraform times out and fails theapplyeven though GKE finishes and reports the poolRUNNINGmoments later. On run 30965268896 thecpu-workerpool did exactly this — and because it was a held/manual run, teardown was skipped and the healthy cluster + its full VPC network group (1 cluster VPC + 8gpu-nicVPCs + router + NAT + firewall + subnets) leaked, contributing to theNETWORKSquota (50) exhaustion that blocked later runs.terraform applyis idempotent, so a second attempt refreshes state, waits out the still-completing (or already-healthy) pool, and converges. Bounded to 2 attempts so a genuinely-stuck provision still fails fast and surfaces (and, on nightly, teardown still runs).Fixes: N/A (mitigates)
Related: #2065 (upstream node-pool timeout bump 30m→60m — the durable fix)
Type of Change
Component(s) Affected
.github/workflows/uat-gcp.yaml)Implementation Notes
Destroy Clusterretry pattern (docker runinside anif, which does not tripset -e, plus an explicit post-loop guard so a real failure stillexit 1s).aicr-<run_id>groups with no live workflow run) is tracked separately.Testing
Risk Assessment
Rollout notes: N/A — CI-only.
Checklist
make lint— yamllint/actionlint clean on the edited step)git commit -S)