From 8ecc658a68a6ebc5ae6568bec5f2485c92ae710d Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Sun, 8 Nov 2026 23:20:43 -0400 Subject: [PATCH 1/6] ci: a runner install recipe with a restart policy, an OOM guard and a dedicated user (Refs #1504) Both self-hosted runners were OOM-killed on 11-08 08:37 and stayed dead until 12:23, with Result=oom-kill and NRestarts=0 - the units have no restart policy, and runsvc.sh exits 0 after the listener stops ("no retry needed"), so even Restart=on-failure would never have fired. Nine hrx-build and fifteen pr-agent jobs queued behind a state nothing reports. This is the recipe for the coming fresh install, as the owner asked: a script that installs a runner as a systemd service owned by a dedicated unprivileged user, plus the docs page explaining every line. scripts/install-ci-runner.sh useradd ghrunner --system --no shell, home /var/lib/ghrunner, and the runner's _work under it, so a job no longer runs as the owner (the 11-08 audit finding: no ssh keys, no ~/models, no fleet remotes) Restart=always + RestartSec=30 - `always`, not `on-failure`, because a killed runsvc.sh reports success; StartLimitIntervalSec=0 so a runner can never be rate-limited out of existence, with the 30 s spacing as the actual bound OOMPolicy=continue - the line the old units lacked: an OOM kill inside the cgroup kills the job, and the runner keeps listening, instead of the unit being stopped with Result=oom-kill MemoryHigh/MemoryMax (43G/48G by default) - the cgroup hits its own ceiling before the box's OOM killer chooses between the biggest allocation on the machine and everything else keeps the shipped units' KillMode=process / 5-minute TimeoutStopSec; idempotent; the registration token is copied to a 0600 file the runner user owns, used and deleted, never echoed --dry-run prints the unit and verifies it, --verify reports Result/NRestarts/OOMPolicy, and a group the box does not have (docker) is a warning, not an install failure docs/ci-runners.md - the two runners and their labels, why each directive is there, the dedicated user with the honest price of it (docker group is root-equivalent; rootless podman is the follow-up that would remove it), the fresh install start to finish, and how to verify an install. CONTRIBUTING.md links it from the automation list. Measured 2026-11-08/09 on this box: systemd 259, and the live units confirm the diagnosis: Restart=no, OOMPolicy=stop, MemoryMax=infinity, StartLimitIntervalUSec=10s the generated unit passes systemd-analyze verify, and a deliberately corrupted directive (OOMPolicy=bogus) is caught by the same check, so the verification is not vacuous gh api repos/1bit-MONSTER/engine/actions/runners: strixhalo-bench and strixhalo-engine both online, labels self-hosted,Linux,X64,{bench,pr-agent},strixhalo - the labels the recipe installs NOT claimed: item 3 of the issue ("something that notices" - a row that asserts both runners are online) is a separate change and stays open, which is why this says Refs rather than Fixes. And the OOM behaviour itself is not demonstrated end to end: a throwaway systemd-run pair with and without OOMPolicy=continue both reported Result=success here (the child's allocation failed as an exception rather than an OOM kill), so that experiment says nothing either way - the docs say so rather than presenting it as measured, and the fresh install's single end-to-end run is where it gets confirmed. The live units were not touched. checks: bash -n clean; shellcheck -S warning clean on the new script; tools/copyright.py --check rc 0. --- CONTRIBUTING.md | 4 + docs/ci-runners.md | 175 +++++++++++++++++++++++++++++ scripts/install-ci-runner.sh | 212 +++++++++++++++++++++++++++++++++++ 3 files changed, 391 insertions(+) create mode 100644 docs/ci-runners.md create mode 100755 scripts/install-ci-runner.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dbd2de9db..71e81462e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,6 +70,10 @@ an RFC or issue. - **PR-Agent** reviews every pull request with a local model (Qwen3-Coder-30B-A3B) on the Strix Halo box (`.github/workflows/pr-agent.yml`); comment `/review`, `/describe`, `/improve` or `/ask` for more. +- **The box's runners.** `pr-agent` and `hrx-build` run on Strix Halo's own self-hosted runners, because those + jobs need the GPU, the NPU and the local LLM. How they are installed — the dedicated user, the restart policy + and the OOM guard that engine [#1504](https://github.com/1bit-MONSTER/engine/issues/1504) asked for — is + [docs/ci-runners.md](docs/ci-runners.md). - **Issue agent** (`tools/issue_agent.py`) takes a first pass on issues opened by people outside the project: one label, a request for the bug-report fields when a bug skipped the form, a registry lookup for linked Hugging Face models, and a pointer to Q&A for questions. The model only picks diff --git a/docs/ci-runners.md b/docs/ci-runners.md new file mode 100644 index 000000000..8aca7f855 --- /dev/null +++ b/docs/ci-runners.md @@ -0,0 +1,175 @@ + + +# Self-hosted CI runners on the Strix Halo box + +Two runners serve this repository, both on Strix Halo, both installed by +`scripts/install-ci-runner.sh`: + +| runner | labels | takes | +|---|---|---| +| `strixhalo-engine` | `self-hosted,Linux,X64,pr-agent,strixhalo` | `pr-agent`, `issue-agent` | +| `strixhalo-bench` | `self-hosted,Linux,X64,strixhalo,bench` | `hrx-build` and the bench/gate jobs | + +Everything the GitHub-hosted jobs need comes from `ubuntu-*` runners; these two exist because those jobs need +*this box* — its GPU, its NPU and the local LLM at `127.0.0.1:8090`. + +## Why this page exists (engine #1504) + +On 2026-11-08 both runners were OOM-killed at 08:37 and **stayed dead until 12:23**: + +``` +$ systemctl show actions.runner.1bit-MONSTER-engine.strixhalo-bench.service -p Result -p NRestarts +Result=oom-kill +NRestarts=0 +``` + +`NRestarts=0` is the whole bug. The units shipped with no restart policy, and `runsvc.sh` exits **0** after the +listener stops ("Runner listener exit with 0 return code, stop the service, no retry needed"), so even +`Restart=on-failure` would never have fired. While they were down, 9 `hrx-build` and 15 `pr-agent` jobs queued, +and every check that needs the box read as *pending* — never green, never red, which looks exactly like "queued +behind a busy runner". It had happened before (2026-11-06, 41 `pr-agent` runs queued behind the same unit). + +A check whose runner is gone is the same silent-absence class as [engine #1077](https://github.com/1bit-MONSTER/engine/issues/1077); +a restart policy is what turns a four-hour outage into a thirty-second one. + +## The unit, and why each line is there + +`scripts/install-ci-runner.sh` writes this (values shown for the defaults): + +```ini +[Unit] +Description=GitHub Actions runner strixhalo-engine (dedicated user ghrunner) +After=network-online.target +Wants=network-online.target +StartLimitIntervalSec=0 + +[Service] +Type=simple +User=ghrunner +WorkingDirectory=/var/lib/ghrunner/actions-runner-strixhalo-engine +ExecStart=/var/lib/ghrunner/actions-runner-strixhalo-engine/runsvc.sh +Restart=always +RestartSec=30 +OOMPolicy=continue +MemoryHigh=43G +MemoryMax=48G +TasksMax=8192 +KillMode=process +KillSignal=SIGTERM +TimeoutStopSec=5min + +[Install] +WantedBy=multi-user.target +``` + +- **`Restart=always`, not `on-failure`** — because `runsvc.sh` reports success when it is killed. `always` is the + only value that brings a runner back from that exit code. `RestartSec=30` keeps a crash-looping runner from + hammering the box and the GitHub API. +- **`StartLimitIntervalSec=0`** — a runner must never be rate-limited out of existence. The 30-second spacing is + the rate limit; systemd giving up after five starts in ten seconds (the default) is how "stays dead" happens + again. +- **`OOMPolicy=continue`** — this is the line the old units lacked. With the default (`stop`), a process in the + cgroup being OOM-killed stops the *unit* with `Result=oom-kill`. With `continue`, the kernel's victim is the + one that dies — the job — and the runner keeps running, so the job fails with a reason instead of the runner + vanishing. +- **`MemoryHigh`/`MemoryMax`** — the cgroup hits its own ceiling before the box's OOM killer has to choose + between "the largest allocation on the machine" and everything else. The default 48G matches the bench rule + (`systemd-run --user --scope -p MemoryMax=48G`), so a heavy job is killed by *its* limit, not by the box's. +- **`KillMode=process`** with the 5-minute stop timeout — the behaviour the shipped units already had: stop the + listener, let the job's own timeout end its children. + +## The dedicated user + +The jobs used to run as the owner (`User=bcloud`). They now run as `ghrunner`: no shell, home +`/var/lib/ghrunner`, and the runner's `_work` tree under it. That account holds no ssh keys, no `~/models` and no +fleet remotes — anything a job writes into `$HOME` lands there, not in the owner's tree. + +What it still needs, and the honest price of each: + +| grant | why | price | +|---|---|---| +| `docker` group | the bench jobs run containers (podman/docker images for the distro proofs) | **docker group is root-equivalent** — a job that runs `docker run -v /:/h …` can read the box. It is narrower than the owner's account but not a sandbox | +| `render` (or `video`) group | `/dev/kfd` and `/dev/dri/renderD*` at `0660` for the GPU jobs | device access only | +| read on the models directory | `pr-agent` starts the local LLM through `1bit serve` | the same models any job could already load | + +If that trade is not wanted, the next step is rootless podman (or a docker socket proxy that only allows the +images the workflows name) and dropping the `docker` group — that is a change to the workflows, not to this +recipe, and it is not in #1504's scope. + +## A fresh install, start to finish + +```bash +# 1. the runner, unpacked and verified. Pin the version; the script checks the release's SHA256SUMS and +# refuses an unverified tarball. +curl -fsSLO https://github.com/actions/runner/releases/download/v2.329.0/actions-runner-linux-x64-2.329.0.tar.gz +mkdir -p /srv/actions-runner-2.329.0 && tar -C /srv/actions-runner-2.329.0 -xzf actions-runner-linux-x64-2.329.0.tar.gz + +# 2. a registration token: repo Settings -> Actions -> Runners -> New self-hosted runner, or +# gh api -X POST repos/1bit-MONSTER/engine/actions/runners/registration-token --jq .token > /root/runner-token +# It is a one-hour secret: write it to a root-only file and delete it after the install. + +# 3. install both runners +sudo scripts/install-ci-runner.sh --name strixhalo-engine --labels pr-agent \ + --token-file /root/runner-token --runner-dir /srv/actions-runner-2.329.0 +sudo scripts/install-ci-runner.sh --name strixhalo-bench --labels bench \ + --token-file /root/runner-token --runner-dir /srv/actions-runner-2.329.0 +sudo rm -f /root/runner-token +``` + +`--dry-run` prints the unit and runs `systemd-analyze verify` on it without changing anything; `--verify` prints +the installed unit's `Result`/`NRestarts`/`OOMPolicy` and exits. The script is idempotent: an existing `ghrunner` +user, an already-configured runner directory and an existing unit are reused or rewritten, not re-created. + +## Verifying an install + +```bash +# the unit is up and has not been restarting +systemctl show actions.runner.1bit-MONSTER-engine.strixhalo-engine.service \ + -p ActiveState -p Result -p NRestarts -p OOMPolicy -p MemoryMax +# ActiveState=active Result=success NRestarts=0 OOMPolicy=continue MemoryMax=51539607552 + +# GitHub agrees it is online and idle +gh api repos/1bit-MONSTER/engine/actions/runners --jq '.runners[] | "\(.name) \(.status) busy=\(.busy)"' +# strixhalo-bench online busy=false +# strixhalo-engine online busy=false +``` + +And the OOM behaviour itself, which is what #1504 is about: with `OOMPolicy=continue`, a process killed inside the +unit's cgroup does not stop the unit (`systemd.service(5)`: "the service manager will not take any action"), so +the job fails and the runner keeps listening; with the default `stop` it ends up exactly as it did on 11-08 — +`Result=oom-kill`, `NRestarts=0`. + +```bash +# the state to read after a heavy job has been killed, on a freshly installed runner: +systemctl show actions.runner.1bit-MONSTER-engine.strixhalo-bench.service -p Result -p NRestarts +# Result=success <- the job died, the runner did not +# NRestarts=0 <- nothing had to restart it +``` + +**Not yet demonstrated end to end here.** A throwaway `systemd-run --user` pair with and without +`OOMPolicy=continue` (128M cap, a child allocating 256M) both came back `Result=success` on this box — the child's +allocation failed as an exception rather than as an OOM kill, so that experiment says nothing either way. The +directive is the documented mechanism and it is what the fresh install's single end-to-end run should confirm, +together with the rest of #1504's acceptance. + +## What a restart policy cannot do + +It cannot notice that a runner is *not* coming back (a version mismatch, a revoked token, a full disk under +`/var/lib/ghrunner`). That is [engine #1504](https://github.com/1bit-MONSTER/engine/issues/1504)'s item 3 and +still open: a cheap row that asserts both self-hosted runners are `online=busy=false` through the API the release +lanes already use, so a runner that is gone turns a check red instead of leaving it pending for ever. diff --git a/scripts/install-ci-runner.sh b/scripts/install-ci-runner.sh new file mode 100755 index 000000000..96c738d87 --- /dev/null +++ b/scripts/install-ci-runner.sh @@ -0,0 +1,212 @@ +#!/usr/bin/env bash +# Copyright 2026 bong-water-water-bong +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# install-ci-runner.sh: install one self-hosted GitHub Actions runner as a systemd service, owned by a +# dedicated unprivileged user, with the restart and OOM policy that engine #1504 asks for. +# +# sudo scripts/install-ci-runner.sh --name strixhalo-engine --labels pr-agent \ +# --token-file /root/runner-token --runner-dir /srv/actions-runner-2.329.0 +# +# It runs on the box the jobs run on (Strix Halo), not from CI. It creates the user, the runner directory, +# the unit, and starts it; it is idempotent (an existing user or runner config is reused) and it never +# prints the registration token. +# +# Why each policy is here, in one line each - the detail is in docs/ci-runners.md: +# Restart=always runsvc.sh exits 0 after an OOM kill ("no retry needed"), so on-failure never fires +# OOMPolicy=continue an OOM kill inside the cgroup kills the JOB; the runner keeps running +# MemoryHigh/Max the cgroup hits its own ceiling first, so the box's OOM killer is not choosing +# between "the biggest allocation on the machine" and everything else +# User=ghrunner the jobs stop running as the owner: no ssh keys, no ~/models, no fleet remotes +# StartLimitIntervalSec=0 a runner must never be rate-limited out of existence; RestartSec bounds the loop +# +# Options: +# --name NAME runner name and unit suffix (default: strixhalo-engine) +# --labels LIST extra labels beyond self-hosted,Linux,X64 (default: pr-agent) +# --token-file FILE file holding a registration token (repo Settings -> Actions -> Runners -> New) +# --runner-dir DIR an already-unpacked runner (config.sh/runsvc.sh in it). Downloaded if omitted, +# with --runner-version, and verified against the release's SHA256SUMS +# --runner-version VER e.g. 2.329.0 (only used when downloading) +# --user USER dedicated user (default: ghrunner) +# --groups LIST extra groups for that user (default: docker; use "" for none) +# --memory-max SIZE MemoryMax for the unit (default: 48G); MemoryHigh is 90% of it +# --env-file FILE optional EnvironmentFile= for the unit +# --dry-run print the unit and verify it, change nothing +# --verify only report the installed unit's state +set -euo pipefail + +name=strixhalo-engine labels=pr-agent user=ghrunner groups=docker memory_max=48G +token_file='' runner_dir='' runner_version='' env_file='' dry=0 verify=0 +usage() { sed -n '16,48p' "$0"; } + +while [ $# -gt 0 ]; do + case $1 in + --name) name=$2; shift 2 ;; + --labels) labels=$2; shift 2 ;; + --token-file) token_file=$2; shift 2 ;; + --runner-dir) runner_dir=$2; shift 2 ;; + --runner-version) runner_version=$2; shift 2 ;; + --user) user=$2; shift 2 ;; + --groups) groups=$2; shift 2 ;; + --memory-max) memory_max=$2; shift 2 ;; + --env-file) env_file=$2; shift 2 ;; + --dry-run) dry=1; shift ;; + --verify) verify=1; shift ;; + -h|--help) usage; exit 0 ;; + *) echo "install-ci-runner.sh: unknown option $1" >&2; usage >&2; exit 2 ;; + esac +done + +unit=actions.runner.1bit-MONSTER-engine.$name.service +unit_path=/etc/systemd/system/$unit +dir=/var/lib/$user/actions-runner-$name +memory_high=$(awk -v m="$memory_max" 'BEGIN{ sub(/G$/,"",m); printf "%dG", int(m * 0.9) }') + +# <-- the unit this recipe installs +unit_text() { + cat </dev/null +} + +if [ "$verify" = 1 ]; then + state + exit 0 +fi + +if [ "$dry" = 1 ]; then + unit_text + # verify the DIRECTIVES, not the install: ExecStart does not exist yet, and systemd-analyze treats a missing + # binary as an error. The copy it verifies points at /bin/true, so an unknown or malformed setting still + # fails here while the not-yet-installed path does not. + tmp=$(mktemp --suffix=.service) + unit_text | sed "s|^ExecStart=.*|ExecStart=/bin/true|" > "$tmp" + if systemd-analyze verify "$tmp"; then echo "dry run: every directive verifies; nothing was changed" + else echo "dry run: the unit does not verify" >&2; rm -f "$tmp"; exit 1; fi + rm -f "$tmp" + exit 0 +fi + +[ "$(id -u)" = 0 ] || { echo "install-ci-runner.sh: run as root (it creates a user and a unit)" >&2; exit 2; } +[ -n "$token_file" ] && [ -s "$token_file" ] || { echo "install-ci-runner.sh: --token-file is required and must not be empty" >&2; exit 2; } + +# the dedicated user (engine #1504's audit finding): the jobs must not run as the owner +if ! id "$user" >/dev/null 2>&1; then + useradd --system --create-home --home-dir "/var/lib/$user" --shell /usr/sbin/nologin "$user" + echo "created user $user (no shell, home /var/lib/$user)" +fi +for g in ${groups//,/ }; do + getent group "$g" >/dev/null 2>&1 || { echo "warning: no group '$g' on this box; $user is not in it (jobs that need it will fail loudly)" >&2; continue; } + id -nG "$user" | tr ' ' '\n' | grep -qx "$g" || usermod -aG "$g" "$user" +done + +# the runner itself +mkdir -p "/var/lib/$user" && chown "$user:" "/var/lib/$user" +if [ ! -x "$dir/config.sh" ]; then + if [ -n "$runner_dir" ]; then + [ -x "$runner_dir/config.sh" ] || { echo "install-ci-runner.sh: $runner_dir is not an unpacked runner" >&2; exit 2; } + mkdir -p "$dir" && cp -a "$runner_dir/." "$dir/" + else + [ -n "$runner_version" ] || { echo "install-ci-runner.sh: --runner-dir or --runner-version is required" >&2; exit 2; } + tarball=actions-runner-linux-x64-$runner_version.tar.gz + tmpd=$(mktemp -d) + curl -fsSL -o "$tmpd/$tarball" \ + "https://github.com/actions/runner/releases/download/v$runner_version/$tarball" + curl -fsSL -o "$tmpd/SHA256SUMS" \ + "https://github.com/actions/runner/releases/download/v$runner_version/SHA256SUMS" || true + if [ -s "$tmpd/SHA256SUMS" ]; then + ( cd "$tmpd" && grep " $tarball\$" SHA256SUMS | sha256sum -c - ) || { + echo "install-ci-runner.sh: $tarball does not match the release's SHA256SUMS" >&2; exit 1; } + else + echo "install-ci-runner.sh: no SHA256SUMS on that release; refusing to install an unverified runner" >&2 + exit 1 + fi + mkdir -p "$dir" && tar -C "$dir" -xzf "$tmpd/$tarball" + rm -rf "$tmpd" + fi + chown -R "$user:" "$dir" +fi + +# register, as the runner's own user, with the token read from the file and never echoed +if [ ! -f "$dir/.runner" ]; then + tok=$(mktemp) + cp "$token_file" "$tok"; chown "$user:" "$tok"; chmod 0600 "$tok" + # config.sh takes the token as an argument and there is no stdin form, so it is in the child's argv for the + # length of the call; the file is 0600 and root-owned, this copy is 0600 and the runner user's, and both the + # copy and the temp dir go away here. The token itself is a one-hour registration token, never a PAT. + sudo -u "$user" bash -c "cd '$dir' && ./config.sh --unattended --replace \ + --url https://github.com/1bit-MONSTER/engine \ + --token \"\$(cat '$tok')\" \ + --name '$name' --labels '$labels' --work '_work'" > /dev/null + rm -f "$tok" + echo "registered runner $name with labels self-hosted,Linux,X64,$labels" +fi +chown -R "$user:" "$dir" + +unit_text > "$unit_path" +chmod 0644 "$unit_path" +systemd-analyze verify "$unit_path" +systemctl daemon-reload +systemctl enable --now "$unit" +sleep 5 +echo +echo "installed $unit as user $user, MemoryMax=$memory_max, OOMPolicy=continue, Restart=always" +state +cat < Date: Mon, 9 Nov 2026 04:34:32 -0400 Subject: [PATCH 2/6] ci: the runner keeps the fleet's GPU/NPU token, or the gate quietly bypasses it (Refs #1504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review's CHANGES, and the second point is the one that made the recipe wrong rather than incomplete: a runner whose home is empty does not merely lose `thermal-run.sh`, it takes a DIFFERENT GPU token. 1. **The fleet's shared state is linked into the runner's home**, and the account joins the owner's group: - `~/lb/thermal-run.sh` is what `hrx-build.yml:111,132` and `hw-gate.yml:33-34` run every build under, and `thermal-run.sh` itself talks to `systemctl --user` — so the installer also does `loginctl enable-linger` for the runner's account, or thermal-run falls back to its weaker process-group mode. - `gpu-hold`/`npu-hold` are resolved by `command -v` (`hw-gate.sh:53-88`), i.e. through `~/.local/bin` on PATH; the unit now sets `Environment=PATH=/var/lib/ghrunner/.local/bin:…`, and that directory is the shared one. - `~/GPU-TOKEN`, `~/GPU-QUEUE`, `~/NPU-TOKEN`, `~/NPU-QUEUE` are linked too, and the installer makes the queue directories and token files group-writable (and no longer world-accessible). Run as `ghrunner` with an empty home, the hardware gate queued on ITS OWN token file while every lane held the real one — GPU work concurrent with a holder, which is the one thing the hold rule exists to prevent. `--no-share` skips it for a box with no fleet. 2. **`~/.cache` is deliberately not shared** (it is 0700 for the owner): the persistent build trees (`~/.cache/hrx-build/bench`, `~/.cache/hw-gate`) and `pr-agent-venv-*` move, so the first runs on a fresh install are cold and `hrx-build`'s "rebuild only what changed" starts from zero once. That is now a sentence in the docs, which is what the review asked for. 3. **The isolation claim is corrected.** "No ssh keys, no ~/models" is true of `ghrunner`'s own home, but `/home/bcloud` is `drwxrwxr-x` here, so a job as `ghrunner` can read whatever in the owner's home is world-readable. The docs say the isolation is per-process/per-account, and that read-isolation would be `chmod o-rx /home/bcloud` — the owner's call, since it changes the box for every other lane. Measured on this box while writing it: `/home/bcloud` 0775, `~/lb` 0775, `~/.local/bin` 0775, `~/GPU-TOKEN` 0664, `~/GPU-QUEUE` 0775, `~/.cache` 0700 — which is what decides, per path, whether a link plus the group is enough or the path has to stay out of the shared set. The live queue and token files were READ, never modified: the chgrp/ chmod lines in the installer are for the fresh install, and they are idempotent. The unit still verifies (`systemd-analyze verify`, and a corrupted directive is still caught); `bash -n` and `shellcheck -S warning` clean; `tools/copyright.py --check` rc 0. --- docs/ci-runners.md | 39 ++++++++++++++++++++++++--- scripts/install-ci-runner.sh | 51 +++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index 8aca7f855..68932fe1e 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -96,8 +96,41 @@ WantedBy=multi-user.target ## The dedicated user The jobs used to run as the owner (`User=bcloud`). They now run as `ghrunner`: no shell, home -`/var/lib/ghrunner`, and the runner's `_work` tree under it. That account holds no ssh keys, no `~/models` and no -fleet remotes — anything a job writes into `$HOME` lands there, not in the owner's tree. +`/var/lib/ghrunner`, and the runner's `_work` tree under it. Anything a job writes into `$HOME` lands there, not +in the owner's tree, and the account holds no ssh keys and no fleet git remotes of its own. + +**It is not read-isolation of the owner's files, though** — `/home/bcloud` is `drwxrwxr-x` on this box, so a job +as `ghrunner` can read whatever in the owner's home is world-readable. The isolation is per-process and +per-account (a job cannot use the owner's keys to push, and its `$HOME` writes are its own); if read-isolation is +wanted too, that is `chmod o-rx /home/bcloud` — the owner's call, because it changes the box for every other lane +as well. + +### The fleet's shared state, which the jobs assume is in `$HOME` + +The workflows and the scripts they call reach the box's shared state through `$HOME`, so a runner with an empty +home does not merely lose the tools — **it takes a different GPU token**: + +| the jobs use | via | +|---|---| +| `~/lb/thermal-run.sh` (`hrx-build.yml:111,132`, `hw-gate.yml:33-34`) | the thermal governor every build runs under | +| `gpu-hold`, `npu-hold` | `command -v`, i.e. `~/.local/bin` on `PATH` (`hw-gate.sh:53-88`) | +| `~/GPU-TOKEN`, `~/GPU-QUEUE`, `~/NPU-TOKEN`, `~/NPU-QUEUE` | the token files the whole fleet queues on | + +Run as `ghrunner` with an empty home, `command -v gpu-hold` finds nothing (or the repo copy beside the script), +`~` resolves to `/var/lib/ghrunner`, and the hardware gate then queues on **its own** token file while every +other lane holds the real one — GPU work running concurrently with a holder, which is exactly what the "devices +only via gpu-hold" rule exists to prevent. + +So the installer links those six paths from the runner's home into the account that already has them +(`--share-with`, default `bcloud`), puts the runner's account in that account's group, makes the queue +directories and token files group-writable (removing world access), sets `PATH` in the unit so `command -v` +resolves the shared `gpu-hold`, and enables lingering for the runner's user so `thermal-run.sh`'s +`systemctl --user` has a user manager to talk to. `--no-share` skips all of that, for a box with no fleet. + +What that deliberately does **not** share is `~/.cache`, which is `0700` on the owner's account: the persistent +build trees (`~/.cache/hrx-build/bench`, `~/.cache/hw-gate`) and the `pr-agent-venv-*` environments therefore +move, and the first runs on a fresh install are cold — `hrx-build`'s "rebuild only what changed" starts from +zero once. What it still needs, and the honest price of each: @@ -105,7 +138,7 @@ What it still needs, and the honest price of each: |---|---|---| | `docker` group | the bench jobs run containers (podman/docker images for the distro proofs) | **docker group is root-equivalent** — a job that runs `docker run -v /:/h …` can read the box. It is narrower than the owner's account but not a sandbox | | `render` (or `video`) group | `/dev/kfd` and `/dev/dri/renderD*` at `0660` for the GPU jobs | device access only | -| read on the models directory | `pr-agent` starts the local LLM through `1bit serve` | the same models any job could already load | +| the fleet group (the owner's primary group) | the shared `~/lb`, `~/.local/bin` and the queue/token files above | jobs write the real queue, so a runner is a full member of the fleet, not a bystander — which is the point | If that trade is not wanted, the next step is rootless podman (or a docker socket proxy that only allows the images the workflows name) and dropping the `docker` group — that is a change to the workflows, not to this diff --git a/scripts/install-ci-runner.sh b/scripts/install-ci-runner.sh index 96c738d87..d1e5153d1 100755 --- a/scripts/install-ci-runner.sh +++ b/scripts/install-ci-runner.sh @@ -42,13 +42,19 @@ # --user USER dedicated user (default: ghrunner) # --groups LIST extra groups for that user (default: docker; use "" for none) # --memory-max SIZE MemoryMax for the unit (default: 48G); MemoryHigh is 90% of it +# --share-with USER the existing user whose fleet state the jobs must keep using (default: the owner +# account, bcloud). Its ~/lb, ~/.local/bin, ~/GPU-TOKEN, ~/GPU-QUEUE, ~/NPU-TOKEN +# and ~/NPU-QUEUE are linked into the runner's home, and the runner's account joins +# that user's group so it can write the queue. Without this a job would take ITS OWN +# GPU token and run GPU work concurrently with every lane holding the real one +# --no-share skip that (only for a box with no fleet to share with) # --env-file FILE optional EnvironmentFile= for the unit # --dry-run print the unit and verify it, change nothing # --verify only report the installed unit's state set -euo pipefail name=strixhalo-engine labels=pr-agent user=ghrunner groups=docker memory_max=48G -token_file='' runner_dir='' runner_version='' env_file='' dry=0 verify=0 +token_file='' runner_dir='' runner_version='' env_file='' share_with=bcloud no_share=0 dry=0 verify=0 usage() { sed -n '16,48p' "$0"; } while [ $# -gt 0 ]; do @@ -62,6 +68,8 @@ while [ $# -gt 0 ]; do --groups) groups=$2; shift 2 ;; --memory-max) memory_max=$2; shift 2 ;; --env-file) env_file=$2; shift 2 ;; + --share-with) share_with=$2; shift 2 ;; + --no-share) no_share=1; shift ;; --dry-run) dry=1; shift ;; --verify) verify=1; shift ;; -h|--help) usage; exit 0 ;; @@ -91,6 +99,8 @@ Type=simple User=$user WorkingDirectory=$dir ExecStart=$dir/runsvc.sh +# hw-gate.sh resolves gpu-hold with `command -v`, and the runner's PATH is what decides which token it takes +Environment=PATH=/var/lib/$user/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin $( [ -n "$env_file" ] && echo "EnvironmentFile=$env_file" ) # runsvc.sh exits 0 after an OOM kill ("no retry needed"), so on-failure would leave the runner dead. Restart=always @@ -191,6 +201,45 @@ if [ ! -f "$dir/.runner" ]; then fi chown -R "$user:" "$dir" +# --- the fleet's state, at the paths the jobs already assume --------------------------------------------- +# The workflows and scripts reach the box's shared state through $HOME: ~/lb/thermal-run.sh (the thermal +# governor every build runs under), ~/.local/bin/gpu-hold (found with `command -v`), and the queue/token files +# ~/GPU-TOKEN, ~/GPU-QUEUE, ~/NPU-TOKEN, ~/NPU-QUEUE. A runner whose home is empty therefore does not just lose +# the tools - it takes a DIFFERENT GPU token and queues beside nobody, which is the one thing the fleet's hold +# rule exists to prevent (engine #1504 review). So the runner's home carries the same absolute paths, linked +# into the account that already holds them, and the runner joins that account's group to write the queue. +if [ "$no_share" = 0 ]; then + id "$share_with" >/dev/null 2>&1 || { echo "install-ci-runner.sh: --share-with user '$share_with' does not exist (use --no-share on a box with no fleet)" >&2; exit 2; } + share_home=$(getent passwd "$share_with" | cut -d: -f6) + share_group=$(id -gn "$share_with") + id -nG "$user" | tr ' ' '\n' | grep -qx "$share_group" || usermod -aG "$share_group" "$user" + # the tools and the hold state, by name, only when they exist: an install on a box that has not set them + # up yet is not an error, it is a missing feature the docs name + for rel in lb .local/bin GPU-TOKEN GPU-QUEUE NPU-TOKEN NPU-QUEUE; do + src="$share_home/$rel" dst="/var/lib/$user/$rel" + if [ -e "$src" ]; then + mkdir -p "$(dirname "$dst")" + [ -L "$dst" ] && [ "$(readlink "$dst")" = "$src" ] && continue + if [ -e "$dst" ] && [ ! -L "$dst" ]; then + echo "warning: /var/lib/$user/$rel exists as a real path; leaving it (move it aside to share $src)" >&2 + continue + fi + ln -sfn "$src" "$dst" + echo "shared: ~/$rel -> $src" + fi + done + # the queue and the token files must be writable by the runner's new group; the modes are the owner's, so + # they are tightened here rather than left to chance (0775 dir / 0664 file: group read+write, no world) + [ -d "$share_home/GPU-QUEUE" ] && chgrp -R "$share_group" "$share_home/GPU-QUEUE" && chmod -R g+rwX,o-rwx "$share_home/GPU-QUEUE" + [ -d "$share_home/NPU-QUEUE" ] && chgrp -R "$share_group" "$share_home/NPU-QUEUE" && chmod -R g+rwX,o-rwx "$share_home/NPU-QUEUE" + for f in GPU-TOKEN NPU-TOKEN; do + if [ -f "$share_home/$f" ]; then chgrp "$share_group" "$share_home/$f"; chmod g+rw,o-rw "$share_home/$f"; fi + done + # thermal-run.sh runs under `systemctl --user`, so the account needs a user manager of its own + loginctl enable-linger "$user" 2>/dev/null || echo "warning: could not enable-linger $user; thermal-run.sh will use its process-group fallback" + echo "shared state: $share_with's group ($share_group), queue and token files group-writable" +fi + unit_text > "$unit_path" chmod 0644 "$unit_path" systemd-analyze verify "$unit_path" From 84272601bbfa409415ab63c514082c1cfc484966 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Mon, 9 Nov 2026 04:51:33 -0400 Subject: [PATCH 3/6] ci: a hold group that owns only the hold state, not the owner's home (Refs #1504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review measured the escalation and it was real: putting the runner's account in the owner's group is group write on /home/bcloud, which is 0775 - so a job running unreviewed PR code could rewrite ~/.local/bin/gpu-hold (which every lane then executes), replace ~/.bashrc, or move ~/.ssh aside. That is an owner-level code path created by a recipe whose point is that jobs stop running as the owner. Removed entirely. And reading the release path adds a constraint the group-on-the-files shape does not cover: gpu-hold releases a token with `rm -f "$TOKEN"`, which is a write to the DIRECTORY, so a hold file that stays inside the owner's home cannot be shared with another account by chgrp'ing the file - and a symlink is worse, because rm removes the link and leaves the real token held for ever. So the hold state moves to its own directory, owned by its own group: /var/lib/1bit-fleet/ 0770 fleethold, setgid gpu-token 0660 fleethold gpu-queue/ 0770 fleethold, setgid (tickets inherit the group) npu-token, npu-queue/ - `ghrunner` joins **fleethold and nothing else**. The shared tools are already world-readable and executable, so running them needs no group at all - only the hold files do. - the unit gets GPU_TOKEN_FILE/GPU_QUEUE_DIR, which gpu-hold honours, so a job queues with the fleet; - the owner's side must name the same two paths or the split reappears in the other direction, so the installer writes /etc/profile.d/1bit-fleet.sh and the docs say what a long-running owner service needs instead; - `npu-hold` wraps gpu-hold with `exec env GPU_QUEUE_DIR=~/NPU-QUEUE …`, overriding the environment, so NPU sharing needs that wrapper changed - named as a follow-up rather than claimed; - `--no-hold-state` for a runner with no hardware jobs (the pr-agent runner talks to the local LLM over HTTP); - `--migrate-hold-state` moves an existing hold state in, and REFUSES when the queue is not empty. Measured: the dry-run unit carries `Environment=GPU_TOKEN_FILE=/var/lib/1bit-fleet/gpu-token GPU_QUEUE_DIR=/var/lib/1bit-fleet/gpu-queue`, and `--no-hold-state` drops it (0 occurrences); the unit still verifies; bash -n and shellcheck -S warning clean; copyright rc 0. Nothing on the live box was modified - the recipe creates the group and directory on the fresh install, and the migration path is opt-in and queue-empty-gated. --- docs/ci-runners.md | 69 +++++++++++++------- scripts/install-ci-runner.sh | 118 +++++++++++++++++++++-------------- 2 files changed, 118 insertions(+), 69 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index 68932fe1e..7c3c99a12 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -105,32 +105,50 @@ per-account (a job cannot use the owner's keys to push, and its `$HOME` writes a wanted too, that is `chmod o-rx /home/bcloud` — the owner's call, because it changes the box for every other lane as well. -### The fleet's shared state, which the jobs assume is in `$HOME` +### One queue, two accounts -The workflows and the scripts they call reach the box's shared state through `$HOME`, so a runner with an empty -home does not merely lose the tools — **it takes a different GPU token**: +The workflows reach the box's hold state through `$HOME`: `~/GPU-TOKEN`, `~/GPU-QUEUE` (and `npu-hold`, which +hard-codes `~/NPU-TOKEN` and `~/NPU-QUEUE` around `gpu-hold`). A runner with a different home therefore does not +merely lose `thermal-run.sh` — it takes **its own** GPU token, and the hardware gate then runs GPU work +concurrently with whatever lane holds the real one, which is the one thing the hold rule exists to prevent. -| the jobs use | via | -|---|---| -| `~/lb/thermal-run.sh` (`hrx-build.yml:111,132`, `hw-gate.yml:33-34`) | the thermal governor every build runs under | -| `gpu-hold`, `npu-hold` | `command -v`, i.e. `~/.local/bin` on `PATH` (`hw-gate.sh:53-88`) | -| `~/GPU-TOKEN`, `~/GPU-QUEUE`, `~/NPU-TOKEN`, `~/NPU-QUEUE` | the token files the whole fleet queues on | +The obvious repair — put the runner's account in the owner's group — is wrong, and the review of this PR measured +why: `/home/bcloud` is `0775`, so group write on the **directory** lets a job rename or replace any entry in it. +A job could rewrite `~/.local/bin/gpu-hold` (which every lane then executes), replace `~/.bashrc`, or move +`~/.ssh` aside and put its own directory in its place. That is an owner-level code path created by a recipe whose +whole point is that jobs no longer run as the owner. -Run as `ghrunner` with an empty home, `command -v gpu-hold` finds nothing (or the repo copy beside the script), -`~` resolves to `/var/lib/ghrunner`, and the hardware gate then queues on **its own** token file while every -other lane holds the real one — GPU work running concurrently with a holder, which is exactly what the "devices -only via gpu-hold" rule exists to prevent. +So the hold state lives in its own directory, owned by its own group: -So the installer links those six paths from the runner's home into the account that already has them -(`--share-with`, default `bcloud`), puts the runner's account in that account's group, makes the queue -directories and token files group-writable (removing world access), sets `PATH` in the unit so `command -v` -resolves the shared `gpu-hold`, and enables lingering for the runner's user so `thermal-run.sh`'s -`systemctl --user` has a user manager to talk to. `--no-share` skips all of that, for a box with no fleet. +``` +/var/lib/1bit-fleet/ 0770 fleethold, setgid + gpu-token 0660 fleethold <- what gpu-hold writes, and rm -f's on release + gpu-queue/ 0770 fleethold, setgid <- tickets inherit the group + npu-token, npu-queue/ +``` -What that deliberately does **not** share is `~/.cache`, which is `0700` on the owner's account: the persistent -build trees (`~/.cache/hrx-build/bench`, `~/.cache/hw-gate`) and the `pr-agent-venv-*` environments therefore -move, and the first runs on a fresh install are cold — `hrx-build`'s "rebuild only what changed" starts from -zero once. +- `ghrunner` joins **`fleethold` and nothing else**. The shared tools (`~/lb`, `~/.local/bin`) are already + world-readable and executable, so no group is needed to *run* them — only the hold files need the group. +- The unit gets `Environment=GPU_TOKEN_FILE=/var/lib/1bit-fleet/gpu-token GPU_QUEUE_DIR=/var/lib/1bit-fleet/gpu-queue`, + which `gpu-hold` honours, so a job queues with the fleet. +- **The owner's side has to name the same two paths**, or the split simply reappears in the other direction. The + installer writes `/etc/profile.d/1bit-fleet.sh` exporting them for the owner's shells; a long-running owner + service needs them in its own unit or environment. +- **Why a shared *file* inside the owner's home cannot work at all:** `gpu-hold` releases with + `rm -f "$TOKEN"`, a write to the containing **directory**. `chgrp` on the file is not enough, and a symlink is + worse — `rm` removes the link and leaves the real token held for ever. +- `npu-hold` wraps `gpu-hold` with `exec env GPU_QUEUE_DIR=~/NPU-QUEUE GPU_TOKEN_FILE=~/NPU-TOKEN …`, so it + overrides whatever the environment says: **NPU sharing needs that wrapper changed** to honour the two + variables. Until then the NPU queue is per-account, and that is a named follow-up rather than a claim. +- `--no-hold-state` exists for a runner that runs no hardware jobs — the `pr-agent` runner does not (it talks to + the local LLM over HTTP on `127.0.0.1:8090`), so it needs none of this. +- `--migrate-hold-state` moves an existing `~/GPU-{TOKEN,QUEUE}`, `~/NPU-{TOKEN,QUEUE}` into `/var/lib/1bit-fleet`, + and **refuses when the queue is not empty**: moving a live queue out from under the lanes is not a step to + take blind. + +What is deliberately **not** shared is `~/.cache`, which is `0700` on the owner's account: the persistent build +trees (`~/.cache/hrx-build/bench`, `~/.cache/hw-gate`) and the `pr-agent-venv-*` environments move, so the first +runs on a fresh install are cold — `hrx-build`'s "rebuild only what changed" starts from zero once. What it still needs, and the honest price of each: @@ -138,7 +156,7 @@ What it still needs, and the honest price of each: |---|---|---| | `docker` group | the bench jobs run containers (podman/docker images for the distro proofs) | **docker group is root-equivalent** — a job that runs `docker run -v /:/h …` can read the box. It is narrower than the owner's account but not a sandbox | | `render` (or `video`) group | `/dev/kfd` and `/dev/dri/renderD*` at `0660` for the GPU jobs | device access only | -| the fleet group (the owner's primary group) | the shared `~/lb`, `~/.local/bin` and the queue/token files above | jobs write the real queue, so a runner is a full member of the fleet, not a bystander — which is the point | +| `fleethold` (its own group) | write the hold state in `/var/lib/1bit-fleet` | the runner can write the queue and the token files **and nothing else** — not the owner's home, not the tools it runs | If that trade is not wanted, the next step is rootless podman (or a docker socket proxy that only allows the images the workflows name) and dropping the `docker` group — that is a change to the workflows, not to this @@ -162,8 +180,15 @@ sudo scripts/install-ci-runner.sh --name strixhalo-engine --labels pr-agent \ sudo scripts/install-ci-runner.sh --name strixhalo-bench --labels bench \ --token-file /root/runner-token --runner-dir /srv/actions-runner-2.329.0 sudo rm -f /root/runner-token + +# 4. the owner's own lanes must name the same hold state (the installer writes this file); a long-running +# owner service needs the two variables in its own unit instead +. /etc/profile.d/1bit-fleet.sh ``` +On a box that already has hold state in the owner's home, add `--migrate-hold-state --owner-user ` to the +first install — with the queue drained, which the installer enforces. + `--dry-run` prints the unit and runs `systemd-analyze verify` on it without changing anything; `--verify` prints the installed unit's `Result`/`NRestarts`/`OOMPolicy` and exits. The script is idempotent: an existing `ghrunner` user, an already-configured runner directory and an existing unit are reused or rewritten, not re-created. diff --git a/scripts/install-ci-runner.sh b/scripts/install-ci-runner.sh index d1e5153d1..ee887d12c 100755 --- a/scripts/install-ci-runner.sh +++ b/scripts/install-ci-runner.sh @@ -42,19 +42,28 @@ # --user USER dedicated user (default: ghrunner) # --groups LIST extra groups for that user (default: docker; use "" for none) # --memory-max SIZE MemoryMax for the unit (default: 48G); MemoryHigh is 90% of it -# --share-with USER the existing user whose fleet state the jobs must keep using (default: the owner -# account, bcloud). Its ~/lb, ~/.local/bin, ~/GPU-TOKEN, ~/GPU-QUEUE, ~/NPU-TOKEN -# and ~/NPU-QUEUE are linked into the runner's home, and the runner's account joins -# that user's group so it can write the queue. Without this a job would take ITS OWN -# GPU token and run GPU work concurrently with every lane holding the real one -# --no-share skip that (only for a box with no fleet to share with) +# --hold-state DIR where the fleet's GPU/NPU token and queue files live (default /var/lib/1bit-fleet). +# The installer creates it group-owned by --hold-group, mode 0770 with the setgid bit so +# tickets inherit the group, and puts the runner's account in that group - NOT in the +# owner's group. The unit is given GPU_TOKEN_FILE/GPU_QUEUE_DIR so the job queues with +# the fleet instead of beside it, and the owner's own environment must name the same +# two variables (docs/ci-runners.md, "One queue, two accounts"). A file inside the +# owner's home cannot be shared this way: gpu-hold RELEASES with `rm -f "$TOKEN"`, +# which needs write on the DIRECTORY, i.e. on the owner's home itself. +# --hold-group GROUP the group that owns the hold state (default fleethold) +# --no-hold-state leave the runner without hold state, for a runner that runs no hardware jobs (the +# pr-agent runner does not: it talks to the local LLM over HTTP). A runner without it +# must not run hw-gate/hrx-build, which is why the default is to set it up +# --migrate-hold-state move an existing ~/GPU-TOKEN, ~/GPU-QUEUE, ~/NPU-TOKEN, ~/NPU-QUEUE of +# --owner-user into --hold-state. Requires a quiet queue: it is the one step that +# changes the paths under every running lane # --env-file FILE optional EnvironmentFile= for the unit # --dry-run print the unit and verify it, change nothing # --verify only report the installed unit's state set -euo pipefail name=strixhalo-engine labels=pr-agent user=ghrunner groups=docker memory_max=48G -token_file='' runner_dir='' runner_version='' env_file='' share_with=bcloud no_share=0 dry=0 verify=0 +token_file='' runner_dir='' runner_version='' env_file='' hold_state=/var/lib/1bit-fleet hold_group=fleethold owner_user=bcloud no_hold=0 migrate=0 dry=0 verify=0 usage() { sed -n '16,48p' "$0"; } while [ $# -gt 0 ]; do @@ -68,8 +77,11 @@ while [ $# -gt 0 ]; do --groups) groups=$2; shift 2 ;; --memory-max) memory_max=$2; shift 2 ;; --env-file) env_file=$2; shift 2 ;; - --share-with) share_with=$2; shift 2 ;; - --no-share) no_share=1; shift ;; + --hold-state) hold_state=$2; shift 2 ;; + --hold-group) hold_group=$2; shift 2 ;; + --owner-user) owner_user=$2; shift 2 ;; + --no-hold-state) no_hold=1; shift ;; + --migrate-hold-state) migrate=1; shift ;; --dry-run) dry=1; shift ;; --verify) verify=1; shift ;; -h|--help) usage; exit 0 ;; @@ -99,8 +111,7 @@ Type=simple User=$user WorkingDirectory=$dir ExecStart=$dir/runsvc.sh -# hw-gate.sh resolves gpu-hold with `command -v`, and the runner's PATH is what decides which token it takes -Environment=PATH=/var/lib/$user/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin +$( [ "$no_hold" = 0 ] && printf 'Environment=GPU_TOKEN_FILE=%s/gpu-token GPU_QUEUE_DIR=%s/gpu-queue\n' "$hold_state" "$hold_state" ) $( [ -n "$env_file" ] && echo "EnvironmentFile=$env_file" ) # runsvc.sh exits 0 after an OOM kill ("no retry needed"), so on-failure would leave the runner dead. Restart=always @@ -201,43 +212,56 @@ if [ ! -f "$dir/.runner" ]; then fi chown -R "$user:" "$dir" -# --- the fleet's state, at the paths the jobs already assume --------------------------------------------- -# The workflows and scripts reach the box's shared state through $HOME: ~/lb/thermal-run.sh (the thermal -# governor every build runs under), ~/.local/bin/gpu-hold (found with `command -v`), and the queue/token files -# ~/GPU-TOKEN, ~/GPU-QUEUE, ~/NPU-TOKEN, ~/NPU-QUEUE. A runner whose home is empty therefore does not just lose -# the tools - it takes a DIFFERENT GPU token and queues beside nobody, which is the one thing the fleet's hold -# rule exists to prevent (engine #1504 review). So the runner's home carries the same absolute paths, linked -# into the account that already holds them, and the runner joins that account's group to write the queue. -if [ "$no_share" = 0 ]; then - id "$share_with" >/dev/null 2>&1 || { echo "install-ci-runner.sh: --share-with user '$share_with' does not exist (use --no-share on a box with no fleet)" >&2; exit 2; } - share_home=$(getent passwd "$share_with" | cut -d: -f6) - share_group=$(id -gn "$share_with") - id -nG "$user" | tr ' ' '\n' | grep -qx "$share_group" || usermod -aG "$share_group" "$user" - # the tools and the hold state, by name, only when they exist: an install on a box that has not set them - # up yet is not an error, it is a missing feature the docs name - for rel in lb .local/bin GPU-TOKEN GPU-QUEUE NPU-TOKEN NPU-QUEUE; do - src="$share_home/$rel" dst="/var/lib/$user/$rel" - if [ -e "$src" ]; then - mkdir -p "$(dirname "$dst")" - [ -L "$dst" ] && [ "$(readlink "$dst")" = "$src" ] && continue - if [ -e "$dst" ] && [ ! -L "$dst" ]; then - echo "warning: /var/lib/$user/$rel exists as a real path; leaving it (move it aside to share $src)" >&2 - continue - fi - ln -sfn "$src" "$dst" - echo "shared: ~/$rel -> $src" - fi - done - # the queue and the token files must be writable by the runner's new group; the modes are the owner's, so - # they are tightened here rather than left to chance (0775 dir / 0664 file: group read+write, no world) - [ -d "$share_home/GPU-QUEUE" ] && chgrp -R "$share_group" "$share_home/GPU-QUEUE" && chmod -R g+rwX,o-rwx "$share_home/GPU-QUEUE" - [ -d "$share_home/NPU-QUEUE" ] && chgrp -R "$share_group" "$share_home/NPU-QUEUE" && chmod -R g+rwX,o-rwx "$share_home/NPU-QUEUE" - for f in GPU-TOKEN NPU-TOKEN; do - if [ -f "$share_home/$f" ]; then chgrp "$share_group" "$share_home/$f"; chmod g+rw,o-rw "$share_home/$f"; fi +# --- the fleet's hold state, in a directory the fleet group owns ----------------------------------------- +# The workflows reach the hold through $HOME (~/GPU-TOKEN, ~/GPU-QUEUE, and npu-hold's hard-coded +# ~/NPU-TOKEN, ~/NPU-QUEUE), and gpu-hold RELEASES a token with `rm -f "$TOKEN"` - a write to the DIRECTORY, +# not to the file. So a hold file that stays inside the owner's home cannot be shared with another account by +# chgrp'ing the file: either the runner is in the owner's group, which is group-write on the owner's whole home +# (/home/bcloud is 0775 here, so a job could replace ~/.bashrc or move ~/.ssh aside - an owner-level escalation +# created by a recipe whose point is that jobs no longer run as the owner), or the file is not shareable at all. +# +# So the hold state moves to a directory the FLEET group owns, and both sides name it: +# /{gpu-token,gpu-queue,npu-token,npu-queue} group-owned, 0770, queues setgid so tickets +# created by either account inherit the group +# The runner's account joins THAT group and nothing else; the shared tools (~/lb, ~/.local/bin) are already +# world-readable and executable, so no group is needed to run them. +if [ "$no_hold" = 0 ]; then + getent group "$hold_group" >/dev/null 2>&1 || groupadd --system "$hold_group" + id -nG "$user" | tr ' ' '\n' | grep -qx "$hold_group" || usermod -aG "$hold_group" "$user" + install -d -m 0770 -g "$hold_group" "$hold_state" "$hold_state/gpu-queue" "$hold_state/npu-queue" + chmod g+s "$hold_state/gpu-queue" "$hold_state/npu-queue" + for f in gpu-token npu-token; do + [ -e "$hold_state/$f" ] || install -m 0660 -g "$hold_group" /dev/null "$hold_state/$f" + chgrp "$hold_group" "$hold_state/$f"; chmod 0660 "$hold_state/$f" done - # thermal-run.sh runs under `systemctl --user`, so the account needs a user manager of its own - loginctl enable-linger "$user" 2>/dev/null || echo "warning: could not enable-linger $user; thermal-run.sh will use its process-group fallback" - echo "shared state: $share_with's group ($share_group), queue and token files group-writable" + if [ "$migrate" = 1 ]; then + owner_home=$(getent passwd "$owner_user" | cut -d: -f6) + [ -n "$owner_home" ] || { echo "install-ci-runner.sh: --migrate-hold-state needs --owner-user that exists" >&2; exit 2; } + # a live queue is not a thing to move under the lanes: refuse unless it is empty + [ -z "$(ls -A "$owner_home/GPU-QUEUE" 2>/dev/null)" ] || { + echo "install-ci-runner.sh: $owner_home/GPU-QUEUE is not empty: let the queue drain before migrating (or migrate by hand)" >&2; exit 1; } + for pair in "GPU-TOKEN:gpu-token" "NPU-TOKEN:npu-token"; do + src="$owner_home/${pair%%:*}"; dst="$hold_state/${pair##*:}" + [ -s "$src" ] && cp -a "$src" "$dst" && chgrp "$hold_group" "$dst" && chmod 0660 "$dst" && rm -f "$src" + done + for pair in "GPU-QUEUE:gpu-queue" "NPU-QUEUE:npu-queue"; do + src="$owner_home/${pair%%:*}"; dst="$hold_state/${pair##*:}" + [ -d "$src" ] && { cp -a "$src/." "$dst/" 2>/dev/null || true; rm -rf "$src"; } + done + chgrp -R "$hold_group" "$hold_state"; chmod -R g+rwX,o-rwx "$hold_state" + echo "migrated the hold state from $owner_home into $hold_state" + fi + # the owner's side has to name the same two paths, or the runner queues with the fleet's files and the + # owner's lanes with their own - which is the bug this whole block exists to prevent + cat > /etc/profile.d/1bit-fleet.sh < "$unit_path" From 0070323df163a31315a8ff6984d81db5582c1041 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Mon, 9 Nov 2026 05:14:44 -0400 Subject: [PATCH 4/6] ci: the queue is the fleet's by default, and the tools are reachable by the path the jobs use (Refs #1504) All four points from the review, and the third one changes the design rather than the recipe. 1. **`~/lb/thermal-run.sh` and `~/.local/bin` are back, as read-only links.** The workflows reach them by a HOME-relative path, not by permission - so the runner's home gets `lb` and `.local/bin` symlinked into the account that owns them, `PATH` puts `.local/bin` first (that is how `hw-gate.sh` resolves `gpu-hold`), and the installer does `loginctl enable-linger` again so `thermal-run.sh`'s `systemctl --user` has a user manager. The runner reaches both through `other` (r-x): it runs the tools and cannot rewrite them, which is what makes this different from the owner's group that was removed. 2. **`hw-gate.sh`'s other two `$HOME` defaults are named in the unit**: `HW_GATE_MODELS=/models` (read-only) and `HW_GATE_LOCK=/npu.lock`. The lock was the NPU version of the bypass - a runner with an empty home took its own NPU lock. And the same preference is applied at source: `scripts/hw-gate.sh` now takes the lock from the hold directory when it exists. 3. **The hold paths are the DEFAULT now, not an environment variable.** `/etc/profile.d` reaches none of the fleet's long-lived tmux/`pi` sessions or `systemd --user` services, so after a migration a lane without the variables would fall back to `~/GPU-TOKEN` and silently fork the queue. `scripts/gpu-hold` therefore prefers `$ONEBIT_HOLD_DIR/{gpu-token,gpu-queue}` (default `/var/lib/1bit-fleet`) when it exists and falls back to `~/GPU-*` otherwise, so every caller agrees with no environment at all and a box that has not been set up behaves exactly as before. `/etc/profile.d/1bit-fleet.sh` stays as belt-and-braces for interactive shells. **`tests/gpu_hold_queue_test.sh`** (ctest `gpu_hold_queue`) pins both arms: with the fleet state installed, a claim with NO environment uses the fleet's queue and token; with it absent, the old `~/GPU-QUEUE` is used. Writing that test found a bug in the installer: `gpu-hold` treats the token file EXISTING as "held" (the holder creates it with noclobber), so pre-creating an empty `gpu-token` made every claim wait for ever - the first version of the installer did exactly that, and `--migrate-hold-state` would have left a ghost holder behind by keeping an empty token. The installer creates the directory and queues and nothing else, and the migration removes the old file whether it was moved or empty. 4. **The MemoryHigh parse is fixed**: awk's numeric coercion read "512M" as 512 and "2T" as 2, so 512M became 460G and 2T became 1G. It parses the unit now, and a value without a K/M/G/T suffix is passed through with MemoryHigh left at systemd's default rather than guessed: 48G -> MemoryHigh=43.2G 512M -> 460.8M 2T -> 1.8T bare bytes -> unchanged Measured: `--dry-run` prints the three Environment lines (PATH, the hold state + HW_GATE_LOCK, HW_GATE_MODELS - the last needed `share_home` resolved before the unit is rendered, or the dry run disagreed with the install); `systemd-analyze verify` passes and still catches a corrupted directive; `tests/gpu_hold_queue_test.sh` 2/2 arms pass; bash -n and shellcheck -S warning clean; copyright rc 0. Nothing on the live box was modified - the hold directory does not exist there yet, so both defaults are inert until a fresh install. --- CMakeLists.txt | 5 +++ docs/ci-runners.md | 31 ++++++++++++++++ scripts/gpu-hold | 16 ++++++-- scripts/hw-gate.sh | 5 ++- scripts/install-ci-runner.sh | 44 ++++++++++++++++++++-- tests/gpu_hold_queue_test.sh | 72 ++++++++++++++++++++++++++++++++++++ 6 files changed, 164 insertions(+), 9 deletions(-) create mode 100755 tests/gpu_hold_queue_test.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index 288321f6a..3225a890e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -265,6 +265,11 @@ add_test(NAME pack_version COMMAND ${CMAKE_SOURCE_DIR}/tests/pack_version_test.s # resolves outside the payload, and a runtime set shipped without the aqlprofile agent libhsa opens. Each # arm asserts the row's message, and a clean tree is the control. No package build, no GPU, runs in CI. add_test(NAME pack_check COMMAND ${CMAKE_SOURCE_DIR}/tests/pack_check_test.sh) +# engine #1504: the GPU hold queue is the FLEET's when the shared state is installed, with no environment to say +# so - the failure it guards against is silent (a caller under another account taking its own token). Two arms: +# fleet state present -> the claim uses the fleet's queue and token; absent -> the old ~/GPU-* paths, so a box +# that has not been set up is unchanged. No GPU: the claim holds the token for three seconds and runs `true`. +add_test(NAME gpu_hold_queue COMMAND ${CMAKE_SOURCE_DIR}/tests/gpu_hold_queue_test.sh) # engine #713: the pinned HRX toolchain (config/hrx-toolchain.config, scripts/fetch-rocm-sdk.sh) -- the # pin file's refusals, the completeness check against a fabricated tree, and that cmake and # weekly.sh both use the pin instead of the box's /opt/rocm-therock. No network, no SDK, no GPU. diff --git a/docs/ci-runners.md b/docs/ci-runners.md index 7c3c99a12..0b9c2d3cf 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -140,6 +140,37 @@ So the hold state lives in its own directory, owned by its own group: - `npu-hold` wraps `gpu-hold` with `exec env GPU_QUEUE_DIR=~/NPU-QUEUE GPU_TOKEN_FILE=~/NPU-TOKEN …`, so it overrides whatever the environment says: **NPU sharing needs that wrapper changed** to honour the two variables. Until then the NPU queue is per-account, and that is a named follow-up rather than a claim. +### The tools are reached by a home-relative path + +`hrx-build.yml:111,132` and `hw-gate.yml` call `~/lb/thermal-run.sh`, and `hw-gate.sh` resolves `gpu-hold` with +`command -v` — a **home-relative path**, not a permission. So the installer links `~/lb` and `~/.local/bin` from the +runner's home into the account that owns them and puts `.local/bin` first on the unit's `PATH`. The runner reaches +both through `other` (`r-x`), so it can **run** the tools and cannot rewrite them — which is the difference between +this and the owner's group, and the reason the links are back after the review's first round. + +`thermal-run.sh` runs its jobs under `systemctl --user`, so the installer also does +`loginctl enable-linger `; without a user manager for that account it falls back to a process-group +`SIGSTOP`, which is the weaker thermal brake the script itself warns about. + +`hw-gate.sh` has two more `$HOME` defaults (`:98-99`), and both resolve into an empty home for a runner under +another account: + +| variable | default | the installer sets it to | +|---|---|---| +| `HW_GATE_MODELS` | `$HOME/models` | `--owner-user`'s `models/` when it exists — read-only | +| `HW_GATE_LOCK` | `$HOME/npu.lock` | `/npu.lock` — the **fleet's** lock, so a runner cannot take NPU work on a lock of its own (the NPU version of the GPU-token bypass) | + +The same reasoning is applied at the other end: `scripts/hw-gate.sh` now prefers `/npu.lock` when that +directory exists, and `scripts/gpu-hold` prefers `/{gpu-token,gpu-queue}`, so **the default is what +moved, not an environment variable**. That matters because the lanes are long-lived tmux/`pi` sessions and +`systemd --user` services, none of them login shells: `/etc/profile.d` reaches none of them, and a caller without +the variables would otherwise fall back to `~/GPU-TOKEN` and silently fork the queue. `ONEBIT_HOLD_DIR` overrides +the directory if it ever lives elsewhere. + +One consequence worth writing down, found by `tests/gpu_hold_queue_test.sh`: `gpu-hold` treats the token file +**existing** as "held" — the holder creates it with `noclobber` — so the installer must not pre-create an empty +`gpu-token`. It creates the directory and the queues, and lets the first claim create the token. + - `--no-hold-state` exists for a runner that runs no hardware jobs — the `pr-agent` runner does not (it talks to the local LLM over HTTP on `127.0.0.1:8090`), so it needs none of this. - `--migrate-hold-state` moves an existing `~/GPU-{TOKEN,QUEUE}`, `~/NPU-{TOKEN,QUEUE}` into `/var/lib/1bit-fleet`, diff --git a/scripts/gpu-hold b/scripts/gpu-hold index b029b12f3..d75b682bf 100755 --- a/scripts/gpu-hold +++ b/scripts/gpu-hold @@ -35,8 +35,14 @@ # -t MINUTES max hold, default $GPU_HOLD_MINUTES or 14 (0.05 = 3 seconds) # -l LABEL label written into the token, default "-$$" # -w SECONDS how long to queue before giving up, default $GPU_HOLD_WAIT or 5400; exit 75 on timeout -# -q DIR queue directory, default $GPU_QUEUE_DIR or ~/GPU-QUEUE -# GPU_TOKEN_FILE the token file, default ~/GPU-TOKEN +# -q DIR queue directory, default $GPU_QUEUE_DIR, else the fleet's queue when it exists +# ($ONEBIT_HOLD_DIR/gpu-queue, /var/lib/1bit-fleet by default), else ~/GPU-QUEUE +# GPU_TOKEN_FILE the token file, same order of preference, else ~/GPU-TOKEN +# ONEBIT_HOLD_DIR where the fleet's hold state lives (engine #1504); when that directory exists it IS +# the queue for every caller, whoever runs: a runner under another account, a tmux lane, a +# systemd --user service. Without it a second account silently takes its own token and runs +# GPU work beside the fleet, and with it a lane that never sourced a profile file still +# agrees, because the default - not an environment variable - is what moved. # GPU_HOLD_RELEASE_WAIT seconds a label must wait after releasing, default 180 # GPU_HOLD_TCTL_START start gate in C, default 75 - a hot box is not taken # GPU_HOLD_TCTL_ABORT abort in C, default 93 @@ -54,8 +60,10 @@ set -u MINUTES=${GPU_HOLD_MINUTES:-14} LABEL="" WAIT=${GPU_HOLD_WAIT:-5400} -QUEUE=${GPU_QUEUE_DIR:-$HOME/GPU-QUEUE} -TOKEN=${GPU_TOKEN_FILE:-$HOME/GPU-TOKEN} +HOLD=${ONEBIT_HOLD_DIR:-/var/lib/1bit-fleet} +# the fleet's state wins when it is installed, and ~ is the fallback for a box that has not been set up that way +QUEUE=${GPU_QUEUE_DIR:-$([ -d "$HOLD/gpu-queue" ] && printf '%s' "$HOLD/gpu-queue" || printf '%s' "$HOME/GPU-QUEUE")} +TOKEN=${GPU_TOKEN_FILE:-$([ -d "$HOLD" ] && printf '%s' "$HOLD/gpu-token" || printf '%s' "$HOME/GPU-TOKEN")} RELEASE_WAIT=${GPU_HOLD_RELEASE_WAIT:-180} TCTL_START=${GPU_HOLD_TCTL_START:-75} TCTL_ABORT=${GPU_HOLD_TCTL_ABORT:-93} diff --git a/scripts/hw-gate.sh b/scripts/hw-gate.sh index 3a8c043c9..62f5c7f2d 100755 --- a/scripts/hw-gate.sh +++ b/scripts/hw-gate.sh @@ -96,7 +96,10 @@ fi ROOT=$(cd "$(dirname "$0")/.." && pwd) STORE=${HW_GATE_STORE:-$HOME/.cache/hw-gate} MODELS=${HW_GATE_MODELS:-$HOME/models} -LOCK=${HW_GATE_LOCK:-$HOME/npu.lock} +# engine #1504: the NPU lock is fleet state, like the GPU token - when the shared hold directory exists it is +# THE lock, so a process running under another account cannot take NPU work on a lock of its own +_HOLD=${ONEBIT_HOLD_DIR:-/var/lib/1bit-fleet} +LOCK=${HW_GATE_LOCK:-$([ -d "$_HOLD" ] && printf '%s' "$_HOLD/npu.lock" || printf '%s' "$HOME/npu.lock")} LOCK_WAIT=${HW_GATE_LOCK_WAIT:-5400} # The NPU array's fair claim, the one the rest of the box uses: npu-hold puts the command on ~/NPU-QUEUE and # ~/NPU-TOKEN, and npu-serve-keeper stops the owner's serve while a ticket waits. ~/npu.lock is a *private* diff --git a/scripts/install-ci-runner.sh b/scripts/install-ci-runner.sh index ee887d12c..fc1f6031b 100755 --- a/scripts/install-ci-runner.sh +++ b/scripts/install-ci-runner.sh @@ -92,7 +92,16 @@ done unit=actions.runner.1bit-MONSTER-engine.$name.service unit_path=/etc/systemd/system/$unit dir=/var/lib/$user/actions-runner-$name -memory_high=$(awk -v m="$memory_max" 'BEGIN{ sub(/G$/,"",m); printf "%dG", int(m * 0.9) }') +# MemoryHigh is 90% of MemoryMax IN ITS OWN UNIT: awk's numeric coercion reads "512M" as 512 and "2T" as 2, +# so the old one-liner turned 512M into 460G and 2T into 1G. Anything without a K/M/G/T suffix is passed through +# unconverted and MemoryHigh is left at systemd's default rather than guessed. +share_home=$(getent passwd "$owner_user" 2>/dev/null | cut -d: -f6) || share_home='' +memory_high=$(awk -v m="$memory_max" 'BEGIN{ + if (m !~ /^[0-9.]+[KMGT]$/) { exit } # no suffix: leave MemoryHigh alone + v = substr(m, 1, length(m) - 1) + 0; u = substr(m, length(m)) + printf "%.1f%s", v * 0.9, u +}') +[ -n "$memory_high" ] || memory_high=$memory_max # <-- the unit this recipe installs unit_text() { @@ -111,7 +120,9 @@ Type=simple User=$user WorkingDirectory=$dir ExecStart=$dir/runsvc.sh -$( [ "$no_hold" = 0 ] && printf 'Environment=GPU_TOKEN_FILE=%s/gpu-token GPU_QUEUE_DIR=%s/gpu-queue\n' "$hold_state" "$hold_state" ) +Environment=PATH=/var/lib/$user/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin +$( [ "$no_hold" = 0 ] && printf 'Environment=GPU_TOKEN_FILE=%s/gpu-token GPU_QUEUE_DIR=%s/gpu-queue HW_GATE_LOCK=%s/npu.lock\n' "$hold_state" "$hold_state" "$hold_state" ) +$( [ -n "${share_home:-}" ] && [ -d "$share_home/models" ] && printf 'Environment=HW_GATE_MODELS=%s/models\n' "$share_home" ) $( [ -n "$env_file" ] && echo "EnvironmentFile=$env_file" ) # runsvc.sh exits 0 after an OOM kill ("no retry needed"), so on-failure would leave the runner dead. Restart=always @@ -225,13 +236,34 @@ chown -R "$user:" "$dir" # created by either account inherit the group # The runner's account joins THAT group and nothing else; the shared tools (~/lb, ~/.local/bin) are already # world-readable and executable, so no group is needed to run them. +# the workflows reach the shared tools by a HOME-RELATIVE path (~/lb/thermal-run.sh, and `command -v gpu-hold` +# for ~/.local/bin), so the runner's home needs those two entries. They are symlinks into the account that owns +# them and the runner reaches them through "other" (r-x), NOT through a group: it can run the tools and cannot +# rewrite them - which is the escalation the first version of this recipe created. +if [ -n "$share_home" ]; then + for rel in lb .local/bin; do + src="$share_home/$rel"; dst="/var/lib/$user/$rel" + [ -d "$src" ] || continue + mkdir -p "$(dirname "$dst")" + if [ -e "$dst" ] && [ ! -L "$dst" ]; then + echo "warning: /var/lib/$user/$rel exists as a real path; leaving it (move it aside to share $src)" >&2 + continue + fi + ln -sfn "$src" "$dst" + echo "tools: ~/$rel -> $src (read-only through \"other\", no group)" + done +fi + if [ "$no_hold" = 0 ]; then getent group "$hold_group" >/dev/null 2>&1 || groupadd --system "$hold_group" id -nG "$user" | tr ' ' '\n' | grep -qx "$hold_group" || usermod -aG "$hold_group" "$user" install -d -m 0770 -g "$hold_group" "$hold_state" "$hold_state/gpu-queue" "$hold_state/npu-queue" chmod g+s "$hold_state/gpu-queue" "$hold_state/npu-queue" + # NOT the token files: `gpu-hold` treats the token's EXISTENCE as "held" (the holder creates it with + # noclobber), so pre-creating an empty one makes every claim wait for ever - found by tests/gpu_hold_queue_test.sh. + # The directory is group-writable so any fleet member can create the token; an existing one is handed over. for f in gpu-token npu-token; do - [ -e "$hold_state/$f" ] || install -m 0660 -g "$hold_group" /dev/null "$hold_state/$f" + [ -e "$hold_state/$f" ] || continue chgrp "$hold_group" "$hold_state/$f"; chmod 0660 "$hold_state/$f" done if [ "$migrate" = 1 ]; then @@ -242,7 +274,8 @@ if [ "$no_hold" = 0 ]; then echo "install-ci-runner.sh: $owner_home/GPU-QUEUE is not empty: let the queue drain before migrating (or migrate by hand)" >&2; exit 1; } for pair in "GPU-TOKEN:gpu-token" "NPU-TOKEN:npu-token"; do src="$owner_home/${pair%%:*}"; dst="$hold_state/${pair##*:}" - [ -s "$src" ] && cp -a "$src" "$dst" && chgrp "$hold_group" "$dst" && chmod 0660 "$dst" && rm -f "$src" + if [ -s "$src" ]; then cp -a "$src" "$dst" && chgrp "$hold_group" "$dst" && chmod 0660 "$dst"; fi + rm -f "$src" # empty or moved: either way no file stays behind to read as a held token done for pair in "GPU-QUEUE:gpu-queue" "NPU-QUEUE:npu-queue"; do src="$owner_home/${pair%%:*}"; dst="$hold_state/${pair##*:}" @@ -259,6 +292,9 @@ if [ "$no_hold" = 0 ]; then export GPU_TOKEN_FILE=$hold_state/gpu-token export GPU_QUEUE_DIR=$hold_state/gpu-queue PROFILE + # thermal-run.sh runs its jobs under `systemctl --user`; without a user manager for this account it falls + # back to a process-group SIGSTOP, which is the weaker mode the file itself warns about + loginctl enable-linger "$user" 2>/dev/null || echo "warning: could not enable-linger $user; thermal-run.sh will use its process-group fallback" echo "hold state: $hold_state (group $hold_group, 0770, queues setgid); $user joined $hold_group" echo "the owner's shells pick up GPU_TOKEN_FILE/GPU_QUEUE_DIR from /etc/profile.d/1bit-fleet.sh;" echo "a long-running owner service needs them in its own unit or environment (docs/ci-runners.md)" diff --git a/tests/gpu_hold_queue_test.sh b/tests/gpu_hold_queue_test.sh new file mode 100755 index 000000000..40da17c6b --- /dev/null +++ b/tests/gpu_hold_queue_test.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# Copyright 2026 bong-water-water-bong +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# gpu_hold_queue_test.sh: the GPU hold queue is the FLEET's when it is installed, whoever runs (engine #1504). +# +# A runner under a different account, a tmux lane and a `systemd --user` service must all queue on the same +# token file. The failure this guards against is silent: a caller whose `$HOME` is somewhere else, and which +# never sourced a profile file, takes its OWN token and runs GPU work beside the fleet. +# +# Arms: +# 1. the fleet state exists -> a claim with NO environment at all lands in the fleet queue and uses the +# fleet token file; +# 2. the fleet state is absent -> the old ~/GPU-* paths are used, so a box that has not been set up is +# unchanged (that is what every caller did before). +set -uo pipefail +here=$(cd "$(dirname "$0")" && pwd) +hold=$(mktemp -d); fake_home=$(mktemp -d) +trap 'rm -rf "$hold" "$fake_home"' EXIT +fail=0 + +claim() { # claim: run one very short claim with the given HOME and hold dir, nothing else in the environment + # the output is kept: gpu-hold logs "took ", which is the only evidence of WHICH token a claim used + # after it released it (release is `rm -f "$TOKEN"`, by design) + HOME=$fake_home ONEBIT_HOLD_DIR=$hold \ + GPU_HOLD_TCTL_START=${GPU_HOLD_TCTL_START:-999} GPU_HOLD_RELEASE_WAIT=0 \ + bash "$here/../scripts/gpu-hold" -l gpu-hold-queue-test -t 0.05 -w 10 true 2>&1 +} + +# 1. the fleet's state is installed: the queue and the token are the fleet's, with no environment to say so +mkdir -p "$hold/gpu-queue" # and NOT the token: its existence means "held", so the holder must create it +if out=$(claim); then + if printf '%s' "$out" | grep -q "took $hold/gpu-token" && + [ -e "$hold/gpu-queue/.released-at.gpu-hold-queue-test" ] && [ ! -d "$fake_home/GPU-QUEUE" ]; then + echo "PASS a claim with no environment queues on the fleet's state and takes the fleet token" + else + echo "FAIL the claim did not use the fleet's state (fleet queue: $(ls -A "$hold/gpu-queue" 2>/dev/null | tr '\n' ' '); ~/GPU-QUEUE: $([ -d "$fake_home/GPU-QUEUE" ] && echo present || echo absent)); gpu-hold said:" + printf '%s\n' "$out" | tail -3 | sed 's/^/ /' + fail=$((fail + 1)) + fi +else + echo "FAIL the claim itself failed (rc $?); see the gpu-hold output above" + fail=$((fail + 1)) +fi + +# 2. nothing installed: the old paths, so this change is inert on a box without the shared state +hold2=$(mktemp -d); rm -rf "$hold2"; mkdir -p "$fake_home" +if HOME=$fake_home ONEBIT_HOLD_DIR=$hold2 GPU_HOLD_TCTL_START=${GPU_HOLD_TCTL_START:-999} GPU_HOLD_RELEASE_WAIT=0 \ + bash "$here/../scripts/gpu-hold" -l gpu-hold-queue-test2 -t 0.05 -w 10 true >/dev/null 2>&1; then + if [ -e "$fake_home/GPU-QUEUE/.released-at.gpu-hold-queue-test2" ]; then + echo "PASS with no fleet state the claim falls back to ~/GPU-QUEUE, as before" + else + echo "FAIL with no fleet state the claim did not use ~/GPU-QUEUE"; fail=$((fail + 1)) + fi +else + echo "FAIL the fallback claim failed (rc $?)"; fail=$((fail + 1)) +fi +r=$([ "$fail" = 0 ] && echo ok || echo no); rm -rf "$hold2" +[ "$r" = ok ] && echo "gpu_hold_queue: all rows PASS" +[ "$fail" = 0 ] From e3fd85c338ce1822d4783aced1df6e0e300c2506 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Mon, 9 Nov 2026 05:47:00 -0400 Subject: [PATCH 5/6] ci: install the repo's gpu-hold, and refuse to migrate the queue behind an old one (Refs #1504) The required change is right, and `cmp` confirms it: the installed ~/.local/bin/gpu-hold and scripts/gpu-hold differ, so the fleet-default fix lived only in the repository while every lane - and this runner, through its own home's symlink - runs the installed copy that still says `$HOME/GPU-TOKEN`. `--migrate-hold-state` would then move the token, and the first lane without GPU_TOKEN_FILE (a tmux lane, `pi`, a systemd --user service) would recreate the old path: two queues again, which is what the default change exists to prevent. Both halves the review offered, because they cover different risks: - the installer **installs** `scripts/gpu-hold` over `$owner_home/.local/bin/gpu-hold` when the two differ, keeping a dated `.bak-*` (the way this box keeps every other tool), so a fresh install has one copy and it is the repository's; - `--migrate-hold-state` **refuses** unless that installed copy resolves the fleet default (`grep -q 'ONEBIT_HOLD_DIR|1bit-fleet'`), naming the reason: it still defaults to ~/GPU-TOKEN, so the first lane without the variable would recreate it. The docs' install sequence now says the order and which step enforces it. Also fixed, from the review's notes: - **`npu-hold`'s hard-coded `$HOME` is a condition, not a footnote**, so the installer warns at install time: `npu-hold` is not in this repository and pins `GPU_QUEUE_DIR="$HOME/NPU-QUEUE" GPU_TOKEN_FILE="$HOME/NPU-TOKEN"`, so NPU work from another account uses a PRIVATE NPU queue; the unit's HW_GATE_LOCK makes the *lock* shared, the queue is not, and the runner must not take NPU work until that wrapper honours the variables. - **the MemoryHigh comment now matches the code**: a size with no K/M/G/T suffix sets MemoryHigh equal to MemoryMax - a hard limit with no throttle band above it, which is a fine choice for a runner, and is what the code does rather than "systemd's default". Measured: `cmp` of the two copies differs (so the install step has work to do); `--dry-run` still verifies and prints the same unit; bash -n and shellcheck clean; copyright rc 0. Nothing on the live box was touched - the overwrite and the migration only run at install time, and the migration is additionally gated on the copy. --- docs/ci-runners.md | 17 +++++++++++++++-- scripts/install-ci-runner.sh | 29 +++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index 0b9c2d3cf..0550eedf4 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -217,8 +217,21 @@ sudo rm -f /root/runner-token . /etc/profile.d/1bit-fleet.sh ``` -On a box that already has hold state in the owner's home, add `--migrate-hold-state --owner-user ` to the -first install — with the queue drained, which the installer enforces. +**Order matters, and the installer enforces it.** The default that keeps the queue un-forked lives in +`scripts/gpu-hold`; what *runs* on the box is `~/.local/bin/gpu-hold`, which every lane and the runner reach +through their own home. So the installer puts the repository's copy in place (keeping a dated `.bak-*`, as this +box does for every other tool) before anything else, and `--migrate-hold-state` **refuses** to run against a copy +that still defaults to `~/GPU-TOKEN` — otherwise the migration moves the token and the first lane without +`GPU_TOKEN_FILE` recreates the old path, which is two queues again. + +On a box that already has hold state in the owner's home, the first install takes +`--migrate-hold-state --owner-user `, with the queue drained — the installer refuses a non-empty queue. + +**NPU is not shared yet, and the installer says so at install time.** `npu-hold` is not in this repository, and it +pins the queue with `exec env GPU_QUEUE_DIR="$HOME/NPU-QUEUE" GPU_TOKEN_FILE="$HOME/NPU-TOKEN" …` +(`~/.local/bin/npu-hold:31`), which resolves to a private NPU queue for any other account whatever `gpu-hold`'s +default is. The unit sets `HW_GATE_LOCK` to the fleet's lock, so the *lock* is shared; the *queue* is not. Until +`npu-hold` honours the two variables, a runner under another account must not take NPU work. `--dry-run` prints the unit and runs `systemd-analyze verify` on it without changing anything; `--verify` prints the installed unit's `Result`/`NRestarts`/`OOMPolicy` and exits. The script is idempotent: an existing `ghrunner` diff --git a/scripts/install-ci-runner.sh b/scripts/install-ci-runner.sh index fc1f6031b..40dcc4b8c 100755 --- a/scripts/install-ci-runner.sh +++ b/scripts/install-ci-runner.sh @@ -93,8 +93,9 @@ unit=actions.runner.1bit-MONSTER-engine.$name.service unit_path=/etc/systemd/system/$unit dir=/var/lib/$user/actions-runner-$name # MemoryHigh is 90% of MemoryMax IN ITS OWN UNIT: awk's numeric coercion reads "512M" as 512 and "2T" as 2, -# so the old one-liner turned 512M into 460G and 2T into 1G. Anything without a K/M/G/T suffix is passed through -# unconverted and MemoryHigh is left at systemd's default rather than guessed. +# so the old one-liner turned 512M into 460G and 2T into 1G. A size with no K/M/G/T suffix is passed through +# unconverted, which makes MemoryHigh equal to MemoryMax: a hard limit with no throttle band above it, which is a +# fine choice for a runner (the job dies at the ceiling) and is what the code does. share_home=$(getent passwd "$owner_user" 2>/dev/null | cut -d: -f6) || share_home='' memory_high=$(awk -v m="$memory_max" 'BEGIN{ if (m !~ /^[0-9.]+[KMGT]$/) { exit } # no suffix: leave MemoryHigh alone @@ -266,7 +267,26 @@ if [ "$no_hold" = 0 ]; then [ -e "$hold_state/$f" ] || continue chgrp "$hold_group" "$hold_state/$f"; chmod 0660 "$hold_state/$f" done + # The default that keeps the queue un-forked lives in scripts/gpu-hold, and what RUNS is the copy on the + # box: ~/.local/bin/gpu-hold, which every lane and this runner reach through its own home. If that copy is + # older it still defaults to ~/GPU-TOKEN, so a migration would move the token and the first lane without + # GPU_TOKEN_FILE would recreate it - two queues again. So the installer puts the repository's copy in place + # (keeping a dated backup, the way this box keeps every other tool), and the migration refuses to run + # against a copy that cannot resolve the fleet default. + tool_dir=$share_home/.local/bin + if [ -d "$tool_dir" ] && [ -f "$(dirname "$0")/gpu-hold" ] && ! cmp -s "$(dirname "$0")/gpu-hold" "$tool_dir/gpu-hold"; then + install -m 0755 "$(dirname "$0")/gpu-hold" "$tool_dir/gpu-hold.new" && + { [ -f "$tool_dir/gpu-hold" ] && cp -a "$tool_dir/gpu-hold" "$tool_dir/gpu-hold.bak-$(date +%Y%m%d-%H%M%S)"; } + mv -f "$tool_dir/gpu-hold.new" "$tool_dir/gpu-hold" + echo "installed scripts/gpu-hold over $tool_dir/gpu-hold (previous copy kept as a .bak-*), so every lane's default is the fleet's" + fi if [ "$migrate" = 1 ]; then + # never migrate against a gpu-hold that would still default to ~/GPU-TOKEN + if [ -f "$tool_dir/gpu-hold" ] && ! grep -q 'ONEBIT_HOLD_DIR\|1bit-fleet' "$tool_dir/gpu-hold"; then + echo "install-ci-runner.sh: refusing --migrate-hold-state: $tool_dir/gpu-hold does not resolve the fleet default" >&2 + echo " (it still defaults to \$HOME/GPU-TOKEN, so the first lane without GPU_TOKEN_FILE would recreate the old path)" >&2 + exit 1 + fi owner_home=$(getent passwd "$owner_user" | cut -d: -f6) [ -n "$owner_home" ] || { echo "install-ci-runner.sh: --migrate-hold-state needs --owner-user that exists" >&2; exit 2; } # a live queue is not a thing to move under the lanes: refuse unless it is empty @@ -295,6 +315,11 @@ PROFILE # thermal-run.sh runs its jobs under `systemctl --user`; without a user manager for this account it falls # back to a process-group SIGSTOP, which is the weaker mode the file itself warns about loginctl enable-linger "$user" 2>/dev/null || echo "warning: could not enable-linger $user; thermal-run.sh will use its process-group fallback" + if [ -f "$share_home/.local/bin/npu-hold" ] && grep -q 'GPU_TOKEN_FILE="\$HOME/NPU-TOKEN"' "$share_home/.local/bin/npu-hold" 2>/dev/null; then + echo "warning: npu-hold pins ~/NPU-QUEUE and ~/NPU-TOKEN, so NPU work from $user uses a PRIVATE NPU queue." >&2 + echo " The unit sets HW_GATE_LOCK to the fleet's lock, so the lock is shared; the queue is not until npu-hold" >&2 + echo " honours GPU_QUEUE_DIR/GPU_TOKEN_FILE (docs/ci-runners.md). Until then do not let this runner take NPU work." >&2 + fi echo "hold state: $hold_state (group $hold_group, 0770, queues setgid); $user joined $hold_group" echo "the owner's shells pick up GPU_TOKEN_FILE/GPU_QUEUE_DIR from /etc/profile.d/1bit-fleet.sh;" echo "a long-running owner service needs them in its own unit or environment (docs/ci-runners.md)" From 2cb9bd473d31ff944be7ff97ac91760d039cba11 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Mon, 9 Nov 2026 06:04:42 -0400 Subject: [PATCH 6/6] install-ci-runner: the installed gpu-hold stays the owner's file (Refs #1504) The installer runs as root, so `install`/`cp -a`/`mv` left a root-owned gpu-hold (and .bak-*) in the owner's ~/.local/bin: replaceable, because they own the directory, but not editable in place by its owner. It is installed -o/-g to the owner now and the backup is chowned to them, so the tool stays theirs in the way every other file in that directory is. --- scripts/install-ci-runner.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/install-ci-runner.sh b/scripts/install-ci-runner.sh index 40dcc4b8c..d50770ef9 100755 --- a/scripts/install-ci-runner.sh +++ b/scripts/install-ci-runner.sh @@ -275,8 +275,12 @@ if [ "$no_hold" = 0 ]; then # against a copy that cannot resolve the fleet default. tool_dir=$share_home/.local/bin if [ -d "$tool_dir" ] && [ -f "$(dirname "$0")/gpu-hold" ] && ! cmp -s "$(dirname "$0")/gpu-hold" "$tool_dir/gpu-hold"; then - install -m 0755 "$(dirname "$0")/gpu-hold" "$tool_dir/gpu-hold.new" && - { [ -f "$tool_dir/gpu-hold" ] && cp -a "$tool_dir/gpu-hold" "$tool_dir/gpu-hold.bak-$(date +%Y%m%d-%H%M%S)"; } + # owned by the account whose tool it is, not by root: this script runs as root, and a root-owned file in + # ~/.local/bin can be replaced (the directory is theirs) but not edited in place by its owner + owner_group=$(id -gn "$owner_user") + install -m 0755 -o "$owner_user" -g "$owner_group" "$(dirname "$0")/gpu-hold" "$tool_dir/gpu-hold.new" && + { [ -f "$tool_dir/gpu-hold" ] && cp -a "$tool_dir/gpu-hold" "$tool_dir/gpu-hold.bak-$(date +%Y%m%d-%H%M%S)" && + chown "$owner_user:$owner_group" "$tool_dir"/gpu-hold.bak-*; } mv -f "$tool_dir/gpu-hold.new" "$tool_dir/gpu-hold" echo "installed scripts/gpu-hold over $tool_dir/gpu-hold (previous copy kept as a .bak-*), so every lane's default is the fleet's" fi