Files
felhom.eu/documentation/audits/SPIKE-controller-agent-channel-health-2026-06-29.md
T

136 lines
10 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 ~12 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:<reason>`, 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.
## IMPLEMENTED 2026-06-29 — controller v0.90.0 (+ hub v0.20.0)
The channel-check slice shipped per this spike's GO: `internal/channelhealth` (a ~60s scheduler job
probing via the memoized `Server.ProbeAgentChannel`, the Q1 classifier, N≥2 debounce for transient
reasons, first-obs seed) + an English operator-only Notifier event + a Hungarian dashboard banner.
Live-validated on guest 9201: a clean agent restart fired **no** alert (debounce); a sustained stop
logged `transient down (unreachable, 1/2) — suppressed` then `agent channel DOWN (up→down:unreachable)`
+ the dashboard banner; start → `recovered`. One thing the spike didn't predict: the hub's
`allowedEventTypes` allowlist rejected the controller-pushed `agent_channel_*` events (HTTP 400) —
fixed in **hub v0.20.0**. See `felhom-controller/REPORT.md` + CHANGELOG v0.90.0.
## Self-health arc COMPLETE (2026-06-29)
This channel-check is one of three independent angles on the original silent-multi-day-outage incident
class, all now shipped + live-validated:
1. **agent** watches its own privileged capabilities (v0.44.0 + the build-time sudoers gate).
2. **controller** watches its link to the agent — this channel-health check (v0.90.0) + F2 born-down
alerting (v0.91.0).
3. **hub** proactively watches every agent's served local-API **leaf fingerprint** fleet-wide
(agent v0.48.0 reports it; hub v0.22.x `HostLeafChecker` → `host_leaf_changed`), independent of any
controller. Live: a leaf regen raised `host_leaf_changed` (`82078fab…→60b5974d…`) AND the controller
`agent_channel_pin_mismatch` — complementary detection.
Prevention also shipped: agent v0.46.0 loud-WARNs a regenerated leaf + `felhom-host-install.sh`
`--preserve-state-from`/populated-host guard. The original incident is now both prevented and caught
from three angles. (Remaining future enhancement: the served-fp-vs-pinned-fp authoritative cross-check.)
## 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).