docs: REPORT.md for controller v0.71.0 guest-reboot recovery fix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,73 +1,82 @@
|
||||
# REPORT — controller v0.70.0: config-apply self-restart + geo-restriction UX fixes (2026-06-16)
|
||||
# REPORT — controller v0.71.0: fix guest-reboot recovery of drive-backed apps (2026-06-16)
|
||||
|
||||
**Deployed:** controller **v0.70.0** on guest 9201 / felhom-pve (bootstrap-managed; healthy, hub-reporting OK).
|
||||
**Scope:** controller-only. Fixes found during live geo testing (rotating the Cloudflare API token).
|
||||
**Commits (trunk, per-commit green gate):** A `ba87412`, B `02a4ba0`, C `09d75c1`, docs `c514c00`, live-fix `9b7a265`.
|
||||
**Deployed:** controller **v0.71.0** on guest 9201 / felhom-pve (bootstrap-managed; healthy).
|
||||
**Commits (trunk):** `25e5cb5` (boot-race fix, first cut) → `e2de234` (full fix: + agent-path blocker + periodic retry + docs/tests).
|
||||
**Live-accepted:** two `pct reboot 9201` cycles, all 8 drive-backed apps recovered automatically, zero manual starts.
|
||||
|
||||
## What shipped
|
||||
## Phase A — diagnosis (pinned live, not guessed): THREE sub-causes
|
||||
|
||||
1. **Config-apply self-restart (core fix).** `POST /api/config/apply` previously wrote the new
|
||||
`controller.yaml` but logged "restart needed" and left stale in-process singletons — the Cloudflare
|
||||
client is built once at startup, so a rotated CF token kept 403'ing until a manual LXC restart. Now:
|
||||
byte-identical re-push → no-op (no write, no restart); changed config → write, respond 200 (flushed),
|
||||
then **graceful self-restart** (`os.Exit(0)` after ~500 ms; container is `restart: unless-stopped`).
|
||||
Exit is behind an injectable `Restarter` seam. Removed the stale wording and the dead `OnConfigApplied`
|
||||
hook (Phase-1-retired infra-backup push).
|
||||
2. **Manual "Vezérlő újraindítása" button** → `POST /api/selfrestart` (auth + CSRF via `/api/` mount),
|
||||
same helper; confirm → POST → poll `GET /` every 2 s → reload.
|
||||
3. **Immediate hub report push on geo change** (`Router.reportPushNow`, wired in `main.go`): a successful
|
||||
geo save and a successful manual geo sync fire a non-blocking report push so the hub reflects the new
|
||||
state within seconds instead of after the ~15-min cycle. Scoped to geo handlers.
|
||||
4. **Always report `geo_restriction`** (`buildGeoRestrictionReport`): always populated (Enabled=false,
|
||||
empty countries when never configured) so the hub always renders the section ("Inaktív" when off).
|
||||
5. **Country autocomplete fixed.** See root cause below.
|
||||
A guest `pct reboot` strands drive-backed apps because in-guest dockerd auto-starts the `unless-stopped`
|
||||
apps ~18s BEFORE the agent re-binds the drive, so the create-time volume bind fails
|
||||
(`mkdir /mnt/felhom-drives/<drive>/userdata: permission denied`) and `RestartCount=0` means it is never
|
||||
retried → stuck `Exited`. The intended recovery (`processGuestBootChange`) did **not** fire. Live repro
|
||||
(`paperless-webserver`: `exit=128, RestartCount=0, State.Error=mkdir…permission denied`) was the fixture.
|
||||
Diagnosis pinned three distinct sub-causes:
|
||||
|
||||
## Country autocomplete — root cause (diagnosed live, NOT the hypothesised JS throw)
|
||||
1. **AGENT-PATH BLOCKER (the live root cause).** `/api/disks` → `{"error":"agent not configured"}`:
|
||||
`agentClient()` requires `cfg.LocalAPI.Endpoint`, which was **empty**, so `processGuestBootChange`
|
||||
(and the **entire** drive gate) returned at its first guard — never reaching any boot-id/bind logic.
|
||||
The authoritative `bootstrap.json` *had* a complete `local_api` block (`endpoint/fingerprint/token`),
|
||||
but `bootstrap.MaybeIngest` (bootstrap.go:101) returned immediately on "already configured"
|
||||
(`cfg.Customer.ID != ""`), so a controller.yaml seeded before `local_api` existed never got it merged.
|
||||
Evidence: `LastGuestBootID` was stuck at the guest's first-boot value across every reboot (it was never
|
||||
updated because the function bailed before reaching the persist).
|
||||
2. **BOOT-RACE READINESS GATE.** `processGuestBootChange` sampled the agent's `BoundUnderParent` **once**
|
||||
during fast startup (racing the ~18s rebind), recreated nothing, and **persisted the new boot-id** —
|
||||
burning its one-shot. (Confirmed by manually wiring `local_api`: the *old* sample-once would still have
|
||||
missed; the new poll caught it.)
|
||||
3. **SINGLE-SHOT FRAGILITY.** `processGuestBootChange` ran only once at startup; right after a guest reboot
|
||||
the agent's local API can be briefly unreachable/stale, so the single attempt bailed with no retry.
|
||||
|
||||
`filterCountries` runs without error and **correctly populates** the list, but reveals it with
|
||||
`list.style.display = ''`. The `.geo-country-list` CSS default is `display:none` (style.css:3084), so
|
||||
clearing the *inline* style falls back to the stylesheet and the populated list stays hidden — **no
|
||||
console error, just an invisible dropdown**. Proven live via `javascript_tool`: calling
|
||||
`filterCountries('Német')` set `innerHTML` to the "Németország (DE)" option but computed `display`
|
||||
stayed `none`; forcing `display:block` revealed it. Latent since the geo feature's first commit
|
||||
(`e1fb852`), not a recent regression; the operator's "1 console error" was unrelated (the stray
|
||||
DevTools filter noted in the task). **Fix:** reveal with `'block'`.
|
||||
(The periodic drive-gate never recovered them either: its first observation was *after* the rebind →
|
||||
present + not-disconnected → no transition; settings showed `felhom-flash disconnected=None`.)
|
||||
|
||||
## Tests (non-hollow, per-commit green gate; companions proven red pre-fix)
|
||||
## Phase B — fix (harden the existing mechanism, no parallel one)
|
||||
|
||||
- A: `selfrestart_test.go` — changed config → restart once; **identical → not called (companion, red
|
||||
without the no-op guard)**; invalid YAML → not called; `/api/selfrestart` → restart once.
|
||||
- B: `geo_test.go` — geo save success → push once; **invalid country → no push (companion)**.
|
||||
`geo_report_test.go` — `buildGeoRestrictionReport(nil)` → non-nil disabled (**companion vs old
|
||||
nil-omit**); passthrough; disabled-nil-countries.
|
||||
- C: template/CSS visibility fix — verified live (no Go unit test applies).
|
||||
1. **`ensureLocalAPI`** (`internal/bootstrap/bootstrap.go`): `MaybeIngest` now calls it on the
|
||||
already-configured path — when `cfg.LocalAPI.Endpoint` is empty it merges `local_api` from
|
||||
bootstrap.json into the existing controller.yaml in place (no hub re-pull, existing config preserved),
|
||||
idempotent + fail-safe.
|
||||
2. **`driveBindLive` + `pollLiveBinds`** (`internal/web/intermediary.go`): `processGuestBootChange` now
|
||||
gates on the **real live in-guest bind** — `driveBindLive` checks whether `/mnt/felhom-drives/<drive>`
|
||||
is an actual mountpoint in the controller's own `/mnt` (rslave) `/proc/self/mountinfo` (true only once
|
||||
the agent's bind propagated, exactly when docker can recreate the app); `pollLiveBinds` waits for it
|
||||
(bounded ~120s, poll 2s) before recreating via the normal pipeline (`compose down`→`up -d`).
|
||||
`shouldRecreateOnBoot` is unchanged and state-independent → stuck-`Exited` create-time-failure apps are
|
||||
included. Drives that never go live in the window are left to the gate.
|
||||
3. **Periodic retry** (`driveGateLoop`): `processGuestBootChange` now runs on every periodic tick too —
|
||||
idempotent (boot-id gated) — so a momentarily-unreachable agent right after a reboot no longer
|
||||
permanently strands recovery.
|
||||
|
||||
## Live verification on guest 9201 (all 5 pass)
|
||||
The host-reboot path the earlier sweep validated is unaffected (same code path, strictly more robust); the
|
||||
**guest-only reboot path** (never exercised by host-reboot sweeps) is now covered.
|
||||
|
||||
- **(a) config-apply auto-restart:** no-op re-push → `"…változatlan…"`, container `StartedAt` unchanged;
|
||||
changed config → `"…a vezérlő újraindul."` + `StartedAt` advanced (11:04:18 → 11:08:50) + healthy — **no
|
||||
manual step**.
|
||||
- **(b) restart button:** renders (`btn-restart-controller` in rendered HTML); `POST /api/selfrestart`
|
||||
→ `"Újraindítás folyamatban…"` + restart (11:14:19 → 11:14:50) + healthy. (Full button click not
|
||||
automated — its `confirm()` dialog blocks the browser extension; the poll-reload JS is code-reviewed.)
|
||||
- **(c) geo save → hub within seconds:** save → controller logs "Hub report pushed successfully" 1 s
|
||||
later; hub's latest demo report `received_at = 11:15:52` (the save time) with
|
||||
`geo_restriction = {"enabled":true,"allowed_countries":["HU"]}`.
|
||||
- **(d) hub "Inaktív" when off:** geo disabled → report carries `geo_restriction = {"enabled":false,…}`
|
||||
(present-but-disabled = the data the hub renders as "Inaktív"; pre-fix this field was omitted and the
|
||||
section hidden). Demo re-enabled (HU) afterwards.
|
||||
- **(e) country dropdown:** typing "Német" lists **Németország (DE)** on the deployed 0.70.0 (the
|
||||
`'block'` fix; confirmed the deployed JS uses `'block'` and the list becomes visible).
|
||||
## Phase C — tests (non-hollow, pre-fix companions, red-proofed)
|
||||
|
||||
## Bug found + fixed during verification
|
||||
- `internal/web/intermediary_test.go`: `pollLiveBinds` waits through the rebind window then reports live
|
||||
(recreate fires); a never-live drive stays absent (no spurious recreate); an explicit companion that a
|
||||
**single early sample misses** the not-yet-live bind. Red-proofed against a no-wait single-sample.
|
||||
- `internal/bootstrap/bootstrap_test.go`: `ensureLocalAPI` merges `local_api` into an already-configured
|
||||
controller.yaml that lacks it (companion: pre-fix `MaybeIngest` left `LocalAPI.Endpoint` empty —
|
||||
red-proofed) and no-ops when already present. Full controller suite green; `go vet` clean.
|
||||
|
||||
The restart button was initially placed inside the `{{if .RetrievalPassword}}` block, so it was hidden
|
||||
on the demo (no retrieval password set). Moved outside the conditional (commit `9b7a265`), rebuilt +
|
||||
redeployed 0.70.0, re-verified it renders.
|
||||
## Phase D — live acceptance (the real gate)
|
||||
|
||||
## Notes / cleanup
|
||||
Built + deployed `felhom-controller:0.71.0` to guest 9201 (the redeploy itself validated `ensureLocalAPI`:
|
||||
the container recreate reset controller.yaml, the code re-merged `local_api`, `/api/disks` → 200). Then
|
||||
**two** `pct reboot 9201` cycles (zero manual intervention):
|
||||
|
||||
- All temp files removed from the build host (incl. a copied `controller.yaml` that held plaintext
|
||||
secrets), guest, and local; the demo's `controller.yaml` was restored to its clean pre-test content.
|
||||
- Out of scope (untouched): the Komga healthcheck; extending the immediate-report-push pattern beyond
|
||||
geo handlers.
|
||||
| reboot | boot-id | gate log | result |
|
||||
|---|---|---|---|
|
||||
| #1 | `…7348791` | "waiting (≤2m0s) for live drive bind(s) … → live bind confirmed — recreating" ×8 | all 8 `Up` |
|
||||
| #2 | `…7367438` | same full sequence ×8 | all 8 `Up` |
|
||||
|
||||
Both recovered audiobookshelf, calibre-web, immich-server, jellyfin, komga, radarr, romm,
|
||||
paperless-webserver automatically. (komga reports its container healthcheck "unhealthy" but is up and
|
||||
serving — a pre-existing, unrelated issue.) The boot-id now advances correctly on each reboot.
|
||||
|
||||
## Notes
|
||||
|
||||
- The Komga healthcheck quirk is pre-existing and out of scope.
|
||||
- The `local_api`-merge gap likely affected the whole drive gate on this golden, not just boot recovery —
|
||||
the `ensureLocalAPI` fix restores the agent path generally.
|
||||
|
||||
Reference in New Issue
Block a user