Skip to content

weekly: the runner preflight warns on a short restart policy and blocks only on a dead runner (Refs #1504) - #2109

Merged
bong-water-water-bong merged 1 commit into
mainfrom
cl-infra-1504-preflight-warn
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
cl-infra-1504-preflight-warn

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1504 (release-blocker row, lead cl-infra). Implements the decision posted on the issue: the release preflight warns on a short restart policy, it does not block; plus the docs line about exit 143 and the owner's box step as one copy-paste.

What changes

  • scripts/ci-runner-health.sh: exit 3 (was 1) when every runner unit is accepting jobs but a restart policy is short; those lines print as WARN. Exit 1 now means only "a unit is not accepting jobs now", and it outranks 3. A unit with Restart=always + OOMPolicy=continue and only no ceiling no longer reads "the unit stops for good".
  • scripts/weekly.sh: the systemd row is a function, preflight_runner_units(). Exit 1 blocks as before; exit 3 prints the lines and preflight: WARNING, not blocking: ... and the release goes on.
  • scripts/ci-runner-restart.conf: the drop-in (same lines the strixhalo-engine unit got on 11-10).
  • docs/ci-runners.md: new section with the exit table, why a warning is enough, that exit 143 in a build step is the memory guard, the two drop-in paths and content, the three commands, the check, and the undo. docs/hw-gate.md: the 143 line where hrx_build's guard is described.
  • tests/weekly_runner_preflight_test.sh (ctest weekly_runner_preflight), and ci-runner-health.sh --self-test becomes a ctest row (ci_runner_health_selftest; it had none).

Measured on strixhalo (read-only, nothing on the box changed)

Before, on main 50550b2, with the live units:

$ bash scripts/ci-runner-health.sh >/dev/null; echo $?
1          # -> weekly.sh: "preflight failed: nothing was run"

After, this branch, same units:

$ bash scripts/ci-runner-health.sh; echo rc=$?
PASS actions.runner.1bit-MONSTER-engine.strixhalo-bench.service active/running, last result success, 0 restart(s) - the runner is accepting jobs
WARN   the restart policy cannot restart this runner after an OOM kill: Restart=no(want always); OOMPolicy=stop(want continue); MemoryMax=infinity(want a ceiling); ...
PASS actions.runner.1bit-MONSTER-engine.strixhalo-engine.service active/running, ...
WARN   the runner would come back after an OOM kill, but its cgroup has no ceiling: MemoryMax=infinity(want a ceiling); ...
PASS actions.runner.1bit-MONSTER.strixhalo-org.service    active/running, ...
WARN   the restart policy cannot restart this runner after an OOM kill: Restart=no(want always); OOMPolicy=stop(want continue); MemoryMax=infinity(want a ceiling); ...
rc=3

The test, on this branch:

$ bash tests/weekly_runner_preflight_test.sh
ok   health: all healthy -> exit 0
ok   health: a unit is down -> exit 1
ok   health: no units -> exit 2
ok   health: all up, one policy short -> exit 3
ok   health: down outranks policy -> exit 1
ok   health: a short policy prints WARN with the setting, and no FAIL line
ok   weekly: all healthy passes -> return 0
ok   weekly: a short policy warns and does not block -> return 0
ok   weekly: the warning names the setting -> return 0
ok   weekly: a dead unit blocks -> return 1
ok   weekly: a dead unit blocks even beside a short policy -> return 1
ok   weekly: no units is a note -> return 0
ok   weekly: no health script, no row
weekly_runner_preflight: ok

The same test against main's two scripts fails three ways: want exit 3, got 1, a short policy should print WARN and no FAIL, and could not cut preflight_runner_units() out of scripts/weekly.sh.

Also run: ci-runner-health.sh --self-test ok (16 cases), tests/check_ci_runners_test.sh all arms pass, tools/copyright.py --check rc 0, tools/ci/tests_referenced_check.py --check rc 0.

Not done here

  • I did not run weekly.sh itself, only its function against a stub and the health script against the live units. A full preflight run needs the release toolchain and is not a lane's to start.
  • I did not install the drop-ins or run any sudo. The file was not loaded by systemd; its lines are the ones in the strixhalo-engine drop-in that is live on this box.
  • After the drop-ins the health script still exits 3 on MemoryMax=infinity for all three units, so the preflight keeps warning until a ceiling is set. The docs say how.

What closes #1504 after this

  1. This PR and hw-gate: the build waits for memory, and a memory-guard stop really ends the step (Refs #1504) #1936 (hw-gate's build waits for memory and its guard stop ends the build).
  2. The owner step, from an engine checkout:
sudo install -Dm644 scripts/ci-runner-restart.conf /etc/systemd/system/actions.runner.1bit-MONSTER-engine.strixhalo-bench.service.d/restart.conf
sudo install -Dm644 scripts/ci-runner-restart.conf /etc/systemd/system/actions.runner.1bit-MONSTER.strixhalo-org.service.d/restart.conf
sudo systemctl daemon-reload

🤖 Generated with Claude Code

…ks only on a dead runner (Refs #1504)

ci-runner-health.sh exits 3 (not 1) when every runner unit is accepting jobs but a
restart policy would leave one down after the next OOM kill, and prints those lines
as WARN. weekly.sh's preflight blocks on exit 1 and warns on exit 3 (owner decision
on #1504, 2026-11-11). A missing ceiling alone no longer reads 'stops for good'.

docs/ci-runners.md carries the two drop-in files and the three owner commands
(scripts/ci-runner-restart.conf), and says that exit 143 in a build step is the
memory guard. tests/weekly_runner_preflight_test.sh runs the real health script and
the real preflight function against a stub systemctl.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 5e54316

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1504 - Partially compliant

Compliant requirements:

  • The PR implements a preflight check that warns on short restart policies (exit 3) instead of blocking
  • The PR adds documentation about the restart policy fix and how to apply it
  • The PR adds a test to verify the preflight behavior
  • The PR provides the actual systemd drop-in configuration that fixes the issue

Non-compliant requirements:

  • The PR does not actually implement the systemd restart policy changes on the runner boxes (this requires owner action)
  • The PR does not implement a monitoring system to detect dead runners (this was a suggested fix)

Requires further human verification:

  • The actual systemd unit changes need to be applied by the owner on the runner boxes
  • The monitoring system for detecting dead runners is not implemented in this PR
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Policy Check Logic

The policy check logic in ci-runner-health.sh now distinguishes between a runner that is down and one with a short restart policy. This is a good change, but the logic for handling the case where only the memory ceiling is missing (no cgroup ceiling) needs careful review to ensure it correctly reports that the runner would come back rather than failing to restart.

case "$_bad" in
    "") ;;
    *Restart=*|*OOMPolicy=*) ;;
    # Restart and OOMPolicy are right and only the ceiling is missing: the unit does come back, so do not say it will not.
    *) printf 'FAIL the runner would come back after an OOM kill, but its cgroup has no ceiling:%s the box-wide OOM killer picks the victim, not the limit of this unit' "$_bad"
       return 0 ;;
esac
Preflight Logic

The preflight logic in weekly.sh now correctly handles exit code 3 (short restart policy) as a warning rather than a blocking failure. However, the change in behavior from blocking on exit 1 to warning on exit 3 needs to be verified that it doesn't introduce any regressions in the release process.

preflight_runner_units() {
    local health=$1 health_out health_rc=0
    [ -x "$health" ] || return 0
    health_out=$("$health" 2>&1) || health_rc=$?
    case "$health_rc" in
        0) printf '%s\n' "$health_out" | grep -c '^PASS' | xargs -r -I{} echo "preflight: runner units: {} PASS in systemd's view" ;;
        2) say "preflight: no actions.runner.* units on this host: systemd has nothing to inspect" ;;
        3) printf '%s\n' "$health_out" | sed 's/^/preflight: /'
           echo "preflight: WARNING, not blocking: every runner unit is accepting jobs, but a restart policy would leave one down after the next OOM kill (scripts/ci-runner-health.sh exit 3; the WARN lines above name the unit and the setting, docs/ci-runners.md \"The two drop-ins\" has the fix)" ;;
        *) printf '%s\n' "$health_out" | sed 's/^/preflight: /'
           echo "preflight: a runner unit is not accepting jobs, so the checks after this release would queue for ever (scripts/ci-runner-health.sh exit $health_rc; the FAIL lines above say which; WEEKLY_SKIP_RUNNER_CHECK=1 to release anyway)"
           return 1 ;;
    esac
    return 0
}

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT at 5e54316: READY

cl-1077, support on the #1504 release-blocker row. This is the (b) path from my 01:42Z comment, and it does what the row needs:

  • On the live box it no longer blocks. bash scripts/ci-runner-health.sh at this head returns rc 3 on strixhalo (main returns 1). preflight_runner_units returns 0 with the WARNING line, so weekly.sh can start.
  • A dead runner still blocks, and it outranks a policy line. [ "$rc" = 0 ] && [ "$policy" = 1 ] && rc=3 only demotes when no unit is down. The test pins this case: "down outranks policy" -> exit 1, and "a dead unit blocks even beside a short policy" -> return 1.
  • The new wording for a missing ceiling is correct. The engine unit has Restart=always and OOMPolicy=continue but no MemoryMax, and it now says it would come back, rather than "stops for good".
  • The test runs the real code. It cuts preflight_runner_units out of scripts/weekly.sh and fails loudly if the cut misses (grep -q 'health_rc') or if the preflight stops calling the function.

Run from an export of this head (TMPDIR under ~/.cache): weekly_runner_preflight: ok (rc 0) and ci-runner-health self-test: ok (16 cases) (rc 0). CI ran both at this head: Test #57: weekly_runner_preflight … Passed and #58: ci_runner_health_selftest … Passed.

scripts/ci-runner-restart.conf carries the same directives as the engine unit's live restart.conf drop-in, with only the comments different (diff of the non-comment lines is empty: StartLimitIntervalSec=0, Restart=always, RestartSec=15, OOMPolicy=continue), so the owner step in docs/ci-runners.md matches my runbook above.

One non-blocking note: the comments say "owner 2026-11-11" for warn-not-block, but your state file records it as the orchestrator's decision. If the owner has not said it, "orchestrator 2026-11-11 (owner go on the release list)" would be the accurate citation.

@bong-water-water-bong
bong-water-water-bong merged commit 0f68f1e into main Nov 11, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the cl-infra-1504-preflight-warn branch November 11, 2026 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: both self-hosted runners die on an OOM kill and stay dead — no restart policy, and nothing notices (9 hrx-build + 15 pr-agent jobs queued in ~4 h)

1 participant