From 8b4b9011ddce52b4be124a12836169c4fd2aeb33 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 22 Jun 2026 20:55:08 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20v0.75.0=20REPORT/CONTEXT=20=E2=80=94=20?= =?UTF-8?q?userdata=20mountpoint=20gate=20+=20boot-ordering=20design=20not?= =?UTF-8?q?e=20+=20live=20C6=20proof?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 22 +++++++++++++ REPORT.md | 93 ++++++++++++++++++++++++------------------------------ 2 files changed, 64 insertions(+), 51 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 171a2db..a5ff93f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,6 +7,28 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" +Last updated: 2026-06-22 (v0.75.0 — gate userdata MkdirAll on a live mountpoint) + +> **2026-06-22 — v0.75.0 (deployed on 9201): userdata MkdirAll gated on a live mountpoint.** +> Both `MkdirAll`-into-`/userdata` sites — the deploy belt (`stacks.ensureUserdataMounts`) and +> the FileBrowser sync (`web.syncFileBrowserMounts`) — now skip when an **external** drive root (under +> `StableParentDir=/mnt/felhom-drives`, `!= sysDataPath`) is **not a live mountpoint** (`system.IsMountPoint`). +> Closes the campaign-#2 hazard where a drive-absent window produced `mkdir …/userdata: permission denied` +> + transient `Created` flapping AND could write app data onto the guest **rootfs** (shadowed when the +> drive returns). The app is held by `planDriveGates` instead. System/local path is never gated. New +> `Manager.isMountPoint` seam + pure `web.skipFileBrowserPath` helper for tests. Live-proven via a C6 +> re-run: 0 permission-denied, 0 shadow dirs on the rootfs, apps recover on reconnect. +> +> **Boot-ordering design note (NOT implemented — decide separately):** the *boot-time* `mkdir … +> permission denied` is a different cause — **docker's** boot-restore auto-starts drive-backed +> containers (`restart: unless-stopped`) before the agent mounts the drives, so docker (not the belt) +> tries to create the bind source; `planDriveGates` recovers them after mount convergence (why a reboot +> ends healthy). Options: (A) accept + suppress (apps self-heal; lowest risk; maybe downgrade the +> boot-window log level) — recommended now; (B) drive-backed app containers don't docker-auto-start at +> boot (restart policy `no`/`on-failure`) so the controller's gate is the sole starter after mounts +> converge — cleaner but bigger (must cover crash-restart too). See +> `felhom.eu/documentation/tests/test-campaign-2-finding1-recovery-diagnosis.md` (boot-ordering observation). + 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.** diff --git a/REPORT.md b/REPORT.md index 49a63aa..e160bec 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,64 +1,55 @@ -# REPORT — controller v0.74.0: fix the controller→agent connection leak +# REPORT — controller v0.75.0: gate userdata MkdirAll on a live mountpoint ## Baseline → target -`felhom-controller` `main` v0.73.0 → **v0.74.0**. Controller-only change; agent/hub/firewall untouched. +`felhom-controller` `main` v0.74.0 → **v0.75.0**. Controller-only; agent/hub untouched. ## 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`. +Two `MkdirAll`-into-`/userdata` sites fired without checking the drive was mounted (campaign-#2 +findings #2/#3): the deploy belt (`ensureUserdataMounts`) and the FileBrowser sync. During a +drive-absent window they logged `mkdir …/userdata: permission denied` + caused transient `Created` +container flapping — and worse, writing into an unmounted mountpoint lands app data on the guest +**rootfs**, shadowed when the drive returns (data-integrity + rootfs-fill hazard). -## 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. +## Change (commit `0b2605c`) +- `internal/stacks/manager.go` — `ensureUserdataMounts` skips when the `HDD_PATH` drive root is an + **external** path (`!= sysDataPath`) that is **not a live mountpoint**; the app is held by + `planDriveGates`. New injectable `Manager.isMountPoint` seam (defaults to `system.IsMountPoint`). + The system/local path is never gated. +- `internal/web/handlers.go` — the FileBrowser sync loop skips (and doesn't mount) a path under + `StableParentDir` that isn't a live mountpoint, via the new pure `skipFileBrowserPath` helper. +- `EnsureUserdataDir`/`EnsureUserdataSkeleton`/`planDriveGates` unchanged (gated the callers). -## 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). +## Tests (green; red-proofed) +`go build ./... && go vet ./... && go test ./...` all green. +- T1 `TestEnsureUserdataMounts_SkipsAbsentExternalDrive` — belt skips when external drive absent. + Red-proof: removing the gate → dir created → FAIL (shown, reverted). +- T2 `TestEnsureUserdataMounts_EnsuresWhenMounted` — belt ensures when drive mounted. +- T3 `TestEnsureUserdataMounts_SystemPathNeverSkipped` — system/local path never gated (must-not-over-gate). +- T4 `TestSkipFileBrowserPath` — only an absent external path is skipped; mounted + system paths kept. + Red-proof: gate→false → absent `usb` not skipped → FAIL (shown, reverted). ## 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. +Built+pushed `gitea.dooplex.hu/admin/felhom-controller:0.75.0` on 192.168.0.180 (digest +`sha256:f735676e…`; **build-server `git pull` done first** — the v0.74.0 stale-source gotcha), +deployed to guest 9201 via the bootstrap path. `docker inspect`: `image=:0.75.0 running healthy`. -## 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 -n ss -tn state established dst 192.168.0.162:8443 | grep -c 192.168`): +## LIVE acceptance — campaign-#2 C6 re-run, now clean +Disconnected felhom-flash (8 apps) and observed the drive-absent window: -| stage | pre-fix image (stale build) | **fixed image (2e85376e)** | +| Check | campaign #2 (pre-fix) | **v0.75.0 (fixed)** | |---|---|---| -| before | 8 | **1** | -| after 40 calls | 48 | **2** | -| after 80 calls | 90 | **2** | -| after 120 calls | 132 | **2** | +| `mkdir …/userdata: permission denied` | many | **0** | +| "could not ensure userdata skeleton" | present | **0** | +| rootfs shadow dir at unmounted mountpoint | created | **none** — `/mnt/felhom-drives/felhom-flash/userdata` does not exist (bare mountpoint) | +| apps during absent window | transient `Created` flapping | held cleanly (down) | +| reconnect | recovered | **recovered: flash remounted /dev/sdc1, 8 apps restarted → 25 running / 0 unhealthy** | -**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). +The integrity hazard (app data shadowed onto the rootfs) is closed: the now-unmounted mountpoint is +empty, no `userdata/...` was written to the rootfs. -## 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. +## Not touched / design note (see CONTEXT.md) +The **boot-time** `mkdir … permission denied` (daemon boot-restore) is a different cause — **docker** +auto-starts drive-backed containers (`restart: unless-stopped`) before the agent mounts the drives, so +docker (not the belt) tries to create the bind source; `planDriveGates` recovers them after mount +convergence. This task ships only the runtime gate; the boot-ordering decision is documented, not +implemented.