fix(ci): replace racy staging-box wait with an atomic lock-slot semaphore - #197
Merged
Conversation
Matches the YYYYMMDD_<username>_<short_description> convention already used in ../ddev/CLAUDE.md, so branches created here follow the same org-wide standard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…hore
ci-wait-for-staging-box.sh polled `coder list` for a zero ci-bot workspace
count before creating a workspace -- a check-then-act race. Jobs come from
three workflow files and both self-hosted and GitHub-hosted runners, all
hitting the same remote Coder server, so multiple jobs could observe "0
workspaces" in the same poll window and all create at once. This happened
in production: three ci-bot workspaces ran concurrently on a box sized for
about two, and the job racing into that window failed its agent connection
("Agent doesn't exist with that id").
Replace it with a real semaphore: a new ci-lock template (no Docker/Sysbox,
just a no-op resource) provisions N fixed-name slot workspaces
(ci-slot-1..N). Coder enforces a unique workspace name per owner, so
claiming a slot via `coder create ci-slot-<i>` is a genuine atomic
compare-and-swap instead of a poll. Abandoned slots self-heal via a
staleness check in the acquire script and, as a backstop, the existing
ci-reap-staging.sh janitor (which already reaps any stale ci-bot workspace
by age, with no changes needed there).
See openspec/changes/add-ci-staging-lock/ for the full design and the
git-ref-based alternative that was considered and rejected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Was only mentioned in the OpenSpec design notes; operators need it in docs/admin/server-setup.md alongside the other CI repository variables to know it exists and how to tune it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
…h step All five PR #197 integration-test jobs failed after burning the full 45-minute retry budget: ci-lock had never been pushed to staging-coder.ddev.com, so every `coder create ci-slot-<i>` failed with "no template found", which the acquire script indistinguishably treated as "slot busy, keep retrying". Add a cheap `coder templates list` preflight so a missing template fails in under a second with an actionable message instead of after 45 minutes. Document the one-time `make push-all-templates` step this depends on in server-setup.md. Also pushed ci-lock (and refreshed the other three) to staging-coder.ddev.com directly and smoke-tested a real create/delete cycle against it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`coder templates list --output json` wraps template fields under `.Template.*` (unlike `coder list` for workspaces, which is flat) -- the previous commit's preflight checked `.name` instead of `.Template.name`, so it always reported ci-lock as missing even though it exists, failing every job immediately. Verified the fix directly against staging-coder.ddev.com: the corrected filter finds ci-lock, a live acquire/release cycle succeeds end to end, and the old filter's false negative is confirmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two real bugs surfaced by staging runs at CI_LOCK_SLOTS=3: 1. ci-reap-staging.sh archived every "unused" template version on each 15-minute sweep, with no age check. A job that pushes its version and then waits behind ci-acquire-staging-lock.sh for a free slot can have that version swept up as "unused" before it ever gets to create a workspace from it -- confirmed directly: the janitor ran mid-wait for two contrib jobs and both then failed with "Archived template versions cannot be used to make a workspace." Now enumerates versions itself and only archives ones older than VERSION_AGE_MINUTES (default 60, well above the lock's 45-minute worst-case wait). 2. The box turns out to tolerate many concurrent workspaces fine -- what it can't take is several hitting their heaviest startup work (Docker image builds, composer installs) at the same instant. Add STAGGER_SECONDS (default 90) to ci-acquire-staging-lock.sh: after claiming a slot, sleep out the remainder of that window since the most recently started sibling slot, so simultaneous starts get spread out even when slots are free. This lets CI_LOCK_SLOTS stay higher without recreating the original startup-burst failures. Both changes verified directly against staging-coder.ddev.com (age-gated dry-run confirmed against real version ages; stagger confirmed with two live acquire cycles). Co-Authored-By: Claude Sonnet 5 <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
1 participant
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.
Summary
ci-wait-for-staging-box.shserialized CI workspace creation on the shared staging box by pollingcoder listfor a zeroci-botworkspace count, then creating — a check-then-act race. Jobs come from three workflow files and both self-hosted and GitHub-hosted runners, all hitting the same remote Coder server, so multiple jobs could observe "0 workspaces" in the same ~30s poll window and all create at once.This happened in production: three
ci-botworkspaces ran concurrently on a box sized for about two, and the job racing into that window failed its agent connection ("Agent doesn't exist with that id" — see run 31230219543).ci-locktemplate (no Docker/Sysbox, just a no-op resource) provisionsNfixed-name slot workspaces (ci-slot-1..ci-slot-N). Coder enforces a unique workspace name per owner, so claiming a slot viacoder create ci-slot-<i>is a genuine compare-and-swap instead of a poll.ci-reap-staging.shjanitor, which already reaps any staleci-botworkspace by age with no changes needed there.openspec/changes/add-ci-staging-lock/.Changing how many CI workspaces can run at once
The limit
Nis theCI_LOCK_SLOTSrepository variable (GitHub → Settings → Secrets and variables → Actions → Variables) — no code change needed to retune it. Unset defaults to2. Now documented indocs/admin/server-setup.md(it wasn't documented anywhere before this PR):Nagainst the box's actual cores/RAM.Takes effect on the next workflow run; no restart or redeploy required.
Test plan
make validate(all four templates, including the newci-lock)make test-templatesterraform fmt -check -recursiveci-acquire-staging-lock.sh/ci-release-staging-lock.shexercised locally against a mockedcoderCLI: confirmed only N of several concurrent contenders acquire distinct slots and the rest wait/retry; confirmed release→acquire handoff to a waiting contender; confirmed a stale abandoned slot gets reclaimedintegration-test.yml/drupal-integration-test.yml/drupal-contrib-integration-test.ymlfor clean acquire/release behavior under real loadNote
Every self-hosted (
sysbox) job in all three workflows is currently disabled (if: false) — only the GitHub-hosted-ghvariants are active, and those are the only ones wired into the lock here (they're what was actually racing). If the self-hosted jobs are ever re-enabled, they'll need the same acquire/release wiring added.