diff --git a/CHANGELOG.md b/CHANGELOG.md index 1571f76..20e85d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,33 +1,46 @@ ## Changelog -### v0.71.0 — fix the guest-reboot boot-race that strands drive-backed apps (2026-06-16) +### v0.71.0 — fix guest-reboot recovery of drive-backed apps (boot-race + the agent-path blocker) (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). +jellyfin, komga, radarr, romm, paperless-webserver) stuck `Exited` forever. 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 intended recovery (`processGuestBootChange`) did not fire. +Live diagnosis pinned **three** sub-causes, fixed together (harden the existing mechanism — no parallel +one): -**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. +1. **The agent-path blocker (the live root cause).** `agentClient()` returned **"agent not configured"** + — `cfg.LocalAPI.Endpoint` was empty — so `processGuestBootChange` (and the **entire** drive gate) + bailed at its first guard, never reaching any boot-id/bind logic. `bootstrap.json` *had* a complete + `local_api` block, but `MaybeIngest` returned immediately on "already configured" (customer.id set), + so a controller.yaml seeded before `local_api` existed never got the agent path merged. **Fix:** + `MaybeIngest` now calls new **`ensureLocalAPI`** on the already-configured path — it merges `local_api` + from bootstrap.json into the existing controller.yaml when missing (no hub re-pull, config preserved), + idempotent + fail-safe. +2. **The boot-race readiness gate.** `processGuestBootChange` sampled the agent's `BoundUnderParent` + **once** during fast startup, racing the ~18s rebind, recreated nothing, and burned its boot-id + one-shot. **Fix:** it now gates on the **REAL live in-guest bind** — 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 propagated, exactly when docker can recreate + the app), and new `pollLiveBinds` **waits** for it (bounded ~120s, poll 2s) before recreating via the + normal pipeline (`compose down`→`up -d`). `shouldRecreateOnBoot` is unchanged and state-independent, + so a stuck-`Exited` create-time-failure app is included. +3. **The 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 one attempt bailed and never + retried. **Fix:** `driveGateLoop` now runs it on every periodic tick too — idempotent (boot-id + gated), so it retries until the agent is reachable. -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`. +Apps on a drive that never goes live in the window are left to the normal gate. 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. + +Tests (non-hollow, with pre-fix companions, red-proofed): `pollLiveBinds` waits through the rebind then +reports live (recreate fires) / never-live stays absent / a single early sample misses the not-yet-live +bind; `ensureLocalAPI` merges `local_api` into an already-configured controller.yaml that lacks it +(companion: pre-fix MaybeIngest left it empty) and no-ops when already present. Live-accepted with +repeated `pct reboot 9201`. ### v0.70.0 — config-apply self-restart + geo-restriction UX fixes (2026-06-16) diff --git a/controller/README.md b/controller/README.md index 3f1e607..d486b80 100644 --- a/controller/README.md +++ b/controller/README.md @@ -655,7 +655,14 @@ not just those with HDD data. Non-HDD apps can configure destination, method, an > 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. +> covered; drives that never go live in the window are left to the drive-absent gate. `processGuestBootChange` +> also runs on every periodic `driveGateLoop` tick now (idempotent, boot-id gated) so a momentarily-unreachable +> agent right after a guest reboot no longer permanently strands recovery. +> **Agent-path prerequisite (also v0.71.0):** the whole drive gate needs `cfg.LocalAPI.Endpoint` (the +> per-guest agent local API). `bootstrap.MaybeIngest` now calls `ensureLocalAPI` on the already-configured +> path — merging `local_api` from `bootstrap.json` into an existing controller.yaml that lacks it (seeded +> before `local_api` existed) — because without it `agentClient()` returns "agent not configured" and the +> entire gate + boot recovery silently die. > > **⚠️ 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/bootstrap/bootstrap.go b/controller/internal/bootstrap/bootstrap.go index fc08062..7af095a 100644 --- a/controller/internal/bootstrap/bootstrap.go +++ b/controller/internal/bootstrap/bootstrap.go @@ -99,7 +99,12 @@ func Path() string { // - On success: writes controller.yaml (0600, atomic), reloads it, and returns the reloaded cfg. func MaybeIngest(configPath string, cfg *config.Config, logger *log.Logger, pull PullFunc) *config.Config { if cfg != nil && cfg.Customer.ID != "" { - return cfg // already configured — do not clobber, do not pull (idempotent) + // Already configured — do NOT re-pull the hub config or clobber controller.yaml. But STILL + // ensure the per-guest local_api block is present: a controller.yaml that was seeded/pulled + // before local_api existed (or by the setup wizard) is "configured" yet has no agent path, so + // agentClient() returns "agent not configured" and the ENTIRE drive gate + guest-reboot + // recovery silently die. ensureLocalAPI merges it in from bootstrap.json if missing. + return ensureLocalAPI(configPath, cfg, logger) } bpath := Path() data, err := os.ReadFile(bpath) @@ -184,6 +189,50 @@ func pullWithRetry(pull PullFunc, hubURL, customerID, password string, logger *l return "", lastErr } +// ensureLocalAPI handles an ALREADY-configured controller whose controller.yaml lacks the per-guest +// local_api block (seeded/pulled before local_api existed, or set up via the wizard). Without it the +// controller cannot reach the host agent at all (agentClient → "agent not configured"), which silently +// kills the whole drive gate + guest-reboot recovery. If bootstrap.json carries a complete local_api +// block, this merges it into the existing controller.yaml in place and reloads. Idempotent + fail-safe: +// returns cfg unchanged when local_api is already present, the bootstrap is absent/incomplete, or any +// step fails (it must never brick a configured guest). +func ensureLocalAPI(configPath string, cfg *config.Config, logger *log.Logger) *config.Config { + if cfg == nil || cfg.LocalAPI.Endpoint != "" { + return cfg // already has the agent path → nothing to do + } + data, err := os.ReadFile(Path()) + if err != nil { + return cfg // no bootstrap → nothing to merge (legacy/manually-configured guest) + } + var b Bootstrap + if err := json.Unmarshal(data, &b); err != nil { + return cfg + } + if b.LocalAPI.Endpoint == "" || b.LocalAPI.Fingerprint == "" || b.LocalAPI.Token == "" { + return cfg // bootstrap has no usable local_api to merge + } + current, err := os.ReadFile(configPath) + if err != nil { + return cfg + } + merged, err := mergeLocalAPI(string(current), b.LocalAPI) + if err != nil { + logger.Printf("[WARN] bootstrap: merging local_api into existing config failed: %v — agent path stays unconfigured", err) + return cfg + } + if err := writeFileAtomic(configPath, merged); err != nil { + logger.Printf("[WARN] bootstrap: could not write %s with local_api: %v", configPath, err) + return cfg + } + reloaded, err := config.LoadPermissive(configPath) + if err != nil { + logger.Printf("[WARN] bootstrap: wrote local_api but reload failed: %v", err) + return cfg + } + logger.Printf("[INFO] bootstrap: existing config was missing local_api — merged from %s (%s); agent path now configured", Path(), b.LocalAPI.Endpoint) + return reloaded +} + // mergeLocalAPI parses the pulled controller.yaml as a generic map, sets the local_api block from the // bootstrap (overwriting any hub-emitted placeholder), and re-marshals. local_api.enabled is NOT set // — it defaults on once endpoint is present (config.LocalAPIConfig). diff --git a/controller/internal/bootstrap/bootstrap_test.go b/controller/internal/bootstrap/bootstrap_test.go index 64a3499..4dd77fb 100644 --- a/controller/internal/bootstrap/bootstrap_test.go +++ b/controller/internal/bootstrap/bootstrap_test.go @@ -137,6 +137,77 @@ func TestMaybeIngest_DoesNotClobberConfigured_NoPull(t *testing.T) { } } +// FIX (v0.71.0): an ALREADY-configured controller.yaml that LACKS local_api still gets the per-guest +// agent path merged from bootstrap.json — without it agentClient() returns "agent not configured" and +// the whole drive gate + guest-reboot recovery silently die. COMPANION: the pre-fix MaybeIngest +// returned immediately on "already configured", so LocalAPI.Endpoint stayed empty (this test fails +// against that). The hub is NEVER re-pulled (the existing config is preserved verbatim). +func TestMaybeIngest_ConfiguredMissingLocalAPI_Merges(t *testing.T) { + dir := t.TempDir() + _, cfgPath := writeBootstrap(t, dir, goodBootstrapV2) // bootstrap HAS local_api + + const configuredNoLocalAPI = `customer: + id: cust-8200 + domain: cust8200.felhom.eu +hub: + enabled: true + url: https://hub.felhom.eu + api_key: CUSTKEY +` + if err := os.WriteFile(cfgPath, []byte(configuredNoLocalAPI), 0o600); err != nil { + t.Fatal(err) + } + existing, err := config.LoadPermissive(cfgPath) + if err != nil { + t.Fatal(err) + } + if existing.LocalAPI.Endpoint != "" { + t.Fatalf("precondition: existing config should lack local_api, got %q", existing.LocalAPI.Endpoint) + } + + pulled := false + pull := func(string, string, string) (string, error) { pulled = true; return hubYAML, nil } + + got := MaybeIngest(cfgPath, existing, testLogger(), pull) + + if pulled { + t.Fatal("must NOT re-pull the hub config for an already-configured controller") + } + if got.LocalAPI.Endpoint != "192.168.0.162:8443" || got.LocalAPI.Token != "PERGUESTTOKEN" || got.LocalAPI.Fingerprint != "ab12" { + t.Fatalf("local_api not merged into the already-configured controller (the live boot-recovery blocker): %+v", got.LocalAPI) + } + if got.Customer.ID != "cust-8200" || got.Hub.APIKey != "CUSTKEY" { + t.Fatalf("merging local_api must preserve the existing config: %+v / %+v", got.Customer, got.Hub) + } + raw, _ := os.ReadFile(cfgPath) + if !strings.Contains(string(raw), "192.168.0.162:8443") || !strings.Contains(string(raw), "CUSTKEY") { + t.Fatalf("controller.yaml must persist local_api + keep the customer key:\n%s", raw) + } +} + +// IDEMPOTENT: a configured controller that ALREADY has local_api is untouched (no re-merge, no pull, +// no rewrite). +func TestMaybeIngest_ConfiguredWithLocalAPI_NoOp(t *testing.T) { + dir := t.TempDir() + _, cfgPath := writeBootstrap(t, dir, goodBootstrapV2) + existing := config.Default() + existing.Customer.ID = "cust-8200" + existing.LocalAPI.Endpoint = "already:9999" + pulled := false + pull := func(string, string, string) (string, error) { pulled = true; return hubYAML, nil } + + got := MaybeIngest(cfgPath, existing, testLogger(), pull) + if pulled { + t.Fatal("must not pull when already configured") + } + if got.LocalAPI.Endpoint != "already:9999" { + t.Fatalf("existing local_api must be preserved, got %q", got.LocalAPI.Endpoint) + } + if _, err := os.Stat(cfgPath); err == nil { + t.Fatal("controller.yaml must not be rewritten when local_api already present") + } +} + // FAIL-SAFE (transient): a persistently-unreachable hub is retried, then leaves cfg in setup mode // (no controller.yaml). Asserts the retry count (1 initial + len(pullRetryDelays)). func TestMaybeIngest_TransientRetriesThenSetup(t *testing.T) { diff --git a/controller/internal/web/intermediary.go b/controller/internal/web/intermediary.go index 9d34314..0cfabfd 100644 --- a/controller/internal/web/intermediary.go +++ b/controller/internal/web/intermediary.go @@ -296,6 +296,12 @@ func (s *Server) driveGateLoop() { t := time.NewTicker(30 * time.Second) defer t.Stop() for range t.C { + // Re-run the boot-change converger every tick (not just once at startup): right after a GUEST + // reboot the agent's local API may briefly be unreachable / its per-guest token stale, so the + // single startup attempt can bail before reading the boot-id. It is idempotent (boot-id gated: + // a no-op once the current boot has been converged), so retrying until the agent is reachable + // is safe and is what makes guest-reboot recovery robust. + s.processGuestBootChange() s.ReconcileDriveGates() } }