diff --git a/README.md b/README.md index d892de9..973fd6f 100644 --- a/README.md +++ b/README.md @@ -60,11 +60,20 @@ with: runs-on: '["self-hosted", "linux", "x64"]' ``` -**Exception:** `claude.yml` and `claude-code-review.yml` default to the -self-hosted pool (`'["self-hosted", "linux", "x64"]'`) since Claude jobs -run on private repos. Public/open-source consumers opt out with +**Exception:** `claude.yml` defaults to the self-hosted pool +(`'["self-hosted", "linux", "x64"]'`). Consumers opt out with `runs-on: '["ubuntu-latest"]'`. +`claude-code-review.yml` **defaulted to self-hosted until v3.4.0** and now +defaults to `'["ubuntu-24.04"]'`. It reads a PR diff and calls the Claude API, +so it needs nothing a self-hosted runner provides — and on a contended pool it +queued 40-77 minutes for 3-6 minutes of work, so reviews landed after the PR +had merged. Pass `runs-on: '["self-hosted", "linux", "x64"]'` to opt back in. + +Consumer-side runner conventions (the `RUNNER_POOL` and `DIND_POOL` variables, +the jobs deliberately pinned to self-hosted, and how to flip or revert either) +are documented in [RUNNERS.md](RUNNERS.md). + The Sproncy self-hosted fleet uses capability labels (`dind`, `fast`, `slow` for AI workloads). `.github/actionlint.yaml` declares them so `actionlint` doesn't false-positive. @@ -115,8 +124,9 @@ with `fromJSON`. Pass a quoted JSON array — not a bare label: A bare string like `runs-on: ubuntu-latest` (or unquoted `'[self-hosted, ...]'`) will not parse and the job will fail to schedule. The default is -`'["ubuntu-latest"]'` for every reusable except the `claude*` bot workflows, -which default to the self-hosted pool. +`'["ubuntu-latest"]'` for every reusable except `claude.yml`, which defaults to +the self-hosted pool. (`claude-code-review.yml` did too until v3.4.0; it now +defaults to `'["ubuntu-24.04"]'` — see [RUNNERS.md](RUNNERS.md).) ## Consuming a composite action @@ -188,8 +198,9 @@ steps: ## Self-hosted runners Every reusable workflow accepts a `runs-on` input. Most default to -`'["ubuntu-latest"]'`; the Claude workflows (`claude.yml`, -`claude-code-review.yml`) default to `'["self-hosted", "linux", "x64"]'`. +`'["ubuntu-latest"]'`; `claude.yml` defaults to +`'["self-hosted", "linux", "x64"]'`. `claude-code-review.yml` did too until +v3.4.0 and now defaults to `'["ubuntu-24.04"]'`. Pass `runs-on: '["self-hosted", "linux", "x64"]'` (or a longer label array like `'["self-hosted", "linux", "x64", "dind"]'`) to route to your own runner pool, or `runs-on: '["ubuntu-latest"]'` to opt back onto diff --git a/RUNNERS.md b/RUNNERS.md new file mode 100644 index 0000000..5fc19a6 --- /dev/null +++ b/RUNNERS.md @@ -0,0 +1,191 @@ +# Runner selection + +Which machine a job runs on, how to change it, and what must not move. + +Two conventions exist, they default in **opposite directions**, and that is +deliberate. There is also one default that lives in this repo rather than in +the consumers. + +--- + +## Why any of this exists + +The self-hosted pool is small and shared across every org. Jobs that need +nothing from it — reading a diff, calling an API, linting — were queueing +behind jobs that genuinely need a real machine. + +Measured on the `sproncy` pool before any of this landed: + +| run created | job started | finished | queued | actually ran | +| ----------- | ----------- | -------- | ------ | ------------ | +| 19:37:04 | 20:54:07 | 20:59:14 | 77 min | 5m 07s | +| 22:18:25 | 23:02:23 | 23:05:28 | 44 min | 3m 05s | +| 17:25:19 | 18:26:13 | 18:29:42 | 61 min | 3m 29s | + +Long queues are the visible cost. The invisible ones were worse: reviews +posting *after* the PR had merged, where nothing surfaces the findings, and +jobs failing outright at runner assignment (`steps_run=0`) in roughly one run +in six. + +--- + +## The three mechanisms + +### 1. `runs-on` input on the reusables — see the README + +Defaults and the JSON-array calling convention are documented in +[README.md](README.md#runner-strategy). The one thing to know here: as of +v3.4.0 `claude-code-review.yml` defaults to a GitHub-hosted image, while +`claude.yml` still defaults to the self-hosted pool. + +The rest of this file covers the two **consumer-side** variables, which the +README does not describe because they live in the consuming repos rather than +in this one. + +### 2. `RUNNER_POOL` — ordinary CI, defaults to **self-hosted** + +```yaml +runs-on: ${{ fromJSON(vars.RUNNER_POOL || '["self-hosted","linux","x64"]') }} +``` + +Unset means today's behaviour. Setting the variable moves the repo. + +**Why this way round:** the safe default is the status quo. A deleted or +mistyped variable degrades to self-hosted rather than somewhere surprising. + +### 3. `DIND_POOL` — service-container jobs, defaults to **hosted** + +```yaml +runs-on: ${{ fromJSON(vars.DIND_POOL || '["ubuntu-24.04"]') }} +``` + +**Why the other way round:** here hosted *is* the fix. Service containers are +native on hosted runners — the runner is itself a container on the same +network, so `services.` resolves without DinD. The org has a single +`dind` runner shared across every repo, so the self-hosted path queues behind +whatever else wants it. Defaulting to self-hosted would preserve the fault. + +The move is worth the asymmetry: + +| job | before (self-hosted) | after (hosted) | +| ---------------------------------- | --------------------- | -------------- | +| distillery `integration-test` | 18m 36s | 59s | +| transcription `Integration tests` | queued / cancelled | 70s | +| scrapers `Integration Tests` | cancelled after 25m | 6m 27s | + +--- + +## Flipping and reverting + +Repo scope (preferred — narrow blast radius): + +```bash +gh variable set RUNNER_POOL --repo / --body '["ubuntu-24.04"]' +gh variable delete RUNNER_POOL --repo / # back to self-hosted +``` + +Org scope (every repo in the org at once): + +```bash +gh variable set RUNNER_POOL --org --visibility all --body '["ubuntu-24.04"]' +``` + +Repo variables take precedence over org variables, so a repo-level value wins. + +**The value must be a valid JSON array.** `fromJSON` throws on anything else, +and because the expression repeats across jobs, one malformed variable fails +them all at runner assignment with an error that names no cause: + +``` +["ubuntu-24.04"] correct +ubuntu-24.04 breaks every job using the variable +``` + +--- + +## Current state + +Deliberately not tabulated here. A hand-maintained inventory of state that +lives in another system goes stale — in this fleet a version-pin table drifted +three times in two days, twice while the PR carrying it was open. + +Read it instead: + +```bash +# which repos consume a toggle. `gh search code` does NOT work here -- it does +# not index these private repos and returns an empty result rather than an +# error, which reads as "nothing uses it". Walk the workflows instead: +for repo in $(gh repo list --limit 100 --json nameWithOwner --jq '.[].nameWithOwner'); do + for f in $(gh api "repos/$repo/contents/.github/workflows" --jq '.[].name' 2>/dev/null); do + gh api "repos/$repo/contents/.github/workflows/$f" --jq '.content' 2>/dev/null \ + | base64 -d 2>/dev/null | grep -q "vars.RUNNER_POOL" && echo "$repo $f" + done +done + +# what a repo or org is currently set to +gh api repos///actions/variables --jq '.variables[]|"\(.name)=\(.value)"' +gh api /orgs//actions/variables --jq '.variables[]|"\(.name)=\(.value)"' + +# what a job actually ran on +gh api repos///actions/runs//jobs \ + --jq '.jobs[]|"\(.name) \(.runner_name)"' +``` + +That last one is the only real answer. Everything else is intent. + +--- + +## Deliberate exceptions — do not "fix" these + +Four jobs are pinned to self-hosted on purpose. Flipping a toggle must not +drag them along, which is why each is hardcoded rather than parameterised. + +| where | why | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | +| `Regularmusic/web-platform` `deploy.yml` | Reaches Komodo on the home-lab network. A hosted runner cannot route there. | +| `sproncy/sproncy-secrets` `infisical-rotate.yml` | Decrypts live secret material. On self-hosted that plaintext exists only on hardware we control. | +| `sproncy/sproncy-secrets` `key-rotation.yml` | Same. Both are scheduled or dispatched rather than per-push, so pinning costs almost no capacity — which is why it is worth spending here. | +| `sproncy/monitoring_stack` `claude-code-review.yml` | Passes `runs-on` explicitly. An existing opt-out, not an oversight. | + +--- + +## Gotchas found the hard way + +**Pin the image, not the alias.** `ubuntu-latest` is floating — GitHub moves it +(24.04 today, 26.04 eventually). A dated label makes an image bump an +intentional, reviewable change instead of a silent environment shift mid-run. + +**Not every ubuntu image is equivalent.** `ubuntu-22.04` ships Python 3.10, +which predates `tomllib`. A job relying on a system interpreter can pass on +`ubuntu-24.04` and fail on `ubuntu-22.04`. Check before pointing a variable at +an older image. + +**Dependabot PRs get no repo secrets.** A job needing a secret fails at the +token step on a Dependabot PR regardless of runner. Scope the guard to the +event, not the actor: + +```yaml +if: github.event_name != 'pull_request' || github.actor != 'dependabot[bot]' +``` + +Testing `github.actor` alone looks equivalent and is not: on a `schedule` run +the actor is `github-actions[bot]`, so a bare actor test silently changes +meaning between triggers. + +**A concurrency guard written for shared hardware may outlive its reason.** +`web-platform`'s E2E jobs serialise on fixed ports 3001–3003 because +self-hosted jobs share a box. Hosted runners get an ephemeral VM each, so the +collision cannot happen — but the group is qualified by neither pool nor ref, +so it keeps serialising anyway. Drop or qualify it when flipping that repo. + +**Hosted minutes are metered on private repos**, and each org has its own +allowance that cannot be pooled without an Enterprise plan. Check headroom +before moving a heavy suite: + +```bash +gh api "/organizations//settings/billing/usage?year=&month=" \ + --jq '[.usageItems[]|select(.product=="actions" and (.unitType|test("Minutes")))|.quantity]|add' +``` + +Note the `year`/`month` arguments. Without them the endpoint returns a +truncated page that looks like a complete per-repo breakdown and is not.