diff --git a/Dockerfile b/Dockerfile index 988616406..82f533b7c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -134,11 +134,10 @@ RUN mkdir -p downloads logs && \ USER bot -# Liveness via Redis TCP PING — confirms node can run AND the bot's -# Redis dependency is reachable. A wedged node process or broken Redis -# link both fail this; the old `console.log` check did neither. -HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \ - CMD node -e "const net=require('net');const s=net.createConnection({host:process.env.REDIS_HOST||'redis',port:+(process.env.REDIS_PORT||6379)},()=>s.write('*1\r\n\$4\r\nPING\r\n'));s.on('data',d=>process.exit(d.toString().startsWith('+PONG')?0:1));s.on('error',()=>process.exit(1));setTimeout(()=>process.exit(1),3000);" || exit 1 +# Gateway readiness via /healthz — returns 200 when client.isReady(), 503 otherwise. +# Covers Redis reachability implicitly (the bot cannot complete login without it). +HEALTHCHECK --interval=15s --timeout=5s --start-period=45s --retries=3 \ + CMD node -e "require('http').get('http://127.0.0.1:9091/healthz',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))" CMD ["sh", "-c", "npx prisma migrate deploy --config prisma/prisma.config.ts && node packages/bot/dist/index.js"] diff --git a/docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md b/docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md new file mode 100644 index 000000000..f536936fb --- /dev/null +++ b/docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md @@ -0,0 +1,61 @@ +# ADR: Bot Docker HEALTHCHECK — Gateway Readiness over Redis TCP Ping + +**Date:** 2026-05-24 +**Status:** Accepted + +## Context + +The `production-bot` Dockerfile stage had a HEALTHCHECK that pinged the Redis TCP socket. This answered "can the bot process reach Redis?" but not "is the Discord gateway connected?" + +A bot container could be `healthy` under this check while: + +- Failing to authenticate with Discord (invalid token) +- Stuck in gateway reconnect backoff +- Blocked by a Discord API outage + +`deploy.sh` already checks for `unhealthy` Docker containers and fails the deploy. The HEALTHCHECK signal is therefore the correct place to surface bot gateway connectivity — no CI or deploy-script changes are needed. + +The bot's metrics server (`metricsServer.ts`) already exposes a `/healthz` route on `0.0.0.0:9091` that returns HTTP 200 when `client.isReady()` is true and 503 otherwise. + +## Decision + +Replace the Redis TCP ping in the bot's HEALTHCHECK with an HTTP GET against `localhost:9091/healthz`. Increase `start-period` from 30 s to 45 s to account for the additional time needed to complete Discord gateway login after process start. + +```dockerfile +HEALTHCHECK --interval=15s --timeout=5s --start-period=45s --retries=3 \ + CMD node -e "require('http').get('http://127.0.0.1:9091/healthz',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))" +``` + +`curl` is not available in the Alpine base image, so the probe uses the native Node.js `http` module. + +## Alternatives Considered + +**Keep Redis TCP ping:** Preserved fast startup health signals (~1 s). Rejected because Redis connectivity does not imply gateway connectivity — the two failure modes are different and independent. + +**Add a second HEALTHCHECK:** Docker only permits one HEALTHCHECK directive per stage. Not possible. + +**Add `botReady` field to backend `/api/health` via Redis flag:** Requires bot + backend changes + deploy-script probe update. Rejected: the gateway check is indirect (mediated by Redis), has a race condition at startup (flag absent before first `ready` event), and does not auto-fail the deploy — an operator must check the JSON manually. See rejected Option B in research notes. + +**`docker exec` introspection from deploy.sh:** Requires importing the full bot module graph; fragile and couples deploy logic to bot internals. Rejected. + +## Consequences + +**Positive:** + +- A bot container that is running but not connected to Discord becomes `unhealthy`, which fails the deploy automatically. No CI or deploy-script changes needed. +- The healthcheck implicitly covers Redis reachability — the bot cannot complete `client.login()` and become ready if Redis is unreachable (session state, queue, etc.). +- Faster failure signal: `interval=15s` means gateway disconnect is detected within 45 s (interval × retries). + +**Negative:** + +- `start-period=45s` delays the first health check pass. If the bot connects to Discord in under 10 s (typical), the container is marked healthy immediately; if Discord is slow (> 45 s), the first check fires while the client is still connecting and is counted as a failure. Three consecutive failures (135 s total) would mark the container `unhealthy`. This is acceptable for a deploy-time gate. + +**Neutral:** + +- The metrics server (`METRICS_DISABLED=true`) is a no-op in tests. The healthcheck fires in production containers only. + +## Revisit When + +- The bot's startup time consistently exceeds 45 s (increase `start-period`). +- Discord gateway latency at cold start grows beyond 30 s routinely — monitor with `HEALTHCHECK_GATEWAY_CONNECT_MS` metric once Prometheus scraping is wired. +- `metricsServer.ts`'s `/healthz` route is removed or the metrics server is disabled by default. diff --git a/docs/decisions/2026-05-24-deploy-lock-contention-signal.md b/docs/decisions/2026-05-24-deploy-lock-contention-signal.md new file mode 100644 index 000000000..1ce7fb176 --- /dev/null +++ b/docs/decisions/2026-05-24-deploy-lock-contention-signal.md @@ -0,0 +1,74 @@ +# ADR: Deploy Lock Contention — Fail Fast with Error Commit Status + +**Date:** 2026-05-24 +**Status:** Accepted + +## Context + +`deploy.sh` uses an atomic `mkdir` lock (`$LOCK_DIR`) to prevent concurrent deploys. When a second webhook fires while a deploy is already running, `acquire_lock` fails and `deploy.sh` exits 1 immediately. + +Before this change, the contention path had two problems: + +1. `DEPLOYED_SHA` is set only after `sync_checkout_to_origin_main` succeeds, so the EXIT trap's `post_deploy_status` call had no SHA to post against — the status was silently dropped. +2. CI polled the `homelab-deploy` commit status for the incoming SHA for up to 5 minutes (20 × 15s), found nothing, and printed "Deploy validation timed out" with exit 1 — giving a confusing signal rather than an actionable one. + +The second commit's deploy was silently dropped; an operator had to dig through deploy logs to understand why. + +## Decision + +Before the lock acquisition attempt, run `git fetch origin main` to determine the incoming SHA regardless of what the working tree contains. On contention, immediately post `state: error` keyed to that SHA via the GitHub Statuses API — then exit 1. + +```bash +incoming_sha="" +if git -C "$DEPLOY_DIR" fetch origin main 2>/dev/null; then + incoming_sha=$(git -C "$DEPLOY_DIR" rev-parse FETCH_HEAD 2>/dev/null || true) +fi + +if ! acquire_lock; then + if [[ -n "$GITHUB_DEPLOY_STATUS_TOKEN" && -n "$incoming_sha" ]]; then + curl -s -o /dev/null \ + -X POST \ + -H "Authorization: token $GITHUB_DEPLOY_STATUS_TOKEN" \ + -H "Content-Type: application/json" \ + "https://api.github.com/repos/${GITHUB_REPO}/statuses/${incoming_sha}" \ + -d '{"state":"error","description":"Deploy skipped — another deploy in progress","context":"homelab-deploy"}' || true + fi + notify 16711680 "Deploy Skipped" "Another deploy is already in progress" + exit 1 +fi +``` + +The `git fetch` result is silently swallowed if the network is unavailable — `incoming_sha` stays empty and the status post is skipped (same behavior as missing token). + +## Alternatives Considered + +**Keep exit-1 with no status post:** Zero change, CI continues to time out confusingly after 5 minutes. Rejected — the root cause (concurrent deploy) is immediately knowable; masking it costs operator time. + +**Post `failure` instead of `error`:** `failure` implies the deploy ran and the service is broken. `error` means the deploy could not start — the correct semantic per the GitHub Statuses API. Operator sees "error" in the deploy check and knows to look at lock contention, not service health. + +**Wait for the running deploy to finish, then re-trigger:** Requires a retry loop inside deploy.sh and a way to re-webhook the same SHA. Over-engineered for the frequency of occurrence. The operator can re-push or re-trigger if needed. + +**Extend EXIT trap to capture incoming SHA:** The EXIT trap fires after lock failure too, but `DEPLOYED_SHA` is empty at that point. Patching the trap to use `incoming_sha` would work but is more invasive and less readable than the targeted pre-lock block. + +## Consequences + +**Positive:** + +- CI exits immediately on `error` status instead of timing out after 5 minutes. +- The error description ("Deploy skipped — another deploy in progress") is surfaced directly in the CI step output via `gh api` polling. +- The operator can re-trigger the second deploy after the first finishes. + +**Negative:** + +- `git fetch origin main` adds one network call before every deploy, including the common non-contention path. The call is a `fetch` (read-only, ~100ms on LAN), not a full clone. A fetch failure is silently ignored and does not abort the deploy. +- If both `GITHUB_DEPLOY_STATUS_TOKEN` is absent and the fetch fails, the error is invisible to CI — same as before this change. The token must be configured for full benefit. + +**Neutral:** + +- The same `homelab-deploy` context string is used, so the error status appears on the same row as the success/failure status in the GitHub UI. + +## Revisit When + +- Rapid-fire deploys (>2 concurrent) become common — the lock pattern itself may need a queue rather than a drop. +- `git fetch` latency from homelab to GitHub increases consistently beyond ~2s (measure via deploy logs). +- `GITHUB_DEPLOY_STATUS_TOKEN` is rotated without updating the homelab environment — contention will revert to silent CI timeout until the token is refreshed. diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 3f931eee1..5f41dfb9c 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -330,8 +330,22 @@ if [[ "$RECEIVED_SECRET" != "$EXPECTED_SECRET" ]]; then exit 1 fi +incoming_sha="" +if git -C "$DEPLOY_DIR" fetch origin main 2>/dev/null; then + incoming_sha=$(git -C "$DEPLOY_DIR" rev-parse FETCH_HEAD 2>/dev/null || true) +fi + if ! acquire_lock; then log "ERROR: LOCK_CONTENTION (another deploy is already running)" + if [[ -n "$GITHUB_DEPLOY_STATUS_TOKEN" && -n "$incoming_sha" ]]; then + curl -s -o /dev/null \ + -X POST \ + -H "Authorization: token $GITHUB_DEPLOY_STATUS_TOKEN" \ + -H "Content-Type: application/json" \ + "https://api.github.com/repos/${GITHUB_REPO}/statuses/${incoming_sha}" \ + -d '{"state":"error","description":"Deploy skipped — another deploy in progress","context":"homelab-deploy"}' || true + log "INFO: posted error status for ${incoming_sha}" + fi notify 16711680 "Deploy Skipped" "Another deploy is already in progress" exit 1 fi