From 979adf82ebe35083435e38264b054181de71f0d4 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 10:35:19 +0700 Subject: [PATCH] fleetd #493: never build into the path a running daemon holds redeploy-fleetd.sh's build step wrote straight into fleetd/target/fleetd.jar via `mvn clean install` while the OLD daemon was still running from that exact path. A JVM loads classes lazily, so a class the daemon had not touched yet could be read from a jar already replaced or removed -- the failure landed on the shutdown drain (NoClassDefFoundError, exit 143, looks clean). Stage the freshly built jar at target/fleetd-new.jar (stage_built_jar), confirm the OLD pid has actually exited (wait_for_daemon_exit, extracted from the existing wait loop), and only then swap it into the live path (swap_staged_jar) -- strictly after the wait, strictly before start. A failed swap dies without starting. --no-build and --check keep their existing, truthful behavior (require_no_build_jar; jar_id still reads the live path by default). A leftover staged jar from an interrupted run is wiped before the next build. The build still runs before anything is stopped, so a failed build still never takes the fleet down. Also fixed: the drain-gate abort message ("aborted -- nothing changed") now names the staged jar when one exists, since staging already moves the freshly built jar off the live path before that prompt runs. Adds unit tests for stage_built_jar, swap_staged_jar, require_no_build_jar, wait_for_daemon_exit, and a source-order test proving swap sits after the wait and before start (sourcing stops before the main flow ever runs, so the ordering itself can only be checked by reading the script's own call sites). --- scripts/redeploy-fleetd.sh | 109 ++++++++++++++++++++--- scripts/test-redeploy-fleetd.sh | 148 ++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 10 deletions(-) diff --git a/scripts/redeploy-fleetd.sh b/scripts/redeploy-fleetd.sh index 08be7af..2c7f0b3 100755 --- a/scripts/redeploy-fleetd.sh +++ b/scripts/redeploy-fleetd.sh @@ -56,6 +56,13 @@ set -euo pipefail REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" MODULE="$REPO/fleetd" JAR="$MODULE/target/fleetd.jar" +# fleetd #493: never build into the path a running process holds. The build writes here first +# (Maven's shade plugin has finalName=fleetd, so `clean install` still lands its output at +# target/fleetd.jar — that part is unchanged and out of this script's control), but this script +# now moves it out to JAR_STAGED immediately, and only swaps it back to JAR (a plain `mv`, so a +# rename, never a byte-by-byte overwrite) after the OLD daemon has been confirmed exited. See +# stage_built_jar/swap_staged_jar below. +JAR_STAGED="$MODULE/target/fleetd-new.jar" OUT="$MODULE/fleetd.out" # Matches BOTH the absolute form and the relative `java -jar target/fleetd.jar` a hand-start # produces from inside fleetd/. Anchoring on the absolute path alone was a real bug: the daemon @@ -115,8 +122,62 @@ ok() { printf ' ok %s\n' "$*"; } warn() { printf ' WARN %s\n' "$*"; } die() { printf '\n FAIL %s\n\n' "$*" >&2; exit 1; } -jar_id() { [ -f "$JAR" ] && shasum -a 256 "$JAR" | cut -c1-12 || echo "absent"; } +# Reports the hash of $JAR by default, or of whatever path is passed — used to report the STAGED +# jar right after a build (before it has been swapped in) without ever changing what a bare +# `jar_id` (no args) means: the live path, $JAR. --check and the final "pid ..., jar ..." line +# both call it with no args on purpose, so neither can ever be fooled by a leftover staged file. +jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && shasum -a 256 "$f" | cut -c1-12 || echo "absent"; } running_pid() { pgrep -f "$PATTERN" || true; } + +# fleetd #493 — three small, independently testable pieces of "never build into the path a +# running process holds": +# +# stage_built_jar moves the jar Maven just produced OUT of the live path and onto the staging +# path, immediately after a successful build. Dies (leaving the OLD daemon +# untouched — this runs before the stop step) if Maven reported success but +# left no jar behind, or if the move itself fails. +# require_no_build_jar the --no-build path never builds or stages anything: it must find a +# jar already sitting at the live path from an earlier successful run, and +# die with the same truthful message this script has always used if not. +# wait_for_daemon_exit polls running_pid() for up to $1 seconds and reports whether the OLD +# daemon actually exited — extracted to its own function so the main flow +# can be relied on to call swap_staged_jar only AFTER this returns success, +# and so a test can prove that ordering by reading the script's own source. +# swap_staged_jar the actual swap: a plain `mv` of the staged jar onto the live path. Called +# only once the OLD daemon is confirmed gone (see wait_for_daemon_exit above), +# so this is never a write into a path a running process holds — by the time +# it runs, nothing holds that path anymore. If it fails, the caller must not +# start a new daemon: die() below already refuses that by exiting the script. +stage_built_jar() { + [ -f "$JAR" ] || die "build succeeded but produced no jar at $JAR — cannot stage it for restart. + The running daemon was NOT touched." + mv -f "$JAR" "$JAR_STAGED" \ + || die "could not move the freshly built jar from $JAR to the staging path $JAR_STAGED. + The running daemon was NOT touched." +} + +require_no_build_jar() { + [ -f "$JAR" ] || die "no jar at $JAR — run without --no-build" +} + +wait_for_daemon_exit() { + local timeout="$1" _i + for _i in $(seq "$timeout"); do + [ -z "$(running_pid)" ] && return 0 + sleep 1 + done + [ -z "$(running_pid)" ] +} + +swap_staged_jar() { + local staged="$1" live="$2" + [ -f "$staged" ] || die "no staged jar at $staged to swap in — the daemon was NOT started." + mv -f "$staged" "$live" \ + || die "could not move the staged jar from $staged into place at $live — the daemon was NOT + started. The built jar is still sitting at $staged; a manual 'mv \"$staged\" \"$live\"' + may recover this once you find out why the move failed." +} + # `launchctl list