CI: retry EC2 runner start with backoff on capacity failures - #758
Closed
harrism wants to merge 1 commit into
Closed
CI: retry EC2 runner start with backoff on capacity failures#758harrism wants to merge 1 commit into
harrism wants to merge 1 commit into
Conversation
Multi-availability-zone configuration makes the runner action walk every AZ
before giving up, but that walk takes seconds. When a whole region is short of
an instance type -- g6.xlarge shortages across all three us-east-2 zones are
happening now -- every zone fails in one burst and the job dies:
Attempting to start EC2 instance using 3 availability zone configuration(s)
Trying availability zone configuration 1/3 ... 2/3 ... 3/3
All availability zone configurations failed
Capacity usually frees within minutes, so retry the whole AZ walk. The action
has no launch retry of its own in either v2.4.3 or v2.6.1;
startup-retry-interval-seconds only governs runner registration after an
instance already exists.
Expand each start step into three attempts with 90s then 180s backoff, each
continue-on-error so a failure falls through to the next, and a final step that
fails only if all three found no capacity. Jobs that expose the runner label as
a job output now select it with `||` across the three attempt ids; jobs that
pass an explicit label input need no change there.
Deliberately inline rather than a shared reusable workflow: fvdb-core's
check_runner_token_policy.py forbids passing EC2_RUNNER_TOKEN through a
reusable-workflow secrets block (rule 2) and forbids `uses: ./...` in any job
that references the token (rule 4). Inline expansion keeps every occurrence as
the exact `github-token: ${{ secrets.<TOKEN> }}` action input, and the same
shape is used in both repositories for consistency.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
Contributor
Author
|
Superseded. The inline retry ladder was ~30 lines of near-identical YAML repeated at nine call sites, which can drift independently — so it is now a reusable workflow instead:
Retry semantics are unchanged from this PR: three attempts, 90s then 180s backoff. |
harrism
added a commit
that referenced
this pull request
Sep 3, 2026
…ckoff (#760) Replaces #758. CI keeps failing because us-east-2 has no capacity for the requested instance type at the moment the runner is requested. `machulav/ec2-github-runner` sweeps the availability zones in `availability-zones-config` within one attempt, but it never retries the sweep, so a transient region-wide shortage fails the whole run. Retrying twice, 90s and then 180s later, turns most of those into a slow pass instead of a red build. #758 inlined that retry ladder at every call site. That is ~30 lines of near-identical YAML in nine places that can drift independently, which is why this replaces it: the ladder goes in a reusable workflow instead. ## What changed Two new local reusable workflows hold the retry policy, the pinned action SHA, the AWS OIDC step and the teardown: * `.github/workflows/start-ec2-runner.yml` * `.github/workflows/stop-ec2-runner.yml` `tests.yml`, `cu130.yml`, `cu132.yml`, `publish.yml` and `nightly-publish.yml` now call them — nine start jobs and nine stop jobs. **401 lines of duplicated YAML removed, 171 added.** The reusable workflows absorb both call shapes the repo already had: * `runner-label` / `aws-resource-tags` for the matrix fan-outs in `publish.yml` and `nightly-publish.yml`, which need a predictable label because they find the instance again by tag at teardown. * `stagger-seconds`, replacing the hand-rolled `RANDOM % 16` sleep those same matrix jobs used to spread out runner registrations. `stop-ec2-runner.yml` takes the instance id directly when the caller has one from a start job's output, and otherwise resolves it from the `RunnerLabel` tag — the lookup the publish workflows had to do inline, because a matrix job has no single output to read. ## Two details worth a reviewer's eye * `aws-resource-tags` defaults to `'[]'`, not `''`. The pinned action does a bare `JSON.parse()` on that input ([`src/config.js`](https://github.com/machulav/ec2-github-runner/blob/343a1b2ae682e681c3cec9a235d882da17ff04ef/src/config.js#L39)) with no fallback, so an empty string throws and would break every caller that does not set tags. An empty `label`, by contrast, is explicitly fine — the action generates a unique one. * Retry semantics are unchanged from #758: only one attempt can succeed, since each is skipped unless every earlier one failed, so at most one of the three step outputs is non-empty and `||` picks it. ## Testing * `check_runner_token_policy.py` (with #759 applied): OK on all 17 workflow files. * Policy unit tests: 26 passed. * `actionlint` 1.7.7: clean. `zizmor` 1.26.1 `--persona=regular`: no findings. * Parsed every reusable-workflow caller job to confirm none carries a key incompatible with `uses:` (`runs-on`, `steps`, `env`, `container`, …). Because CI runs on `pull_request_target` from the base branch, this change cannot exercise itself on its own PR — the first real run of the new provisioning path is the one after merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Update: retry-path fix folded in #759 has merged, so the Workflow Security gate now passes here. While confirming the ladder had not yet fired in production (it hasn't — every start since the merge succeeded on attempt 1), I audited the never-executed path and found a defect, now fixed in `c90f2d7`: The action publishes its outputs as soon as the instance launches, **before** waiting for the runner to register. So a "no capacity" failure leaves outputs empty, but a *registration* failure leaves them populated. Selecting outputs with `a1 || a2 || a3` takes the first **non-empty** value rather than the **successful** one — which in that second case hands back a dead label (dependent job queues until timeout instead of failing fast) and the wrong instance id (the retry's instance leaks). Fixed by selecting on `outcome`, and by stopping the instance a failed attempt left running before retrying. Verified against all five outcome combinations. Same fix for frc is openvdb/fvdb-reality-capture#330. Not a regression from this refactor — the inline version in #758 had the same `||` chain. The refactor is why the fix is one file rather than nine. --------- Signed-off-by: Mark Harris <mharris@nvidia.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Multi-AZ configuration makes the runner action walk every availability zone before giving up — but that walk takes seconds. When a whole region is short of an instance type, every zone fails in one burst and the job dies:
g6.xlargeis currently unavailable across all three us-east-2 zones, so multi-AZ alone doesn't help. Capacity typically frees within minutes.The action has no launch retry of its own — in either
v2.4.3orv2.6.1,startup-retry-interval-secondsonly governs runner registration after an instance already exists.Change
Each start step becomes three attempts with 90s then 180s backoff:
continue-on-error: true, so a failure falls through to the next||across the three attempt ids; jobs passing an explicitlabel:input need no changeWhy inline rather than a shared reusable workflow
I first built this as a reusable workflow (much DRYer), then found fvdb-core's
check_runner_token_policy.pyforbids exactly that:Calling a local reusable workflow is
uses: ./...and passes the token viasecrets:— two violations, and the policy anticipates the pattern by name. Inline expansion keeps every occurrence as the exactgithub-token: ${{ secrets.<TOKEN> }}action input. The same shape is used in both repos for consistency, even though only fvdb-core enforces the policy.Testing
CI provisioning can't be fully proven without a capacity failure to retry against, but everything statically checkable was verified:
continue-on-error||-joined outputs or an explicitlabel:inputmode: stopsteps untouchedcheck_runner_token_policy.pypasses (✅ EC2 runner token policy: OK)Note the failing GPU checks on this PR are the capacity outage itself, not a defect here; and for
pull_request_targetworkflows these changes only take effect after merge.🤖 Generated with Claude Code