From 7868feea62125ccc37367696fca10f438d1f7183 Mon Sep 17 00:00:00 2001 From: August Cayzer Date: Sat, 5 Sep 2026 20:59:06 +0100 Subject: [PATCH] feat: custom runner labels, and a rename command Labels could only be set by hand-editing a pool config. register could not produce one, the pools file could not express one, apply could not see one, and anything that recreated the pool dropped it silently, which is a workflow queuing for ever against a pool reporting perfect health. --labels now appends to the base set on register and in the pools file. POOL_LABELS stays the full authoritative list and the extras are derived back out of it every time, so a config edited by hand is respected rather than reverted. runpool pools shows them. rename moves a pool's directories, config, launch agents and state, then re-registers every runner with GitHub. It deletes the old registrations rather than relying on --replace, which only covers a name collision and so would strand them: offline, unreachable, and still advertising the old pool name as a label. reregister now takes the reconfiguration lock, which it never did. Closes #66 Closes #67 --- .github/workflows/ci.yml | 4 + AGENTS.md | 21 ++- README.md | 5 +- bin/runpool | 29 ++++- lib/apply.sh | 78 +++++++++-- lib/common.sh | 66 ++++++++++ lib/lifecycle.sh | 271 +++++++++++++++++++++++++++++++++++++-- lib/scheduler.sh | 10 +- runpool.pools.example | 21 ++- skills/runpool/SKILL.md | 29 ++++- tests/pool-labels.sh | 185 ++++++++++++++++++++++++++ tests/pool-rename.sh | 246 +++++++++++++++++++++++++++++++++++ 12 files changed, 938 insertions(+), 27 deletions(-) create mode 100755 tests/pool-labels.sh create mode 100755 tests/pool-rename.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1268e1..f0955b5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,3 +55,7 @@ jobs: run: tests/pool-settling-window.sh - name: stuck queue guard run: tests/stuck-queue-guard.sh + - name: pool labels + run: tests/pool-labels.sh + - name: pool rename + run: tests/pool-rename.sh diff --git a/AGENTS.md b/AGENTS.md index 8eff6f2..5e0ff76 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,7 @@ Break any of these and the tool stops being what it is. ``` bin/runpool the executable, its dispatcher, its help text, and RUNPOOL_VERSION lib/common.sh config, logging, pool loading, launch agents, deregistration -lib/lifecycle.sh register, set-count, up, down, reregister, remove +lib/lifecycle.sh register, set-count, up, down, reregister, rename, remove lib/apply.sh the pools file, and reconciling the machine to it lib/scheduler.sh status, doctor, autoscale, sweep, clean, schedule lib/notify.sh the optional notifier hook and what triggers it @@ -59,6 +59,24 @@ assets/icon.svg the icon, source of truth; PNGs are rendered from it **The consequence is that an agent already loaded is not necessarily an agent that behaves correctly.** A plist rewritten on disk changes nothing until the pool cycles. Anything depending on agent behaviour must therefore read the *loaded* environment with `launchctl print`, not the file. `_rp_agent_traps_signals` is the example, and `_rp_drain_pool` refuses per runner on the strength of it. The file on disk is what somebody intended; the loaded environment is what is true. +## The pool name is a label + +**`POOL_LABELS` is the full list handed to `config.sh`, and the extras are derived back out of it.** There is deliberately no `POOL_EXTRA_LABELS`. A second field would be a frozen copy of `_rp_extra_labels`, and configs get edited by hand: the copy would go stale on the next hand edit and the following `apply` would silently revert it, which is the very defect `--labels` exists to fix. + +- **`--labels` appends and cannot replace.** GitHub assigns `self-hosted`, the OS and the architecture to every self-hosted runner whatever it is told, so a replacing flag would promise something GitHub overrides. The pool name is in the base set too, because it is the routing contract. +- **An implicit label is refused, not dropped.** A token accepted and then normalised away would leave what the pools file declares and what the config holds permanently unequal, and `apply` would re-register the pool on every run. +- **Extras are compared sorted and stored in declared order.** Unlike `--watch`, which is compared as written: the costs are not symmetric, because a spurious watch difference is one file write and a spurious label difference stands the whole pool down. +- **`_rp_extra_labels` is tolerant and `_rp_valid_label` is strict.** The first runs against whatever a human left in a config and must only filter; the second guards the way in. That character class is what keeps a sourced config safe, `apply`'s `|` separator intact, and `rename`'s `find -name` free of globs. + +## Renaming moves, and must deregister + +**`rename` uses `mv`, unlike `migrate-storage`, which copies.** The copy there is because it crosses storage roots, where a partial copy is real and the old tree is the safety net. A rename keeps the same parent by construction, so `mv` is atomic and a second on-disk copy of runner credentials buys nothing. + +- **It holds two locks, old then new, released in reverse.** From the moment the new config exists, `_rp_pool_names` returns it and autoscale would start it mid-rename. The second acquisition failing has to release the first. +- **`config.sh --replace` cannot help.** It replaces a registration of the same name, and the name is what changes, so GitHub keeps the old one: permanently offline, still carrying the old pool name as a label, and unreachable afterwards because `config.sh` overwrites the `.runner` holding its `agentId`. The old registrations are therefore deleted explicitly, before reconfiguring. +- **It iterates the runner directories that exist, not `1..POOL_COUNT`.** A count lowered by hand leaves higher-numbered runners on disk and registered; those are deregistered and deliberately not re-registered, which would be a permanent `miscount`. +- **`_rp_migrate_update_pool_conf` is not reusable here.** Its `END` clause adds `POOL_CACHE_DIR` when absent, and that absence is exactly how `_rp_load_pool` recognises a legacy pool. `rename` writes its own config instead, in one write, omitting the field when the pool did not have it. + ## The stuck-queue guard subtracts, it does not suppress **A queued run that never starts would otherwise wake a pool for ever**, every `RUNPOOL_IDLE_SECS` plus a tick, and nothing reports it because a pool that wakes and stands down is behaving as designed. `_rp_autoscale` therefore computes `queued > held` rather than deciding whether the pool is allowed to wake. @@ -75,6 +93,7 @@ assets/icon.svg the icon, source of truth; PNGs are rendered from it - **The holder writes its pid into the lock.** `_rp_set_count_locked` calls `_rp_up` at the end of its own work while still holding the lock, so a test for mere existence would deadlock every resize against itself. `_rp_resize_locked_by_other` is the predicate to use. - **Staleness is measured from the lock's mtime**, and a drain refreshes it every poll. So an old lock means nobody is tending it, not that the work is slow. Anything that can hold the lock for a long time must call `_rp_resize_lock_touch`. +- **`rename` holds two, old then new, released in reverse.** The lock directory carries the pool name, so one lock cannot cover both. They cannot deadlock, because `_rp_resize_lock` refuses rather than blocks, and the second acquisition failing must release the first. - **A dead holder is not an obstacle.** It left the directory behind, and the stale break in `_rp_resize_lock` is what clears it. ## Configuration precedence diff --git a/README.md b/README.md index d2ea679..92ec3b6 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ The first job after a quiet spell waits about a minute for its pool to come up. | Command | | |---|---| -| `register --repo OWNER/REPO\|--org ORG [--count N] [--watch OWNER/REPO,...] [--allow-public]` | Create a pool and configure its runners | +| `register --repo OWNER/REPO\|--org ORG [--count N] [--watch OWNER/REPO,...] [--labels LABEL,...] [--allow-public]` | Create a pool and configure its runners | | `set-count N [--if-count M] [--drain]` | Change a pool's runner count. `--if-count` refuses unless it is currently M; `--drain` lets running jobs finish first | | `apply [--dry-run] [--file PATH]` | Reconcile the machine to a file describing its pools | | `up` / `down [--drain\|--force]` | Bring a pool online, or stand it down. `--drain` waits for running jobs; `--force` ends them | @@ -67,6 +67,7 @@ The first job after a quiet spell waits about a minute for its pool to come up. | `stats [--queue] [--days N \| --all]` | What jobs cost, from recorded telemetry. `--queue` adds the wait before each job started, over the last 7 days unless widened | | `pause [pool]` / `resume [pool]` | Global kill switch, or persistent per-pool pause | | `reregister ` | Recreate GitHub registrations, keeping the local install | +| `rename [--drain]` | Rename a pool, locally and at GitHub | | `rewrite-agents` | Regenerate the launch agents after changing hook settings | | `remove ` | Deregister and delete a pool | | `clean [pool]` | Prune work directories, temp, diagnostics, old binaries, caches | @@ -83,6 +84,8 @@ Three commands earn a note beyond the table: - **The pools file is intent; the running pool is state.** `set-count` changes the pool and deliberately does not write the file, so the two disagree after any resize. That is the normal condition between them rather than a fault: the file records the shape you want a machine to have and is what you copy between machines, while the pool records what is running right now. `apply` is where they are reconciled, and it resolves the difference in the file's favour, so `apply --dry-run` first is not a formality. A `count 3 -> 4` line in that plan is the drift, and applying it would undo a deliberate resize. - **`set-count` is absolute, so a caller that reads a count and acts on it later needs `--if-count`.** A pool changed in between turns a growth into a shrink, and shrinking deregisters runners. `--if-count M` refuses unless the pool is still at M, and one resize per pool runs at a time so two callers cannot interleave. A runner deregistered locally that GitHub still holds is reported as a failure, not logged and passed over: a stale registration attracts jobs that then queue forever. - **`status --json --local` skips the GitHub query**, reporting those fields as `null`. The root `paused` field is the global kill switch; every pool also carries its own additive `paused` field. Anything refreshing on a timer should use `--local`, since one API call per pool per minute is thousands a day and makes a passive readout fail whenever the network does. +- **The pool's name is one of its runners' labels, so renaming changes routing.** Every runner carries `self-hosted`, the machine's OS and architecture, the pool's name, and anything `--labels` adds. `rename` moves the pool's directories, config, launch agents and state, then re-registers every runner under the new name, which means `runs-on: [self-hosted, ]` stops matching and has to be updated. It deletes the old GitHub registrations rather than replacing them, because `--replace` only covers a name collision and the name is what is changing; a registration left behind would be permanently offline while still advertising the old label. `rename` does not touch the pools file, so update that too. +- **A label change re-registers the whole pool.** Labels live on GitHub's registration, not in a file GitHub reads, so `apply` applies one by standing the pool down and re-registering every runner. That is far heavier than a count or watch-list change and the plan says so. Absent `--labels` means no extra labels, the same way absent `--count` would mean the default: a pool whose config was edited by hand to add a label needs that label declared in the file, or the next `apply` removes it. - **`doctor` answers "why is nothing picking this up" in one command.** It checks `gh` and its authentication, that GitHub still holds the registrations, that the launch agents exist, and then disk headroom, config permissions and the organisation's runner-group setting. Each failure comes with what to do about it, and it exits non-zero when something is actually wrong. It repairs nothing, so it is safe at any moment including mid-job. ## Describing a machine's pools diff --git a/bin/runpool b/bin/runpool index 5190559..40b8189 100755 --- a/bin/runpool +++ b/bin/runpool @@ -43,7 +43,7 @@ export RUNPOOL_INVOKED # The released version, and the only place it is written. The Homebrew formula # builds from a git tag, so a tag without a matching bump here ships a binary # that misreports itself. -RUNPOOL_VERSION="0.11.0" +RUNPOOL_VERSION="0.12.0" # shellcheck source=lib/common.sh . "${RUNPOOL_ROOT}/lib/common.sh" @@ -83,6 +83,7 @@ Commands: pools List registered pools stats [options] Show recorded job durations and queue times reregister Recreate a pool's GitHub registrations + rename Rename a pool, locally and at GitHub remove Deregister and delete a pool tick Run autoscaling and pool health checks autoscale Bring pools up when work is queued @@ -110,7 +111,32 @@ Options: --org ORG Register an organisation-scoped pool --count Set the runner count (default: 2) --watch OWNER/REPO,... Repositories an organisation pool watches for work + --labels LABEL,... Extra runner labels, on top of self-hosted, macOS, ARM64 + and the pool's own name. Letters, digits, dot, + underscore and hyphen; several separated by commas --allow-public Allow a public repository (repository scope only) +HELP + ;; + rename) + cat <<'HELP' +Usage: runpool rename [options] + +Rename a pool. Moves its directories, config, launch agents and state, then +re-registers every runner with GitHub under the new name. + +Options: + --drain Let running jobs finish first, then rename + --timeout How long --drain waits + +The pool name is one of the runner's labels, so 'runs-on: [self-hosted, ]' +stops matching and has to be changed to the new name. A workflow asking only +for 'self-hosted' is unaffected. + +The old GitHub registrations are deleted rather than replaced, because +--replace only covers a name collision and the name is what is changing. + +The pools file is not touched. Update it too, or the next 'runpool apply' will +create the old pool again. HELP ;; set-count) @@ -234,6 +260,7 @@ case "${cmd}" in set-count) _rp_set_count "$@" ;; apply) _rp_apply "$@" ;; reregister) _rp_reregister "$@" ;; + rename) _rp_rename "$@" ;; up) _rp_up "$@" ;; down) _rp_down "$@" ;; up-all) _rp_up_all ;; diff --git a/lib/apply.sh b/lib/apply.sh index 9ef7ded..389e36a 100644 --- a/lib/apply.sh +++ b/lib/apply.sh @@ -131,11 +131,11 @@ _rp_parse_pools_file() ( esac declared="${declared}${name} " - scope=""; target=""; count="2"; watch=""; allow="0" + scope=""; target=""; count="2"; watch=""; labels=""; allow="0" while [ $# -gt 0 ]; do tok="$1" case "${tok}" in - --org|--repo|--count|--watch) + --org|--repo|--count|--watch|--labels) # Checked before shifting two. 'shift 2' with one argument left is a # failure that does not shift, which turns this into an infinite loop # rather than an error. @@ -150,6 +150,7 @@ _rp_parse_pools_file() ( ;; --count) count="${val}" ;; --watch) watch="${watch},${val}" ;; + --labels) labels="${labels},${val}" ;; esac ;; --allow-public) allow="1"; shift ;; *) _rp_err "${file}:${lineno}: unknown field '${tok}'"; return 1 ;; @@ -210,7 +211,31 @@ _rp_parse_pools_file() ( watch="${clean#,}" fi - printf '%s|%s|%s|%s|%s|%s\n' "${name}" "${scope}" "${target}" "${count}" "${watch}" "${allow}" + # Extra labels only. The base three and the pool's own name are on every + # runner regardless, so naming one here is refused rather than dropped, for + # the reason register states: a token accepted and then normalised away + # would leave declared and derived permanently unequal and re-register the + # pool on every apply. Rebuilt from what was validated, like the watch list + # above and for the same reason. + labels="${labels#,}"; labels="${labels// /}" + if [ -n "${labels}" ]; then + clean="" + for tok in $(echo "${labels}" | tr ',' ' '); do + _rp_valid_label "${tok}" || { + _rp_err "${file}:${lineno}: --labels: $(_rp_label_rule), got '${tok}'"; return 1; } + case ",${RUNPOOL_BASE_LABELS},${name}," in + *",${tok},"*) _rp_err "${file}:${lineno}: --labels: '${tok}' is on every runner in this pool already and cannot be given again"; return 1 ;; + esac + case ",${clean}," in *",${tok},"*) continue ;; esac + clean="${clean},${tok}" + done + labels="${clean#,}" + fi + + # Labels are APPENDED as a seventh field rather than inserted. An empty + # field in the middle is fine, which is why '|' beat a tab, but reordering + # would silently shift every variable in both reads below. + printf '%s|%s|%s|%s|%s|%s|%s\n' "${name}" "${scope}" "${target}" "${count}" "${watch}" "${allow}" "${labels}" count_declared=$(( count_declared + 1 )) done < "${file}" || { _rp_err "${file}: could not be read"; return 1; } @@ -229,8 +254,8 @@ _rp_plan_line() { printf " %s %-12s %-4s %-24s %s\n" "$1" "$2" "$3" "$4" "$5"; _rp_apply() { # Every local declared once, at the top. local dry=0 file="${RUNPOOL_POOLS_FILE}" records actions="" rc=0 seen=" " \ - name scope target count watch allow verb chg_count chg_watch \ - have_count have_watch what p \ + name scope target count watch allow labels verb chg_count chg_watch chg_labels \ + have_count have_watch have_labels what p n_reregister=0 \ n_create=0 n_change=0 n_same=0 n_conflict=0 n_absent=0 n_failed=0 while [ $# -gt 0 ]; do @@ -281,7 +306,7 @@ _rp_apply() { # Records arrive on fd 3 rather than stdin. `register` shells out to the # runner's config.sh and to gh, and anything in the loop body that read stdin # would eat the rest of the plan. - while IFS='|' read -r name scope target count watch allow <&3; do + while IFS='|' read -r name scope target count watch allow labels <&3; do # A file that declares no pools is a valid state, not an error, but an # empty record set still feeds one empty line through the printf below, # and that reads back as a pool with no name and plans a create for it. @@ -292,9 +317,10 @@ _rp_apply() { n_create=$(( n_create + 1 )) what="create with ${count} runner(s)" [ -n "${watch}" ] && what="${what}, watching ${watch}" + [ -n "${labels}" ] && what="${what}, labelled ${labels}" [ "${allow}" = "1" ] && what="${what} [--allow-public]" _rp_plan_line "+" "${name}" "${scope}" "${target}" "${what}" - actions="${actions}create|${name}|${scope}|${target}|${count}|${watch}|${allow}|0|0 + actions="${actions}create|${name}|${scope}|${target}|${count}|${watch}|${allow}|${labels}|0|0|0 " continue fi @@ -321,7 +347,11 @@ _rp_apply() { # not recorded anywhere: it is permission to perform a create, consulted # once by `register` and meaningless afterwards. Adding or removing it on a # pool that already exists changes nothing, so '=' is the truthful answer. - chg_count=0; chg_watch=0 + # Derived from POOL_LABELS every time rather than stored beside it, so a + # config somebody has edited by hand is respected rather than reverted. + have_labels="$(_rp_extra_labels "${POOL_LABELS:-}" "${name}")" + + chg_count=0; chg_watch=0; chg_labels=0 what="" [ "${count}" != "${have_count}" ] && { chg_count=1; what="count ${have_count} -> ${count}"; } if [ "${watch}" != "${have_watch}" ]; then @@ -329,6 +359,19 @@ _rp_apply() { [ -n "${what}" ] && what="${what}; " what="${what}watch ${have_watch:-(none)} -> ${watch:-(none)}" fi + # Compared sorted while stored in the order declared, unlike the watch list + # just above. The asymmetry is deliberate and the cost is not symmetric: a + # spurious watch difference costs one file write, a spurious label + # difference stands the whole pool down and re-registers every runner. + if [ "$(_rp_sorted_labels "${labels}")" != "$(_rp_sorted_labels "${have_labels}")" ]; then + chg_labels=1 + n_reregister=$(( n_reregister + 1 )) + [ -n "${what}" ] && what="${what}; " + what="${what}labels ${have_labels:-(none)} -> ${labels:-(none)}" + # Say what stops matching, not just what changed. A label removed here is + # a workflow that queues for ever against a pool reporting perfect health. + [ -n "${have_labels}" ] && what="${what} (a workflow using 'runs-on: [self-hosted, ${have_labels%%,*}]' stops matching)" + fi if [ -z "${what}" ]; then n_same=$(( n_same + 1 )) @@ -338,7 +381,7 @@ _rp_apply() { n_change=$(( n_change + 1 )) _rp_plan_line "~" "${name}" "${scope}" "${target}" "${what}" - actions="${actions}change|${name}|${scope}|${target}|${count}|${watch}|0|${chg_count}|${chg_watch} + actions="${actions}change|${name}|${scope}|${target}|${count}|${watch}|0|${labels}|${chg_count}|${chg_watch}|${chg_labels} " done 3< <(printf '%s\n' "${records}") @@ -357,7 +400,7 @@ _rp_apply() { if [ "${dry}" != "1" ] && [ -n "${actions}" ]; then echo echo " applying:" - while IFS='|' read -r verb name scope target count watch allow chg_count chg_watch <&3; do + while IFS='|' read -r verb name scope target count watch allow labels chg_count chg_watch chg_labels <&3; do [ -n "${verb}" ] || continue case "${verb}" in create) @@ -369,10 +412,24 @@ _rp_apply() { # which left a window in which a create could succeed and the watch # list never arrive. [ -n "${watch}" ] && set -- "$@" --watch "${watch}" + [ -n "${labels}" ] && set -- "$@" --labels "${labels}" [ "${allow}" = "1" ] && set -- "$@" --allow-public _rp_register "$@" || { n_failed=$(( n_failed + 1 )); rc=1; } ;; change) + # Labels first, and much the heaviest of the three: they live on + # GitHub's registration, so changing them means standing the pool + # down and re-registering every runner. The config is written before + # that, because _rp_reregister reads POOL_LABELS back through + # _rp_load_pool; if the write lands and the re-register does not, + # 'runpool reregister ' finishes the job. + if [ "${chg_labels}" = "1" ]; then + if _rp_write_pool_labels "${name}" "$(_rp_pool_labels "${name}" "${labels}")"; then + _rp_reregister "${name}" || { n_failed=$(( n_failed + 1 )); rc=1; } + else + n_failed=$(( n_failed + 1 )); rc=1 + fi + fi if [ "${chg_watch}" = "1" ]; then _rp_write_pool_watch "${name}" "${watch}" || { n_failed=$(( n_failed + 1 )); rc=1; } fi @@ -388,6 +445,7 @@ _rp_apply() { echo echo " ${n_create} to create, ${n_change} to change, ${n_same} unchanged, ${n_absent} not in the file" + [ "${n_reregister}" -gt 0 ] && echo " ${n_reregister} pool(s) will be re-registered: a label change recreates every runner's registration with GitHub and leaves the pool stopped" [ "${n_conflict}" -gt 0 ] && echo " ${n_conflict} conflict(s): scope or target differs. See the '!' lines above" [ "${n_failed}" -gt 0 ] && echo " ${n_failed} failed: see the messages above" if [ "${dry}" = "1" ]; then diff --git a/lib/common.sh b/lib/common.sh index aa2ed19..e8e4a53 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -277,6 +277,72 @@ _rp_valid_count() { # rule differently. _rp_count_rule() { echo "a runner count is a whole number from 1 to 9999, written without a leading zero"; } +# --------------------------------------------------------------------------- +# Runner labels +# --------------------------------------------------------------------------- +# GitHub gives every self-hosted runner these three whatever `config.sh` is +# told, so they are structural rather than ours to choose. RunPool contributes +# the pool name and whatever `--labels` adds. +RUNPOOL_BASE_LABELS="self-hosted,macOS,ARM64" + +# The same character class as a pool name, and for sharper reasons. A label +# reaches four hazardous places: POOL_LABELS is written into a config file that +# every command SOURCES, so a '$' or a backtick there is command execution on +# every invocation and on every 60-second tick; `apply` separates its records +# with '|'; the pools file is tokenised by word splitting, so whitespace +# silently becomes two fields; and `rename` matches plists with `find -name`. +# One rule closes all four. The 256 is GitHub's own cap, enforced here because +# past it `config.sh` fails deep inside the runner's own log. +_rp_valid_label() { + case "$1" in + ''|.|..) return 1 ;; + *[!A-Za-z0-9._-]*) return 1 ;; + esac + [ "${#1}" -le 256 ] || return 1 + return 0 +} +_rp_label_rule() { echo "a label is letters, digits, dot, underscore and hyphen, up to 256 characters, and several are separated by commas without spaces"; } + +# The full list handed to config.sh, and its inverse. +# +# POOL_LABELS holds the whole list because that is what config.sh needs and +# what anyone reading the config wants to see. The extras are DERIVED from it +# every time rather than stored alongside it: the config is hand-editable, and +# a second field would be a frozen copy of a cheap pure function that goes +# stale the moment somebody edits the first one, which is the very defect this +# feature exists to fix. +# +# $1 pool name, $2 extras (may be empty). +_rp_pool_labels() { + if [ -n "${2:-}" ]; then echo "${RUNPOOL_BASE_LABELS},$1,$2"; else echo "${RUNPOOL_BASE_LABELS},$1"; fi +} + +# $1 the stored list, $2 the pool name. Tolerant on purpose: a hand-edited +# config may hold anything, and this must never fail, only filter. Strictness +# belongs on the way in, in _rp_valid_label. +# +# The pool name's FIRST occurrence is what gets removed, not every one. A pool +# named 'xcode' carrying a genuine 'xcode' label would otherwise lose it on +# rename, and one occurrence is by definition the name label. +_rp_extra_labels() { + local list="$1" name="$2" tok out="" seen_name=0 + for tok in $(echo "${list}" | tr ',' ' '); do + _rp_valid_label "${tok}" || continue + case ",${RUNPOOL_BASE_LABELS}," in *",${tok},"*) continue ;; esac + if [ "${tok}" = "${name}" ] && [ "${seen_name}" = "0" ]; then seen_name=1; continue; fi + case ",${out}," in *",${tok},"*) continue ;; esac + out="${out}${out:+,}${tok}" + done + echo "${out}" +} + +# For comparison only, never for storage. `apply` compares what the pools file +# declares against what the config holds, and an order difference there would +# re-register the whole pool for nothing. +_rp_sorted_labels() { + echo "$1" | tr ',' '\n' | grep -v '^$' | sort | tr '\n' ',' | sed 's/,$//' +} + # Load POOL_* for pool $1 into the caller's scope. POOL_WATCH is set only on # org pools, so every reader still uses "${POOL_WATCH:-}". # diff --git a/lib/lifecycle.sh b/lib/lifecycle.sh index 5843923..dab2f1a 100644 --- a/lib/lifecycle.sh +++ b/lib/lifecycle.sh @@ -15,17 +15,17 @@ _rp_register() { _rp_require tar || return 1 local name="${1:-}"; [ $# -gt 0 ] && shift - [ -n "${name}" ] || { _rp_err "usage: runpool register --repo OWNER/REPO|--org ORG [--count N] [--watch OWNER/REPO,...] [--allow-public]"; return 1; } + [ -n "${name}" ] || { _rp_err "usage: runpool register --repo OWNER/REPO|--org ORG [--count N] [--watch OWNER/REPO,...] [--labels LABEL,...] [--allow-public]"; return 1; } _rp_valid_pool_name "${name}" || { _rp_err "invalid pool name: '${name}'" _rp_err "Letters, digits, dot, underscore and hyphen only: the name becomes a directory, a launch-agent label and a JSON field." return 1 } - local scope="" target="" count="2" watch="" clean="" tok allow_public=0 vis="" + local scope="" target="" count="2" watch="" labels="" clean="" tok allow_public=0 vis="" while [ $# -gt 0 ]; do case "$1" in - --repo|--org|--count|--watch) + --repo|--org|--count|--watch|--labels) # Checked before shifting two, the same way lib/apply.sh checks it. # Without this 'runpool register zz --org' died on a raw '$2: unbound # variable' with an internal line number and no usage message. @@ -38,6 +38,7 @@ _rp_register() { # vocabulary means the same flag given twice means the same thing # in both places. --watch) watch="${watch},$2" ;; + --labels) labels="${labels},$2" ;; esac shift 2 ;; @@ -78,6 +79,34 @@ _rp_register() { watch="${clean#,}" fi + # Extra labels, appended to the base set rather than replacing it. + # + # Replacing is not on offer, and the reason is GitHub's rather than ours: it + # assigns self-hosted, the OS and the architecture to every self-hosted + # runner whatever config.sh is told, so a flag claiming to replace them would + # promise something GitHub silently overrides. The pool name is here too + # because it is the routing contract: a pool that could drop it would go on + # being listed by a name nothing routes to, and its jobs would queue for ever + # against a pool reporting perfect health. + # + # So an implicit label is REFUSED rather than quietly dropped, the same way + # --watch is refused on a repo pool. It also keeps `apply` idempotent: a + # token accepted here and normalised away would leave declared and derived + # permanently unequal, re-registering the pool on every run. + labels="${labels#,}"; labels="${labels// /}" + if [ -n "${labels}" ]; then + clean="" + for tok in $(echo "${labels}" | tr ',' ' '); do + _rp_valid_label "${tok}" || { _rp_err "--labels: $(_rp_label_rule), got '${tok}'"; return 1; } + case ",${RUNPOOL_BASE_LABELS},${name}," in + *",${tok},"*) _rp_err "--labels: '${tok}' is already on every runner in this pool and cannot be given again"; return 1 ;; + esac + case ",${clean}," in *",${tok},"*) continue ;; esac + clean="${clean},${tok}" + done + labels="${clean#,}" + fi + # Whose job the public-repository check is depends on the scope, and the two # cases are genuinely different. # @@ -136,10 +165,10 @@ _rp_register() { esac fi - local dir_base cache_base labels tarball i runner_dir cache_dir runner_name token + local dir_base cache_base tarball i runner_dir cache_dir runner_name token dir_base="${RUNPOOL_RUNNER_DIR}/${name}" cache_base="${RUNPOOL_CACHE_DIR}/pools/${name}" - labels="self-hosted,macOS,ARM64,${name}" + labels="$(_rp_pool_labels "${name}" "${labels}")" tarball="$(_rp_fetch_runner_tarball)" || return 1 mkdir -p "${dir_base}" @@ -232,6 +261,20 @@ _rp_write_pool_watch() { ' "${conf}" >| "${conf}.tmp" && mv -f "${conf}.tmp" "${conf}" } +# The full label list. Rebuilt whole for exactly the reason above, and here the +# corruption that reasoning describes would land on this very field. +# +# $2 is the whole list, not the extras: POOL_LABELS is what config.sh is handed +# and the extras are derived back out of it wherever they are wanted. +_rp_write_pool_labels() { + local conf; conf="$(_rp_pool_conf "$1")" + awk -v v="$2" ' + /^POOL_LABELS=/ { print "POOL_LABELS=\"" v "\""; seen = 1; next } + { print } + END { if (!seen) print "POOL_LABELS=\"" v "\"" } + ' "${conf}" >| "${conf}.tmp" && mv -f "${conf}.tmp" "${conf}" +} + # Arguments used to be read as $1 and $2 with anything further ignored in # silence, which is how a mistyped flag became invisible. They are parsed # properly now and anything unrecognised is refused. @@ -412,10 +455,25 @@ _rp_set_count_locked() { # needs changing, because routing lives in the workflow; only GitHub's record # has to be recreated. # --------------------------------------------------------------------------- +# Held for the whole re-registration, released by the caller rather than a +# trap, for the reason `_rp_set_count` states. Without it this stands the pool +# down and then spends a long time in config.sh with nothing stopping autoscale +# or `up` starting it back up on top of registrations being rewritten. That was +# latent while this was only ever run by hand; `apply` now calls it. _rp_reregister() { + local name="$1" rc _rp_require gh || return 1 - local name="$1" [ -n "${name}" ] || { _rp_err "usage: runpool reregister "; return 1; } + _rp_valid_pool_name "${name}" || { _rp_err "invalid pool name: '${name}'"; return 1; } + _rp_resize_lock "${name}" || return 1 + _rp_reregister_locked "${name}" + rc=$? + _rp_resize_unlock "${name}" + return ${rc} +} + +_rp_reregister_locked() { + local name="$1" _rp_load_pool "${name}" || return 1 _rp_down "${name}" || return 1 @@ -500,10 +558,15 @@ _rp_migrate_update_work_folder() { "${runner_dir}/.runner" >| "${tmp}" && mv -f "${tmp}" "${runner_dir}/.runner" } -_rp_migrate_move_cache_dir() { +# Move a directory, idempotently. Source gone means the move already happened, +# target present means something is in the way. That shape is what makes an +# interrupted `rename` resumable: re-running it finds each completed step done +# and carries on. Two callers now, and it moves runner trees as well as caches, +# which is why it is no longer named for one of them. +_rp_move_dir() { local from="$1" to="$2" [ -d "${from}" ] || return 0 - [ ! -e "${to}" ] || { _rp_err "migration target already has ${to}"; return 1; } + [ ! -e "${to}" ] || { _rp_err "cannot move ${from}: ${to} already exists"; return 1; } mkdir -p "$(dirname "${to}")" || return 1 mv "${from}" "${to}" } @@ -643,8 +706,8 @@ _rp_migrate_storage() { if [ -d "${runner_dir}/_work" ]; then rm -rf "${runner_dir}/_work" || return 1 fi - _rp_migrate_move_cache_dir "${runner_dir}/.pnpm-store" "${cache_dir}/pnpm" || return 1 - _rp_migrate_move_cache_dir "${runner_dir}/.npm-cache" "${cache_dir}/npm" || return 1 + _rp_move_dir "${runner_dir}/.pnpm-store" "${cache_dir}/pnpm" || return 1 + _rp_move_dir "${runner_dir}/.npm-cache" "${cache_dir}/npm" || return 1 # Job temp belongs to the old workspace in the same way. Nothing in it # is durable runner state, so carrying it across creates risk for no gain. rm -rf "${runner_dir}/tmp" "${runner_dir}/.tmp" || return 1 @@ -856,6 +919,194 @@ _rp_down() { _rp_up_all() { local p; for p in $(_rp_pool_names); do _rp_pool_paused "${p}" || _rp_up "${p}"; done; } _rp_down_all() { local p; for p in $(_rp_pool_names); do _rp_down "${p}" "${1:-}"; done; } +# --------------------------------------------------------------------------- +# rename: give a pool a different name, locally and at GitHub +# +# The name is not decoration. It is the config filename, the runner directory, +# the cache directory, the launch-agent labels and their log files, three state +# files, the lock, each runner's registered name at GitHub, and one of the +# labels those runners carry. Changing it by hand means all of that, twice if +# there is a second machine, and getting one wrong is silent. +# +# Moves rather than copies. `migrate-storage` copies because it crosses storage +# roots, where a partial copy is real and the old tree is the safety net. A +# rename keeps the same parent by construction, so `mv` is atomic, and a second +# copy of a tree carrying runner credentials is a cost with nothing to buy. +# --------------------------------------------------------------------------- +_rp_rename_usage() { echo "usage: runpool rename [--drain] [--timeout ]"; } + +_rp_rename() { + local old="" new="" drain=0 timeout="${RUNPOOL_DRAIN_TIMEOUT}" rc + _rp_require gh || return 1 + while [ $# -gt 0 ]; do + case "$1" in + --drain) drain=1; shift ;; + --timeout) + [ $# -ge 2 ] || { _rp_err "--timeout needs a value ($(_rp_rename_usage))"; return 1; } + timeout="$2"; shift 2 ;; + --timeout=*) timeout="${1#*=}"; shift ;; + -*) _rp_err "unknown flag: $1 ($(_rp_rename_usage))"; return 1 ;; + *) + if [ -z "${old}" ]; then old="$1" + elif [ -z "${new}" ]; then new="$1" + else _rp_err "$(_rp_rename_usage)"; return 1 + fi + shift ;; + esac + done + + [ -n "${old}" ] && [ -n "${new}" ] || { _rp_err "$(_rp_rename_usage)"; return 1; } + # Both names, and before either reaches a lock path, a directory or the + # `find -name` pattern that removes the old plists. + _rp_valid_pool_name "${old}" || { _rp_err "invalid pool name: '${old}'"; return 1; } + _rp_valid_pool_name "${new}" || { _rp_err "invalid pool name: '${new}'"; return 1; } + [ "${old}" != "${new}" ] || { _rp_err "'${old}' is already called that"; return 1; } + case "${timeout}" in ''|*[!0-9]*|0) _rp_err "--timeout: whole seconds above zero, got '${timeout}'"; return 1 ;; esac + + _rp_load_pool "${old}" || return 1 + [ ! -f "$(_rp_pool_conf "${new}")" ] || { + _rp_err "a pool called '${new}' already exists" + _rp_err "If a rename was interrupted, one of '${old}' and '${new}' is a leftover; remove that one and retry." + return 1 + } + + # Both names, old first. The new one is not decoration: from the moment its + # config exists, `_rp_pool_names` returns it and autoscale would otherwise + # start it in the middle of the move. Two locks cannot deadlock, because + # _rp_resize_lock refuses rather than blocks; the second failing has to + # release the first. + _rp_resize_lock "${old}" || return 1 + _rp_resize_lock "${new}" || { _rp_resize_unlock "${old}"; return 1; } + _rp_rename_locked "${old}" "${new}" "${drain}" "${timeout}" + rc=$? + _rp_resize_unlock "${new}" + _rp_resize_unlock "${old}" + return ${rc} +} + +_rp_rename_locked() { + local old="$1" new="$2" drain="$3" timeout="$4" was_up=0 busy i + local new_dir new_cache extras runner_dir runner_name token orphaned=0 configured=0 + + # Read before any drain, because draining unloads the agents and the answer + # would then always be no. + _rp_agent_loaded "$(_rp_label "${old}" 1)" && was_up=1 + + if [ "${drain}" = "1" ]; then + _rp_drain_pool "${old}" "${timeout}" || return 1 + fi + busy="$(_rp_busy_in "${POOL_DIR}")" + [ "${busy}" -eq 0 ] || { + _rp_err "'${old}' has ${busy} job(s) running, so refusing to rename (they would fail). Wait for them, or retry with --drain to let them finish first." + return 1 + } + _rp_down "${old}" || return 1 + + # In place, not into RUNPOOL_RUNNER_DIR. A pool migrated from the legacy + # layout has POOL_DIR under the legacy base, and hardcoding the native root + # would drag half of it into Application Support while the rest stayed put. + # It also guarantees the same filesystem, which is what makes mv atomic. + new_dir="$(dirname "${POOL_DIR}")/${new}" + _rp_move_dir "${POOL_DIR}" "${new_dir}" || return 1 + new_cache="${POOL_CACHE_DIR}" + if [ "${POOL_LEGACY_LAYOUT}" != "1" ]; then + new_cache="$(dirname "${POOL_CACHE_DIR}")/${new}" + _rp_move_dir "${POOL_CACHE_DIR}" "${new_cache}" || return 1 + fi + + # One write, as register does, rather than a series of edits: a crash between + # two of them leaves a config that loads and is wrong. + # + # POOL_CACHE_DIR is written only when the pool had one. Its ABSENCE is how + # _rp_load_pool recognises the legacy layout, so adding it here would quietly + # convert a legacy pool while its files stayed where they were. + extras="$(_rp_extra_labels "${POOL_LABELS}" "${old}")" + { + cat <| "$(_rp_pool_conf "${new}")" + rm -f "$(_rp_pool_conf "${old}")" + + # Reload under the new name, which both re-points every POOL_* the rest of + # this function reads and catches a config we just wrote that will not load, + # before anything at GitHub is touched. + _rp_load_pool "${new}" || return 1 + + [ -f "$(_rp_pool_pause_flag "${old}")" ] && mv -f "$(_rp_pool_pause_flag "${old}")" "$(_rp_pool_pause_flag "${new}")" + rm -f "$(_rp_pool_started_flag "${old}")" "$(_rp_pool_stuck_file "${old}")" + + # Every runner directory that exists, not 1..POOL_COUNT. A count lowered by + # hand leaves higher-numbered runners on disk and still registered, and those + # have to be deregistered here or they are stranded for ever under a name + # nothing records. They are deliberately NOT re-registered: a leftover + # runner-5 in a four-runner pool would be a permanent miscount. + while IFS= read -r runner_dir; do + [ -n "${runner_dir}" ] || continue + i="${runner_dir##*/runner-}" + case "${i}" in ''|*[!0-9]*) continue ;; esac + + # The token first: a token failure must not cost a registration we have + # already deleted. + token="$(_rp_registration_token "${POOL_SCOPE}" "${POOL_TARGET}")" \ + || { _rp_err "${new} runner-${i}: no registration token. Check 'gh auth status', that ${POOL_TARGET} exists, and that you have admin on it"; return 1; } + + # --replace cannot help here, unlike in reregister. It replaces a + # registration OF THE SAME NAME, and the name is exactly what is changing, + # so GitHub would keep the old one: permanently offline, still carrying + # '${old}' as a label so any surviving runs-on matches a dead runner, and + # unreachable afterwards because config.sh overwrites the .runner holding + # its agentId. Counted rather than fatal, for the reason set-count states: + # stopping half way leaves the pool half renamed. + _rp_deregister_runner "${runner_dir}" "${POOL_SCOPE}" "${POOL_TARGET}" || orphaned=$(( orphaned + 1 )) + rm -f "${runner_dir}/.runner" "${runner_dir}/.credentials" \ + "${runner_dir}/.credentials_rsaparams" "${runner_dir}/.runner_migrated" \ + "${runner_dir}/.service" + + [ "${i}" -le "${POOL_COUNT}" ] || { + _rp_log "${new} runner-${i}: past the pool's count of ${POOL_COUNT}, deregistered and left on disk" + continue + } + _rp_migrate_update_work_folder "${runner_dir}" "$(_rp_runner_work_dir "${new}" "${i}")" || return 1 + runner_name="$(hostname -s)-${new}-${i}" + _rp_log "${new} runner-${i}: registering as '${runner_name}'" + ( cd "${runner_dir}" && ./config.sh --unattended --replace \ + --url "https://github.com/${POOL_TARGET}" --token "${token}" \ + --name "${runner_name}" --labels "${POOL_LABELS}" --work "$(_rp_runner_work_dir "${new}" "${i}")" \ + >> "${RUNPOOL_LOG}" 2>&1 ) || return 1 + _rp_prepare_runner_cache "${new}" "${i}" + _rp_write_plist "$(_rp_label "${new}" "${i}")" "${runner_dir}" \ + "$(_rp_runner_cache_dir "${new}" "${i}")" "${POOL_LEGACY_LAYOUT}" + configured=$(( configured + 1 )) + done </dev/null | sort) +EOF + + # _rp_rewrite_plists only ever writes, so the old ones have to go explicitly. + # `find` rather than a glob, which would expand to a literal against an empty + # agent directory. + find "${RUNPOOL_AGENT_DIR}" -maxdepth 1 -name "${RUNPOOL_LABEL_NS}.${old}.*.plist" -exec rm -f {} + 2>/dev/null + + _rp_log "pool '${old}' renamed to '${new}': ${configured} runner(s) re-registered" + _rp_log "workflows using 'runs-on: [self-hosted, ${old}]' no longer match; the pool now carries '${new}'" + _rp_log "${RUNPOOL_POOLS_FILE} still names '${old}'; update it or the next 'runpool apply' will create that pool again" + + if [ "${orphaned}" -gt 0 ]; then + _rp_err "${orphaned} runner(s) are still registered on ${POOL_TARGET} under the old name. They are offline and still carry '${old}' as a label, so a workflow still routing to it would match one and queue for ever. Run the DELETE commands above, or remove them from GitHub's runner settings." + return 1 + fi + + [ "${was_up}" = "1" ] && ! _rp_pool_paused "${new}" && _rp_up "${new}" + return 0 +} + # --------------------------------------------------------------------------- # remove: deregister and delete a pool entirely # --------------------------------------------------------------------------- diff --git a/lib/scheduler.sh b/lib/scheduler.sh index 395ce04..54487e3 100644 --- a/lib/scheduler.sh +++ b/lib/scheduler.sh @@ -195,10 +195,16 @@ _rp_status_json() { } _rp_pools() { - local p found=0 + local p found=0 extras for p in $(_rp_pool_names); do found=1; _rp_load_pool "${p}" || continue - printf " %-10s %-4s %-20s count=%s\n" "${p}" "${POOL_SCOPE}" "${POOL_TARGET}" "${POOL_COUNT}" + # Extra labels shown, the implicit ones not: every runner carries + # self-hosted, the OS, the architecture and the pool name, so printing them + # would be noise on every line. Without this, "why does my runs-on not + # match" cannot be answered without reading the config by hand. + extras="$(_rp_extra_labels "${POOL_LABELS:-}" "${p}")" + printf " %-10s %-4s %-20s count=%s%s\n" "${p}" "${POOL_SCOPE}" "${POOL_TARGET}" "${POOL_COUNT}" \ + "${extras:+ labels=${extras}}" done [ "${found}" = "0" ] && echo " (no pools registered: 'runpool register --repo OWNER/REPO')" return 0 diff --git a/runpool.pools.example b/runpool.pools.example index cec6eaf..0cfa146 100644 --- a/runpool.pools.example +++ b/runpool.pools.example @@ -23,6 +23,7 @@ # --org ORG | --repo OWNER/REPO # [--count N] runners in the pool; default 2 # [--watch OWNER/REPO,...] org pools only, see below +# [--labels LABEL,...] extra runner labels, see below # [--allow-public] repo pools only, and only consulted when # the pool is created # @@ -41,6 +42,23 @@ # applied, because the runners are registered against the old one. Remove the # pool and apply again. +# --labels adds to what every runner in the pool carries already: self-hosted, +# the machine's OS and architecture, and the pool's own name. It cannot replace +# them, because GitHub assigns the first three whatever it is told, and the +# pool name is the routing contract. Naming one of them here is an error rather +# than a no-op. +# +# A label is letters, digits, dot, underscore and hyphen. That is narrower than +# GitHub allows, and deliberately: this file is word-split, and the value is +# written into a config file that every command SOURCES, so a space, a quote or +# a '$' would be a broken pool or worse. 'xcode-16.2' works; 'xcode 16' cannot. +# +# Applying a label change is the heaviest thing in this file. Labels live on +# GitHub's registration, so `apply` stands the pool down and re-registers every +# runner. Absent --labels means no extra labels, so a pool whose config was +# edited by hand to add one needs it declared here, or the next apply takes it +# away and whatever routed to it queues for ever. + # --------------------------------------------------------------------------- # An organisation pool. Every repository in the org can use these runners. # --------------------------------------------------------------------------- @@ -53,7 +71,8 @@ # A repo pool needs no --watch. It polls its own target, and giving it one is # refused rather than ignored. -# acme --org acme-inc --count 4 --watch acme-inc/api, acme-inc/web +# acme --org acme-inc --count 4 --watch acme-inc/api, acme-inc/web \ +# --labels xcode # Long lists are easier to read split across lines. Uncomment every line of it # or none of them: diff --git a/skills/runpool/SKILL.md b/skills/runpool/SKILL.md index 209ba3b..7c871e8 100644 --- a/skills/runpool/SKILL.md +++ b/skills/runpool/SKILL.md @@ -50,6 +50,14 @@ runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }} Then set the repository variable `CI_RUNNER` to `self-hosted`. The fallback keeps the workflow working for anyone without the pool, and the variable means switching back to hosted is one setting rather than a commit. +**A pool can carry extra labels, and `runs-on` can ask for them.** Every runner carries `self-hosted`, the machine's OS and architecture, and the pool's own name; `register --labels` and the pools file add to that. A job wanting a particular capability then asks for it: + +```yaml +runs-on: ${{ vars.CI_RUNNER == 'self-hosted' && fromJSON('["self-hosted", "xcode"]') || 'macos-latest' }} +``` + +`runpool pools` shows a pool's extra labels, which is how to answer "why does my `runs-on` not match" without reading a config file. **Declare them in the pools file too**, or the next `apply` removes them: absent `--labels` means none, and applying that stands the pool down and re-registers every runner without the label. + **Never add an automatic fallback to hosted runners.** On macOS that silently costs ten times as much, and if the hosted allowance is exhausted it fails anyway. A job waiting for a pool that is down is the correct behaviour. Keep publish, deploy and OIDC jobs hosted regardless: npm provenance requires it. @@ -131,7 +139,7 @@ It works down the whole list below in one pass, prints a remedy against each fai **Three situations `doctor` deliberately reports as healthy, because they are.** - **`running 0/N` with a job genuinely queued:** the tick brings a pool up within about a minute. Wait before intervening; `runpool up ` forces it. -- **A clean report and the job still waits:** the problem is routing, not capacity. The workflow's `runs-on` may not resolve to `self-hosted`, or its labels may not match the pool's. RunPool controls only whether the runners are up and cannot see either. +- **A clean report and the job still waits:** the problem is routing, not capacity. The workflow's `runs-on` may not resolve to `self-hosted`, or its labels may not match the pool's, which `runpool pools` will show you. RunPool controls only whether the runners are up and cannot see either. - **a run held down and the rest of the queue moving:** intended. The guard subtracts held runs from the queued count rather than silencing the pool, and a run that is merely waiting its turn behind another gets a fresh chance once a day. ```bash @@ -143,6 +151,24 @@ tail -50 ~/Library/Logs/runpool/runpool.log The JSON carries each pool's watched repositories and the paths to its logs, so a wrapper never has to guess either. **The root `paused` field is global; each pool has its own `paused` boolean.** Read both: a paused pool is intentional state, not an unreachable runner. +## Renaming a pool + +```bash +runpool rename [--drain] +``` + +Moves the pool's directories, config, launch agents and state, then re-registers every runner with GitHub under the new name. `--drain` lets running jobs finish first; without it a busy pool is refused. + +**The pool name is one of its runners' labels, so this is a routing change.** Any workflow with `runs-on: [self-hosted, ]` stops matching and has to be updated. One asking only for `self-hosted` is unaffected. + +Do it in this order, and the middle step is the one people forget: + +1. `runpool rename ` on each machine that hosts the pool. +2. Update any workflow whose `runs-on` names the old pool. +3. Update the pools file, or the next `runpool apply` sees a pool it does not recognise and creates the old one again. + +The old GitHub registrations are deleted rather than replaced, because `--replace` only covers a name collision and the name is exactly what changed. A registration left behind would sit permanently offline while still advertising the old label, which is a job queued for ever against a runner that will never answer. + ## Capacity ```bash @@ -194,4 +220,5 @@ RunPool ships with **no notifier** and works fully without one. Set `RUNPOOL_NOT - **Pool names are validated at `register`**: letters, digits, dot, underscore and hyphen. The name becomes a directory, a launch-agent label and a JSON field, so anything else is refused rather than sanitised. - **All runners share one HOME**, so each needs its own package store and cache. runpool sets this in the launch agent; if you hand-edit an agent, preserve it or concurrent installs collide. - **Ephemeral macOS VMs are capped at two per machine** by Apple's licence. If someone suggests Tart, Tartelet or Cilicon for more than two parallel macOS jobs, that ceiling is why it will not work. +- **The pool name is a runner label, not just a local identifier.** It is part of the routing contract, which is why renaming a pool is a change to every workflow that names it and not an act of tidying. - **Nothing watches RunPool itself.** This is an accepted gap, not an oversight. Do not build a heartbeat for it. diff --git a/tests/pool-labels.sh b/tests/pool-labels.sh new file mode 100755 index 0000000..f80ff9e --- /dev/null +++ b/tests/pool-labels.sh @@ -0,0 +1,185 @@ +#!/bin/bash +# Custom runner labels: the rules that derive and validate them, and the drift +# `apply` reports against a pool that has some. +# +# Two halves, both offline. The first is pure functions. The second is +# `apply --dry-run`, which reads the pools file and the configs and calls +# nothing, so the whole plan can be asserted with no GitHub account. +set -uo pipefail + +repo_dir=$(cd -P "$(dirname "$0")/.." && pwd) +scratch_dir=$(mktemp -d) +trap 'rm -rf "${scratch_dir}"' EXIT INT TERM + +export RUNPOOL_BASE="${scratch_dir}/base" +export RUNPOOL_STATE_DIR="${scratch_dir}/base/state" +export RUNPOOL_CACHE_DIR="${scratch_dir}/cache" +export RUNPOOL_CONFIG="${scratch_dir}/runpool.conf" +export RUNPOOL_POOLS_FILE="${scratch_dir}/pools" +export RUNPOOL_LOG_DIR="${scratch_dir}/logs" +export RUNPOOL_LOG="${scratch_dir}/logs/runpool.log" +export RUNPOOL_AGENT_DIR="${scratch_dir}/agents" +mkdir -p "${RUNPOOL_BASE}/pools" "${RUNPOOL_STATE_DIR}/pools" "${RUNPOOL_CACHE_DIR}" \ + "${RUNPOOL_LOG_DIR}" "${RUNPOOL_AGENT_DIR}" + +# shellcheck source=/dev/null +. "${repo_dir}/lib/common.sh" 2>/dev/null || true + +fail() { echo "FAIL: $*" >&2; exit 1; } +pass=0 +check() { + local label="$1" expected="$2" actual="$3" + if [ "${expected}" = "${actual}" ]; then + pass=$(( pass + 1 )) + else + fail "${label}: expected '${expected}', got '${actual}'" + fi +} + +# --------------------------------------------------------------------------- +# deriving the extras back out of the stored list +# --------------------------------------------------------------------------- +# The case this exists for: a label added by hand to a config, which `register` +# could never have produced and nothing else can see. +check "an extra label is recovered" "xcode" \ + "$(_rp_extra_labels "self-hosted,macOS,ARM64,acme,xcode" acme)" +check "a plain pool has no extras" "" \ + "$(_rp_extra_labels "self-hosted,macOS,ARM64,acme" acme)" + +# Only the FIRST occurrence of the name is the name label. A pool called xcode +# carrying a genuine xcode label must keep it, or rename loses it silently. +check "a pool named after its label keeps it" "xcode" \ + "$(_rp_extra_labels "self-hosted,macOS,ARM64,xcode,xcode" xcode)" + +check "spaces, empties and duplicates are filtered" "a,b" \ + "$(_rp_extra_labels "self-hosted, macOS ,ARM64,,acme,,a,a,b" acme)" +check "a config missing a base label still derives" "xcode" \ + "$(_rp_extra_labels "self-hosted,acme,xcode" acme)" +check "order is preserved" "z,a" \ + "$(_rp_extra_labels "self-hosted,macOS,ARM64,acme,z,a" acme)" + +# A hand-mangled config must filter, never fail: this runs on every apply, and +# a config nobody can load is worse than a label nobody asked for. A space is +# not mangling, though: the list is split on whitespace as well as commas, so +# 'a b' is two labels and both survive. +check "an unusable token is dropped rather than fatal" "ok" \ + "$(_rp_extra_labels "self-hosted,acme,a|b,ok" acme)" +check "a space reads as two labels" "a,b" \ + "$(_rp_extra_labels "self-hosted,acme,a b" acme)" + +# Round trip. +check "building then deriving is the identity" "xcode,gpu" \ + "$(_rp_extra_labels "$(_rp_pool_labels acme "xcode,gpu")" acme)" +check "the built list carries the base three and the name" "self-hosted,macOS,ARM64,acme,xcode" \ + "$(_rp_pool_labels acme xcode)" +check "and just the name with no extras" "self-hosted,macOS,ARM64,acme" \ + "$(_rp_pool_labels acme "")" + +check "sorting is for comparison only" "a,b,c" "$(_rp_sorted_labels "c,a,b")" +check "and ignores empty entries" "a,b" "$(_rp_sorted_labels "b,,a")" + +# --------------------------------------------------------------------------- +# validation on the way in +# --------------------------------------------------------------------------- +# '|' is apply's record separator, '"' and '$' reach a config that is SOURCED +# on every invocation and every tick, whitespace is two fields in a file that +# is word-split, and '*' would reach rename's find -name. +for bad in "a|b" 'a"b' 'a$(id)' 'a`id`' "a b" "a,b" "" "." ".." "a*b" "a[b"; do + if _rp_valid_label "${bad}"; then fail "_rp_valid_label accepted '${bad}'"; fi + pass=$(( pass + 1 )) +done +for good in xcode xcode-16.2 gpu_2 a.b; do + _rp_valid_label "${good}" || fail "_rp_valid_label rejected '${good}'" + pass=$(( pass + 1 )) +done +long=$(printf 'a%.0s' $(seq 1 256)); _rp_valid_label "${long}" || fail "256 characters should be allowed" +pass=$(( pass + 1 )) +toolong=$(printf 'a%.0s' $(seq 1 257)); _rp_valid_label "${toolong}" && fail "257 characters should be refused" +pass=$(( pass + 1 )) + +# --------------------------------------------------------------------------- +# what apply plans, with no GitHub account +# --------------------------------------------------------------------------- +# RUNPOOL_POOLS_FILE and RUNPOOL_LOG_DIR are isolated separately on purpose: +# RUNPOOL_BASE moves neither, so without both this would read the real pools +# file and write to a real installation's log. +runpool() { "${repo_dir}/bin/runpool" "$@"; } + +cat >"${RUNPOOL_BASE}/pools/acme.conf" <"${RUNPOOL_POOLS_FILE}" +out=$(runpool apply --dry-run 2>&1) || fail "dry run failed: ${out}" +case "${out}" in + *"labels xcode -> (none)"*) pass=$(( pass + 1 )) ;; + *) fail "the plan does not name the label being dropped: ${out}" ;; +esac +case "${out}" in + *"stops matching"*) pass=$(( pass + 1 )) ;; + *) fail "the plan does not say what stops matching: ${out}" ;; +esac +case "${out}" in + *"will be re-registered"*) pass=$(( pass + 1 )) ;; + *) fail "the plan does not warn that a label change re-registers: ${out}" ;; +esac + +# Declared to match, so nothing to do. +printf 'acme --org acme-inc --count 2 --labels xcode\n' >"${RUNPOOL_POOLS_FILE}" +out=$(runpool apply --dry-run 2>&1) || fail "dry run failed: ${out}" +case "${out}" in + *"up to date"*) pass=$(( pass + 1 )) ;; + *) fail "a matching label set should be up to date: ${out}" ;; +esac + +# Order alone is not drift, or every apply would re-register the pool. +cat >"${RUNPOOL_BASE}/pools/acme.conf" <"${RUNPOOL_POOLS_FILE}" +out=$(runpool apply --dry-run 2>&1) || fail "dry run failed: ${out}" +case "${out}" in + *"up to date"*) pass=$(( pass + 1 )) ;; + *) fail "label order should not read as drift: ${out}" ;; +esac + +# An implicit label is refused rather than dropped, so declared and derived +# cannot end up permanently unequal. +printf 'acme --org acme-inc --count 2 --labels self-hosted\n' >"${RUNPOOL_POOLS_FILE}" +out=$(runpool apply --dry-run 2>&1) && fail "a base label should be refused" +case "${out}" in + *"pools:1:"*"already"*) pass=$(( pass + 1 )) ;; + *) fail "the refusal should name the line: ${out}" ;; +esac +printf 'acme --org acme-inc --count 2 --labels acme\n' >"${RUNPOOL_POOLS_FILE}" +runpool apply --dry-run >/dev/null 2>&1 && fail "the pool's own name should be refused" +pass=$(( pass + 1 )) + +# The record separator, which would otherwise shift every field in both reads. +printf 'acme --org acme-inc --count 2 --labels a|b\n' >"${RUNPOOL_POOLS_FILE}" +runpool apply --dry-run >/dev/null 2>&1 && fail "a label containing '|' should be refused" +pass=$(( pass + 1 )) + +# A create carries its labels through to the plan. +rm -f "${RUNPOOL_BASE}/pools/acme.conf" +printf 'acme --org acme-inc --count 2 --labels xcode\n' >"${RUNPOOL_POOLS_FILE}" +out=$(runpool apply --dry-run 2>&1) || fail "dry run failed: ${out}" +case "${out}" in + *"labelled xcode"*) pass=$(( pass + 1 )) ;; + *) fail "a create should say what it will be labelled: ${out}" ;; +esac + +echo "ok: ${pass} case(s)" diff --git a/tests/pool-rename.sh b/tests/pool-rename.sh new file mode 100755 index 0000000..1d16082 --- /dev/null +++ b/tests/pool-rename.sh @@ -0,0 +1,246 @@ +#!/bin/bash +# Renaming a pool: what moves, what is re-registered, and what it refuses. +# +# The name is a config filename, two directories, the launch-agent labels and +# their logs, three state files, a lock, each runner's registered name, and one +# of the labels those runners carry. Getting one of them wrong is silent, which +# is what this is for. +# +# Offline: gh and each runner's config.sh are stubbed, and both log their +# arguments so the GitHub-facing contract can be asserted without a network. +set -uo pipefail + +repo_dir=$(cd -P "$(dirname "$0")/.." && pwd) +scratch_dir=$(mktemp -d) +bin_dir="${scratch_dir}/bin" +gh_log="${scratch_dir}/gh.log" +cfg_log="${scratch_dir}/config.log" +trap 'rm -rf "${scratch_dir}"' EXIT INT TERM + +export RUNPOOL_BASE="${scratch_dir}/base" +export RUNPOOL_STATE_DIR="${scratch_dir}/base/state" +export RUNPOOL_CACHE_DIR="${scratch_dir}/cache" +export RUNPOOL_CONFIG="${scratch_dir}/runpool.conf" +export RUNPOOL_POOLS_FILE="${scratch_dir}/pools" +export RUNPOOL_LOG_DIR="${scratch_dir}/logs" +export RUNPOOL_LOG="${scratch_dir}/logs/runpool.log" +# Derived from RUNPOOL_BASE inside common.sh rather than taken from the +# environment, so this has to agree with it or the assertions below look at an +# empty directory while the tool writes somewhere else. +export RUNPOOL_AGENT_DIR="${scratch_dir}/base/agents" +mkdir -p "${RUNPOOL_BASE}/pools" "${RUNPOOL_STATE_DIR}/pools" "${RUNPOOL_CACHE_DIR}" \ + "${RUNPOOL_LOG_DIR}" "${RUNPOOL_AGENT_DIR}" "${bin_dir}" + +fail() { echo "FAIL: $*" >&2; exit 1; } +pass=0 +ok() { pass=$(( pass + 1 )); } +present() { [ -e "$1" ] || fail "$2: expected $1 to exist"; ok; } +absent() { [ ! -e "$1" ] || fail "$2: expected $1 to be gone"; ok; } +holds() { grep -q -- "$2" "$1" || fail "$3: '$2' not in $1"; ok; } +lacks() { grep -q -- "$2" "$1" && fail "$3: '$2' should not be in $1"; ok; } + +# gh: a registration token for the POST, success for the DELETE, and a record +# of every call. The DELETE is what proves the old registrations are removed +# rather than left behind, which --replace cannot do when the name changes. +cat >"${bin_dir}/gh" <> "${gh_log}" +case "\$*" in + *registration-token*) echo "TOKEN123" ;; + *) exit 0 ;; +esac +STUB +chmod +x "${bin_dir}/gh" +export PATH="${bin_dir}:${PATH}" + +# --------------------------------------------------------------------------- +# a pool to rename +# --------------------------------------------------------------------------- +build_pool() { + local name="$1" count="$2" legacy="${3:-0}" i dir cache + rm -rf "${RUNPOOL_BASE}/runners" "${RUNPOOL_CACHE_DIR}/pools" "${RUNPOOL_AGENT_DIR:?}"/* + rm -f "${RUNPOOL_BASE}/pools"/*.conf "${RUNPOOL_STATE_DIR}/pools"/* + : >"${gh_log}"; : >"${cfg_log}" + dir="${RUNPOOL_BASE}/runners/${name}" + cache="${RUNPOOL_CACHE_DIR}/pools/${name}" + { + echo "POOL_NAME=\"${name}\"" + echo "POOL_SCOPE=\"org\"" + echo "POOL_TARGET=\"acme-inc\"" + echo "POOL_COUNT=\"${count}\"" + echo "POOL_DIR=\"${dir}\"" + [ "${legacy}" = "1" ] || echo "POOL_CACHE_DIR=\"${cache}\"" + echo "POOL_LABELS=\"self-hosted,macOS,ARM64,${name},xcode\"" + echo "POOL_WATCH=\"acme-inc/api,acme-inc/web\"" + } >"${RUNPOOL_BASE}/pools/${name}.conf" + + i=1 + while [ "${i}" -le "${count}" ]; do + mkdir -p "${dir}/runner-${i}" "${cache}/runner-${i}/work" + printf '{"agentId": %s0, "workFolder": "%s/runner-%s/work"}\n' "${i}" "${cache}" "${i}" \ + >"${dir}/runner-${i}/.runner" + # A config.sh that behaves like the real one: refuses nothing, writes a + # .runner, and records exactly what it was asked for. + cat >"${dir}/runner-${i}/config.sh" <> "${cfg_log}" +printf '{"agentId": 99, "workFolder": "x"}\n' > .runner +STUB + chmod +x "${dir}/runner-${i}/config.sh" + printf 'plist for %s runner-%s\n' "${name}" "${i}" \ + >"${RUNPOOL_AGENT_DIR}/runpool.${name}.${i}.plist" + i=$(( i + 1 )) + done + : >"${RUNPOOL_STATE_DIR}/pools/${name}.paused" + : >"${RUNPOOL_STATE_DIR}/pools/${name}.started" +} + +# launchd and process inspection are the two things a test cannot have, and +# neither is what is under test here. +probe() { + ( + # shellcheck source=/dev/null + . "${repo_dir}/lib/common.sh" 2>/dev/null || true + # shellcheck source=/dev/null + . "${repo_dir}/lib/lifecycle.sh" + # shellcheck source=/dev/null + . "${repo_dir}/lib/scheduler.sh" + _rp_agent_loaded() { return 1; } + _rp_down() { return 0; } + _rp_busy_in() { echo "${BUSY:-0}"; } + _rp_up() { return 0; } + eval "${EXTRA:-:}" + _rp_rename "$@" + ) +} + +# --------------------------------------------------------------------------- +# the rename itself +# --------------------------------------------------------------------------- +build_pool alpha 2 +probe alpha bravo >"${scratch_dir}/out" 2>&1 || fail "rename failed: $(cat "${scratch_dir}/out")" + +conf="${RUNPOOL_BASE}/pools/bravo.conf" +present "${conf}" "the new config" +absent "${RUNPOOL_BASE}/pools/alpha.conf" "the old config" +holds "${conf}" 'POOL_NAME="bravo"' "the config names the new pool" +holds "${conf}" "runners/bravo" "POOL_DIR moved" +holds "${conf}" "pools/bravo" "POOL_CACHE_DIR moved" +holds "${conf}" 'POOL_COUNT="2"' "the count survives" +holds "${conf}" "acme-inc/api" "the watch list survives" +# The whole point of deriving extras rather than storing them. +holds "${conf}" 'POOL_LABELS="self-hosted,macOS,ARM64,bravo,xcode"' "the extra label survives the rename" + +present "${RUNPOOL_BASE}/runners/bravo/runner-1" "the runner tree moved" +absent "${RUNPOOL_BASE}/runners/alpha" "the old runner tree" +present "${RUNPOOL_CACHE_DIR}/pools/bravo" "the cache moved" +absent "${RUNPOOL_CACHE_DIR}/pools/alpha" "the old cache" + +# Written by _rp_write_plist against the new label, and the old ones removed by +# hand: _rp_rewrite_plists only ever writes. +present "${RUNPOOL_AGENT_DIR}/runpool.bravo.1.plist" "the new launch agent" +absent "${RUNPOOL_AGENT_DIR}/runpool.alpha.1.plist" "the old launch agent" +absent "${RUNPOOL_AGENT_DIR}/runpool.alpha.2.plist" "the second old launch agent" + +present "${RUNPOOL_STATE_DIR}/pools/bravo.paused" "the pause flag moved" +absent "${RUNPOOL_STATE_DIR}/pools/alpha.paused" "the old pause flag" +absent "${RUNPOOL_STATE_DIR}/pools/alpha.started" "the old started stamp" + +# The GitHub half. --replace cannot cover a name change, so the old +# registrations have to be deleted explicitly or they are stranded offline +# still carrying the old pool name as a label. +holds "${gh_log}" "DELETE /orgs/acme-inc/actions/runners/10" "runner 1 deregistered" +holds "${gh_log}" "DELETE /orgs/acme-inc/actions/runners/20" "runner 2 deregistered" +holds "${cfg_log}" "$(hostname -s)-bravo-1" "runner 1 registered under the new name" +holds "${cfg_log}" "$(hostname -s)-bravo-2" "runner 2 registered under the new name" +holds "${cfg_log}" "self-hosted,macOS,ARM64,bravo,xcode" "registered with the new label set" +lacks "${cfg_log}" "alpha" "nothing was registered under the old name" + +# The lock is taken under both names and released on the way out. +absent "${RUNPOOL_STATE_DIR}/resize.alpha.lock" "the old lock" +absent "${RUNPOOL_STATE_DIR}/resize.bravo.lock" "the new lock" + +# --------------------------------------------------------------------------- +# a legacy pool must not gain a cache directory +# --------------------------------------------------------------------------- +# POOL_CACHE_DIR being ABSENT is how _rp_load_pool recognises the legacy +# layout. Writing one here would silently convert the pool while its files +# stayed where they were. +build_pool alpha 1 1 +probe alpha bravo >"${scratch_dir}/out" 2>&1 || fail "legacy rename failed: $(cat "${scratch_dir}/out")" +lacks "${RUNPOOL_BASE}/pools/bravo.conf" "POOL_CACHE_DIR" "a legacy pool stays legacy" + +# --------------------------------------------------------------------------- +# a count lowered by hand +# --------------------------------------------------------------------------- +# runner-2 is on disk and registered but past the pool's count. It has to be +# deregistered, or it is stranded under a name nothing records; and it must not +# be re-registered, or the pool is a permanent miscount. +build_pool alpha 2 +sed -i '' 's/POOL_COUNT="2"/POOL_COUNT="1"/' "${RUNPOOL_BASE}/pools/alpha.conf" +probe alpha bravo >"${scratch_dir}/out" 2>&1 || fail "rename failed: $(cat "${scratch_dir}/out")" +holds "${gh_log}" "DELETE /orgs/acme-inc/actions/runners/20" "the surplus runner is deregistered" +lacks "${cfg_log}" "bravo-2" "the surplus runner is not re-registered" + +# --------------------------------------------------------------------------- +# resumability +# --------------------------------------------------------------------------- +# _rp_move_dir returns success when the source is gone, which is what lets a +# rename interrupted after the move be finished by running it again. +build_pool alpha 1 +mv "${RUNPOOL_BASE}/runners/alpha" "${RUNPOOL_BASE}/runners/bravo" +probe alpha bravo >"${scratch_dir}/out" 2>&1 || fail "an interrupted rename did not resume: $(cat "${scratch_dir}/out")" +present "${RUNPOOL_BASE}/pools/bravo.conf" "the resumed rename completed" + +# --------------------------------------------------------------------------- +# refusals +# --------------------------------------------------------------------------- +refused() { + local what="$1"; shift + local out + out=$(probe "$@" 2>&1) && fail "${what}: should have been refused" + [ -f "${RUNPOOL_BASE}/pools/alpha.conf" ] || fail "${what}: the pool was touched" + [ ! -d "${RUNPOOL_STATE_DIR}/resize.alpha.lock" ] || fail "${what}: left the old lock behind" + [ ! -d "${RUNPOOL_STATE_DIR}/resize.bravo.lock" ] || fail "${what}: left the new lock behind" + ok + printf '%s\n' "${out}" +} + +build_pool alpha 1 +refused "unknown source pool" nosuch bravo >/dev/null +refused "invalid target name" alpha "b ravo" >/dev/null +refused "renaming to itself" alpha alpha >/dev/null +refused "one name only" alpha >/dev/null +refused "an unknown flag" alpha bravo --wat >/dev/null +refused "a zero timeout" alpha bravo --timeout 0 >/dev/null + +# A busy pool, and the message has to name the way through. +out=$(BUSY=1 probe alpha bravo 2>&1) && fail "a busy pool should refuse" +case "${out}" in + *"--drain"*) ok ;; + *) fail "the busy refusal does not name --drain: ${out}" ;; +esac + +# A name already in use, in both directions. +build_pool alpha 1 +cp "${RUNPOOL_BASE}/pools/alpha.conf" "${RUNPOOL_BASE}/pools/bravo.conf" +probe alpha bravo >/dev/null 2>&1 && fail "an existing target name should refuse" +ok +rm -f "${RUNPOOL_BASE}/pools/bravo.conf" + +# Both locks are genuinely taken, which only a test holding each in turn shows. +build_pool alpha 1 +mkdir -p "${RUNPOOL_STATE_DIR}/resize.alpha.lock" +echo $$ >"${RUNPOOL_STATE_DIR}/resize.alpha.lock/pid" +probe alpha bravo >/dev/null 2>&1 && fail "a locked source should refuse" +ok +rm -rf "${RUNPOOL_STATE_DIR}/resize.alpha.lock" +mkdir -p "${RUNPOOL_STATE_DIR}/resize.bravo.lock" +echo $$ >"${RUNPOOL_STATE_DIR}/resize.bravo.lock/pid" +probe alpha bravo >/dev/null 2>&1 && fail "a locked target should refuse" +ok +[ ! -d "${RUNPOOL_STATE_DIR}/resize.alpha.lock" ] || fail "the source lock was not released after the target lock failed" +ok +rm -rf "${RUNPOOL_STATE_DIR}/resize.bravo.lock" + +echo "ok: ${pass} case(s)"