From 502a75b2fda16411813ebffff5181e003cff8082 Mon Sep 17 00:00:00 2001 From: Claude Agent Date: Fri, 14 Aug 2026 19:56:10 +0000 Subject: [PATCH] =?UTF-8?q?show-status:=20exit=20code=20was=20always=200?= =?UTF-8?q?=20=E2=80=94=20three=20compounding=20bugs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit show-status.sh could never report failure. The ansible task that wraps it ('Execute show-status.sh and fail on failure') therefore always passed, on every host, regardless of node state. Three bugs, each masking the next: 1. $? read too late. `code=0` sits between the sync-status.sh call and `if [ $? -ne 0 ]`. A plain assignment succeeds and overwrites $? with 0, so the condition was ALWAYS false and the else branch always taken. 2. The else branch was inverted. It is the sync-status-SUCCEEDED path, yet it set `code=1; any_failure=true` — marking healthy nodes as failures. 3. any_failure could never propagate. check_sync_status runs backgrounded (`&`), i.e. in a subshell, so `any_failure=true` inside it is discarded; and the `wait "$pid"` loop threw away each job's exit status. (3) hid (1) and (2): a script that believed every node had failed still exited 0, so nobody saw it. Fix: capture rc immediately; restore the intended logic (success => 0, syncing or lagging => tolerated, anything else => failure); propagate failure in the PARENT via `wait "$pid" || any_failure=true`, since the subshell cannot. Verified with a stubbed sync-status.sh: scenario before after all online 0 0 one syncing 0 0 (tolerated) one lagging 0 0 (tolerated) one ERROR 0 1 ALL error 0 1 Behaviour for healthy fleets is unchanged; only genuine failures now surface. Co-Authored-By: Claude Opus 5 (1M context) --- show-status.sh | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/show-status.sh b/show-status.sh index ef68d945..ce026a24 100755 --- a/show-status.sh +++ b/show-status.sh @@ -22,26 +22,25 @@ check_sync_status() { # Cap the whole per-node branch (belt-and-suspenders over check-health's own cap), so no single # node can ever block the 'wait' below — that is what wedged the fleet rpc-update for hours. result=$(timeout "${SYNC_TIMEOUT:-60}" "$BASEPATH/sync-status.sh" "${part%.yml}") + # Capture the status IMMEDIATELY. Any command in between - including a plain + # assignment like `code=0` - overwrites $? with its own (always 0) status. + rc=$? code=0 - - if [ $? -ne 0 ]; then - if [[ "$result" == *"syncing"* ]]; then - # Allow exit status 1 if result contains "syncing" - code=0 - elif [[ "$result" == *"lagging"* ]]; then - # Allow exit status 1 if result contains "lagging" + if [ "$rc" -ne 0 ]; then + if [[ "$result" == *"syncing"* ]] || [[ "$result" == *"lagging"* ]]; then + # sync-status exits 1 for syncing/lagging; those are expected states, + # not failures. code=0 else - any_failure=true code=1 fi - else - code=1 - any_failure=true fi echo "${part%.yml}: $result" + # NOTE: do NOT set any_failure here. This function runs backgrounded (`&`), so + # it executes in a subshell and any variable it sets is discarded. Failure is + # propagated to the parent through this return code, collected by `wait` below. return "$code" } @@ -74,9 +73,12 @@ for part in "${parts[@]}"; do fi done -# Wait for all background processes to finish +# Wait for all background processes to finish. `wait` runs in the PARENT shell, so +# this is where a failing node can actually flip any_failure - the checker itself +# cannot, being a subshell. Previously the status was discarded here, which silently +# neutered the exit code. for pid in "${pids[@]}"; do - wait "$pid" + wait "$pid" || any_failure=true done # Fenced nodes (fleet-state maintenance windows) are dropped from COMPOSE_FILE