From ddc00056c30cee85481dda55ec5429ab32a04e50 Mon Sep 17 00:00:00 2001 From: rfay Date: Sat, 8 Aug 2026 03:05:03 +0000 Subject: [PATCH 1/6] docs: document DDEV standard branch naming convention Matches the YYYYMMDD__ convention already used in ../ddev/CLAUDE.md, so branches created here follow the same org-wide standard. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 1 + scripts/ci-wait-for-staging-box.sh | 57 ------------------------------ 2 files changed, 1 insertion(+), 57 deletions(-) delete mode 100755 scripts/ci-wait-for-staging-box.sh diff --git a/CLAUDE.md b/CLAUDE.md index b194e64..64c0495 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -263,6 +263,7 @@ Key template variables (e.g. in `freeform/template.tf`): - Use feature branches for changes - **Never use the local `main` branch** — always `git fetch upstream` and base branches on `upstream/main`. Use `upstream/main` for comparisons (e.g. `git diff upstream/main...HEAD`), not local `main`. - **Never run `git push`, under any circumstances** — not to `main`, not to a feature branch, not to a fork. Commit locally and hand off to the user (or open a PR only if explicitly asked and only via a mechanism that doesn't require you to push, e.g. `gh pr create` from a branch the user has already pushed) — always let the user push. +- **Branch naming**: `YYYYMMDD__` (standard DDEV convention, matches `../ddev/CLAUDE.md`). Example: `20250108_rfay_fix_networking`. Create with `git fetch upstream && git checkout -b upstream/main --no-track`. - Always use OpenSpec for architectural changes (see AGENTS.md) ### OpenSpec Integration diff --git a/scripts/ci-wait-for-staging-box.sh b/scripts/ci-wait-for-staging-box.sh deleted file mode 100755 index f50417c..0000000 --- a/scripts/ci-wait-for-staging-box.sh +++ /dev/null @@ -1,57 +0,0 @@ -#!/usr/bin/env bash -# Wait until no other CI workspace exists on the shared staging box before this -# job creates its own workspace. -# -# Why this exists: -# GitHub Actions' `concurrency:` keyword only keeps one job running plus one -# pending per group -- any additional job that tries to join while one is -# already pending gets CANCELLED outright, not queued behind it. A single PR -# push (or push to main) triggers up to five box-provisioning jobs across -# integration-test.yml, drupal-integration-test.yml, and -# drupal-contrib-integration-test.yml at nearly the same instant, so sharing -# one concurrency group silently dropped most of them instead of running -# them in turn. Polling the box's actual ci-bot workspace count avoids that: -# every contender waits and none are cancelled. -# -# Usage: -# ci-wait-for-staging-box.sh -# -# Environment: -# CI_OWNER Coder username that owns CI workspaces (default: ci-bot) -# MAX_WAIT_SECONDS Give up and fail after this long (default: 2700 = 45m) -# POLL_SECONDS Delay between checks (default: 30) - -set -uo pipefail - -CI_OWNER="${CI_OWNER:-ci-bot}" -MAX_WAIT_SECONDS="${MAX_WAIT_SECONDS:-2700}" -POLL_SECONDS="${POLL_SECONDS:-30}" - -# Jitter so jobs that all started in the same instant don't all sample the -# workspace count (and all see "clear") at the exact same moment. -sleep "$((RANDOM % 15))" - -waited=0 -while true; do - if ! workspaces_json=$(coder list --all --output json 2>/dev/null); then - echo "WARN: 'coder list' failed; proceeding without the box-busy check" >&2 - exit 0 - fi - - count=$(echo "$workspaces_json" | jq -r --arg owner "$CI_OWNER" \ - '[.[] | select(.owner_name==$owner)] | length') - - if [[ "$count" -eq 0 ]]; then - echo "Staging box is free (0 ci-bot workspaces) -- proceeding" - exit 0 - fi - - if [[ "$waited" -ge "$MAX_WAIT_SECONDS" ]]; then - echo "ERROR: staging box still busy ($count ci-bot workspace(s)) after ${MAX_WAIT_SECONDS}s -- giving up" >&2 - exit 1 - fi - - echo "Staging box busy ($count ci-bot workspace(s)); waiting ${POLL_SECONDS}s (${waited}s/${MAX_WAIT_SECONDS}s so far)..." - sleep "$POLL_SECONDS" - waited=$((waited + POLL_SECONDS)) -done From 1bcd6d1e6a69a56b594e190835f6a5eb654d63e5 Mon Sep 17 00:00:00 2001 From: rfay Date: Sat, 8 Aug 2026 03:05:50 +0000 Subject: [PATCH 2/6] fix(ci): replace racy staging-box wait with an atomic lock-slot semaphore 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-` 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 --- .../drupal-contrib-integration-test.yml | 37 ++++-- .github/workflows/drupal-integration-test.yml | 35 ++++-- .github/workflows/integration-test.yml | 22 ++-- Makefile | 17 ++- ci-lock/.terraform.lock.hcl | 24 ++++ ci-lock/template.tf | 27 ++++ .../changes/add-ci-staging-lock/design.md | 30 +++++ .../changes/add-ci-staging-lock/proposal.md | 19 +++ .../specs/ci-staging-concurrency/spec.md | 21 ++++ openspec/changes/add-ci-staging-lock/tasks.md | 21 ++++ scripts/ci-acquire-staging-lock.sh | 117 ++++++++++++++++++ scripts/ci-release-staging-lock.sh | 24 ++++ 12 files changed, 362 insertions(+), 32 deletions(-) create mode 100644 ci-lock/.terraform.lock.hcl create mode 100644 ci-lock/template.tf create mode 100644 openspec/changes/add-ci-staging-lock/design.md create mode 100644 openspec/changes/add-ci-staging-lock/proposal.md create mode 100644 openspec/changes/add-ci-staging-lock/specs/ci-staging-concurrency/spec.md create mode 100644 openspec/changes/add-ci-staging-lock/tasks.md create mode 100755 scripts/ci-acquire-staging-lock.sh create mode 100755 scripts/ci-release-staging-lock.sh diff --git a/.github/workflows/drupal-contrib-integration-test.yml b/.github/workflows/drupal-contrib-integration-test.yml index 00af395..9705c9b 100644 --- a/.github/workflows/drupal-contrib-integration-test.yml +++ b/.github/workflows/drupal-contrib-integration-test.yml @@ -37,10 +37,11 @@ env: # running plus one pending; every other simultaneous contender gets silently # CANCELLED, not queued. Instead, each box-touching job below (including # contrib-plain-gh and contrib-issue-fork-gh, which otherwise run in parallel -# within a single workflow run) calls scripts/ci-wait-for-staging-box.sh right -# before creating its workspace, which polls the Coder server's actual -# workspace count and waits its turn -- see that script's header comment for -# detail. +# within a single workflow run) calls scripts/ci-acquire-staging-lock.sh right +# before creating its workspace, which claims one of a fixed number of +# lock-slot workspaces (an atomic Coder-side compare-and-swap on workspace +# name) and waits its turn if all slots are held -- see that script's header +# comment for detail. concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} cancel-in-progress: true @@ -67,7 +68,7 @@ jobs: if: false runs-on: [self-hosted, sysbox] # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 strategy: matrix: @@ -230,7 +231,7 @@ jobs: if: false runs-on: [self-hosted, sysbox] # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 defaults: run: @@ -410,7 +411,7 @@ jobs: if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == github.repository_owner }} runs-on: ubuntu-latest # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 strategy: # PRs run a single module (smoke); push-to-main and nightly run the full @@ -460,8 +461,10 @@ jobs: - name: Reap predecessor workspaces (cancelled prior runs of this cell) run: ./scripts/ci-reap-family.sh "gc-${{ matrix.project }}-d${{ matrix.drupal_version }}-" "${{ env.WORKSPACE_NAME }}" - - name: Wait for staging box to be free - run: ./scripts/ci-wait-for-staging-box.sh + - name: Acquire staging box lock + run: ./scripts/ci-acquire-staging-lock.sh + env: + CI_LOCK_SLOTS: ${{ vars.CI_LOCK_SLOTS }} - name: Create workspace run: | @@ -547,6 +550,10 @@ jobs: if: always() run: coder delete ${{ env.WORKSPACE_NAME }} --yes || true + - name: Release staging box lock + if: always() + run: ./scripts/ci-release-staging-lock.sh + - name: Archive CI template version if: always() run: coder templates versions archive drupal-contrib ${{ env.WORKSPACE_NAME }} --yes || true @@ -556,7 +563,7 @@ jobs: if: ${{ vars.CONTRIB_TEST_ISSUE_FORK != '' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == github.repository_owner) }} runs-on: ubuntu-latest # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 defaults: run: @@ -620,8 +627,10 @@ jobs: - name: Reap predecessor workspaces (cancelled prior runs of this cell) run: ./scripts/ci-reap-family.sh "gc-fork-" "${{ env.WORKSPACE_NAME }}" - - name: Wait for staging box to be free - run: ./scripts/ci-wait-for-staging-box.sh + - name: Acquire staging box lock + run: ./scripts/ci-acquire-staging-lock.sh + env: + CI_LOCK_SLOTS: ${{ vars.CI_LOCK_SLOTS }} - name: Create workspace run: | @@ -705,6 +714,10 @@ jobs: if: always() run: coder delete ${{ env.WORKSPACE_NAME }} --yes || true + - name: Release staging box lock + if: always() + run: ./scripts/ci-release-staging-lock.sh + - name: Archive CI template version if: always() run: coder templates versions archive drupal-contrib gc-${{ github.run_number }}-${{ github.run_attempt }} --yes || true diff --git a/.github/workflows/drupal-integration-test.yml b/.github/workflows/drupal-integration-test.yml index b9f1840..a5e2754 100644 --- a/.github/workflows/drupal-integration-test.yml +++ b/.github/workflows/drupal-integration-test.yml @@ -43,9 +43,10 @@ env: # simultaneous contender gets silently CANCELLED, not queued. Instead, each # box-touching job below (including drupal-plain-gh and drupal-issue-fork-gh, # which otherwise run in parallel within a single workflow run) calls -# scripts/ci-wait-for-staging-box.sh right before creating its workspace, -# which polls the Coder server's actual workspace count and waits its turn -- -# see that script's header comment for detail. +# scripts/ci-acquire-staging-lock.sh right before creating its workspace, +# which claims one of a fixed number of lock-slot workspaces (an atomic +# Coder-side compare-and-swap on workspace name) and waits its turn if all +# slots are held -- see that script's header comment for detail. concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} cancel-in-progress: true @@ -72,7 +73,7 @@ jobs: if: false runs-on: [self-hosted, sysbox] # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 strategy: matrix: @@ -218,7 +219,7 @@ jobs: if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == github.repository_owner }} runs-on: ubuntu-latest # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 strategy: # PRs run a single Drupal version (smoke); push-to-main and nightly run the @@ -266,8 +267,10 @@ jobs: - name: Reap predecessor workspaces (cancelled prior runs of this cell) run: ./scripts/ci-reap-family.sh "gd-${{ matrix.drupal_version }}-" "${{ env.WORKSPACE_NAME }}" - - name: Wait for staging box to be free - run: ./scripts/ci-wait-for-staging-box.sh + - name: Acquire staging box lock + run: ./scripts/ci-acquire-staging-lock.sh + env: + CI_LOCK_SLOTS: ${{ vars.CI_LOCK_SLOTS }} - name: Create workspace run: | @@ -292,6 +295,10 @@ jobs: - *verify-http - *delete-workspace + - name: Release staging box lock + if: always() + run: ./scripts/ci-release-staging-lock.sh + - name: Archive CI template version if: always() run: coder templates versions archive drupal-core ${{ env.WORKSPACE_NAME }} --yes || true @@ -303,7 +310,7 @@ jobs: if: false runs-on: [self-hosted, sysbox] # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 defaults: run: @@ -467,7 +474,7 @@ jobs: if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == github.repository_owner }} runs-on: ubuntu-latest # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 defaults: run: @@ -528,8 +535,10 @@ jobs: - name: Reap predecessor workspaces (cancelled prior runs of this cell) run: ./scripts/ci-reap-family.sh "gd-fork-" "${{ env.WORKSPACE_NAME }}" - - name: Wait for staging box to be free - run: ./scripts/ci-wait-for-staging-box.sh + - name: Acquire staging box lock + run: ./scripts/ci-acquire-staging-lock.sh + env: + CI_LOCK_SLOTS: ${{ vars.CI_LOCK_SLOTS }} - name: Create workspace run: | @@ -609,6 +618,10 @@ jobs: if: always() run: coder delete ${{ env.WORKSPACE_NAME }} --yes || true + - name: Release staging box lock + if: always() + run: ./scripts/ci-release-staging-lock.sh + - name: Archive CI template version if: always() run: coder templates versions archive drupal-core gd-${{ github.run_number }}-${{ github.run_attempt }} --yes || true diff --git a/.github/workflows/integration-test.yml b/.github/workflows/integration-test.yml index 7ac7c6c..09f92c7 100644 --- a/.github/workflows/integration-test.yml +++ b/.github/workflows/integration-test.yml @@ -71,9 +71,11 @@ on: # drupal-contrib-integration-test.yml at nearly the same instant, and GitHub's # concurrency groups only keep one job running plus one pending; every other # simultaneous contender gets silently CANCELLED, not queued. Instead, each -# box-touching job below runs scripts/ci-wait-for-staging-box.sh right before -# creating its workspace, which polls the Coder server's actual workspace -# count and waits its turn -- see that script's header comment for detail. +# box-touching job below runs scripts/ci-acquire-staging-lock.sh right before +# creating its workspace, which claims one of a fixed number of lock-slot +# workspaces (an atomic Coder-side compare-and-swap on workspace name) and +# waits its turn if all slots are held -- see that script's header comment +# for detail. concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} cancel-in-progress: true @@ -86,7 +88,7 @@ jobs: if: false runs-on: [self-hosted, sysbox] # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 strategy: matrix: @@ -302,7 +304,7 @@ jobs: if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == github.repository_owner }} runs-on: ubuntu-latest # Long enough to cover queued time behind other box-provisioning jobs - # (see scripts/ci-wait-for-staging-box.sh) plus this job's own runtime. + # (see scripts/ci-acquire-staging-lock.sh) plus this job's own runtime. timeout-minutes: 75 strategy: matrix: @@ -361,8 +363,10 @@ jobs: - name: Reap predecessor workspaces (cancelled prior runs of this cell) run: ./scripts/ci-reap-family.sh "gh-${{ matrix.ws_name }}-" "${{ env.WORKSPACE_NAME }}" - - name: Wait for staging box to be free - run: ./scripts/ci-wait-for-staging-box.sh + - name: Acquire staging box lock + run: ./scripts/ci-acquire-staging-lock.sh + env: + CI_LOCK_SLOTS: ${{ vars.CI_LOCK_SLOTS }} - name: Create workspace if: ${{ matrix.template != 'freeform' }} @@ -497,6 +501,10 @@ jobs: if: always() run: coder delete ${{ env.WORKSPACE_NAME }} --yes || true + - name: Release staging box lock + if: always() + run: ./scripts/ci-release-staging-lock.sh + - name: Archive CI template version if: always() run: coder templates versions archive ${{ matrix.template }} ci-gh-${{ env.CI_TAG }} --yes || true diff --git a/Makefile b/Makefile index f8ae2f4..ab52604 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,10 @@ VERSION := $(shell cat VERSION 2>/dev/null || echo "1.0.0-beta1") DOCKERFILE_DIR := image DOCKERFILE := $(DOCKERFILE_DIR)/Dockerfile -# Template directories (name == directory name == Coder template name) +# Template directories (name == directory name == Coder template name). +# ci-lock is a separate CI-only mutex template (see ci-lock/template.tf) -- +# deliberately excluded here since it has no shared assets to vendor and no +# image_version variable; it gets its own validate/push handling below. TEMPLATES := drupal-core drupal-contrib freeform # Host path to the drupal-core seed cache (bind-mounted read-only into workspaces). @@ -144,6 +147,8 @@ validate: sync-shared ## Validate all Terraform templates (requires terraform in echo "--- Validating $$t ---"; \ (cd $$t && terraform init -backend=false -input=false -no-color && terraform validate -no-color) || exit 1; \ done + @echo "--- Validating ci-lock ---" + @(cd ci-lock && terraform init -backend=false -input=false -no-color && terraform validate -no-color) || exit 1 @echo "All templates valid." .PHONY: fmt-check @@ -207,8 +212,16 @@ push-template-drupal-contrib: sync-shared ## Push drupal-contrib template to Cod push-template-freeform: sync-shared ## Push freeform template to Coder $(call push_template,freeform) +.PHONY: push-template-ci-lock +push-template-ci-lock: ## Push the ci-lock CI-mutex template to Coder (no image, no shared assets -- see ci-lock/template.tf) + @echo "Pushing Coder template ci-lock..." + coder templates push --directory ci-lock ci-lock --yes --activate=$(ACTIVATE) + @echo "Setting template metadata for ci-lock..." + coder templates edit ci-lock --yes --display-name "CI Lock (internal)" --description "CI-internal mutex for the staging box. Not a development environment -- do not use." + @echo "Template ci-lock push complete" + .PHONY: push-all-templates -push-all-templates: push-template-drupal-core push-template-drupal-contrib push-template-freeform ## Push all templates to Coder (no image build) +push-all-templates: push-template-drupal-core push-template-drupal-contrib push-template-freeform push-template-ci-lock ## Push all templates to Coder (no image build) @echo "All templates pushed!" # --- Deploy targets --- diff --git a/ci-lock/.terraform.lock.hcl b/ci-lock/.terraform.lock.hcl new file mode 100644 index 0000000..ae3a16d --- /dev/null +++ b/ci-lock/.terraform.lock.hcl @@ -0,0 +1,24 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/coder/coder" { + version = "2.18.0" + constraints = ">= 2.13.0" + hashes = [ + "h1:O25nA2tkM1JFJtQpaX2TOTeWvsSyuxDoJo6/QTv78hM=", + "zh:107c4eb7a36335ac94637679845c040759e21d3fb108d07081e0689cb93121b1", + "zh:1f8d904d59e35948f8f795e58d7895b791bd9a12c8409079f5bb90221303c1b6", + "zh:3942d5c6b6fad45b3b92f82c7b3f40a2d7f2b7b72da07f1ac63e1022b46bdbc7", + "zh:464f3b53eccfefc3ca02b46da07fb277362c16f81f45cc4570150520e5b9bf75", + "zh:6ae37bef989559a212ce04a615444d40452425e01081d8499843a24db0a09e85", + "zh:6f1410911124b0078a0f1c077899b4d834cd6821596ed350e4b4ad8f5b0030e7", + "zh:9ca356eb0e23bc034091fc9672bbe307a69e242c341a669f10db81e9a75fab2d", + "zh:b36687f0793295337286ae71d8157dc4c78a13fb5a9b1276093db82cdb8edd76", + "zh:c60a39cbde9fb473bfd0c2ad1685a0ce8d4a56408f060c6d1b858bae2e535d98", + "zh:d26ffdb52e095662e16b19942b7579bea7dc60577bd81f5cd671e6632ce48a0b", + "zh:da5414d74b21ecdcf8f93adc2199925c530cde1f53cde014201713d315916366", + "zh:e7e801f91b8afba90d40aa390c8b822ba064a54932a4a25bbff1593942e9bf72", + "zh:f064fb800fc687b2d60d4c12e1e65875a2f4b21cf8bb0b5e018d3ecbf7d29de2", + "zh:f569b65999264a9416862bca5cd2a6177d94ccb0424f3a4ef424428912b9cb3c", + ] +} diff --git a/ci-lock/template.tf b/ci-lock/template.tf new file mode 100644 index 0000000..7c2d1ce --- /dev/null +++ b/ci-lock/template.tf @@ -0,0 +1,27 @@ +# CI-internal mutex, not a development environment. +# +# Integration-test CI jobs create real, resource-heavy (Sysbox + Docker-in-Docker) +# workspaces on a single shared staging Coder box, from both self-hosted runners +# and GitHub-hosted runners. To bound how many of those run at once without a +# check-then-act race, jobs claim one of N fixed-name workspaces provisioned from +# THIS template (ci-slot-1..N) before creating their real workspace, and delete it +# when done. Coder enforces a unique workspace name per owner, so `coder create +# ci-slot-` is an atomic compare-and-swap: only one concurrent caller can win +# for a given . See scripts/ci-acquire-staging-lock.sh / +# scripts/ci-release-staging-lock.sh for the protocol, and +# openspec/changes/add-ci-staging-lock/design.md for why this approach was chosen +# over a git-ref-based lock. +# +# Deliberately has no coder_agent: a lock slot is never connected to, only +# created and deleted, so it provisions and tears down in a second or two. + +terraform { + required_providers { + coder = { + source = "coder/coder" + version = ">= 2.13" + } + } +} + +resource "terraform_data" "lock" {} diff --git a/openspec/changes/add-ci-staging-lock/design.md b/openspec/changes/add-ci-staging-lock/design.md new file mode 100644 index 0000000..4dc8e45 --- /dev/null +++ b/openspec/changes/add-ci-staging-lock/design.md @@ -0,0 +1,30 @@ +# Design: CI Staging-Box Concurrency Lock + +## Context +Three integration-test workflows create real, resource-heavy (Sysbox + Docker-in-Docker) workspaces on one shared staging Coder deployment. Jobs originate from a mix of self-hosted runners (which happen to run on the staging box itself) and GitHub-hosted `ubuntu-latest` runners (ephemeral VMs with no shared filesystem with anything). Any lock mechanism must therefore be reachable over the network by every contender — a local `flock`/`mkdir` lock on the staging box would not see GitHub-hosted contenders at all. + +## Goals / Non-Goals +- Goal: bound the number of concurrently-provisioning/running `ci-bot` workspaces to a configurable `N`, closing the check-then-act race that let it go unbounded. +- Goal: self-heal if a job dies mid-hold without releasing its slot (crashed job, cancelled run, killed runner). +- Non-goal: fine-grained resource accounting (summing actual CPU/RAM requests). A fixed slot count is a coarse but sufficient proxy, tunable as real headroom is learned. +- Non-goal: unifying self-hosted vs. GitHub-hosted execution for these jobs. That's a plausible future simplification (run everything on self-hosted runners on the staging box itself, since a local lock would then suffice) but out of scope here. + +## Decision: Coder-workspace-slot CAS, not a git-ref lock +Two network-reachable atomic primitives were considered: + +1. **Chosen: N fixed-name Coder workspaces** (`ci-slot-1`..`ci-slot-N`), created from a new minimal `ci-lock` template. Coder enforces a unique workspace name per owner — confirmed behavior, not an assumption (it's how `ci-reap-family.sh` already identifies workspace "families" and how `coder create` collisions are known to fail today). Attempting `coder create ci-slot-` is therefore a genuine atomic compare-and-swap: exactly one concurrent caller can win for a given `i`. Releasing is `coder delete ci-slot-`. Because the slot workspaces are owned by `ci-bot` like every other CI workspace, the existing `scripts/ci-reap-staging.sh` janitor (runs every 15 minutes, reaps `ci-bot`-owned workspaces by state/age) already self-heals abandoned slots with zero changes to that script. + +2. **Rejected: N fixed git branch names**, claimed via `git push origin HEAD:refs/heads/ci-slot-N` (atomic ref creation) and released by deleting the branch. This avoids adding a new Coder template, but: (a) none of the three workflow files currently set a `permissions:` block, so the default `GITHUB_TOKEN` likely lacks `contents: write` — this option requires explicitly granting that; (b) it needs its own bespoke stale-lock reaper (age-check a branch's last-commit timestamp, force-delete), duplicating logic that already exists for Coder workspaces rather than reusing it. + +3. **Considered and discarded without enough confidence: named Coder API tokens** (`coder tokens create --name ci-slot-N`) as the CAS, avoiding a Terraform template entirely. Whether token names are enforced unique per user (and thus safe as a CAS) could not be confirmed from the CLI help text or public docs in the time available. Workspace-name uniqueness, by contrast, is directly evidenced in this repo's existing scripts and is a well-known Coder invariant. Given a broken CAS would silently reintroduce the exact race this change exists to fix, the better-evidenced primitive was chosen. + +## Mechanism +- `ci-lock/template.tf`: minimal template, no `coder_agent`, just the `coder` provider and a single no-op resource (`terraform_data`). Provisions/destroys in ~1-2s, touches no Docker/Sysbox. +- `scripts/ci-acquire-staging-lock.sh`: + - `N` from `CI_LOCK_SLOTS` (default 2), `MAX_WAIT_SECONDS` / `POLL_SECONDS` matching the old script's conventions. + - Loop: for `i` in a randomized order over `1..N`, attempt `coder create ci-slot- --template ci-lock --yes`. Success = lock acquired; print which slot for the release step to reuse (write to `$GITHUB_ENV` as `CI_LOCK_SLOT`). + - If all `N` slots are taken: check each slot's build age; if older than `STALE_MINUTES` (default 30 — comfortably longer than any real job), force `coder delete` it and retry immediately (self-heal). Otherwise sleep `POLL_SECONDS` + jitter and retry, up to `MAX_WAIT_SECONDS`. +- `scripts/ci-release-staging-lock.sh`: `coder delete "$CI_LOCK_SLOT" --yes`. Called in an `if: always()` step so normal completion, test failure, and most cancellations all release; the staleness check above is the backstop for the remaining case (hard-killed runner, no steps execute at all). + +## Rollout +`N` starts at 2 (default), given the staging box's 4-core/8-thread/64GB spec and each workspace's default 4-CPU/8GB request. Adjust via the `CI_LOCK_SLOTS` env/repo variable as real utilization data comes in — no code change needed to retune. diff --git a/openspec/changes/add-ci-staging-lock/proposal.md b/openspec/changes/add-ci-staging-lock/proposal.md new file mode 100644 index 0000000..c40c95e --- /dev/null +++ b/openspec/changes/add-ci-staging-lock/proposal.md @@ -0,0 +1,19 @@ +# Change: Add CI Staging-Box Concurrency Lock + +## Why +Integration-test CI jobs that create real workspaces on the shared staging Coder box (`staging-coder.ddev.com`) currently serialize via `scripts/ci-wait-for-staging-box.sh`, which polls `coder list` for a zero count of `ci-bot`-owned workspaces before proceeding to `coder create`. This is a check-then-act race: multiple jobs — from different workflow files, and from both self-hosted `sysbox` runners and GitHub-hosted `ubuntu-latest` runners, all reaching the same remote Coder server — can observe "0 workspaces" within the same ~30s poll window and all create at once. + +This was observed directly in production: on 2026-08-08, three `ci-bot` workspaces existed simultaneously on staging (the count jumped from 1 to 3 within one poll interval — see `Contrib skipto D12 (plain, GH)`'s log in run 31230219543), and the job racing into that window (`Contrib token issue fork (GH)`) failed its `coder ssh --wait=yes` connection with "Agent doesn't exist with that id" — a symptom of the box being oversubscribed (each workspace defaults to 4 CPU / 8GB against a single 4-core/8-thread, 64GB host). + +The intent was never strict one-at-a-time serialization — jobs that don't compete for the same real resources are fine running concurrently — it was to cap concurrent *heavy* workspaces at whatever the box can actually sustain. The current mechanism achieves neither: it's racy, and (at its effective N=1) stricter than the box's real headroom requires. + +## What Changes +- Replace the racy poll-then-create check with a true atomic semaphore of size `N` (repo variable, default 2), implemented via a new minimal `ci-lock` Coder template (no Docker/Sysbox, provisions in ~1-2s) whose only purpose is to let jobs claim one of `N` fixed-name lock-slot workspaces (`ci-slot-1`..`ci-slot-N`) using Coder's existing per-owner unique-workspace-name constraint as the atomic compare-and-swap. +- Add `scripts/ci-acquire-staging-lock.sh` (claim a free slot; retry with backoff + jitter up to a timeout; self-heal by force-releasing a slot whose holder is stale) and `scripts/ci-release-staging-lock.sh` (delete the claimed slot workspace). +- Replace all call sites of `scripts/ci-wait-for-staging-box.sh` across `integration-test.yml`, `drupal-integration-test.yml`, and `drupal-contrib-integration-test.yml` with the new acquire script, and add a matching `if: always()` release step alongside each job's existing "Delete workspace" cleanup. +- Remove `scripts/ci-wait-for-staging-box.sh` (superseded). +- No changes needed to `scripts/ci-reap-staging.sh` — it already reaps any stale `ci-bot`-owned workspace by age, which covers abandoned lock-slot workspaces for free. + +## Impact +- Affected specs: `ci-staging-concurrency` (new capability) +- Affected code: new `ci-lock/template.tf` (+ Makefile wiring to validate/push it alongside the other three templates), `scripts/ci-wait-for-staging-box.sh` (removed), `scripts/ci-acquire-staging-lock.sh` (new), `scripts/ci-release-staging-lock.sh` (new), `.github/workflows/integration-test.yml`, `.github/workflows/drupal-integration-test.yml`, `.github/workflows/drupal-contrib-integration-test.yml` diff --git a/openspec/changes/add-ci-staging-lock/specs/ci-staging-concurrency/spec.md b/openspec/changes/add-ci-staging-lock/specs/ci-staging-concurrency/spec.md new file mode 100644 index 0000000..4bd3368 --- /dev/null +++ b/openspec/changes/add-ci-staging-lock/specs/ci-staging-concurrency/spec.md @@ -0,0 +1,21 @@ +## ADDED Requirements + +### Requirement: Bounded Concurrent CI Workspaces on Staging +The system SHALL ensure no more than a configurable limit `N` of `ci-bot`-owned workspaces are concurrently created/running on the shared staging Coder box, regardless of which workflow file or runner type (self-hosted or GitHub-hosted) initiates them. + +#### Scenario: Two jobs start within the same instant +- **WHEN** two or more CI jobs, from any of the integration-test workflows and any runner type, attempt to create a staging workspace at nearly the same time +- **THEN** at most `N` of them SHALL be allowed to proceed to `coder create` concurrently +- **AND** the rest SHALL wait until a slot frees up rather than proceeding or being silently dropped + +#### Scenario: A slot is free +- **WHEN** fewer than `N` lock slots are currently held +- **THEN** a job requesting a slot SHALL acquire one and proceed without unnecessary delay + +### Requirement: Self-Healing Lock Slots +The locking mechanism SHALL NOT deadlock indefinitely if a job crashes or is force-cancelled after acquiring a slot. + +#### Scenario: A job is killed after acquiring a slot +- **WHEN** a CI job acquires a lock slot and is then killed (e.g. a cancelled workflow run or a hard-killed runner) before its release step executes +- **THEN** the abandoned slot SHALL be reclaimed automatically — either by a later contender's staleness check or by the existing staging janitor — within a bounded amount of time +- **AND** no manual intervention SHALL be required to restore CI throughput diff --git a/openspec/changes/add-ci-staging-lock/tasks.md b/openspec/changes/add-ci-staging-lock/tasks.md new file mode 100644 index 0000000..666b87e --- /dev/null +++ b/openspec/changes/add-ci-staging-lock/tasks.md @@ -0,0 +1,21 @@ +## 1. `ci-lock` template +- [x] 1.1 Create `ci-lock/template.tf`: `coder` provider only, single no-op `terraform_data` resource, no `coder_agent` +- [x] 1.2 Wire `ci-lock` into the Makefile's validate / test-templates / push-all-templates targets alongside the other three templates +- [x] 1.3 `terraform fmt -recursive` and `make validate` pass for the new template + +## 2. Lock scripts +- [x] 2.1 Add `scripts/ci-acquire-staging-lock.sh`: randomized slot order over `1..N` (`CI_LOCK_SLOTS`, default 2), atomic `coder create ci-slot-`, staleness self-heal (`STALE_MINUTES`, default 30), `MAX_WAIT_SECONDS`/`POLL_SECONDS` matching prior conventions, writes acquired slot name to `$GITHUB_ENV` +- [x] 2.2 Add `scripts/ci-release-staging-lock.sh`: `coder delete "$CI_LOCK_SLOT" --yes`, best-effort (never fails the job) +- [x] 2.3 Remove `scripts/ci-wait-for-staging-box.sh` (superseded) + +## 3. Workflow wiring +- [x] 3.1 Enumerate every job in `integration-test.yml`, `drupal-integration-test.yml`, `drupal-contrib-integration-test.yml` that currently calls `ci-wait-for-staging-box.sh` +- [x] 3.2 Replace each with a call to `ci-acquire-staging-lock.sh` +- [x] 3.3 Add an `if: always()` "Release staging box lock" step (calling `ci-release-staging-lock.sh`) to each of those jobs, ordered alongside the existing "Delete workspace" cleanup +- [x] 3.4 Add the `CI_LOCK_SLOTS` repo variable reference (default fallback `2` if unset) + +## 4. Validation +- [x] 4.1 `make validate` and `make test-templates` pass repo-wide +- [x] 4.2 `terraform fmt -check -recursive` clean +- [x] 4.3 Update `docs/admin/server-setup.md` / any doc referencing `ci-wait-for-staging-box.sh` to reference the new scripts + (no such references existed; nothing to change) diff --git a/scripts/ci-acquire-staging-lock.sh b/scripts/ci-acquire-staging-lock.sh new file mode 100755 index 0000000..2952665 --- /dev/null +++ b/scripts/ci-acquire-staging-lock.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash +# Claim one of N fixed-name "lock slot" workspaces before creating a real +# workspace on the shared staging box, bounding how many heavy (Sysbox + +# Docker-in-Docker) CI workspaces can run there at once. +# +# Why this exists: +# The box-busy check this replaces (ci-wait-for-staging-box.sh) polled +# `coder list` for a zero count, then created its own workspace -- a +# check-then-act race. Jobs come from different workflow files and from +# both self-hosted `sysbox` runners and GitHub-hosted `ubuntu-latest` +# runners, all reaching 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-bot workspaces existed +# simultaneously on a box sized for about two, and the job racing into that +# window failed its agent connection. +# +# Coder enforces a unique workspace name per owner, so `coder create +# ci-slot-` is a genuine atomic compare-and-swap: exactly one concurrent +# caller can win for a given . Trying each of N slots in turn gives a +# real bounded semaphore instead of a racy poll. The ci-lock template +# (see ci-lock/template.tf) that these slots are provisioned from has no +# Docker/Sysbox/agent, so claiming a slot is near-instant. +# +# Usage: +# ci-acquire-staging-lock.sh +# # on success, prints the acquired slot name and (if $GITHUB_ENV is set) +# # writes CI_LOCK_SLOT= there for the matching release step to use. +# +# Environment: +# CI_LOCK_SLOTS Number of concurrent slots (default: 2) +# CI_OWNER Coder username that owns CI workspaces (default: ci-bot) +# MAX_WAIT_SECONDS Give up and fail after this long (default: 2700 = 45m) +# POLL_SECONDS Delay between full passes over all slots (default: 30) +# STALE_MINUTES Force-reclaim a held slot older than this (default: 30) + +set -uo pipefail + +CI_LOCK_SLOTS="${CI_LOCK_SLOTS:-2}" +CI_OWNER="${CI_OWNER:-ci-bot}" +MAX_WAIT_SECONDS="${MAX_WAIT_SECONDS:-2700}" +POLL_SECONDS="${POLL_SECONDS:-30}" +STALE_MINUTES="${STALE_MINUTES:-30}" + +acquired="" +last_err="" + +# Fisher-Yates-ish shuffle of 1..N so contenders don't all hammer slot 1 first. +shuffled_slots() { + seq 1 "$CI_LOCK_SLOTS" | shuf +} + +try_acquire_pass() { + local i name out + for i in $(shuffled_slots); do + name="ci-slot-$i" + if out=$(coder create "$name" --template ci-lock --yes 2>&1); then + acquired="$name" + return 0 + fi + last_err="$out" + done + return 1 +} + +# Echoes the number of slots it force-reclaimed, so the caller can retry +# immediately instead of sleeping a full poll interval on a freed slot. +reclaim_stale_slots() { + local slots_json now cutoff reclaimed + reclaimed=0 + if ! slots_json=$(coder list --all --output json 2>/dev/null); then + echo 0 + return + fi + now=$(date +%s) + cutoff=$((now - STALE_MINUTES * 60)) + while IFS=$'\t' read -r name build_created; do + [[ -z "$name" ]] && continue + build_epoch=$(date -d "$build_created" +%s 2>/dev/null || echo 0) + if [[ "$build_epoch" -gt 0 && "$build_epoch" -lt "$cutoff" ]]; then + echo "Reclaiming stale lock slot: $name (build older than ${STALE_MINUTES}m)" >&2 + coder delete "$name" --yes >&2 || echo " WARN: delete failed for $name" >&2 + reclaimed=$((reclaimed + 1)) + fi + done < <(echo "$slots_json" | + jq -r --arg owner "$CI_OWNER" \ + '.[] | select(.owner_name==$owner) | select(.name | startswith("ci-slot-")) | [.name, .latest_build.created_at] | @tsv') + echo "$reclaimed" +} + +# Jitter so contenders that all started in the same instant don't all sample +# slot state at the exact same moment. +sleep "$((RANDOM % 15))" + +waited=0 +while true; do + if try_acquire_pass; then + echo "Acquired staging box lock slot: $acquired" + if [[ -n "${GITHUB_ENV:-}" ]]; then + echo "CI_LOCK_SLOT=$acquired" >>"$GITHUB_ENV" + fi + exit 0 + fi + + if [[ "$(reclaim_stale_slots)" -gt 0 ]]; then + continue # a slot just freed up -- retry now instead of sleeping + fi + + if [[ "$waited" -ge "$MAX_WAIT_SECONDS" ]]; then + echo "ERROR: all $CI_LOCK_SLOTS staging box lock slot(s) still busy after ${MAX_WAIT_SECONDS}s -- giving up" >&2 + echo "Last error: $last_err" >&2 + exit 1 + fi + + echo "All $CI_LOCK_SLOTS staging box lock slot(s) busy; waiting ${POLL_SECONDS}s (${waited}s/${MAX_WAIT_SECONDS}s so far)..." + sleep "$POLL_SECONDS" + waited=$((waited + POLL_SECONDS)) +done diff --git a/scripts/ci-release-staging-lock.sh b/scripts/ci-release-staging-lock.sh new file mode 100755 index 0000000..59eca23 --- /dev/null +++ b/scripts/ci-release-staging-lock.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# Release the staging box lock slot claimed by ci-acquire-staging-lock.sh. +# +# Usage: +# ci-release-staging-lock.sh +# # reads $CI_LOCK_SLOT (set by the acquire script, normally via +# # $GITHUB_ENV so it survives into this later step) +# +# Always exits 0 -- releasing is best-effort and must never fail a run. If a +# job dies before this step runs at all, the slot is reclaimed automatically: +# either by a later contender's staleness check in ci-acquire-staging-lock.sh, +# or by the existing scripts/ci-reap-staging.sh janitor, which already reaps +# any stale ci-bot-owned workspace by age. + +set -uo pipefail + +if [[ -z "${CI_LOCK_SLOT:-}" ]]; then + echo "No CI_LOCK_SLOT set; nothing to release" + exit 0 +fi + +echo "Releasing staging box lock slot: $CI_LOCK_SLOT" +coder delete "$CI_LOCK_SLOT" --yes || echo "WARN: delete failed for $CI_LOCK_SLOT" >&2 +exit 0 From 268d56d6c87cafc6a69872a20a9220f5eed969c2 Mon Sep 17 00:00:00 2001 From: rfay Date: Sat, 8 Aug 2026 03:15:43 +0000 Subject: [PATCH 3/6] docs: document CI_LOCK_SLOTS in the staging server setup guide 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 --- docs/admin/server-setup.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/admin/server-setup.md b/docs/admin/server-setup.md index b4a92ca..d8b3c1f 100644 --- a/docs/admin/server-setup.md +++ b/docs/admin/server-setup.md @@ -1259,6 +1259,19 @@ Go to **GitHub → Settings → Secrets and variables → Actions** and add: |--------------------------|-----------------------------------------| | `TEST_CODER_URL` | `https://staging-coder.ddev.com` | | `DRUPAL_TEST_ISSUE_FORK` | A drupal.org issue number (see below) | +| `CI_LOCK_SLOTS` | Max concurrent CI workspaces on staging (see below) | + +### Tuning how many CI workspaces can run at once (`CI_LOCK_SLOTS`) + +`integration-test.yml`, `drupal-integration-test.yml`, and `drupal-contrib-integration-test.yml` all create real, resource-heavy (Sysbox + Docker-in-Docker) workspaces on the single shared staging box. Before creating one, each job runs `scripts/ci-acquire-staging-lock.sh`, which claims one of a fixed number of lock-slot workspaces (`ci-slot-1`..`ci-slot-N`) and waits if all are held — see that script's header comment and `openspec/changes/add-ci-staging-lock/design.md` for the full mechanism. + +`N` is controlled entirely by the `CI_LOCK_SLOTS` repository variable — no code change needed to retune it: + +- **Unset**: defaults to `2`. +- **Raise it** if staging has spare CPU/RAM headroom and jobs are spending a lot of time queued behind `ci-acquire-staging-lock.sh` waiting for a slot. +- **Lower it** if jobs are failing with agent-connection errors (e.g. "Agent doesn't exist with that id") that trace back to the box being oversubscribed — each workspace's default request is 4 CPU / 8GB, so pick `N` with that against the box's actual cores/RAM in mind. + +Change it in **GitHub → Settings → Secrets and variables → Actions → Variables**; it takes effect on the next workflow run, no restart or redeploy required. ### Choosing a test issue for `DRUPAL_TEST_ISSUE_FORK` From 439634b1aaccf0ef4ced228e650824800a39f0e5 Mon Sep 17 00:00:00 2001 From: rfay Date: Sat, 8 Aug 2026 04:15:08 +0000 Subject: [PATCH 4/6] fix(ci): fail fast when the ci-lock template is missing, document push 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-` 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 --- docs/admin/server-setup.md | 11 +++++++++++ scripts/ci-acquire-staging-lock.sh | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/docs/admin/server-setup.md b/docs/admin/server-setup.md index d8b3c1f..b9e3f89 100644 --- a/docs/admin/server-setup.md +++ b/docs/admin/server-setup.md @@ -1243,6 +1243,17 @@ coder tokens create --user ci-bot --lifetime 8760h Store the token in 1Password at `op://test-secrets/TEST_CODER_SESSION_TOKEN/credential`. +#### 3. Push the `ci-lock` template + +The integration-test workflows serialize access to this box through `scripts/ci-acquire-staging-lock.sh`, which claims one of a fixed set of lock-slot workspaces provisioned from the `ci-lock` template (see "Tuning how many CI workspaces can run at once" below). That template is CI-only plumbing — no workflow pushes it automatically — so it needs to exist before the first CI run. `make push-all-templates` (already the standard way to push templates to an environment) covers it along with the rest; log in as a template-admin (e.g. `ci-bot`) first: + +```bash +coder login +make push-all-templates +``` + +Without this, every `ci-acquire-staging-lock.sh` invocation fails fast with "no 'ci-lock' template found". + ### GitHub repository configuration Go to **GitHub → Settings → Secrets and variables → Actions** and add: diff --git a/scripts/ci-acquire-staging-lock.sh b/scripts/ci-acquire-staging-lock.sh index 2952665..ecb42c2 100755 --- a/scripts/ci-acquire-staging-lock.sh +++ b/scripts/ci-acquire-staging-lock.sh @@ -87,6 +87,22 @@ reclaim_stale_slots() { echo "$reclaimed" } +# Fail fast if the ci-lock template itself is missing (e.g. never pushed to +# this Coder deployment) rather than retrying a doomed `coder create` for the +# full MAX_WAIT_SECONDS -- that failure mode wastes 45 minutes per job with no +# indication of the real problem. See docs/admin/server-setup.md for the +# one-time `make push-all-templates` setup step. +if ! templates_json=$(coder templates list --output json 2>&1); then + echo "ERROR: 'coder templates list' failed; cannot verify ci-lock template exists:" >&2 + echo "$templates_json" >&2 + exit 1 +fi +if ! echo "$templates_json" | jq -e '[.[] | select(.name=="ci-lock")] | length > 0' >/dev/null 2>&1; then + echo "ERROR: no 'ci-lock' template found on this Coder deployment." >&2 + echo "Run 'make push-all-templates' against it once (see docs/admin/server-setup.md)." >&2 + exit 1 +fi + # Jitter so contenders that all started in the same instant don't all sample # slot state at the exact same moment. sleep "$((RANDOM % 15))" From 7c095659a56836212e80241b614f4cf8ffabaf8e Mon Sep 17 00:00:00 2001 From: rfay Date: Sat, 8 Aug 2026 04:27:28 +0000 Subject: [PATCH 5/6] fix(ci): fix wrong JSON field path in the ci-lock preflight check `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 --- scripts/ci-acquire-staging-lock.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci-acquire-staging-lock.sh b/scripts/ci-acquire-staging-lock.sh index ecb42c2..1e6c8b8 100755 --- a/scripts/ci-acquire-staging-lock.sh +++ b/scripts/ci-acquire-staging-lock.sh @@ -97,7 +97,7 @@ if ! templates_json=$(coder templates list --output json 2>&1); then echo "$templates_json" >&2 exit 1 fi -if ! echo "$templates_json" | jq -e '[.[] | select(.name=="ci-lock")] | length > 0' >/dev/null 2>&1; then +if ! echo "$templates_json" | jq -e '[.[] | select(.Template.name=="ci-lock")] | length > 0' >/dev/null 2>&1; then echo "ERROR: no 'ci-lock' template found on this Coder deployment." >&2 echo "Run 'make push-all-templates' against it once (see docs/admin/server-setup.md)." >&2 exit 1 From e4565e2b6236a875330dd06a520cf73962cf5730 Mon Sep 17 00:00:00 2001 From: rfay Date: Sat, 8 Aug 2026 12:36:53 +0000 Subject: [PATCH 6/6] fix(ci): age-gate version archiving, stagger concurrent workspace starts 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 --- scripts/ci-acquire-staging-lock.sh | 32 +++++++++++++++++++ scripts/ci-reap-staging.sh | 51 +++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/scripts/ci-acquire-staging-lock.sh b/scripts/ci-acquire-staging-lock.sh index 1e6c8b8..3cf82aa 100755 --- a/scripts/ci-acquire-staging-lock.sh +++ b/scripts/ci-acquire-staging-lock.sh @@ -21,6 +21,12 @@ # (see ci-lock/template.tf) that these slots are provisioned from has no # Docker/Sysbox/agent, so claiming a slot is near-instant. # +# The box can comfortably run several workspaces at once once they're up -- +# the resource spike is the *start* of each one (Docker image builds, +# composer installs). So on top of the slot count, STAGGER_SECONDS enforces +# a minimum gap between successive workspace starts even when slots are +# free, so simultaneous starts don't all hit their heaviest work together. +# # Usage: # ci-acquire-staging-lock.sh # # on success, prints the acquired slot name and (if $GITHUB_ENV is set) @@ -32,6 +38,8 @@ # MAX_WAIT_SECONDS Give up and fail after this long (default: 2700 = 45m) # POLL_SECONDS Delay between full passes over all slots (default: 30) # STALE_MINUTES Force-reclaim a held slot older than this (default: 30) +# STAGGER_SECONDS Minimum gap enforced between two workspace starts, even +# when slots are available (default: 90) set -uo pipefail @@ -40,6 +48,7 @@ CI_OWNER="${CI_OWNER:-ci-bot}" MAX_WAIT_SECONDS="${MAX_WAIT_SECONDS:-2700}" POLL_SECONDS="${POLL_SECONDS:-30}" STALE_MINUTES="${STALE_MINUTES:-30}" +STAGGER_SECONDS="${STAGGER_SECONDS:-90}" acquired="" last_err="" @@ -87,6 +96,28 @@ reclaim_stale_slots() { echo "$reclaimed" } +# Sleeps out the remainder of STAGGER_SECONDS since the most recently created +# *other* lock slot, if any, so this job's workspace create doesn't land in +# the same CPU spike as one that just started. A no-op when no other slot was +# created recently (the common case outside a start burst). +stagger_if_needed() { + local slots_json newest_other_created other_epoch now gap sleep_for + slots_json=$(coder list --all --output json 2>/dev/null) || return + newest_other_created=$(echo "$slots_json" | + jq -r --arg owner "$CI_OWNER" --arg mine "$acquired" \ + '[.[] | select(.owner_name==$owner) | select(.name | startswith("ci-slot-")) | select(.name != $mine) | .latest_build.created_at] | max // empty') + [[ -z "$newest_other_created" ]] && return + other_epoch=$(date -d "$newest_other_created" +%s 2>/dev/null || echo 0) + [[ "$other_epoch" -eq 0 ]] && return + now=$(date +%s) + gap=$((now - other_epoch)) + if [[ "$gap" -lt "$STAGGER_SECONDS" ]]; then + sleep_for=$((STAGGER_SECONDS - gap)) + echo "Staggering start: another job's slot started ${gap}s ago; waiting ${sleep_for}s more" + sleep "$sleep_for" + fi +} + # Fail fast if the ci-lock template itself is missing (e.g. never pushed to # this Coder deployment) rather than retrying a doomed `coder create` for the # full MAX_WAIT_SECONDS -- that failure mode wastes 45 minutes per job with no @@ -111,6 +142,7 @@ waited=0 while true; do if try_acquire_pass; then echo "Acquired staging box lock slot: $acquired" + stagger_if_needed if [[ -n "${GITHUB_ENV:-}" ]]; then echo "CI_LOCK_SLOT=$acquired" >>"$GITHUB_ENV" fi diff --git a/scripts/ci-reap-staging.sh b/scripts/ci-reap-staging.sh index bab6d5f..468fd9f 100755 --- a/scripts/ci-reap-staging.sh +++ b/scripts/ci-reap-staging.sh @@ -22,7 +22,16 @@ # $AGE_MINUTES (default 20) # - pending / starting / stopping / canceling / deleting -> left alone # (transitional; caught on a later pass) -# Then archives all unused template versions for each CI template. +# Then archives unused template versions older than $VERSION_AGE_MINUTES for +# each CI template. +# +# Why the version-age floor matters: a job pushes an inactive template +# version, then may wait behind ci-acquire-staging-lock.sh for a free slot +# (up to MAX_WAIT_SECONDS there, currently 45m) before creating a workspace +# from it. Archiving "unused" versions the instant this janitor runs -- as it +# used to -- can archive a version that's mid-wait, not abandoned, causing +# `coder create` to fail with "template version archived". VERSION_AGE_MINUTES +# must stay comfortably above that worst-case wait. # # By default runs in dry-run mode (prints what would be deleted). # Pass --force (or set DRY_RUN=false) to actually delete/archive. @@ -30,10 +39,11 @@ # Requires: coder CLI authenticated as a template-admin who owns the CI workspaces. # # Environment overrides: -# CI_OWNER Coder username that owns CI workspaces (default: ci-bot) -# AGE_MINUTES Age threshold in minutes for running workspaces (default: 20) -# DRY_RUN true|false (default: true; --force sets false) -# TEMPLATES space-separated template names to archive versions for +# CI_OWNER Coder username that owns CI workspaces (default: ci-bot) +# AGE_MINUTES Age threshold in minutes for running workspaces (default: 20) +# VERSION_AGE_MINUTES Age threshold in minutes for unused template versions (default: 60) +# DRY_RUN true|false (default: true; --force sets false) +# TEMPLATES space-separated template names to archive versions for # # Usage: # ./scripts/ci-reap-staging.sh # dry run @@ -44,6 +54,7 @@ set -euo pipefail CI_OWNER="${CI_OWNER:-ci-bot}" AGE_MINUTES="${AGE_MINUTES:-20}" +VERSION_AGE_MINUTES="${VERSION_AGE_MINUTES:-60}" DRY_RUN="${DRY_RUN:-true}" TEMPLATES="${TEMPLATES:-drupal-core drupal-contrib freeform}" @@ -115,15 +126,31 @@ echo "Workspaces: reaped=$reaped kept=$kept (owner=$CI_OWNER, age=${AGE_MINUTES} echo # --- Template versions --- -# `coder templates archive --all` archives every unused version (never the -# active one, never one with a live workspace), which is exactly what we want. -echo "Archiving unused template versions..." +# `coder templates archive --all` would archive every unused version +# (never the active one, never one with a live workspace) immediately -- but +# "unused right now" also matches a version that was just pushed and is still +# waiting behind ci-acquire-staging-lock.sh for a free slot. Enumerate and +# filter by age ourselves instead, mirroring the workspace loop above. +version_cutoff=$((now - VERSION_AGE_MINUTES * 60)) +echo "Archiving unused template versions older than ${VERSION_AGE_MINUTES}m..." for t in $TEMPLATES; do - if [[ "$DRY_RUN" == "true" ]]; then - echo " would: coder templates archive $t --all --yes" - else - coder templates archive "$t" --all --yes || echo " WARN: archive failed for $t" >&2 + if ! versions_json=$(coder templates versions list "$t" --output json 2>/dev/null); then + echo " WARN: 'coder templates versions list $t' failed; skipping" >&2 + continue fi + while IFS=$'\t' read -r name created_at; do + [[ -z "$name" ]] && continue + version_epoch=$(date -d "$created_at" +%s 2>/dev/null || echo 0) + if [[ "$version_epoch" -eq 0 || "$version_epoch" -ge "$version_cutoff" ]]; then + continue + fi + if [[ "$DRY_RUN" == "true" ]]; then + echo " would archive: $t $name" + else + coder templates versions archive "$t" --yes "$name" || echo " WARN: archive failed for $t $name" >&2 + fi + done < <(echo "$versions_json" | + jq -r '.[] | select(.active == false) | select(.TemplateVersion.archived == false) | [.TemplateVersion.name, .TemplateVersion.created_at] | @tsv') done echo