docs: v0.74.0 REPORT/CONTEXT/README — agent connection-leak fix + live proof

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-22 17:52:12 +02:00
parent 2a5b88fbf9
commit c208ddf308
3 changed files with 75 additions and 49 deletions
+14
View File
@@ -7,6 +7,20 @@
>
> Ask Claude Code: "Please update CONTEXT.md with what we did today"
Last updated: 2026-06-22 (v0.74.0 — controller→agent connection-leak fix)
> **2026-06-22 — v0.74.0 (deployed on demo guest 9201): fixed the controller→agent connection leak.**
> `Server.agentClient()` built a new `agentapi.Client` (new bare `http.Transport`, `IdleConnTimeout:0`)
> per call → one leaked idle ESTABLISHED socket per agent call to `192.168.0.162:8443`, exhausting the
> ephemeral source-port range after ~5 days of controller uptime → EADDRNOTAVAIL, which had taken down
> storage UI + host-metrics + whole-guest backup (found in the 2026-06-22 unattended campaign). Fix:
> memoize ONE shared client via `sync.Once` + harden the Transport (`MaxIdleConnsPerHost:2`,
> `IdleConnTimeout:90s`). Live-proven: idle sockets to `:8443` stay flat at 2 across a 120-call burst
> (pre-fix grew ~1/call → 132). Agent/firewall untouched. Diagnosis + campaign findings live in
> `felhom.eu/documentation/tests/unattended-test-campaign-2026-06-22-*.md`. **Separate open item:** the
> defense-in-depth host firewall rule scoping `:8443` to the guest bridge subnet is still absent
> (pve-firewall disabled) — to be closed independently.
Last updated: 2026-06-13 (v0.60.0 backlog-Medium cleanup)
> **Live version: controller v0.60.0** (deployed on demo guest 9201), agent **v0.30.0** (AGENT-001 deployed), hub **v0.11.0**.
+55 -49
View File
@@ -1,58 +1,64 @@
# REPORT — controller v0.73.0: DR recipe (customer + apps half)
# REPORT — controller v0.74.0: fix the controller→agent connection leak
**TASK — DR recipe slice (controller half).** Emit the secret-free customer + apps half of the
reconstruction recipe as an additive hub-report section. Grounded in `SPIKE-dr-recipe-2026-06-16.md`.
**The controller emitter is the boundary enforcement point** — it is the component that distinguishes
secret from non-secret deploy fields, so v1 ships only an allowlist and the no-secrets boundary test
lives here.
## Baseline → target
`felhom-controller` `main` v0.73.0 → **v0.74.0**. Controller-only change; agent/hub/firewall untouched.
## Implementation
## Problem
`Server.agentClient()` built a fresh `agentapi.Client` (hence a fresh bare `http.Transport`,
`IdleConnTimeout:0`) on **every** agent API call and discarded it without closing idle connections.
The agent's keep-alive left one idle `ESTABLISHED` socket per call to `192.168.0.162:8443`; they
accumulated (~5.8k/day) until the ephemeral source-port range for that tuple exhausted →
`connect: cannot assign requested address` (EADDRNOTAVAIL), disabling storage UI, host-metrics, and
whole-guest backup after ~5 days of controller uptime. (`:8006`/pveproxy was immune — the controller
never dials it.) Diagnosis:
`felhom.eu/documentation/tests/unattended-test-campaign-2026-06-22-8443-diagnosis.md`.
- `internal/report/dr_recipe.go`:
- `DRRecipeAppHalf{recipe_version, customer{id,display,domain}, apps[]}`.
- `BuildDRRecipeAppHalf(custID, custName, domain, stacks, composeReader)` — pure given the reader;
one `AppRecipe` per DEPLOYED, non-protected stack.
- `AppRecipe{catalog_ref, enabled, storage_bindings}` — the **entire v1 surface**. `buildAppRecipe`
reads NOTHING from `AppConfig.Env`.
- `appStorageBindings(composeYAML, hddPath)` — pure compose parser; each `${HDD_PATH}`/`${USERDATA_PATH}`
volume bind → `{container_path, drive=basename(HDD_PATH), subpath}`; named volumes excluded.
- `readComposeFile` — the production reader (best-effort; unreadable → no bindings, never a failure).
- `Report.DRRecipe *DRRecipeAppHalf` wired into `BuildReport` (customer fields + `GetStacks()`).
## Change (commit `2a5b88f`)
- `internal/web/server.go``Server` gains `agentCli *agentapi.Client`, `agentCliErr error`,
`agentCliOnce sync.Once` (+ the `agentapi` import).
- `internal/web/agent_disk_handlers.go``agentClient()` memoizes the build via `agentCliOnce` and
**returns one shared client** (cfg.LocalAPI is static per process; a config-apply self-restarts the
controller). The empty-endpoint "not configured" guard stays OUTSIDE the `Once`. Signature and all
19 call sites unchanged; `*agentapi.Client`/`*http.Client` are concurrency-safe so no extra locking.
- `internal/agentapi/client.go``New` Transport hardened: `MaxIdleConns:4`,
`MaxIdleConnsPerHost:2`, `IdleConnTimeout:90s` (was bare, `IdleConnTimeout:0`). Added optional
`Client.Close()` (CloseIdleConnections) hygiene helper.
## The boundary (the Phase-1 lesson)
## Tests (green; both red-proofed)
- `go build ./... && go vet ./... && go test ./...` — all green.
- **T1** `TestAgentClient_ReusesSameInstance` (web) — two `agentClient()` calls return the identical
pointer; `TestAgentClient_UnconfiguredErrors` — empty endpoint still errors.
Red-proof: reverting to per-call `agentapi.New` → pointers differ → FAIL (shown, reverted).
- **T2** `TestNew_TransportIdlePoolBounded` (agentapi, white-box) — `IdleConnTimeout>0` AND
`MaxIdleConnsPerHost>0`. Red-proof: bare Transport → `IdleConnTimeout==0` → FAIL (shown, reverted).
The recipe carries ONLY `{catalog_ref, enabled, storage_bindings}` — identifiers/paths. It NEVER touches
`AppConfig.Env`, where the controller keeps `ENC:` secrets. This is an **allowlist** (a new field is
excluded by default), the inverse of the retired infra-backup that shipped `restic_password` /
`cf_api_token`. Secrets stay in the PBS whole-CT snapshot + escrow, recovered with R, never here.
## Deploy
Built+pushed `gitea.dooplex.hu/admin/felhom-controller:0.74.0` on 192.168.0.180 (digest
`sha256:2e85376e…`), deployed to guest 9201 via the bootstrap path (`docker pull` → pin
`/etc/felhom-controller-image` → restart `felhom-controller-bootstrap.service`). `docker inspect`:
`image=:0.74.0 status=running health=healthy`.
> Process note: the first build packaged stale source (the build server's `~/git/felhom-controller`
> was at v0.73.0 — the required `git -C ~/git/felhom-controller pull` step had been skipped, so the
> image was the old code mislabeled `:0.74.0`; the live test still leaked). Pulled the source to
> `2a5b88f` and rebuilt — second image (`sha256:2e85376e…`) is the real fix.
## Tests (the load-bearing boundary test + companion)
## LIVE acceptance — leak is gone (the real proof)
Burst of **120 agent calls** (alternating `/api/disks` + `/api/host-metrics`) against the controller in
guest 9201, sampling idle `ESTABLISHED` sockets to `192.168.0.162:8443` from the controller's netns
(`nsenter -t <pid> -n ss -tn state established dst 192.168.0.162:8443 | grep -c 192.168`):
- `TestBuildAppRecipe_NoSecrets` — emit a recipe for an app whose `Env` carries an `ENC:` value + a
token-shaped value; assert NONE of the values and NO credential-shaped key survive; assert the
allowlisted facts DID emit (non-vacuous).
- `TestBuildAppRecipe_AllowlistIsLoadBearing` — the companion/red-proof: a guard-removed shape leaks the
token; the production emitter does not.
- `TestAppStorageBindings` (+ `_NoHDD`) — pins the compose parse (roms + resources bindings; named volume
excluded; rootfs app → 0 bindings).
- `TestBuildDRRecipeAppHalf` — assemble-correctness (deployed + non-protected only) + whole-half secret
sweep.
- **Live red-proof:** forcing `buildAppRecipe` to dump `Env` made `TestBuildAppRecipe_NoSecrets` and the
companion FAIL (caught the token value + the `DB_PASSWORD`/`SECRET_KEY`/`IGDB_CLIENT_SECRET` keys);
reverted → green.
| stage | pre-fix image (stale build) | **fixed image (2e85376e)** |
|---|---|---|
| before | 8 | **1** |
| after 40 calls | 48 | **2** |
| after 80 calls | 90 | **2** |
| after 120 calls | 132 | **2** |
## Versioning
**Fixed: flat at 2 (= MaxIdleConnsPerHost), independent of call count.** Pre-fix grew ~1/call. Agent
endpoints still return real data (`/api/disks` lists disks, `/api/host-metrics` returns host CPU/mem).
`recipe_version=1`; read is ignore-unknown for forward-compat. The hub assembles this half with the
agent's storage/guest/PBS half (agent v0.38.0) into one customer recipe.
## Gate / deploy
`go build`, `go vet`, `go test ./...` all green (local + build server). Built + pushed image v0.73.0;
deployed to guest 9201 (bootstrap-managed).
## Deferred (NOT in this slice)
Free-form non-secret deploy fields (the SPIKE's `non_secret_deploy_fields`) — v1 is the three-field
allowlist only; additional fields land incrementally behind the same allowlist test. No re-provisioning
automation; no recovery-mode consumption.
## Not touched / separate open item
The agent, its bridge-IP `ListenAddr` bind (deliberate defense-in-depth), and all firewall rules were
**not** changed (controller-only fix). **Separate open item (NOT addressed here):** the defense-in-depth
host firewall rule scoping `:8443` to the guest bridge subnet is still absent (`pve-firewall` disabled,
no 8443 rule) — close it independently of this fix.
+6
View File
@@ -717,6 +717,12 @@ not just those with HDD data. Non-HDD apps can configure destination, method, an
> `EjectDisk`/`FormatDisk(…, confirmed, durableID)`; `DiskInfo.role`+capacity;
> `FormatResult.{role,needs_confirmation,durable_id}`; `ErrNeedsConfirmation` (user-data) vs
> `ErrFormatRefused` (system/backup). `FormatResult.PendingOp.OpsignCommand()` for the operator path.
> - **(v0.74.0) Client lifecycle — ONE shared client, reused.** `Server.agentClient()` builds the
> `agentapi.Client` once (memoized via `sync.Once`) and returns the same instance to all ~19 call
> sites; the `http.Transport` is bounded + expiring (`MaxIdleConnsPerHost:2`, `IdleConnTimeout:90s`).
> This replaced a per-call `agentapi.New(...)` that leaked one idle ESTABLISHED socket per call and
> exhausted the ephemeral source-port range to the agent's `:8443` after ~5 days (EADDRNOTAVAIL).
> Safe because `cfg.LocalAPI` is static per process (a config-apply triggers a graceful self-restart).
> - The **`StoragePath` registry** (`settings.go`: `AddStoragePath`/default/schedulable/label) is unchanged.
> - **(v0.64.0) `AutoDiscoverStoragePaths` is now ADDITIVE** — it no longer bails when the registry is
> non-empty; instead it registers only deployed-app paths missing from the registry. It never removes