From 049dd0d4ced9c92aac35b285ff56d2b70bae3c75 Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Sat, 5 Sep 2026 11:19:03 +0700 Subject: [PATCH] docs(ops): correct the mechanism my own comment got wrong, and name the blast radius MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two non-blocking readings from the #318 review, both verified here before being written down. 1. The comment claimed a container with no healthcheck yields an EMPTY status. It does not. `.State.Health` is ABSENT on such a container, so `docker inspect -f '{{.State.Health.Status}}'` fails: template parsing error: ... map has no entry for key "Health" exiting 1, with the message swallowed by the `2>/dev/null`. `st` ends up empty as a side effect of the command failing, not because the field is empty. The fail-closed OUTCOME was right, so nothing behaves differently — but anyone who removes that redirect while debugging will start seeing template errors, and the comment as written would send them hunting a new bug instead of recognising a pre-existing path. Measured with `docker run --no-healthcheck`. Also measured, and worth recording because the first attempt to reproduce this got it wrong: `aastar-dvt:latest` carries its own HEALTHCHECK, which compose inherits even when the service does not declare one — so reaching this state at all needs an explicit `healthcheck: {disable: true}`. 2. The exit-3 message said "the tunnel is not the problem", which is true and incomplete. Because compose gates cloudflared on ALL THREE nodes being healthy, one unhealthy node means cloudflared cannot start at all — so if the tunnel also drops in that window, the two HEALTHY nodes have no public entry either. A reader could take "the tunnel is not the problem" to mean "the other two are fine publicly". Both the log line and the README row now say so. Not changing the gate: relaxing `depends_on` would buy back exactly the "tunnel serving in front of an empty origin" that exit 3 exists to prevent. Naming the trade-off is the fix; removing it is not. Refs #318 Claude-Session: https://claude.ai/code/session_01Pdq9rgkZq9M4JFeTZD7yYs --- deploy/README-heartbeat.md | 2 +- deploy/tunnel-keepalive.sh | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/deploy/README-heartbeat.md b/deploy/README-heartbeat.md index c976f53..350e026 100644 --- a/deploy/README-heartbeat.md +++ b/deploy/README-heartbeat.md @@ -61,7 +61,7 @@ Four launchd agents, sources version-controlled in `deploy/launchd/`: | -------------------------------- | ---------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `io.aastar.dvt-committee-health` | every 900 s | `committee-health.mjs` against the router; opens/updates a GitHub issue on any non-zero exit | | `io.aastar.dvt-apply-rotation` | every 3600 s | `apply-verifier-rotation.mjs --broadcast`. **Its rotation is done** — applied externally at 2026-09-04T05:37:12Z, not by this job — so runs from here do not broadcast: they check the active verifier against the address the job is pinned to, print "already applied" and exit 0. Green means the observed state matched that pin (it fails loudly on a disarm, a different verifier, or an unpinned new rotation); it does **not** mean this job applied anything. Repoint the expected verifier before the next rotation | -| `io.aastar.dvt-tunnel-keepalive` | every 300 s | `tunnel-keepalive.sh`: probes the **public** `dvt{1,2,3}.aastar.io/health` and restarts the cloudflared container when they stop serving, then re-probes to confirm it worked | +| `io.aastar.dvt-tunnel-keepalive` | every 300 s | `tunnel-keepalive.sh`: probes the **public** `dvt{1,2,3}.aastar.io/health` and restarts the cloudflared container when they stop serving, then re-probes to confirm it worked. Refuses to restart while any node is unhealthy (exit 3) — compose destroys the tunnel _before_ checking `service_healthy`, so restarting there would turn a partial outage into a total one. Worth knowing: **one** unhealthy node blocks cloudflared from starting at all, so in that window the other two nodes have no public entry either | | `io.aastar.dvt-committee-keeper` | `KeepAlive` (resident) | keeps `committee-keeper.mjs --watch` alive; restarts it if it dies | The first two run `deploy/local-heartbeat.sh`, which is **independent of diff --git a/deploy/tunnel-keepalive.sh b/deploy/tunnel-keepalive.sh index aaca305..89a64cc 100755 --- a/deploy/tunnel-keepalive.sh +++ b/deploy/tunnel-keepalive.sh @@ -91,8 +91,17 @@ internet_up() { # therefore the condition that makes compose refuse to start. A 2-of-3 degradation would have been # converted by this script into a 3-of-3 outage. # -# Empty status (a container with no healthcheck defined) counts as NOT healthy: compose's -# service_healthy would not be satisfiable either, so fail-closed is the truthful direction. +# A container with no healthcheck counts as NOT healthy, which is the right direction (compose's +# service_healthy would not be satisfiable either). But note the MECHANISM, because the obvious +# reading is wrong: `.State.Health` is absent rather than empty on such a container, so the template +# does not return "" -- `docker inspect` FAILS with +# template parsing error: ... map has no entry for key "Health" +# and exits 1, and the `2>/dev/null` below swallows it. `st` ends up empty as a side effect of the +# command failing, not because the field is empty. Anyone who removes that redirect to debug +# something else will suddenly see template errors here; they are pre-existing, not a new fault. +# (Measured. Also measured: aastar-dvt:latest carries its own HEALTHCHECK, which compose inherits +# even when the service does not declare one, so reaching this state at all needs an explicit +# `healthcheck: {disable: true}`.) nodes_healthy() { local c st for c in "${CONTAINERS[@]}"; do @@ -148,7 +157,10 @@ if ! nodes_healthy; then say "NODES DOWN or NOT HEALTHY: one or more of ${CONTAINERS[*]} is not reporting healthy. The \ tunnel is not the problem; restarting it would front an empty origin -- and compose would destroy \ the running tunnel and then refuse to recreate it, turning a partial outage into a total one. \ -Start or repair the node stack first." +Start or repair the node stack first. NOTE: while ONE node is unhealthy, cloudflared cannot be \ +started at all (compose gates it on all three being healthy), so if the tunnel also goes down in \ +that window, the healthy nodes' public endpoints cannot come back either -- do not read 'the \ +tunnel is not the problem' as 'the other two are fine publicly'." exit 3 fi