show-status: exit code was always 0 — three compounding bugs #66
Reference in New Issue
Block a user
Delete Branch "fix-show-status-exit-code"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
ASSET-only —
show-status.sh. Script-only, so no./update.sh, no gitlink change.The problem
show-status.shcould never report failure. The ansible task that wraps it — "Execute show-status.sh and fail on failure" — therefore passed on every host regardless of node state.Three bugs, each masking the next:
1.
$?read too late.2. The else branch is inverted. That branch is the sync-status succeeded path, yet it set
code=1; any_failure=true— marking healthy nodes as failures.3.
any_failurecould never propagate.check_sync_statusruns backgrounded (&), i.e. in a subshell, so any variable it sets is discarded — and thewait "$pid"loop threw the status away too.Bug 3 hides 1 and 2: a script that believed every node had failed still exited 0, so the contradiction never surfaced.
Why it matters
The hourly fleet-health cron reports
8 BROKEN … 4 past broken thresholdand nobody acts on it. This is part of why: the one mechanism that could turn "a node is broken" into an ansible failure was inert.It also produced a false sense of safety in the opposite direction — a green ansible run on a host whose nodes were in
error(observed today on rpc-us-49, where both nodes reporterrorwhile being demonstrably healthy).The fix
rc=$?immediately after the callsyncing/lagging→ tolerated, anything else → failurewait "$pid" || any_failure=true, since the subshell cannotComments added at each site explaining why, so the
$?and subshell traps don't get reintroduced.Verification
Stubbed
sync-status.shdriving status by node name; identical matrix run against both versions:bash -nclean. Behaviour for healthy fleets is unchanged — only genuine failures newly surface, which is the point.⚠️ Expect this to make some hosts start failing
run-rpc-updatewhere they previously passed silently. That is the fix working, not a regression — but it is a behaviour change worth knowing about before a fleet rollout, and it argues for landing this on one host first.Related
Found while investigating rpc-us-49, whose two nodes report
errorwhile healthy. That turned out to be a separate issue — us-49/us-50/us-45 are absent fromproduction/hosts, so they never get traefik and their HTTP probes 404. Escalated to the operator inbox ashosts-missing-from-ansible-inventory; this PR does not address that.🤖 Generated with Claude Code
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) <noreply@anthropic.com>⏸ HELD — do not merge yet (rob, 2026-08-14)
Blocked pending Sebastian's decision on operator claim
hosts-missing-from-ansible-inventory.Why the dependency is real. This fix makes
show-status.shexit non-zero when a node reportserror, which makesrun-rpc-updatefail on that host. Today three live hosts — rpc-us-49, rpc-us-50, rpc-us-45 — are absent fromproduction/hosts, never receive traefik, and therefore return 404 on every HTTP probe, so every node on them reportserrordespite being healthy.Merging this first would convert that into hard ansible failures on those hosts. Sebastian's answer determines which:
Either way the sequencing is: inventory decision → then this.
The change itself is verified and ready (see matrix above); it is only the rollout that waits. When it does land, do a single host first — some hosts will legitimately start failing where they previously passed silently, and that wants observing before a fleet run.
merge-bot (del-018): classified B (hand-maintained rpc file(s), single scope). Auto-merge after a 72h objection window - comment 'hold' to block.