fleetd #492 follow-up: detect_supervisor must never read "could not tell" as "none"
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.
This commit is contained in:
Dai Ha
+110
-10
@@ -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 <label>` succeeding. Measured on the second host: `systemctl --user is-active
|
||||
# fleetd` -> "active".
|
||||
# fleetd` -> "active". A clean "no" (inactive/failed/activating/deactivating) exits non-zero with
|
||||
# nothing on stderr; a probe that could not reach systemd at all exits non-zero WITH a stderr
|
||||
# message — see the fleetd #492 follow-up note above.
|
||||
systemd_loaded() {
|
||||
command -v systemctl >/dev/null 2>&1 && systemctl --user is-active "$SYSTEMD_UNIT" >/dev/null 2>&1
|
||||
SYSTEMD_LOADED_ERRORED=0
|
||||
command -v systemctl >/dev/null 2>&1 || return 1
|
||||
local err_file rc=0
|
||||
if ! err_file="$(mktemp -t systemd-loaded-err)"; then
|
||||
SYSTEMD_LOADED_ERRORED=1
|
||||
return 1
|
||||
fi
|
||||
systemctl --user is-active "$SYSTEMD_UNIT" >/dev/null 2>"$err_file" || rc=$?
|
||||
if [ "$rc" -ne 0 ] && [ -s "$err_file" ]; then
|
||||
SYSTEMD_LOADED_ERRORED=1
|
||||
fi
|
||||
rm -f "$err_file"
|
||||
return "$rc"
|
||||
}
|
||||
|
||||
# fleetd #492: three real answers, not two — launchd, systemd, or genuinely unsupervised — plus a
|
||||
# fourth, "ambiguous", for the one case this script cannot tell apart: both signals firing at once.
|
||||
# That is exactly "I cannot tell who supervises this process", and guessing wrong here is how two
|
||||
# daemons end up running against one herdr session (see trap 7 in the header). Pure and
|
||||
# side-effect-free: reads the two probes above and decides — never mutates anything, so it is safe
|
||||
# under --check and testable by overriding launchd_loaded/systemd_loaded after sourcing.
|
||||
# daemons end up running against one herdr session (see trap 7 in the header).
|
||||
#
|
||||
# fleetd #492 follow-up: a fifth answer, "unclear", for two more situations that must NEVER be read
|
||||
# as "none" (measured — see the report this ticket is a follow-up to):
|
||||
# - installed-but-not-loaded, on EITHER supervisor. `systemctl --user is-active` answers "no" for
|
||||
# `activating`, `deactivating`, `failed`, and while an auto-restart is pending — every one of
|
||||
# those is a host that IS under systemd (or launchd) and whose supervisor is about to act again.
|
||||
# `*_installed` already knows the unit/agent exists; this is the first place that fact is
|
||||
# actually consulted in the decision, not just printed as a warning.
|
||||
# - a probe that could not answer at all. systemd_loaded/systemd_installed set their own
|
||||
# *_ERRORED flag (see the comment above them) when `systemctl` exits non-zero WITH a stderr
|
||||
# message — a real tool failure, e.g. it cannot reach the user bus over a non-lingering ssh
|
||||
# session — never conflated with a clean negative answer.
|
||||
# "none" now means only: neither supervisor is installed, neither is loaded, and neither probe
|
||||
# errored. SUPERVISOR_UNCLEAR_DETAIL is set alongside "unclear" so require_drivable_supervisor's
|
||||
# die() can name the specific supervisor and reason, not just the bare word "unclear".
|
||||
#
|
||||
# Pure and side-effect-free besides setting the two globals above: reads the four probes and
|
||||
# decides — never mutates anything, so it is safe under --check and testable by overriding
|
||||
# launchd_installed/launchd_loaded/systemd_installed/systemd_loaded after sourcing.
|
||||
detect_supervisor() {
|
||||
local ld=0 sd=0
|
||||
local ld=0 sd=0 li=0 si=0
|
||||
SYSTEMD_LOADED_ERRORED=0
|
||||
SYSTEMD_INSTALLED_ERRORED=0
|
||||
SUPERVISOR_UNCLEAR_DETAIL=""
|
||||
|
||||
launchd_loaded && ld=1
|
||||
systemd_loaded && sd=1
|
||||
if [ "$ld" = 1 ] && [ "$sd" = 1 ]; then
|
||||
launchd_installed && li=1
|
||||
systemd_installed && si=1
|
||||
|
||||
if [ "$SYSTEMD_LOADED_ERRORED" = 1 ] || [ "$SYSTEMD_INSTALLED_ERRORED" = 1 ]; then
|
||||
SUPERVISOR_UNCLEAR_DETAIL="the systemd --user probe for '$SYSTEMD_UNIT' could not answer cleanly (systemctl exited non-zero and reported an error on stderr, not a clean negative — e.g. it cannot reach the user bus)"
|
||||
echo "unclear"
|
||||
elif [ "$ld" = 1 ] && [ "$sd" = 1 ]; then
|
||||
echo "ambiguous"
|
||||
elif [ "$li" = 1 ] && [ "$ld" = 0 ]; then
|
||||
SUPERVISOR_UNCLEAR_DETAIL="the launchd agent ($LAUNCHD_LABEL) is installed ($LAUNCHD_PLIST exists) but is not currently loaded"
|
||||
echo "unclear"
|
||||
elif [ "$si" = 1 ] && [ "$sd" = 0 ]; then
|
||||
SUPERVISOR_UNCLEAR_DETAIL="the systemd --user unit ($SYSTEMD_UNIT) is installed but not currently active — it may be activating, deactivating, failed, or waiting on an auto-restart"
|
||||
echo "unclear"
|
||||
elif [ "$ld" = 1 ]; then
|
||||
echo "launchd"
|
||||
elif [ "$sd" = 1 ]; then
|
||||
@@ -150,6 +239,17 @@ require_drivable_supervisor() {
|
||||
OLD jar out from under it — the exact failure this ticket (fleetd #492) exists to
|
||||
prevent. Stop one of the two supervisors by hand, confirm only one remains loaded, then
|
||||
rerun." ;;
|
||||
unclear)
|
||||
# fleetd #492 follow-up: SUPERVISOR_UNCLEAR_DETAIL is set by detect_supervisor right before
|
||||
# it returns "unclear"; the fallback text below only fires if this is ever called directly
|
||||
# (as a test does) without going through detect_supervisor first.
|
||||
die "a supervisor looks present but this script cannot tell whether it actually drives this
|
||||
daemon: ${SUPERVISOR_UNCLEAR_DETAIL:-launchd ($LAUNCHD_LABEL) or systemd --user
|
||||
($SYSTEMD_UNIT) reported something other than a clean 'loaded' or a clean 'not loaded'}.
|
||||
Guessing wrong here is the same failure 'ambiguous' above exists to prevent: driving the
|
||||
daemon while an unseen supervisor revives the OLD jar out from under it (fleetd #492).
|
||||
Check 'launchctl list $LAUNCHD_LABEL' and 'systemctl --user status $SYSTEMD_UNIT' by
|
||||
hand, resolve whichever looks unclear, then rerun." ;;
|
||||
*)
|
||||
die "detect_supervisor returned an unrecognized value '$kind' — refusing to guess which
|
||||
supervisor, if any, controls this daemon." ;;
|
||||
|
||||
@@ -56,6 +56,78 @@ test_detect_supervisor_none() {
|
||||
assert_equals "none" "$(detect_supervisor)" "unsupervised detection"
|
||||
}
|
||||
|
||||
# fleetd #492 follow-up — detect_supervisor must never answer "none" when the truth is "could not
|
||||
# tell". `systemd_installed`/`systemd_loaded` already know a unit file exists; this proves that
|
||||
# fact is now actually consulted, not just printed as a warning: an installed-but-not-loaded unit
|
||||
# reads as unclear, because is-active answers "no" for activating/deactivating/failed/pending
|
||||
# auto-restart too, and every one of those is a host that IS under systemd.
|
||||
test_detect_supervisor_systemd_installed_not_loaded_is_unclear() {
|
||||
launchd_installed() { return 1; }
|
||||
launchd_loaded() { return 1; }
|
||||
systemd_installed() { return 0; } # the unit file IS there
|
||||
systemd_loaded() { return 1; } # is-active says no — could be activating/failed/pending restart
|
||||
assert_equals "unclear" "$(detect_supervisor)" "systemd installed-but-not-loaded must read as unclear, not none"
|
||||
}
|
||||
|
||||
# Same fact, the launchd side: a plist on disk that is not currently loaded (unloaded without being
|
||||
# removed, or about to be reloaded) must not read as "no supervisor" either.
|
||||
test_detect_supervisor_launchd_installed_not_loaded_is_unclear() {
|
||||
launchd_installed() { return 0; } # the plist IS there
|
||||
launchd_loaded() { return 1; } # launchctl list says not loaded
|
||||
systemd_installed() { return 1; }
|
||||
systemd_loaded() { return 1; }
|
||||
assert_equals "unclear" "$(detect_supervisor)" "launchd installed-but-not-loaded must read as unclear, not none"
|
||||
}
|
||||
|
||||
# Drives the REAL systemd_loaded/systemd_installed bodies (never stubbed) through a `systemctl`
|
||||
# stub placed first on PATH that exits non-zero AND writes to stderr — the shape of a systemctl
|
||||
# that runs but cannot reach the user bus (measured elsewhere as a headless ssh session with no
|
||||
# lingering). This must read as unclear, never none: a probe that could not answer at all is not
|
||||
# the same fact as "no supervisor is loaded".
|
||||
test_detect_supervisor_systemd_probe_error_is_unclear() {
|
||||
# Re-source first to restore the REAL launchd_*/systemd_* probe bodies. Earlier tests in this
|
||||
# file permanently override them with stub `return 0`/`return 1` bodies (that is the whole point
|
||||
# of those tests), and a bash function definition is global for the rest of the process — without
|
||||
# this, systemd_loaded here would still be whatever the previous test left it as, never touching
|
||||
# a real `systemctl` call at all.
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
local bin_dir result rc=0
|
||||
bin_dir="$TMP/stub-bin-systemctl-errors"
|
||||
mkdir -p "$bin_dir"
|
||||
cat > "$bin_dir/systemctl" <<'STUB'
|
||||
#!/usr/bin/env bash
|
||||
echo "Failed to connect to bus: No such file or directory" >&2
|
||||
exit 1
|
||||
STUB
|
||||
chmod +x "$bin_dir/systemctl"
|
||||
|
||||
PATH="$bin_dir:$PATH" systemd_loaded && rc=0 || rc=$?
|
||||
[ "$rc" -ne 0 ] || fail "systemd_loaded must not report loaded=true when systemctl only errored"
|
||||
assert_equals "1" "$SYSTEMD_LOADED_ERRORED" "systemd_loaded must flag a probe error, not a clean negative"
|
||||
|
||||
launchd_installed() { return 1; }
|
||||
launchd_loaded() { return 1; }
|
||||
result="$(PATH="$bin_dir:$PATH" detect_supervisor)"
|
||||
assert_equals "unclear" "$result" "a systemd probe error must read as unclear, not none"
|
||||
}
|
||||
|
||||
# The other half of the ticket: an "unclear" supervisor must refuse exactly like "ambiguous" does —
|
||||
# die(), never fall through to the `kill` path — and the refusal must name the specific supervisor
|
||||
# and reason detect_supervisor found, not just the bare word "unclear".
|
||||
test_require_drivable_supervisor_refuses_unclear() {
|
||||
launchd_installed() { return 1; }
|
||||
launchd_loaded() { return 1; }
|
||||
systemd_installed() { return 0; }
|
||||
systemd_loaded() { return 1; }
|
||||
local kind output rc=0
|
||||
kind="$(detect_supervisor)"
|
||||
assert_equals "unclear" "$kind" "setup: expected unclear before testing the refusal"
|
||||
output="$(require_drivable_supervisor "$kind" 2>&1)" || rc=$?
|
||||
[ "$rc" -ne 0 ] || fail "require_drivable_supervisor accepted an unclear (undrivable) supervisor"
|
||||
printf '%s' "$output" | grep -qF "$SYSTEMD_UNIT" \
|
||||
|| fail "refusal message does not name the systemd unit it found installed-but-not-loaded"
|
||||
}
|
||||
|
||||
# The heart of the ticket: a supervisor this script cannot drive must refuse, never fall through to
|
||||
# `kill`. require_drivable_supervisor die()s, so it is invoked inside a command substitution — that
|
||||
# forks a subshell, so its exit() only ends the subshell and this test script keeps running under
|
||||
@@ -299,7 +371,11 @@ test_unattributable_quiet_mutation_is_caught() {
|
||||
test_detect_supervisor_launchd_only
|
||||
test_detect_supervisor_systemd_only
|
||||
test_detect_supervisor_none
|
||||
test_detect_supervisor_systemd_installed_not_loaded_is_unclear
|
||||
test_detect_supervisor_launchd_installed_not_loaded_is_unclear
|
||||
test_detect_supervisor_systemd_probe_error_is_unclear
|
||||
test_require_drivable_supervisor_refuses_ambiguous
|
||||
test_require_drivable_supervisor_refuses_unclear
|
||||
test_require_drivable_supervisor_accepts_known_kinds
|
||||
test_count_daemon_pids
|
||||
test_assert_single_daemon_accepts_one_pid
|
||||
|
||||
Reference in New Issue
Block a user