feat: custom runner labels, and a rename command - #72
Merged
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #66 and #67.
Labels
register --labelsand a--labelsfield in the pools file, appended to the base set. Previously the only way to give a pool an extra label was to editPOOL_LABELSin its config by hand, after whichregistercould not reproduce it,applycould not see it,poolsdid not show it, and anything that recreated the pool dropped it. Nothing failed when that happened: the runners came up healthy and the workflow'sruns-onsimply stopped matching.One field, not two.
POOL_LABELSstays the full authoritative list handed toconfig.sh, and the extras are derived back out of it at every point of use. A second field caching that derivation would go stale the first time somebody edited the config by hand, and the followingapplywould silently revert it, which is the same defect in a new place.Append, never replace. GitHub assigns
self-hosted, the OS and the architecture regardless, so a replacing flag would promise something GitHub overrides. The pool name is in the base set too, because it is the routing contract. Naming an implicit label is refused rather than dropped, which also keepsapplyidempotent: a token accepted and normalised away would leave declared and derived permanently unequal and re-register the pool for ever.Validation is the pool-name character class. Narrower than GitHub allows, deliberately:
POOL_LABELSis written into a config every command sources, so$or a backtick there is command execution on every invocation and every 60-second tick;applyseparates records with|; the pools file is word-split; andrenamematches plists withfind -name. One rule closes all four.Applying a label change re-registers the pool, because labels live on GitHub's registration. The plan says so, names what stops matching, and the summary counts the pools affected. Absent
--labelsmeans none, so a pool whose config was hand-edited needs the label declared in the file.Rename
runpool rename <old> <new> [--drain]. Moves the config, runner tree, cache tree, launch agents and state, then re-registers every runner.mv, not copy.migrate-storagecopies because it crosses storage roots; a rename keeps the same parent by construction, somvis atomic and a second on-disk copy of runner credentials buys nothing. Reusing the idempotent move helper also makes an interrupted rename resumable, which is covered by a test.It must deregister explicitly.
config.sh --replacereplaces a registration of the same name, and the name is what changes, so GitHub would keep the old one: permanently offline, still carrying the old pool name as a label so a survivingruns-onmatches a dead runner, and unreachable afterwards becauseconfig.shoverwrites the.runnerholding itsagentId.Two locks, old then new, released in reverse. Runner directories are iterated as they exist rather than
1..POOL_COUNT, so a count lowered by hand does not strand a registration; those surplus runners are deregistered and deliberately not re-registered._rp_migrate_update_pool_confis not reused: itsENDclause addsPOOL_CACHE_DIRwhen absent, and that absence is exactly how a legacy pool is recognised. There is a test for it.Also here
_rp_reregisternow takes the reconfiguration lock. It stood the pool down and then spent a long time inconfig.shwith nothing stopping autoscale oruprestarting it. Latent while it was only hand-run;applynow calls it.runpool poolsprints a pool's extra labels, which is how "why does myruns-onnot match" gets answered without reading a config.Verification
Two new offline tests, 80 cases between them.
tests/pool-labels.shcovers the derivation (including recovering a hand-added label, and the first-occurrence-only rule for a pool named after its own label), the validator, and the repo's firstapplycoverage via--dry-run.tests/pool-rename.shstubsghand each runner'sconfig.shso both log their arguments, which is what proves the GitHub contract offline: theDELETEcalls, the new runner names, and the new label set.bash -n,shellcheck --severity=warningand all ten tests pass.