diff --git a/CHANGELOG.md b/CHANGELOG.md index 0847c1c..1571f76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,34 @@ ## Changelog +### v0.71.0 — fix the guest-reboot boot-race that strands drive-backed apps (2026-06-16) + +A `pct reboot` of the guest left drive-backed apps (audiobookshelf, calibre-web, immich-server, +jellyfin, komga, radarr, romm, paperless-webserver) stuck `Exited` forever. **Sub-cause (diagnosed, +not guessed):** on guest boot the in-guest dockerd auto-starts the `unless-stopped` apps **~18s before** +the agent re-binds the drive under the stable parent; the create-time volume bind fails +(`mkdir /mnt/felhom-drives//userdata: permission denied` on the empty fail-closed placeholder) +and, being a create-time failure (`RestartCount=0`), is **never retried**. The existing recovery, +`processGuestBootChange`, *ran* but **raced the rebind**: it sampled the agent's `BoundUnderParent` +**once** during fast controller startup (bind not live yet) → recreated nothing → **persisted the new +boot-id**, burning its one-shot. The periodic drive-gate never recovered them either (its first +observation was *after* the rebind: present + not-disconnected → no transition). + +**Fix (harden the existing mechanism — no parallel one):** `processGuestBootChange` now gates on the +**REAL live in-guest bind** instead of the once-sampled agent view. New `driveBindLive` checks whether +`/mnt/felhom-drives/` is an actual mountpoint in the controller's own `/mnt` (rslave) +`/proc/self/mountinfo` — true only once the agent's bind has propagated, exactly when docker can +recreate the app. New `pollLiveBinds` waits for that (bounded ~120s, polling 2s; the rebind lands ~18s) +and only then recreates the deployed drive-backed apps via the normal pipeline (`compose down`→`up -d`). +`shouldRecreateOnBoot` is unchanged and state-independent, so a stuck-`Exited` create-time-failure app +is included. Single-flight (runs once before the periodic gate); apps on a drive that never goes live +in the window are left to the gate (drive-absent → stop→return→restart). The host-reboot path the +earlier sweep validated is unaffected (same code path, now strictly more robust — it waits for the +bind). The **guest-only reboot path** (which the host-reboot sweep never exercised) is now covered. + +Tests: `pollLiveBinds` waits through the rebind window then reports live (recreate fires); +never-live drive stays absent (no spurious recreate); plus an explicit pre-fix companion that a single +early sample misses the not-yet-live bind. Live-accepted with repeated `pct reboot 9201`. + ### v0.70.0 — config-apply self-restart + geo-restriction UX fixes (2026-06-16) Fixes found during live geo testing (rotating the Cloudflare API token). diff --git a/controller/README.md b/controller/README.md index af2240f..3f1e607 100644 --- a/controller/README.md +++ b/controller/README.md @@ -646,6 +646,16 @@ not just those with HDD data. Non-HDD apps can configure destination, method, an > - **Guest-reboot convergence is DETERMINISTIC** via the agent's `guest_boot_id`: the controller persists > `LastGuestBootID` and, when it changes, recreates EVERY deployed drive-backed app onto the > re-propagated drive (`processGuestBootChange` — no fragile container-uptime sampling). +> **v0.71.0 — boot-race fix:** on a guest `pct reboot`, in-guest dockerd auto-starts the apps ~18s +> BEFORE the agent re-binds the drive, so their volume bind fails at create-time +> (`mkdir …/userdata: permission denied`, `RestartCount=0` → never retried → stuck `Exited`). The old +> recovery sampled the agent's `BoundUnderParent` ONCE, raced that rebind, recreated nothing, and burned +> its boot-id one-shot. `processGuestBootChange` now **gates on the REAL live in-guest bind** +> (`driveBindLive`: is `/mnt/felhom-drives/` an actual mountpoint in the controller's own `/mnt` +> rslave `/proc/self/mountinfo`?) and **waits** for it (`pollLiveBinds`, bounded ~120s) before recreating +> — including apps stuck `Exited` with a create-time mount failure (`shouldRecreateOnBoot` is +> state-independent). The **guest-only reboot path** (which the host-reboot sweep never exercised) is now +> covered; drives that never go live in the window are left to the drive-absent gate. > > **⚠️ Rebuilt on the agent-delegated disk model (v0.43.0), made ROLE-AWARE in v0.44.0, UX-polished in > v0.45.0.** After the 8C diff --git a/controller/internal/web/intermediary.go b/controller/internal/web/intermediary.go index a7db504..9d34314 100644 --- a/controller/internal/web/intermediary.go +++ b/controller/internal/web/intermediary.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "os" "path" "strings" "time" @@ -12,6 +13,63 @@ import ( "gitea.dooplex.hu/admin/felhom-controller/internal/settings" ) +// bootBindWait / bootBindPoll bound the readiness gate in processGuestBootChange. On a guest reboot +// the agent re-binds each drive under the stable parent ~18s after boot (well after docker has tried +// and permanently failed to auto-start the drive-backed apps); the wait must comfortably cover that +// with margin, the poll be cheap. +const ( + bootBindWait = 120 * time.Second + bootBindPoll = 2 * time.Second +) + +// driveBindLive reports whether `root` (a /mnt/felhom-drives/ stable path) is a REAL live +// mountpoint in this process's own mount namespace — i.e. the agent's per-drive bind has actually +// propagated into the controller's `/mnt` (rslave) view, which is exactly the moment docker can +// (re)create an app whose volume binds under it. It reads /proc/self/mountinfo (pure Go, no syscall; +// the mount point is always field index 4). Before the agent re-binds the drive post-boot, the stable +// path is the empty fail-closed placeholder (a plain subdir of the parent, NOT its own mount) → false. +// This is the load-bearing signal the old code lacked: it sampled the agent's host-side +// BoundUnderParent ONCE during fast controller startup, racing the ~18s rebind. +func driveBindLive(root string) bool { + data, err := os.ReadFile("/proc/self/mountinfo") + if err != nil { + return false + } + for _, line := range strings.Split(string(data), "\n") { + f := strings.Fields(line) + if len(f) >= 5 && f[4] == root { + return true + } + } + return false +} + +// pollLiveBinds waits (bounded by `wait`, polling every `poll`) for each path's drive bind to go LIVE, +// returning the final liveness map. It breaks early once every path is live. bindLive/sleep/now are +// injected so the readiness gate is deterministically unit-testable. A path that never goes live +// within the window stays false (its apps are left to the normal drive gate). +func pollLiveBinds(paths []string, bindLive func(string) bool, sleep func(time.Duration), now func() time.Time, wait, poll time.Duration) map[string]bool { + live := make(map[string]bool, len(paths)) + deadline := now().Add(wait) + for { + all := true + for _, p := range paths { + if live[p] { + continue + } + if bindLive(p) { + live[p] = true + } else { + all = false + } + } + if all || !now().Before(deadline) { + return live + } + sleep(poll) + } +} + // Intermediary-mount model (controller side). Post-migration a drive is visible in the guest ONLY at its // STABLE path /mnt/felhom-drives/ (the host swaps the backing drive underneath it; see the agent's // internal/localapi/intermediary.go + SPIKE-intermediary-mount). So: @@ -243,14 +301,24 @@ func (s *Server) driveGateLoop() { } // processGuestBootChange converges a GUEST REBOOT DETERMINISTICALLY. On a guest reboot docker auto-starts -// the app containers (restart:unless-stopped) potentially BEFORE the agent re-propagates the drive under -// the parent — so they bind the empty fail-closed stable dir and (non-recursive parent bind + leaf-bind -// pinning) never pick up the later propagation. The agent reports a `guest_boot_id` that changes on every -// guest boot but is stable across a controller-only restart; the controller persists the last-seen value. -// When it changes (the controller restarts WITH the guest), this recreates every deployed drive-backed -// app whose drive is present + that docker brought back (`shouldRecreateOnBoot`) onto the populated path, -// then persists the new boot-id. Apps on a still-absent drive are handled by the normal gate -// (stop→return→restart). Replaces the old fragile container-uptime sample. Best-effort. +// the app containers (restart:unless-stopped) BEFORE the agent re-propagates the drive under the parent +// (~18s later) — so the create-time bind of an app's volume fails (`mkdir …/userdata: permission denied` +// on the empty fail-closed placeholder) and, being a create-time failure (RestartCount=0), is NEVER +// retried → the app is stuck Exited forever even after the bind lands. The agent reports a `guest_boot_id` +// that changes on every guest boot but is stable across a controller-only restart; the controller +// persists the last-seen value. When it changes (the controller restarts WITH the guest), this: +// 1. gathers the deployed drive-backed apps' stable drive paths, +// 2. GATES on the REAL live in-guest bind — `pollLiveBinds`/`driveBindLive` wait (bounded) until each +// drive's stable path is an actual mountpoint in the controller's own /mnt (rslave) view, which is +// exactly when docker can recreate the app. (The old code sampled the agent's BoundUnderParent +// ONCE during fast startup, raced the ~18s rebind, recreated nothing, and persisted the boot-id — +// burning its one-shot. That is the bug this fixes.) +// 3. recreates every deployed drive-backed app whose bind is now live (`shouldRecreateOnBoot` is +// state-independent, so a stuck-Exited create-time-failure app is included), then persists the +// new boot-id. +// +// Apps on a drive that never goes live within the window are left to the normal gate (stop→return→ +// restart). Single-flight (runs once, before the periodic gate, in driveGateLoop). Best-effort. func (s *Server) processGuestBootChange() { if s.settings == nil || s.stackMgr == nil { return @@ -268,26 +336,55 @@ func (s *Server) processGuestBootChange() { if resp.GuestBootID == "" || resp.GuestBootID == s.settings.GetLastGuestBootID() { return // no boot-id, or unchanged (controller-only restart) → no recreate } - presentStable := map[string]bool{} - for _, d := range resp.Disks { - if d.GuestPath != "" && d.BoundUnderParent { - presentStable[d.GuestPath] = true + + // Distinct stable drive paths that deployed drive-backed apps depend on (HDD_PATH is the drive root). + needed := map[string]bool{} + for _, st := range s.stackMgr.GetStacks() { + cfg := s.stackMgr.LoadAppConfigByName(st.Name) + if cfg == nil { + continue + } + hdd := cfg.Env["HDD_PATH"] + if cfg.Deployed && hdd != "" && strings.HasPrefix(hdd, StableParentDir+"/") { + needed[hdd] = true } } + if len(needed) == 0 { + if serr := s.settings.SetLastGuestBootID(resp.GuestBootID); serr != nil { + s.logger.Printf("[WARN] [gate] persist boot-id: %v", serr) + } + return + } + paths := make([]string, 0, len(needed)) + for p := range needed { + paths = append(paths, p) + } + + // READINESS GATE (the fix): wait for the REAL live in-guest bind, not the once-sampled agent view. + s.logger.Printf("[INFO] [gate] boot %s: waiting (≤%s) for live drive bind(s) %v before recreating drive-backed apps", resp.GuestBootID, bootBindWait, paths) + presentStable := pollLiveBinds(paths, driveBindLive, time.Sleep, time.Now, bootBindWait, bootBindPoll) + + skipped := 0 for _, st := range s.stackMgr.GetStacks() { cfg := s.stackMgr.LoadAppConfigByName(st.Name) if cfg == nil { continue } if !shouldRecreateOnBoot(cfg.Deployed, cfg.Env["HDD_PATH"], presentStable) { + if cfg.Deployed && strings.HasPrefix(cfg.Env["HDD_PATH"], StableParentDir+"/") { + skipped++ // a deployed drive-backed app whose bind never went live → gate's job + } continue } - s.logger.Printf("[INFO] [gate] boot %s: recreating drive-backed app %s (state=%s) onto %s", resp.GuestBootID, st.Name, st.State, cfg.Env["HDD_PATH"]) + s.logger.Printf("[INFO] [gate] boot %s: live bind confirmed — recreating drive-backed app %s (state=%s) onto %s", resp.GuestBootID, st.Name, st.State, cfg.Env["HDD_PATH"]) _ = s.stackMgr.StopStack(st.Name) if serr := s.stackMgr.StartStack(st.Name); serr != nil { s.logger.Printf("[WARN] [gate] boot recreate %s: %v", st.Name, serr) } } + if skipped > 0 { + s.logger.Printf("[WARN] [gate] boot %s: %d drive-backed app(s) had no live bind within %s — leaving to the drive gate", resp.GuestBootID, skipped, bootBindWait) + } if serr := s.settings.SetLastGuestBootID(resp.GuestBootID); serr != nil { s.logger.Printf("[WARN] [gate] persist boot-id: %v", serr) } diff --git a/controller/internal/web/intermediary_test.go b/controller/internal/web/intermediary_test.go index 71a090e..83d1dd8 100644 --- a/controller/internal/web/intermediary_test.go +++ b/controller/internal/web/intermediary_test.go @@ -2,6 +2,7 @@ package web import ( "testing" + "time" "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" @@ -75,6 +76,69 @@ func TestDefaultPromotionTarget(t *testing.T) { } } +// TestPollLiveBinds_WaitsForLateBind is the boot-race fix's core: the readiness gate must WAIT for the +// drive bind to actually go live (the agent re-binds it ~18s post-boot) and only THEN report it present, +// so shouldRecreateOnBoot fires and the create-time-failed app is recreated. +// +// COMPANION (must fail pre-fix): the old readiness was a SINGLE early sample of the agent's +// BoundUnderParent — taken before the ~18s rebind, so it read the bind ABSENT, recreated nothing, and +// burned the boot-id one-shot. TestSingleEarlySample_MissesLateBind_Companion pins that broken outcome; +// and if pollLiveBinds is reverted to a no-wait single check, this test fails its "did it wait" assertion. +func TestPollLiveBinds_WaitsForLateBind(t *testing.T) { + flash := "/mnt/felhom-drives/felhom-flash" + var nowT time.Duration // fake monotonic clock since boot + now := func() time.Time { return time.Unix(0, 0).Add(nowT) } + sleep := func(d time.Duration) { nowT += d } + const goLive = 18 * time.Second // the real agent rebind lag + bindLive := func(string) bool { return nowT >= goLive } + + live := pollLiveBinds([]string{flash}, bindLive, sleep, now, bootBindWait, bootBindPoll) + + if !live[flash] { + t.Fatalf("poll must detect the bind once live; got %v", live) + } + if nowT < goLive { // proves it WAITED through the rebind window (a single sample would return at t=0) + t.Fatalf("poll returned at t=%s before the bind went live at %s — it did not wait (regression)", nowT, goLive) + } + if !shouldRecreateOnBoot(true, flash, live) { + t.Fatalf("with the live bind present, the drive-backed app MUST be recreated") + } +} + +// TestSingleEarlySample_MissesLateBind_Companion is the explicit pre-fix companion: a single sample of +// the bind at boot (t=0), before the ~18s rebind, reads it ABSENT → the app is NOT recreated and stays +// Exited. This is exactly what stranded paperless-webserver et al. live. +func TestSingleEarlySample_MissesLateBind_Companion(t *testing.T) { + flash := "/mnt/felhom-drives/felhom-flash" + bindLiveAtBoot := func(string) bool { return false } // not yet live at the boot instant + oldPresent := map[string]bool{flash: bindLiveAtBoot(flash)} + if shouldRecreateOnBoot(true, flash, oldPresent) { + t.Fatalf("companion: a single early sample reads the not-yet-live bind as absent and must MISS it") + } +} + +// TestPollLiveBinds_TimeoutLeavesAbsent: a drive that never comes live within the window stays absent, +// so its apps are NOT recreated here (left to the normal drive gate) — no spurious recreate/loop. +func TestPollLiveBinds_TimeoutLeavesAbsent(t *testing.T) { + usb := "/mnt/felhom-drives/felhom-usb" + var nowT time.Duration + now := func() time.Time { return time.Unix(0, 0).Add(nowT) } + sleep := func(d time.Duration) { nowT += d } + bindLive := func(string) bool { return false } // never live + + live := pollLiveBinds([]string{usb}, bindLive, sleep, now, bootBindWait, bootBindPoll) + + if live[usb] { + t.Fatalf("a never-live bind must remain absent after the bounded wait") + } + if nowT < bootBindWait { + t.Fatalf("poll must run to the deadline for an absent drive, t=%s", nowT) + } + if shouldRecreateOnBoot(true, usb, live) { + t.Fatalf("an absent drive's app must NOT be recreated here (the gate owns drive-absent)") + } +} + func TestStablePathForName(t *testing.T) { if got := stablePathForName("felhom-usb"); got != "/mnt/felhom-drives/felhom-usb" { t.Errorf("stablePathForName = %q", got)