From b17f37a683d3f941d9ecf172f67301c96bdf3449 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 09:23:21 +0700 Subject: [PATCH] fleetd #492 follow-up: detect_supervisor must never read "could not tell" as "none" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two situations were silently landing in the "none" answer, which require_drivable_supervisor accepts and the script then falls back to a raw kill + nohup — exactly the wrong move when a supervisor actually IS present: - installed-but-not-loaded, on either supervisor. `systemctl --user is-active` answers "no" for activating/deactivating/failed and while an auto-restart is pending too, and every one of those is a host that IS under systemd (or launchd) and about to act again. `*_installed` already knew this; it was only ever consulted for a warning line, never by the decision itself. - a systemd probe that could not answer at all (e.g. systemctl cannot reach the user bus over a non-lingering ssh session) looked identical to a clean negative, because both probes redirected stderr straight to /dev/null. detect_supervisor now returns a fifth answer, "unclear", for both cases. systemd_loaded/systemd_installed capture systemctl's exit status and stderr separately and set their own *_ERRORED flag only on a real tool failure (non-zero exit WITH stderr), never on a clean negative. "none" now means only: neither supervisor installed, neither loaded, neither probe errored. require_drivable_supervisor die()s on "unclear" exactly like it already does on "ambiguous", naming the specific supervisor and reason via the new SUPERVISOR_UNCLEAR_DETAIL global. Tests: 4 new cases (systemd/launchd installed-but-not-loaded, a real systemd_loaded run through a systemctl stub that errors on stderr, and the die() refusal for "unclear" naming the unit). All 3 new guards were verified by mutation: each was removed from the real script, the suite caught it (a new FAIL line naming the exact broken assertion), then the file was restored byte-identically and the suite went green again. --- scripts/redeploy-fleetd.sh | 120 +++++++++++++++++++++++++++++--- scripts/test-redeploy-fleetd.sh | 76 ++++++++++++++++++++ 2 files changed, 186 insertions(+), 10 deletions(-) diff --git a/scripts/redeploy-fleetd.sh b/scripts/redeploy-fleetd.sh index e034029..bffed1e 100755 --- a/scripts/redeploy-fleetd.sh +++ b/scripts/redeploy-fleetd.sh @@ -34,7 +34,11 @@ # three different answers, drives whichever one it finds through its own control plane # (`launchctl` / `systemctl --user`), and REFUSES outright — never falls back to `kill` — when # it finds a supervision signal it cannot map to exactly one of the two it knows how to drive. -# A wrong guess here is how two daemons end up running against one herdr session. +# A wrong guess here is how two daemons end up running against one herdr session. Follow-up: +# "not currently loaded" is not the same fact as "unsupervised" — a unit that is installed but +# activating/failed/pending-restart, or a `systemctl` call that could not answer at all (e.g. +# no user-bus access), both now read as a fifth answer, "unclear", and REFUSE the same way +# "ambiguous" does, rather than silently falling through to "none". # 8. fleetd #492 — a post-restart check counts running fleetd processes and fails the whole run if # more than one is alive. That is the one thing none of the checks above (healthz 200, jar id, # the fresh "listening" line) can see: every one of them is satisfied by EITHER daemon. @@ -71,6 +75,20 @@ LAUNCHD_PLIST="$HOME/Library/LaunchAgents/$LAUNCHD_LABEL.plist" # "dev.ltms.fleetd" — systemd user units here are not namespaced the way the launchd label is). SYSTEMD_UNIT='fleetd' +# fleetd #492 follow-up: set by systemd_loaded/systemd_installed when the underlying `systemctl` +# call could not answer cleanly — it exited non-zero AND wrote something to stderr, which is a real +# tool failure (e.g. it cannot reach the user bus over a non-lingering ssh session), never the same +# fact as a clean negative answer ("not active", no stderr). Initialized here, not just inside the +# probes, so detect_supervisor can read them under `set -u` even before either probe has ever run, +# and so a test that stubs a probe with a plain `return 0`/`return 1` body (leaving these untouched) +# reads a deterministic 0 rather than whatever a previous probe call left behind. +SYSTEMD_LOADED_ERRORED=0 +SYSTEMD_INSTALLED_ERRORED=0 +# fleetd #492 follow-up: set by detect_supervisor alongside an "unclear" answer, naming the specific +# supervisor and reason so require_drivable_supervisor's die() message is not just the bare word +# "unclear". Initialized empty for the same `set -u` reason as above. +SUPERVISOR_UNCLEAR_DETAIL="" + DO_BUILD=1; ASSUME_YES=0; CHECK_ONLY=0 for arg in "$@"; do case "$arg" in @@ -100,32 +118,103 @@ launchd_loaded() { launchctl list "$LAUNCHD_LABEL" >/dev/null 2>&1; } # (this repo is developed on macOS) can substitute each one independently — the same seam # launchd_installed/launchd_loaded above already use. # +# fleetd #492 follow-up: both functions used to throw `systemctl`'s stderr straight into +# /dev/null, which meant "systemctl answered no" and "systemctl could not answer at all" (e.g. it +# cannot reach the user bus over a non-lingering ssh session) looked identical — both a plain +# nonzero exit. They now capture stderr separately and set their own *_ERRORED flag ONLY when the +# call exited non-zero AND wrote something to stderr — a real tool failure, never a clean "not +# installed"/"not active" answer (which exits non-zero with empty stderr). detect_supervisor reads +# the flag right after calling the probe, so a probe that could not answer routes to "unclear", +# never silently becomes "none". +# # "installed": a unit FILE by this name exists, regardless of its current state — the systemd # analogue of the plist file existing on disk. `list-unit-files` reads unit definitions without # depending on runtime state, so this stays read-only and safe under --check. systemd_installed() { - command -v systemctl >/dev/null 2>&1 \ - && systemctl --user list-unit-files "$SYSTEMD_UNIT.service" --no-legend 2>/dev/null | grep -q . + SYSTEMD_INSTALLED_ERRORED=0 + command -v systemctl >/dev/null 2>&1 || return 1 + local err_file out rc=0 + if ! err_file="$(mktemp -t systemd-installed-err)"; then + SYSTEMD_INSTALLED_ERRORED=1 + return 1 + fi + out="$(systemctl --user list-unit-files "$SYSTEMD_UNIT.service" --no-legend 2>"$err_file")" || rc=$? + if [ "$rc" -ne 0 ]; then + if [ -s "$err_file" ]; then + SYSTEMD_INSTALLED_ERRORED=1 + fi + rm -f "$err_file" + return "$rc" + fi + rm -f "$err_file" + printf '%s' "$out" | grep -q . } # "loaded": systemd currently supervises this unit as an active job — the systemd analogue of # `launchctl list