diff --git a/documentation/audits/SPIKE-controller-agent-channel-health-2026-06-29.md b/documentation/audits/SPIKE-controller-agent-channel-health-2026-06-29.md new file mode 100644 index 0000000..cb69110 --- /dev/null +++ b/documentation/audits/SPIKE-controller-agent-channel-health-2026-06-29.md @@ -0,0 +1,108 @@ +# SPIKE — Controller→agent channel health-check mechanics (failure classification + self-heal) + +- **Date:** 2026-06-29 +- **Class:** Spike / empirical, mostly read-only. One mutation: a clean `systemctl restart + felhom-agent` (no backup in flight; same as R1). Throwaway probe (uncommitted `channelspike_test.go` + in the controller's `agentapi` package on the build server) removed in teardown. No change to + `controller.yaml`, the agent config, mounts, the leaf, or sudoers. +- **Verdict: GO** — a periodic controller→agent channel check is a straightforward slice. The failure + surface classifies cleanly (Q1), and the memoized production client **self-heals** after an agent + restart with no controller restart (Q2) — no latching-client wrinkle to fix first. One nuance: the + only *latching* failure is a `New()` **construction** error (bad fingerprint), which must be surfaced + as a distinct, louder alert. + +--- + +## Q1 — failure classification map + +Throwaway `agentapi.New(...) + client.Storage(ctx)` against the live agent, one mode each. Exact +errors (verbatim): + +| Mode | Exact error | Kind | Match on | Proposed operator alert | Severity | +|---|---|---|---|---|---| +| baseline (real ep/token/fp) | `Storage() OK (channel up)` | — | — | — | — | +| **(a) pin mismatch** (valid-hex wrong fp) | `agentapi: GET /storage: Get "https://192.168.0.162:8443/storage": agentapi: TLS pin mismatch: agent leaf SHA-256 does not match the bootstrap fingerprint` | runtime | `TLS pin mismatch` | Agent leaf cert no longer matches the controller's bootstrap fingerprint — re-pin / re-bootstrap (the R1 incident class) | **critical** | +| **(b) token rejected** (real fp, wrong token) | `agentapi: GET /storage: HTTP 401` | runtime | `HTTP 401` | Agent rejected the controller's bearer token (stale/rotated) — re-bootstrap the guest token | **critical** | +| **(c) agent down / refused** (closed port) | `agentapi: GET /storage: Get "https://…:9/storage": dial tcp …:9: connect: connection refused` | runtime | `connection refused` | felhom-agent down or `:8443` closed | **warning** (transient) → critical if sustained N cycles | +| **(d) timeout** (black-hole `192.0.2.1`) | `agentapi: GET /storage: Get "https://…/storage": context deadline exceeded` | runtime | `context deadline exceeded` / `i/o timeout` | Host unreachable / packets dropped (firewall, host down) | **warning** → critical if sustained | +| **(e) wrong endpoint** (bogus host) | `agentapi: GET /storage: Get "https://nonexistent.invalid:8443/storage": dial tcp: lookup nonexistent.invalid …: no such host` | runtime | `no such host` / `lookup` | `local_api.endpoint` misconfigured / unresolvable | **critical** (config) | +| **(f) malformed fingerprint** (non-hex) | `agentapi: fingerprint must be a SHA-256 (64 hex chars), got 10` (also: `fingerprint is not valid hex`) — **returned by `New()`, not `Storage()`** | **construction (LATCHES)** | error from `agentClient()` itself (before any call) | `local_api.fingerprint` is not a valid SHA-256 — the controller can't build the client at all (needs a config fix + restart) | **critical** (config) | + +**Construction vs runtime is the load-bearing distinction.** Only (f) fails at +`agentapi.New(...)` — and `agentClient()` (`agent_disk_handlers.go:51`) memoizes that via `sync.Once`, +so it **latches** until a controller restart. (a)–(e) are all per-call `Storage()` errors that clear +when the underlying condition does. The classifier keys on the substrings above; all six are +distinguishable. + +--- + +## Q2 — self-heal across a clean agent restart (the load-bearing question) + +Two probes run concurrently across **one** `systemctl restart felhom-agent` (no controller restart): +- **Memoized production path:** `GET /api/disks` through Traefik in-guest (the exact UI call → the + `sync.Once` memoized `agentClient()`), once/sec. 200 = up, non-200 = down. +- **Fresh client:** a new `agentapi.New(...)+Storage()` each second (the `probeLocalAPI` shape). + +Aligned by the restart event (~t+04): + +``` +MEMOIZED (/api/disks): t+00..03 http=200 | t+04 http=502 | t+05..25 http=200 ← 1 failed probe, then up +FRESH client: t+00..03 UP | t+04,05 DOWN (connection refused) | t+06..27 UP +controller container: Up 7 hours (healthy) — DID NOT restart +controller log: one "[web] disk list via agent failed: … connect: connection refused" at the + restart instant, then silence (recovered on its own) +``` + +**Verdict:** +- **The memoized production client SELF-HEALS** after a clean agent restart **without a controller + restart** — one failed call at the restart instant, 200 immediately after. The controller container + never bounced (Up 7h). +- **The fresh-client probe AGREES with the production path at every phase** (both up before, both down + during the ~1–2 s socket gap, both up after). A fresh-client probe is therefore a **faithful + predictor** of the UI's reality — it does **not** report "up" while the UI stays broken. +- **Why it self-heals:** the pin is verified **per TLS handshake** (`client.go:71`), the pooled + transport simply re-dials after `connection refused`, and a clean restart serves the **same** leaf + cert (`60b5974d…`) so the pin still matches. The `*agentapi.Client` reuse only caches the transport, + not a verdict. +- **The R1 incident did NOT contradict this:** there the disk UI stayed broken because the agent served + a genuinely **different** cert (a regenerated leaf) → a real *persistent* pin mismatch, not memoized + stickiness. The memoized client correctly reports "down" while that condition holds and "up" once it + clears — it never lies. + +--- + +## Impl recommendation (for the channel-check slice) + +1. **Probe target — reuse `GET /storage`.** It is the existing `probeLocalAPI` probe: cheap, + read-only, no side-effect (returns this guest's mounts). No new `/healthz` needed. (`/backup/due` is + an equally cheap alternative.) +2. **Reuse the MEMOIZED production client (`s.agentClient()`), NOT a fresh client per probe.** Q2 shows + they agree, so reusing the production client makes the health-check reflect *exactly* what the UI + experiences (zero divergence risk) AND avoids reintroducing the per-call `http.Transport` leak that + `agentClient()` was made a singleton to fix. Bonus: a construction error (mode f) is already surfaced + as the memoized `agentClient()` error, so the check distinguishes "can't even build the client + (latched config error)" from a runtime channel failure for free. **No memoized-client recovery fix + is required** (it self-heals). +3. **Cadence + transition/cooldown — mirror `HostCapabilityChecker`/`HostStalenessChecker`.** A periodic + goroutine (~60 s) calls the probe, classifies the error (Q1 substrings) into a state + `up | down:`, and on a **transition** (up→down, down→up, or reason-change) emits via the + existing controller→hub event relay → operator alert (English) with the hub-side 1 h cooldown. To + avoid alerting on a 1-cycle blip (the ~1 s restart gap above would otherwise page), require **N≥2 + consecutive** down cycles before the `down` transition for the transient reasons (refused/timeout); + pin-mismatch / 401 / construction-error can alert on the first observation (they don't self-clear). +4. **Failure→alert mapping:** the Q1 table (event type per reason, e.g. `agent_channel_pin_mismatch`, + `agent_channel_unauthorized`, `agent_channel_unreachable`, `agent_channel_misconfigured`, + `agent_channel_recovered`). + +## Go / No-go — **GO** + +No wrinkle blocks a straightforward slice. The single thing to get right: **classify the construction +error (latching, config) distinctly from runtime errors (mostly transient)**, and **probe via the +memoized client** so the check can't lie. Debounce transient reasons (N≥2) to avoid restart-blip noise. + +## Teardown — confirmed + +Throwaway `channelspike_test.go` removed (build-server working tree clean — `git status` shows it +gone); `/tmp/spike.env` (held the token, 0600) + both Q2 logs removed; agent restarted clean and +`active`. No config/secret/agent/leaf/sudoers changes. The per-guest token was read out-of-band and is +**not** recorded here (the leaf fingerprint is not a secret).