From 057f34f92471aea7f86a5b4701b98080ee49cbde Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 27 Aug 2026 23:45:57 +0200 Subject: [PATCH] The init is asked for start and restart; a launcher does the rest ADR 0061. Recovery was the most systemd-specific part of the host, and it is the part that must work on a machine where nothing else does -- which made unit-file syntax a poor place for it, because syntax cannot be tested and the one time it runs is the one time nobody can afford it wrong. So StartLimitBurst and OnFailure move into a launcher script that init starts instead of the host. The unit drops to start-at-boot and restart-on-exit, which OpenRC, runit, s6 and an Android init.rc can all express. Everything 0059 decided is kept: two watchdogs, roll back once, recovery is local, the rollback shares no code with the host. The counter is the whole mechanism, so it is what the tests are mostly about. Three real problems came out of writing them: A counter file holding "1 2" became "12" -- `tr -d [:space:]` concatenates rather than rejecting -- which is past the limit, so a HEALTHY node rolled itself back. Now it reads the first field and insists on a plain integer. The corrupt-counter test used "not-a-number", which shell arithmetic happens to evaluate to 0, so it passed with the guard removed and proved nothing. Replaced with values that discriminate: "5x" errors under set -e and kills the launcher, and "0x10" is read as HEX 16 -- past the limit, so again a healthy node rolls back. And the test harness itself was wrong. With `set -e` and a bare launcher call, removing a guard killed the script at the first corrupt case and silently skipped everything after -- reporting a full pass over tests that never ran. Every launcher call now records its failure instead of aborting. Same class as the placebo assertion found last time, and the reason to keep injecting faults rather than trusting green. Both scripts run in `make check`. 27 launcher tests, 9 rollback tests, all confirmed to bite. --- Makefile | 1 + packaging/launch_test.sh | 135 +++++++++++++++++++++++ packaging/nox-mesh-host-launch | 72 ++++++++++++ packaging/nox-mesh-host-rollback | 9 +- packaging/nox-mesh-host-rollback.service | 8 -- packaging/nox-mesh-host.service | 13 +-- packaging/rollback_test.sh | 12 +- 7 files changed, 222 insertions(+), 28 deletions(-) create mode 100755 packaging/launch_test.sh create mode 100755 packaging/nox-mesh-host-launch delete mode 100644 packaging/nox-mesh-host-rollback.service diff --git a/Makefile b/Makefile index a62a6ac..486603a 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,7 @@ check: fmt vet test packaging-test build packaging-test: @./packaging/rollback_test.sh + @./packaging/launch_test.sh fmt: @test -z "$$(gofmt -l . )" || { echo "unformatted:"; gofmt -l . ; exit 1; } diff --git a/packaging/launch_test.sh b/packaging/launch_test.sh new file mode 100755 index 0000000..8194ece --- /dev/null +++ b/packaging/launch_test.sh @@ -0,0 +1,135 @@ +#!/bin/sh +# Tests for nox-mesh-host-launch. +# +# The counter is the whole mechanism and it is the part to get wrong: never cleared and a node +# rolls back on a healthy boot; cleared too eagerly and it never rolls back at all. So the +# counter is what most of these assert. +set -eu +cd "$(dirname "$0")" +LAUNCH="$PWD/nox-mesh-host-launch" +PASS=0; FAIL=0 + +setup() { + WORK="$(mktemp -d)" + export MESH_HOST_STATE_DIR="$WORK/state" + export MESH_HOST_LIBEXEC="$WORK/libexec" + export MESH_HOST_BIN="$WORK/bin/nox-mesh-host" + export MESH_HOST_START_LIMIT=3 + mkdir -p "$MESH_HOST_STATE_DIR" "$MESH_HOST_LIBEXEC" "$WORK/bin" + + # A host that records being started. It exits immediately, which is what the launcher's + # exec makes indistinguishable from a host that ran for a week — the launcher is gone by + # then either way. + cat > "$MESH_HOST_BIN" <<'STUB' +#!/bin/sh +echo "$@" >> "$MESH_HOST_STATE_DIR/host.starts" +exit 0 +STUB + cat > "$MESH_HOST_LIBEXEC/rollback" <<'STUB' +#!/bin/sh +echo rolled-back >> "$MESH_HOST_STATE_DIR/rollback.calls" +[ -n "${STUB_ROLLBACK_FAILS:-}" ] && exit 1 +echo "$(cat "$MESH_HOST_STATE_DIR/known-good" 2>/dev/null)" > "$MESH_HOST_STATE_DIR/rollback-attempted" +exit 0 +STUB + chmod +x "$MESH_HOST_BIN" "$MESH_HOST_LIBEXEC/rollback" + unset STUB_ROLLBACK_FAILS || true +} + +# `|| true` on every launcher call above: a launcher that exits non-zero is something to +# ASSERT, not something to abort on. With `set -e` and a bare call, removing a guard from the +# launcher killed this script at the first corrupt-counter case and silently skipped the rest — +# reporting a full pass over tests that never ran. +check() { if [ "$3" = "$4" ]; then PASS=$((PASS+1)); printf ' ok %s\n' "$1" + else FAIL=$((FAIL+1)); printf ' FAIL %s\n %s\n got: %s\n expected: %s\n' "$1" "$2" "$3" "$4"; fi; } + +count() { cat "$MESH_HOST_STATE_DIR/start-attempts" 2>/dev/null || echo MISSING; } +started() { [ -f "$MESH_HOST_STATE_DIR/host.starts" ] && echo yes || echo no; } +rolled() { [ -f "$MESH_HOST_STATE_DIR/rollback.calls" ] && echo yes || echo no; } + +# --- the ordinary start ----------------------------------------------------------------------- +setup +"$LAUNCH" >/dev/null 2>&1 || true +check "starts the host" "the common case, every boot" "$(started)" "yes" +check "counts the attempt" "the counter is what decides a rollback later" "$(count)" "1" +check "does not roll back" "a first start is not a failure" "$(rolled)" "no" + +# --- failures below the limit ----------------------------------------------------------------- +setup +i=1; while [ $i -le 3 ]; do "$LAUNCH" >/dev/null 2>&1 || true; i=$((i+1)); done +check "three starts do not trigger a rollback" "the limit is exceeded, not reached" "$(rolled)" "no" +check "counts them all" "" "$(count)" "3" + +# --- past the limit --------------------------------------------------------------------------- +setup +echo "1.4.2" > "$MESH_HOST_STATE_DIR/known-good" +i=1; while [ $i -le 4 ]; do "$LAUNCH" >/dev/null 2>&1 || true; i=$((i+1)); done +check "the fourth start rolls back" "three failures is a binary that does not work" "$(rolled)" "yes" +check "and still starts the host" "the rolled-back version has to be run" "$(started)" "yes" +check "resets the counter after rolling back" "the new version deserves its own attempts, or it halts at once" \ + "$(count)" "0" + +# --- the host clears the counter on success ---------------------------------------------------- +setup +i=1; while [ $i -le 2 ]; do "$LAUNCH" >/dev/null 2>&1 || true; i=$((i+1)); done +printf '0\n' > "$MESH_HOST_STATE_DIR/start-attempts" # what the host does on a completed reconcile +i=1; while [ $i -le 3 ]; do "$LAUNCH" >/dev/null 2>&1 || true; i=$((i+1)); done +check "a cleared counter prevents a rollback" "a node up for months must not roll back on a healthy boot" \ + "$(rolled)" "no" + +# --- rolled back once already ------------------------------------------------------------------- +setup +echo "1.4.2" > "$MESH_HOST_STATE_DIR/known-good" +echo "1.4.2" > "$MESH_HOST_STATE_DIR/rollback-attempted" +i=1; while [ $i -le 4 ]; do "$LAUNCH" >/dev/null 2>&1 || true; i=$((i+1)); done +check "does not roll back twice" "the previous version failing too means the machine, not the binary" \ + "$(rolled)" "no" +check "halts instead" "" "$([ -f "$MESH_HOST_STATE_DIR/halted" ] && echo halted || echo running)" "halted" + +# --- halted stays halted -------------------------------------------------------------------------- +setup +echo "rolled back and still failing" > "$MESH_HOST_STATE_DIR/halted" +"$LAUNCH" >/dev/null 2>&1 || true +check "a halted node does not start the host" "nothing further is tried automatically" "$(started)" "no" +set +e; "$LAUNCH" >/dev/null 2>&1; RC=$?; set -e +check "a halted node exits zero" "a supervisor loop that is slow and visible beats a crash loop" "$RC" "0" + +# --- the rollback itself fails ---------------------------------------------------------------------- +setup +echo "1.4.2" > "$MESH_HOST_STATE_DIR/known-good" +STUB_ROLLBACK_FAILS=1; export STUB_ROLLBACK_FAILS +i=1; while [ $i -le 4 ]; do "$LAUNCH" >/dev/null 2>&1 || true; i=$((i+1)); done +check "a failed rollback halts" "restarting into the same failure would loop forever" \ + "$([ -f "$MESH_HOST_STATE_DIR/halted" ] && echo halted || echo running)" "halted" +# Counted, not "was it ever started": the first three attempts DID start it, correctly, and +# only the fourth must not. An earlier version of this asserted the host was never started and +# failed for that reason rather than for a fault. +check "and does not start it on the halting attempt" "three starts, not four" \ + "$(wc -l < "$MESH_HOST_STATE_DIR/host.starts" 2>/dev/null || echo 0)" "3" + +# --- a corrupt counter ------------------------------------------------------------------------------ +# +# The values here are chosen because they DISCRIMINATE. An earlier version used +# "not-a-number", which shell arithmetic happens to evaluate to 0 — so the test passed with the +# guard removed and proved nothing. These two do not: +# +# 5x shell arithmetic errors, and under `set -e` the launcher dies without starting the host +# 0x10 is read as HEX 16 — past the limit, so a healthy node would roll back for no reason +for corrupt in "5x" "0x10" "1 2" ""; do + setup + echo "1.4.2" > "$MESH_HOST_STATE_DIR/known-good" + printf '%s\n' "$corrupt" > "$MESH_HOST_STATE_DIR/start-attempts" + "$LAUNCH" >/dev/null 2>&1 || true + # The exact number is not the property — "1 2" legitimately recovers a leading 1, while + # "5x" is rejected to 0. What must hold for every one of them is that the launcher + # survives its own state and does not read it as "past the limit". + check "corrupt counter [$corrupt]: starts the host" "the launcher must not die on its own state" \ + "$(started)" "yes" + check "corrupt counter [$corrupt]: does not roll back" "a healthy node must not roll back on a bad counter" \ + "$(rolled)" "no" + check "corrupt counter [$corrupt]: counter is a sane integer" "it is written back for the next start to read" \ + "$(count | grep -cE '^[0-9]+$')" "1" +done + +printf '\nlaunch: %d passed, %d failed\n' "$PASS" "$FAIL" +[ "$FAIL" -eq 0 ] diff --git a/packaging/nox-mesh-host-launch b/packaging/nox-mesh-host-launch new file mode 100755 index 0000000..2c261a2 --- /dev/null +++ b/packaging/nox-mesh-host-launch @@ -0,0 +1,72 @@ +#!/bin/sh +# Start the host, and decide what to do when it will not start. +# +# novox/hq ADR 0061. The init is asked for two things — start this at boot, start it again if it +# exits — and everything else is here, because this is the one piece that has to work on a +# machine where the host does not. Unit-file syntax cannot be tested; this can. +# +# POSIX sh, no bashisms, nothing that has to be installed. +set -eu + +STATE_DIR="${MESH_HOST_STATE_DIR:-/var/lib/mesh-host}" +LIBEXEC="${MESH_HOST_LIBEXEC:-/usr/lib/nox-mesh-host}" +HOST="${MESH_HOST_BIN:-/usr/bin/nox-mesh-host}" +LIMIT="${MESH_HOST_START_LIMIT:-3}" + +ATTEMPTS="$STATE_DIR/start-attempts" +HALTED="$STATE_DIR/halted" + +say() { echo "nox-mesh-host-launch: $*" >&2; } + +mkdir -p "$STATE_DIR" + +# Halted: rolled back once and the previous version failed too, so the binary is not the problem. +# Nothing further is tried automatically. Exit zero — a supervisor restarting this forever is a +# slow visible loop rather than a crash loop, and the node stays down until a person looks. +if [ -e "$HALTED" ]; then + say "halted: $(cat "$HALTED" 2>/dev/null || echo 'reason not recorded')" + say "not starting the host. this node needs a person." + exit 0 +fi + +# Read the FIRST FIELD, then insist it is a plain integer. +# +# Stripping whitespace instead concatenates, and that is not a hypothetical: a counter file +# holding "1 2" became "12", which is past the limit, so a healthy node rolled itself back. An +# unreadable counter must fail towards "start normally", never towards "give up". +count=0 +if [ -s "$ATTEMPTS" ]; then + read -r count _ < "$ATTEMPTS" 2>/dev/null || count=0 +fi +case "${count:-}" in + '' | *[!0-9]*) count=0 ;; +esac + +count=$((count + 1)) +printf '%s\n' "$count" > "$ATTEMPTS" + +if [ "$count" -gt "$LIMIT" ]; then + # The host has failed to get through a reconcile $LIMIT times running. The counter is + # cleared by the host itself on success, so reaching here means none of those starts + # worked — not that the machine has been up a long time. + if [ -e "$STATE_DIR/rollback-attempted" ]; then + say "the host failed $count times after a rollback. the previous version does not" + say "start either, so this is the machine and not the binary." + printf 'rolled back and still failing\n' > "$HALTED" + exit 0 + fi + + say "the host failed $count times. rolling back." + if "$LIBEXEC/rollback"; then + # Fresh count for the version we just installed: it deserves its own attempts, and + # without this it inherits a count already over the limit and halts immediately. + printf '0\n' > "$ATTEMPTS" + else + say "rollback failed. halting rather than restarting into the same failure." + printf 'rollback failed\n' > "$HALTED" + exit 0 + fi +fi + +# exec, so the host is what the supervisor watches and signals reach it directly. +exec "$HOST" run diff --git a/packaging/nox-mesh-host-rollback b/packaging/nox-mesh-host-rollback index 290c6be..7c44591 100755 --- a/packaging/nox-mesh-host-rollback +++ b/packaging/nox-mesh-host-rollback @@ -59,8 +59,7 @@ if ! pacman -U --noconfirm "$PKG"; then exit 1 fi -# reset-failed first, or the start limit that brought us here is still in force. -systemctl reset-failed "$PACKAGE".service 2>/dev/null || true -systemctl start "$PACKAGE".service - -say "rolled back to $VERSION and started it. the node is on the previous version." +# Deliberately does NOT start anything. The launcher called this and will exec the host next, +# so starting it here would run two. novox/hq ADR 0061 moved that responsibility; this script +# installs a version and says so, and nothing else. +say "rolled back to $VERSION. the launcher will start it." diff --git a/packaging/nox-mesh-host-rollback.service b/packaging/nox-mesh-host-rollback.service deleted file mode 100644 index d594792..0000000 --- a/packaging/nox-mesh-host-rollback.service +++ /dev/null @@ -1,8 +0,0 @@ -[Unit] -Description=Roll the Novox Mesh node host back to the last version that started -# No OnFailure of its own. If the rollback fails there is nothing further to try -# automatically, and the node needs a person. - -[Service] -Type=oneshot -ExecStart=/usr/lib/nox-mesh-host/rollback diff --git a/packaging/nox-mesh-host.service b/packaging/nox-mesh-host.service index 795a153..080496f 100644 --- a/packaging/nox-mesh-host.service +++ b/packaging/nox-mesh-host.service @@ -2,19 +2,16 @@ Description=Novox Mesh node host After=network-online.target Wants=network-online.target -# When the supervisor gives up, recover rather than leaving the node quiet — a host that will -# not start looks exactly like a machine somebody switched off (novox/hq ADR 0059). -OnFailure=nox-mesh-host-rollback.service +# Two lines of policy and no more (novox/hq ADR 0061). Counting failed starts and rolling back +# lives in the launcher, where it can be tested — so this file is transcription for any other +# init rather than design. [Service] -Type=notify -ExecStart=/usr/bin/nox-mesh-host run +ExecStart=/usr/lib/nox-mesh-host/launch # always, NOT on-failure: the host restarts onto a new binary by exiting CLEANLY -# (novox/hq ADR 0057), and on-failure would leave an upgraded node stopped. +# (novox/hq ADR 0057), and on-failure would leave every upgraded node stopped. Restart=always RestartSec=5s -StartLimitBurst=3 -StartLimitIntervalSec=120 StateDirectory=mesh-host [Install] diff --git a/packaging/rollback_test.sh b/packaging/rollback_test.sh index 8eb07ec..6fcb7aa 100755 --- a/packaging/rollback_test.sh +++ b/packaging/rollback_test.sh @@ -46,10 +46,10 @@ touch "$MESH_HOST_PKG_CACHE/nox-mesh-host-1.4.2-1-x86_64.pkg.tar.zst" "$SCRIPT" >/dev/null 2>&1 check "installs the known-good version" "pacman is asked to install the cached package" \ "$(grep -c 'nox-mesh-host-1.4.2' "$MESH_HOST_STATE_DIR/pacman.calls" 2>/dev/null || echo 0)" "1" -check "resets the start limit before starting" "reset-failed precedes start" \ - "$(head -1 "$MESH_HOST_STATE_DIR/systemctl.calls" | cut -d' ' -f1)" "reset-failed" -check "starts the host again" "systemctl start is called" \ - "$(grep -c '^start ' "$MESH_HOST_STATE_DIR/systemctl.calls" 2>/dev/null || echo 0)" "1" +# It installs and stops. The launcher execs the host next, and starting it here would run two +# (novox/hq ADR 0061). +check "does not start anything itself" "the launcher owns starting" \ + "$([ -f "$MESH_HOST_STATE_DIR/systemctl.calls" ] && echo started || echo not-started)" "not-started" check "records that it rolled back" "the attempted marker holds the version" \ "$(cat "$MESH_HOST_STATE_DIR/rollback-attempted" 2>/dev/null || echo MISSING)" "1.4.2" @@ -90,9 +90,7 @@ echo "1.4.2" > "$MESH_HOST_STATE_DIR/known-good" touch "$MESH_HOST_PKG_CACHE/nox-mesh-host-1.4.2-1-x86_64.pkg.tar.zst" STUB_PACMAN_FAILS=1 ; export STUB_PACMAN_FAILS set +e; "$SCRIPT" >/dev/null 2>&1; RC=$?; set -e -check "pacman fails: does not start the host" "starting the broken binary again would loop" \ - "$([ -f "$MESH_HOST_STATE_DIR/systemctl.calls" ] && echo started || echo not-started)" "not-started" -check "pacman fails: exits non-zero" "a failed rollback is a failure" "$RC" "1" +check "pacman fails: exits non-zero" "a failed rollback is a failure the launcher must see" "$RC" "1" printf '\nrollback: %d passed, %d failed\n' "$PASS" "$FAIL" [ "$FAIL" -eq 0 ]