controller v0.71.0: fix guest-reboot recovery (boot-race + agent-path blocker)

Live diagnosis of drive-backed apps stuck Exited after a pct reboot pinned THREE sub-causes, fixed together (hardening the existing processGuestBootChange, not a parallel mechanism): 1. Agent-path blocker (live root cause): agentClient() returned "agent not configured" (cfg.LocalAPI.Endpoint empty), so processGuestBootChange AND the whole drive gate bailed at the first guard. bootstrap.json had a complete local_api block, but MaybeIngest returned immediately on "already configured" so a controller.yaml seeded before local_api existed never got the agent path. Fix: MaybeIngest now calls ensureLocalAPI on the already-configured path, merging local_api from bootstrap.json into the existing controller.yaml when missing (no hub re-pull, config preserved; idempotent + fail-safe). 2. Boot-race readiness gate: processGuestBootChange sampled BoundUnderParent once during fast startup, racing the ~18s rebind, recreated nothing, burned its boot-id one-shot. Fix: gate on the REAL live in-guest bind -- driveBindLive checks /mnt/felhom-drives/<drive> is a mountpoint in the controller's own /mnt rslave /proc/self/mountinfo; pollLiveBinds waits for it (bounded ~120s) before recreating via the normal pipeline. shouldRecreateOnBoot stays state-independent so stuck-Exited create-time-failure apps are included. 3. Single-shot fragility: processGuestBootChange ran only once at startup; a briefly-unreachable agent right after a guest reboot stranded recovery. Fix: driveGateLoop runs it every periodic tick too (idempotent, boot-id gated). Tests (non-hollow, pre-fix companions, red-proofed): pollLiveBinds waits then reports live / never-live stays absent / single early sample misses; ensureLocalAPI merges local_api into a configured controller.yaml that lacks it / no-ops when present. Live-accepted with repeated pct reboot 9201. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 16:12:49 +02:00
parent 25e5cb5850
commit e2de234325
5 changed files with 172 additions and 26 deletions
@@ -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).
@@ -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) {