Skip to content

fix(ci): retry GKE UAT bringup on transient node-pool timeout - #2066

Merged
mchmarny merged 2 commits into
NVIDIA:mainfrom
njhensley:ci/uat-bringup-retry-transient-provisioning
Aug 5, 2026
Merged

mchmarny merged 2 commits into
NVIDIA:mainfrom
njhensley:ci/uat-bringup-retry-transient-provisioning

Conversation

@njhensley

Copy link
Copy Markdown
Member

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-minute create timeout on google_container_node_pool (terraform/compute.tf). A slow GPU pool can take ~35 min: Terraform times out and fails the apply even though GKE finishes and reports the pool RUNNING moments later. On run 30965268896 the cpu-worker pool 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 + 8 gpu-nic VPCs + router + NAT + firewall + subnets) leaked, contributing to the NETWORKS quota (50) exhaustion that blocked later runs.

terraform apply is 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

  • Bug fix (non-breaking change that fixes an issue)
  • Build/CI/tooling

Component(s) Affected

  • Other: UAT GCP workflow (.github/workflows/uat-gcp.yaml)

Implementation Notes

  • Mirrors the existing Destroy Cluster retry pattern (docker run inside an if, which does not trip set -e, plus an explicit post-loop guard so a real failure still exit 1s).
  • Scoped to GCP, where the 30m timeout and the evidence are. AKS already carries 60m worker-pool timeouts (fix(aks): widen worker-pool create/update timeouts to 60m mchmarny/cluster#31); EKS parity can follow if the same symptom appears there.
  • This reduces the orphan rate for one mode; the general backstop (a scheduled janitor that reaps orphaned aicr-<run_id> groups with no live workflow run) is tracked separately.

Testing

yamllint .github/workflows/uat-gcp.yaml    # clean
actionlint .github/workflows/uat-gcp.yaml  # no new findings (3 pre-existing info-level SC2016 in an unrelated step)

Risk Assessment

  • Low — Adds a bounded retry around an idempotent apply; worst case is one extra apply attempt (~within the 280m job budget). Easy to revert.

Rollout notes: N/A — CI-only.

Checklist

  • Linter passes (make lint — yamllint/actionlint clean on the edited step)
  • I did not skip/disable tests to make CI green
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S)
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>
@njhensley
njhensley requested a review from a team as a code owner August 5, 2026 16:28
@njhensley njhensley added the theme/ci-dx CI pipelines, developer experience, and build tooling label Aug 5, 2026
@njhensley njhensley self-assigned this Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 9c1d9cdb-2d93-4aa2-a8d1-41f5dd9f99a4

📥 Commits

Reviewing files that changed from the base of the PR and between cad5708 and 71e2922.

📒 Files selected for processing (1)
  • .github/workflows/uat-gcp.yaml

📝 Walkthrough

Walkthrough

The GCP Bringup Infra workflow step now performs up to two actuator apply attempts. It waits 60 seconds after the first failure. It explicitly fails the step after two unsuccessful attempts.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Suggested reviewers: mchmarny

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the GKE UAT bring-up retry for transient node-pool timeouts.
Description check ✅ Passed The description directly explains the retry behavior, motivation, scope, implementation, testing, and risk.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 9b180ac and cad5708.

📒 Files selected for processing (1)
  • .github/workflows/uat-gcp.yaml
Comment on lines +471 to +486
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/uat-gcp.yaml | head -100

Repository: NVIDIA/aicr

Length of output: 5524


🏁 Script executed:

sed -n '70,90p' .github/workflows/uat-gcp.yaml

Repository: NVIDIA/aicr

Length of output: 1289


🏁 Script executed:

sed -n '471,486p' .github/workflows/uat-gcp.yaml

Repository: NVIDIA/aicr

Length of output: 856


🏁 Script executed:

# Check for always() blocks and teardown steps
rg "always\(\)" .github/workflows/uat-gcp.yaml -A 5

Repository: 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.yaml

Repository: 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.yaml

Repository: 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 -30

Repository: 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.yaml

Repository: 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.
@mchmarny
mchmarny merged commit 00cb051 into NVIDIA:main Aug 5, 2026
2 of 6 checks passed
@yuanchen8911

Copy link
Copy Markdown
Contributor

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 timeout-minutes need to be recalculated against v0.5.17's actual timeouts, and the stale 30m/pending-fix language removed. The robust fix is independently budgeted teardown; if the single-job design stays, the budget must be recomputed.

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. brought_up=true is then set and kubectl get nodes passes (cluster API reachable, healthy nodes visible from other pools), so the run proceeds or holds without detecting the broken pool. On a skip_tests=true or daytime-up run this holds an unusable cluster indefinitely — the failure() teardown added by #2067 does not fire on a zero exit. Fix: verify every configured GKE node pool reports RUNNING after the actuator succeeds before setting brought_up=true.

@yuanchen8911

Copy link
Copy Markdown
Contributor

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 always() steps — so the budget needs to be recalculated or teardown moved to an independent workflow.

False-green on retained ERROR pool (lines 472–491). brought_up=true is set on any exit 0 from the actuator. The Connect step runs kubectl get nodes, which verifies only that the cluster API is reachable — not that every node pool is healthy. Terraform provider Google v7.35 retains a node pool in ERROR state and produces a no-change plan (exit 0) on a subsequent apply. On a skip_tests=true or daytime-up run, the step sets brought_up=true and the cluster is held with a broken pool. failure() is false, so #2067's teardown gate does not fire. Fix: after the actuator exits 0, verify every configured GKE node pool reports RUNNING before setting brought_up=true.

lockwobr added a commit that referenced this pull request Aug 5, 2026
#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci size/S theme/ci-dx CI pipelines, developer experience, and build tooling

3 participants