From faedc6fc49d0b08160390ecd0268f135777f5fb7 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 27 Aug 2026 07:59:24 +0000 Subject: [PATCH] fix(deploy-ha): guard against double-launching a live HA deploy start.sh/start-worker.sh/start-scheduler.sh/start-apiserver.sh had no check for an already-running instance, so re-running start-all.sh (or a generated per-component script) against a still-live deploy silently spawned a full duplicate process set under the identical --myname/ --peers/ports. Combined with SO_REUSEPORT on every listening socket (src/lib/netio.c), the duplicate's bind()/listen() succeeded instead of failing loudly, so two live controllers per node ended up voting into the same Raft group with independently diverged term/log state -- a split-brain that surfaced as non-converging elections, a bouncing redirect chain, and ctrlstatus reporting an unknown primary for 40+ seconds on a freshly deployed cluster. Each generated launcher now writes a pidfile under $TOP/run/ and refuses to start if a live process already owns it, verified via /proc//cmdline identity match (not just liveness) to avoid a PID-reuse false positive. start-all.sh preflights all expected pidfiles before launching anything, failing fast and loud instead of partially duplicating some nodes. Each script's final invocation now uses exec so the pidfile records the daemon's own PID directly, which also collapses the previous wrapper+child two-process-per-component layout into one process. cleanup.sh clears stale pidfiles for hygiene. Verified live in-sandbox: normal start-all.sh still elects a primary and starts all 6 components; a second start-all.sh run against the same live deploy now refuses cleanly with all 6 already-running components identified, process count unchanged. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg --- src/tools/deploy-ha.sh | 133 ++++++++++++++++++++++++++++++++++------- 1 file changed, 110 insertions(+), 23 deletions(-) diff --git a/src/tools/deploy-ha.sh b/src/tools/deploy-ha.sh index 7b62665d..e0bb57b3 100755 --- a/src/tools/deploy-ha.sh +++ b/src/tools/deploy-ha.sh @@ -136,24 +136,32 @@ if [ ! -x "$SCHED_BIN" ]; then fi # Apiserver binary check -- same opt-in-by-presence spirit as WORKER_BIN/ -# SCHED_BIN above, but NOT a same-build-generation sibling lookup: the -# apiserver is a SEPARATE build system (Go, `make build-server` in -# src/frontend/Makefile), not part of the meson build dir $BINARY lives in, -# so there is no "$BINARY_BUILD_DIR/apiserver/..." to check. Its own build -# output always lands at a fixed path (`/tmp/apiserver`, build-server's -# `go build -o` target) regardless of which meson profile/build dir $BINARY -# came from. deploy-ha.sh does NOT invoke `make build-server` itself -- -# building it remains a caller prerequisite (same "run /xce-build controller -# first if stale" convention SKILL.md's Prerequisites section already -# documents for the controller, just a different build command). Soft-check -# only, same reasoning as worker/scheduler: xce-test-ha never builds an -# apiserver and calls this same script, so a missing binary must not fail -# the deploy -- it just skips generating scripts/start-apiserver.sh, with a -# warning. -APISERVER_BIN="/tmp/apiserver" -if [ ! -x "$APISERVER_BIN" ]; then - echo "Warning: no apiserver binary found at $APISERVER_BIN -- skipping scripts/start-apiserver.sh generation (run 'cd src/frontend && make build-server' first if you need it)" - APISERVER_BIN="" +# SCHED_BIN above. The apiserver is buildable TWO ways (tech.md's Build +# Systems table): (1) meson, via `tools/go_build` (`/xce-build apiserver` +# or `/xce-build all`) -- lands in $BINARY_BUILD_DIR like every other +# component this script resolves, so it IS a same-build-generation sibling +# lookup, same as xceadmin/worker/scheduler above; (2) the standalone Go +# workflow (`cd src/frontend && make build-server`), whose Makefile target +# hardcodes its output to a fixed `/tmp/apiserver` regardless of meson +# profile -- NOT guaranteed to be the same build generation as $BINARY. +# Prefer (1) exactly like xceadmin's own same-generation-first preference +# above; fall back to (2) with a caveat echoed, matching that precedent. +# deploy-ha.sh does NOT invoke either build itself -- building remains a +# caller prerequisite (same "run /xce-build controller first if stale" +# convention SKILL.md's Prerequisites section documents for the controller). +# Soft-check only, same reasoning as worker/scheduler: xce-test-ha never +# builds an apiserver and calls this same script, so a missing binary must +# not fail the deploy -- it just skips generating scripts/start-apiserver.sh, +# with a warning. +APISERVER_BIN="" +if [ -x "$BINARY_BUILD_DIR/frontend/apiserver/xce_apiserver" ]; then + APISERVER_BIN="$BINARY_BUILD_DIR/frontend/apiserver/xce_apiserver" + echo "Using apiserver: $APISERVER_BIN (same build dir as $BINARY)" +elif [ -x "/tmp/apiserver" ]; then + APISERVER_BIN="/tmp/apiserver" + echo "Using apiserver: $APISERVER_BIN (make build-server output; NOT guaranteed to be the same build generation as $BINARY)" +else + echo "Warning: no apiserver binary found at $BINARY_BUILD_DIR/frontend/apiserver/xce_apiserver or /tmp/apiserver -- skipping scripts/start-apiserver.sh generation (run '/xce-build apiserver' or 'cd src/frontend && make build-server' first if you need it)" fi # xceadmin wheel (same-build-generation sibling again): /xce-build xceadmin @@ -184,6 +192,7 @@ N=${#PEER_ARRAY[@]} mkdir -p "$TOP/certs" mkdir -p "$TOP/logs" mkdir -p "$TOP/scripts" +mkdir -p "$TOP/run" for i in $(seq 1 $N); do mkdir -p "$TOP/controller/ctrl-$i/conf" mkdir -p "$TOP/controller/ctrl-$i/data" @@ -247,7 +256,18 @@ if [ ! -d "\$CTRL_DIR/conf" ] || [ ! -d "\$CTRL_DIR/data" ]; then echo "Error: \$CTRL_DIR/conf or \$CTRL_DIR/data does not exist" exit 1 fi -$BINARY --myname \$NAME --peers "$PEERS_STR" --workdir $TOP/controller --certdir $TOP/certs --logfile $TOP/logs/\$NAME.log --vcpus 4 \$* +PIDFILE=$TOP/run/\$NAME.pid +if [ -f "\$PIDFILE" ]; then + OLD_PID=\$(cat "\$PIDFILE" 2>/dev/null || true) + if [ -n "\$OLD_PID" ] && kill -0 "\$OLD_PID" 2>/dev/null && \\ + tr '\0' ' ' < /proc/\$OLD_PID/cmdline 2>/dev/null | grep -q -- "--myname \$NAME " && \\ + tr '\0' ' ' < /proc/\$OLD_PID/cmdline 2>/dev/null | grep -q -- "--workdir $TOP/controller "; then + echo "Error: \$NAME already running as PID \$OLD_PID (workdir $TOP/controller) -- stop it first (kill \$OLD_PID) before restarting" + exit 1 + fi +fi +echo \$\$ > "\$PIDFILE" +exec $BINARY --myname \$NAME --peers "$PEERS_STR" --workdir $TOP/controller --certdir $TOP/certs --logfile $TOP/logs/\$NAME.log --vcpus 4 \$* EOF chmod +x "$TOP/scripts/start.sh" @@ -256,6 +276,7 @@ cat > "$TOP/scripts/cleanup.sh" << EOF #!$GEN_BASH set -e rm -f $TOP/logs/*.log +rm -f $TOP/run/*.pid for i in \$(seq 1 $N); do rm -rf $TOP/controller/ctrl-\$i/data/* done @@ -283,11 +304,21 @@ NAME=\${1:-real-worker} [ \$# -gt 0 ] && shift WORKER_BIN="\${XCE_WORKER_BIN:-$WORKER_BIN}" mkdir -p $TOP/logs +PIDFILE=$TOP/run/\$NAME.pid +if [ -f "\$PIDFILE" ]; then + OLD_PID=\$(cat "\$PIDFILE" 2>/dev/null || true) + if [ -n "\$OLD_PID" ] && kill -0 "\$OLD_PID" 2>/dev/null && \\ + tr '\0' ' ' < /proc/\$OLD_PID/cmdline 2>/dev/null | grep -q -- "-n \$NAME "; then + echo "Error: worker \$NAME already running as PID \$OLD_PID -- stop it first (sudo kill \$OLD_PID) before restarting" + exit 1 + fi +fi +echo \$\$ > "\$PIDFILE" # xcecd (production) requires root (worker/src/main.c:check_user()) and # hardcodes db_path=/var/run/xcecd.db + domain_sock_file=/var/run/.xcecd.sock # -- neither is a flag, so /var/run must exist before it will start. sudo mkdir -p /var/run -sudo "\$WORKER_BIN" --peers "$PEERS_STR" -n "\$NAME" -l $TOP/logs \$* +exec sudo "\$WORKER_BIN" --peers "$PEERS_STR" -n "\$NAME" -l $TOP/logs \$* EOF else cat > "$TOP/scripts/start-worker.sh" << EOF @@ -297,7 +328,17 @@ NAME=\${1:-worker-1} [ \$# -gt 0 ] && shift WORKER_BIN="\${XCE_WORKER_BIN:-$WORKER_BIN}" mkdir -p $TOP/logs -"\$WORKER_BIN" --peers "$PEERS_STR" -n "\$NAME" -l $TOP/logs \$* +PIDFILE=$TOP/run/\$NAME.pid +if [ -f "\$PIDFILE" ]; then + OLD_PID=\$(cat "\$PIDFILE" 2>/dev/null || true) + if [ -n "\$OLD_PID" ] && kill -0 "\$OLD_PID" 2>/dev/null && \\ + tr '\0' ' ' < /proc/\$OLD_PID/cmdline 2>/dev/null | grep -q -- "-n \$NAME "; then + echo "Error: worker \$NAME already running as PID \$OLD_PID -- stop it first (kill \$OLD_PID) before restarting" + exit 1 + fi +fi +echo \$\$ > "\$PIDFILE" +exec "\$WORKER_BIN" --peers "$PEERS_STR" -n "\$NAME" -l $TOP/logs \$* EOF fi chmod +x "$TOP/scripts/start-worker.sh" @@ -311,7 +352,17 @@ if [ -n "$SCHED_BIN" ]; then #!$GEN_BASH set -e mkdir -p $TOP/logs -$SCHED_BIN --peers "$PEERS_STR" --logfile $TOP/logs/scheduler.log -v \$* +PIDFILE=$TOP/run/scheduler.pid +if [ -f "\$PIDFILE" ]; then + OLD_PID=\$(cat "\$PIDFILE" 2>/dev/null || true) + if [ -n "\$OLD_PID" ] && kill -0 "\$OLD_PID" 2>/dev/null && \\ + tr '\0' ' ' < /proc/\$OLD_PID/cmdline 2>/dev/null | grep -q -- "--logfile $TOP/logs/scheduler.log "; then + echo "Error: scheduler already running as PID \$OLD_PID -- stop it first (kill \$OLD_PID) before restarting" + exit 1 + fi +fi +echo \$\$ > "\$PIDFILE" +exec $SCHED_BIN --peers "$PEERS_STR" --logfile $TOP/logs/scheduler.log -v \$* EOF chmod +x "$TOP/scripts/start-scheduler.sh" fi @@ -424,7 +475,17 @@ EOF #!$GEN_BASH set -e mkdir -p $TOP/logs -$APISERVER_BIN -d $TOP/apiserver \$* +PIDFILE=$TOP/run/apiserver.pid +if [ -f "\$PIDFILE" ]; then + OLD_PID=\$(cat "\$PIDFILE" 2>/dev/null || true) + if [ -n "\$OLD_PID" ] && kill -0 "\$OLD_PID" 2>/dev/null && \\ + tr '\0' ' ' < /proc/\$OLD_PID/cmdline 2>/dev/null | grep -q -- "-d $TOP/apiserver "; then + echo "Error: apiserver already running as PID \$OLD_PID -- stop it first (kill \$OLD_PID) before restarting" + exit 1 + fi +fi +echo \$\$ > "\$PIDFILE" +exec $APISERVER_BIN -d $TOP/apiserver \$* EOF chmod +x "$TOP/scripts/start-apiserver.sh" fi @@ -516,6 +577,32 @@ cat > "$TOP/scripts/start-all.sh" << EOF #!$GEN_BASH set -e +echo "=== start-all.sh: checking for already-running processes ===" +ALREADY_RUNNING=0 +for i in \$(seq 1 $N); do + PF=$TOP/run/ctrl-\$i.pid + if [ -f "\$PF" ]; then + PID=\$(cat "\$PF" 2>/dev/null || true) + if [ -n "\$PID" ] && kill -0 "\$PID" 2>/dev/null; then + echo "Error: ctrl-\$i already running as PID \$PID" + ALREADY_RUNNING=1 + fi + fi +done +for PF in $TOP/run/worker-1.pid $TOP/run/scheduler.pid $TOP/run/apiserver.pid; do + if [ -f "\$PF" ]; then + PID=\$(cat "\$PF" 2>/dev/null || true) + if [ -n "\$PID" ] && kill -0 "\$PID" 2>/dev/null; then + echo "Error: \$(basename "\$PF" .pid) already running as PID \$PID" + ALREADY_RUNNING=1 + fi + fi +done +if [ "\$ALREADY_RUNNING" -eq 1 ]; then + echo "Refusing to start -- one or more components from a previous deploy are still running against $TOP. Stop them first (kill , or pkill -f $TOP), then re-run." + exit 1 +fi + echo "=== start-all.sh: starting controller node(s) ===" for i in \$(seq 1 $N); do setsid nohup "$GEN_BASH" "$TOP/scripts/start.sh" ctrl-\$i -vvv /dev/null 2>&1 &