From 4cd1d024e9b664754e3b2b81a11a33a0c26de343 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 14 Jun 2026 15:07:37 +0200 Subject: [PATCH] F9: auto-re-assert enrolled guest data-drive binds on agent startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-guest bind (pct set -mpN) is config state that a destroy+re-provision drops, and nothing restored it — so a re-provisioned guest came up with its enrolled HDD unattached (the live-drive F9 finding). New GuestBindStore persists, per guest, the durable-ids of enrolled drives (recorded at guest-attach); ReassertGuestBinds runs on agent startup (the host's bring-up/reconcile trigger) and re-adds any bind a guest is MISSING — but ONLY when the durable-id still resolves to a present, mounted drive (a swapped/absent drive is never auto-bound) and the guest lacks it (idempotent). The re-added bind activates on the guest's next reboot, like the enroll flow. Wired in main.go (store opened beside drive-intents.json; ReassertGuestBinds called before the local API serves). Tests: restores a missing bind with no manual call (the operator's real-trigger proof); skips absent/swapped durable-id; no-op when already bound; store survives reopen (restart). --- cmd/felhom-agent/main.go | 16 ++- internal/localapi/disks.go | 83 +++++++++++- internal/localapi/guestbinds_reassert_test.go | 126 ++++++++++++++++++ internal/localapi/guestbindstore.go | 103 ++++++++++++++ internal/localapi/server.go | 6 + 5 files changed, 331 insertions(+), 3 deletions(-) create mode 100644 internal/localapi/guestbinds_reassert_test.go create mode 100644 internal/localapi/guestbindstore.go diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 64cc634..d072455 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -340,6 +340,13 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { } else { intentReader = intentStore } + // F9: per-guest enrolled-bind record, replayed by ReassertGuestBinds at startup to restore a + // guest data-drive bind that a re-provision dropped (durable-id-keyed; absent/swapped drives skipped). + guestBindStore, gbErr := localapi.OpenGuestBindStore(filepath.Join(intentStateDir, "guest-binds.json")) + if gbErr != nil { + logger.Warn("storage: guest-bind store unavailable — startup bind re-assert disabled (F9)", "err", gbErr) + guestBindStore = nil + } watchdog := storage.NewWatchdog(storage.WatchdogOptions{ Targets: storage.NewCachingKnownTargets(observer, cfg.Storage.KnownRefresh()), Liveness: storage.NewHostLiveness(hostReader, 0), @@ -424,7 +431,7 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { jobsRunner := signedjobs.NewRunner(client, gate, signedjobs.ExecutorChain{wipeExec, decommExec}, cfg.Hub.HostID, logger) loop.SetEnvelopeObserver(hub.MultiObserver(desiredSyncer, jobsRunner)) - localSrv := buildLocalAPIServer(cfg, px, backupStore, observer, hostOps, gate, collector, intentRec, logger, &localTokens) + localSrv := buildLocalAPIServer(cfg, px, backupStore, observer, hostOps, gate, collector, intentRec, guestBindStore, logger, &localTokens) if localTokens != nil { defer localTokens.Close() } @@ -462,6 +469,10 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { go func() { errc <- pbsLoop.Run(ctx) }() if localSrv != nil { localServers = 1 + // F9: on startup (the host's bring-up/reconcile trigger), re-assert any enrolled guest data-drive + // bind that a re-provision dropped — before serving, so the drive is back in the guest config + // (activates on the guest's next reboot). On-durable-id-match; absent/swapped drives are skipped. + localSrv.ReassertGuestBinds(ctx) go func() { errc <- localSrv.Run(ctx) }() } if lanLoop != nil { @@ -576,7 +587,7 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re // leaf (stable fingerprint). Any failure DISABLES the server (returns nil) WITHOUT crashing the // daemon — the host still reports/reconciles; only the controller channel is unavailable until // fixed. The opened token store is returned via outTokens so the caller can Close it. -func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.Store, observer *storage.Observer, hostOps storage.HostOps, gate *reconcile.Gate, collector *hub.Collector, intent localapi.IntentRecorder, logger *slog.Logger, outTokens **localapi.TokenStore) *localapi.Server { +func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.Store, observer *storage.Observer, hostOps storage.HostOps, gate *reconcile.Gate, collector *hub.Collector, intent localapi.IntentRecorder, guestBinds *localapi.GuestBindStore, logger *slog.Logger, outTokens **localapi.TokenStore) *localapi.Server { if !cfg.LocalAPI.Enabled() { return nil } @@ -620,6 +631,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St Guests2: px, GuestAttach: guestBinder, // slice 10 P2: bind enrolled data drives into the guest Intent: intent, // slice 10 P3: record enroll/eject intent for self-heal + GuestBinds: guestBinds, // F9: per-guest bind record for the startup re-assert // Host metrics (slice 9): the shared collector serves GET /host/metrics — a fresh host + // per-storage view to the customer's monitoring page (reuses the slice-4 collector). diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go index 7d48ef9..c26cd3e 100644 --- a/internal/localapi/disks.go +++ b/internal/localapi/disks.go @@ -286,6 +286,7 @@ func (s *Server) handleDiskGuestAttach(w http.ResponseWriter, r *http.Request, v for key, spec := range mounts { if _, mp, _ := parseMount(spec); mp == where { s.recordIntent(r.Context(), where, "enrolled") + s.recordGuestBind(r.Context(), vmid, where) s.logger.Info("local-api: guest-attach idempotent (already bound)", "vmid", vmid, "where", where, "slot", key) writeOK(w, map[string]any{"vmid": vmid, "attached": where, "slot": key, "already": true}) return @@ -301,8 +302,10 @@ func (s *Server) handleDiskGuestAttach(w http.ResponseWriter, r *http.Request, v writeErr(w, http.StatusBadGateway, "guest-attach failed: "+err.Error()) return } - // Record the drive as ENROLLED so the self-heal watchdog will reconcile it (P3). + // Record the drive as ENROLLED so the self-heal watchdog will reconcile it (P3), and persist the + // per-guest bind so the startup re-assert can restore it after a re-provision (F9). s.recordIntent(r.Context(), where, "enrolled") + s.recordGuestBind(r.Context(), vmid, where) writeOK(w, map[string]any{"vmid": vmid, "attached": where, "slot": slot}) } @@ -561,6 +564,84 @@ func (s *Server) guestBoundPaths(ctx context.Context, vmid int) map[string]bool return out } +// recordGuestBind persists that the drive at `where` (by its durable-id) is enrolled into `vmid`, so the +// startup re-assert (ReassertGuestBinds) can restore the bind after a re-provision (F9). Best-effort. +func (s *Server) recordGuestBind(ctx context.Context, vmid int, where string) { + if s.guestBinds == nil { + return + } + id := s.durableIDForMount(ctx, where) + if id == "" { + s.logger.Warn("local-api: guest-bind not recorded — durable-id unresolved", "vmid", vmid, "where", where) + return + } + if err := s.guestBinds.Record(vmid, id); err != nil { + s.logger.Warn("local-api: guest-bind record failed", "vmid", vmid, "where", where, "durable_id", id, "err", err) + return + } + s.logger.Info("local-api: guest-bind recorded for startup re-assert", "vmid", vmid, "where", where, "durable_id", id) +} + +// ReassertGuestBinds re-adds, on agent startup (the host's bring-up/reconcile trigger), any enrolled +// user-data drive bind a guest is MISSING from its config (F9 — a re-provision drops the mp, and nothing +// previously restored it). For each recorded (vmid, durable-id): only when the durable-id STILL resolves +// to a present, mounted drive AND the guest lacks the bind, it re-runs AttachBind. "On durable-id match" +// — a swapped or absent drive is never auto-bound. The re-added bind is config state; it activates on the +// guest's next reboot (logged), exactly like the enroll flow. Safe to call repeatedly (idempotent). +func (s *Server) ReassertGuestBinds(ctx context.Context) { + if s.guestBinds == nil || s.guestAttach == nil || s.guests == nil { + return + } + // durable-id -> current host mount path (present drives only), from the agent's own storage view. + mountByDurable := map[string]string{} + if targets, err := s.storage.Observe(ctx); err == nil { + for _, t := range targets { + if t.DurableID != "" && t.MountPath != "" { + mountByDurable[t.DurableID] = t.MountPath + } + } + } else { + s.logger.Warn("F9 re-assert: storage view unavailable — skipping", "err", err) + return + } + for vmid, ids := range s.guestBinds.Guests() { + cfg, err := s.guests.GuestConfig(ctx, vmid) + if err != nil { + s.logger.Warn("F9 re-assert: skip guest (config read failed)", "vmid", vmid, "err", err) + continue + } + mounts := cfg.MountPoints() + boundPaths := map[string]bool{} + for _, spec := range mounts { + if _, mp, _ := parseMount(spec); mp != "" { + boundPaths[mp] = true + } + } + for _, id := range ids { + where, present := mountByDurable[id] + if !present { + s.logger.Warn("F9 re-assert: enrolled drive not present (durable-id absent) — skipping", "vmid", vmid, "durable_id", id) + continue + } + if boundPaths[where] { + continue // already bound — nothing to re-assert + } + slot, ok := freeMountSlot(mounts) + if !ok { + s.logger.Warn("F9 re-assert: no free mountpoint slot on guest", "vmid", vmid, "where", where) + continue + } + if err := s.guestAttach.AttachBind(ctx, vmid, slot, where); err != nil { + s.logger.Error("F9 re-assert: AttachBind failed", "vmid", vmid, "where", where, "slot", slot, "err", err) + continue + } + mounts[slot] = where // reserve the slot so a second enrolled drive takes the next one + s.logger.Warn("F9 re-assert: re-attached enrolled drive into guest config (reboot to activate)", + "vmid", vmid, "where", where, "slot", slot, "durable_id", id) + } + } +} + // durableIDForMount resolves the durable-id of the storage mounted at `where` (from the agent's own // storage view) — the key the intent store records enroll/eject against. "" if not resolvable. func (s *Server) durableIDForMount(ctx context.Context, where string) string { diff --git a/internal/localapi/guestbinds_reassert_test.go b/internal/localapi/guestbinds_reassert_test.go new file mode 100644 index 0000000..63888f4 --- /dev/null +++ b/internal/localapi/guestbinds_reassert_test.go @@ -0,0 +1,126 @@ +package localapi + +import ( + "context" + "io" + "log/slog" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +func tempBindStore(t *testing.T) *GuestBindStore { + t.Helper() + gb, err := OpenGuestBindStore(filepath.Join(t.TempDir(), "guest-binds.json")) + if err != nil { + t.Fatal(err) + } + return gb +} + +func reassertServer(t *testing.T, ga GuestAttacher, sv StorageView, mounts map[int]map[string]string, gb *GuestBindStore) *Server { + t.Helper() + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", + Guests: &fakeGuestsCfg{mounts: mounts}, + Backups: &fakeBackups{}, Store: &fakeStore{}, + Storage: sv, Tokens: staticTokens{"A": 8200}, + GuestAttach: ga, GuestBinds: gb, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatal(err) + } + srv.baseCtx = context.Background() + return srv +} + +// usbPresent is a storage view with felhom-usb present at /mnt/felhom-usb (durable uuid:usb-1). +func usbPresent() fakeStorage { + return fakeStorage{targets: []hub.StorageTarget{ + {Name: "usb", Type: hub.StorageTypeUSB, BackingDevice: "/dev/sdb1", MountPath: "/mnt/felhom-usb", DurableID: "uuid:usb-1"}, + }} +} + +// TestReassertGuestBinds_RestoresMissingBind is the F9 core proof (the operator's "fire the real +// trigger" requirement): an enrolled drive that is present on the host but MISSING from the guest config +// (the post-re-provision gap) is auto-re-attached on the startup re-assert — with NO manual guest-attach +// call. Fails on the pre-fix code (no re-assert existed). +func TestReassertGuestBinds_RestoresMissingBind(t *testing.T) { + gb := tempBindStore(t) + if err := gb.Record(8200, "uuid:usb-1"); err != nil { // enrolled at a prior boot + t.Fatal(err) + } + ga := &fakeGuestAttacher{} + // guest 8200 has docker-data only — the felhom-usb bind was dropped by the re-provision. + srv := reassertServer(t, ga, usbPresent(), map[int]map[string]string{ + 8200: {"mp0": "local-lvm:8,mp=/var/lib/docker"}, + }, gb) + + srv.ReassertGuestBinds(context.Background()) + + if ga.count() != 1 { + t.Fatalf("AttachBind called %d times, want 1 (auto-re-assert on startup)", ga.count()) + } + if ga.calls[0].vmid != 8200 || ga.calls[0].where != "/mnt/felhom-usb" { + t.Fatalf("re-asserted bind = %+v, want vmid 8200 where /mnt/felhom-usb", ga.calls[0]) + } +} + +// TestReassertGuestBinds_SkipsAbsentDurable: an enrolled drive whose durable-id is NOT currently present +// (unplugged / swapped for a different disk) must NOT be auto-bound — the "on durable-id match" safety. +func TestReassertGuestBinds_SkipsAbsentDurable(t *testing.T) { + gb := tempBindStore(t) + _ = gb.Record(8200, "uuid:usb-1") + ga := &fakeGuestAttacher{} + srv := reassertServer(t, ga, fakeStorage{}, map[int]map[string]string{ // empty storage view → absent + 8200: {"mp0": "local-lvm:8,mp=/var/lib/docker"}, + }, gb) + + srv.ReassertGuestBinds(context.Background()) + if ga.count() != 0 { + t.Fatalf("AttachBind called %d times — must NOT auto-bind an absent/swapped drive", ga.count()) + } +} + +// TestReassertGuestBinds_SkipsAlreadyBound: when the guest already has the bind, the re-assert is a no-op. +func TestReassertGuestBinds_SkipsAlreadyBound(t *testing.T) { + gb := tempBindStore(t) + _ = gb.Record(8200, "uuid:usb-1") + ga := &fakeGuestAttacher{} + srv := reassertServer(t, ga, usbPresent(), map[int]map[string]string{ + 8200: {"mp0": "local-lvm:8,mp=/var/lib/docker", "mp3": "/mnt/felhom-usb/felhom-data,mp=/mnt/felhom-usb"}, + }, gb) + + srv.ReassertGuestBinds(context.Background()) + if ga.count() != 0 { + t.Fatalf("AttachBind called %d times — already bound, must be a no-op", ga.count()) + } +} + +// TestGuestBindStore_Persist round-trips the store across reopen (the record must survive an agent +// restart, since that is exactly when the re-assert runs). +func TestGuestBindStore_Persist(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "guest-binds.json") + gb, err := OpenGuestBindStore(path) + if err != nil { + t.Fatal(err) + } + _ = gb.Record(8200, "uuid:usb-1") + _ = gb.Record(8200, "uuid:usb-1") // idempotent + _ = gb.Record(9300, "byid:wwn-x") + + re, err := OpenGuestBindStore(path) // simulate restart + if err != nil { + t.Fatal(err) + } + g := re.Guests() + if len(g[8200]) != 1 || g[8200][0] != "uuid:usb-1" { + t.Fatalf("vmid 8200 = %v, want [uuid:usb-1]", g[8200]) + } + if len(g[9300]) != 1 || g[9300][0] != "byid:wwn-x" { + t.Fatalf("vmid 9300 = %v, want [byid:wwn-x]", g[9300]) + } +} diff --git a/internal/localapi/guestbindstore.go b/internal/localapi/guestbindstore.go new file mode 100644 index 0000000..db784f1 --- /dev/null +++ b/internal/localapi/guestbindstore.go @@ -0,0 +1,103 @@ +package localapi + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strconv" + "sync" +) + +// GuestBindStore persists, per guest, the DURABLE-IDs of the user-data drives enrolled (guest-attached) +// into it. F9: the in-guest bind (`pct set -mpN`) is config state that does NOT survive a destroy + +// re-provision, and nothing re-asserted it — so a re-provisioned guest came up with the HDD unattached +// even though it had been enrolled. This store is the record the startup re-assert (ReassertGuestBinds) +// replays: for each enrolled durable-id still physically present, re-add the bind if the guest lacks it. +// +// Keyed by durable-id (NOT host path or sdX) so the re-assert is "on durable-id match" — a swapped or +// absent drive is never auto-bound. Thread-safe; atomic file writes (tmp+rename), 0600. Mirrors +// storage.IntentStore. +type GuestBindStore struct { + path string + mu sync.Mutex + m map[int][]string // vmid -> sorted set of enrolled durable-ids +} + +// OpenGuestBindStore loads (or initializes) the store. Missing file = empty store; corrupt file = error +// (fail loud — losing a bind record would silently drop the re-assert). +func OpenGuestBindStore(path string) (*GuestBindStore, error) { + s := &GuestBindStore{path: path, m: map[int][]string{}} + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return s, nil + } + return nil, fmt.Errorf("guest-bind store: read %s: %w", path, err) + } + if len(data) > 0 { + // stored as {"": ["durable-id", ...]} (string keys — JSON object keys are strings) + raw := map[string][]string{} + if err := json.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("guest-bind store: parse %s: %w", path, err) + } + for k, v := range raw { + vmid, err := strconv.Atoi(k) + if err != nil { + return nil, fmt.Errorf("guest-bind store: bad vmid key %q: %w", k, err) + } + s.m[vmid] = v + } + } + return s, nil +} + +// Record adds (vmid, durableID) to the enrolled set. Idempotent — no write if already present. A blank +// durable-id is refused (the re-assert must never act on a drive whose identity it can't pin). +func (s *GuestBindStore) Record(vmid int, durableID string) error { + if durableID == "" { + return fmt.Errorf("guest-bind store: refusing to record an empty durable-id for vmid %d", vmid) + } + s.mu.Lock() + defer s.mu.Unlock() + for _, id := range s.m[vmid] { + if id == durableID { + return nil // idempotent + } + } + s.m[vmid] = append(s.m[vmid], durableID) + sort.Strings(s.m[vmid]) + return s.saveLocked() +} + +// Guests returns a copy of the vmid → enrolled-durable-ids map. +func (s *GuestBindStore) Guests() map[int][]string { + s.mu.Lock() + defer s.mu.Unlock() + out := make(map[int][]string, len(s.m)) + for vmid, ids := range s.m { + out[vmid] = append([]string(nil), ids...) + } + return out +} + +func (s *GuestBindStore) saveLocked() error { + raw := make(map[string][]string, len(s.m)) + for vmid, ids := range s.m { + raw[strconv.Itoa(vmid)] = ids + } + data, err := json.MarshalIndent(raw, "", " ") + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(s.path), 0o755); err != nil { + return err + } + tmp := s.path + ".tmp" + if err := os.WriteFile(tmp, data, 0o600); err != nil { + os.Remove(tmp) + return err + } + return os.Rename(tmp, s.path) +} diff --git a/internal/localapi/server.go b/internal/localapi/server.go index 6fbd147..8ffff5e 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -87,6 +87,10 @@ type Options struct { // Intent records drive enroll/eject intent for the self-heal watchdog (slice 10 P3). OPTIONAL — // when nil, no intent is recorded (self-heal runs ungated). Intent IntentRecorder + // GuestBinds persists which user-data drives (by durable-id) are enrolled into each guest, so the + // startup re-assert (ReassertGuestBinds) can restore a bind that a re-provision dropped (F9). + // OPTIONAL — when nil, guest binds are not recorded and the startup re-assert is a no-op. + GuestBinds *GuestBindStore // HostReader is the root-free host topology reader used to classify a device/mount's protection // ROLE (it backs SystemDisks for the eject role-gate + the /disks role hints). OPTIONAL — when nil // it defaults to the production *storage.ProcHostReader. Injectable so the role-gate is testable. @@ -143,6 +147,7 @@ type Server struct { guestList GuestLister // slice 8C (optional) guestAttach GuestAttacher // slice 10 P2 (optional) intent IntentRecorder // slice 10 P3 (optional) + guestBinds *GuestBindStore // F9 startup bind re-assert record (optional) host storage.HostReader // role classification source (optional; defaults to ProcHostReader) hostMetrics HostMetricsProvider // slice 9 (optional) @@ -198,6 +203,7 @@ func NewServer(o Options) (*Server, error) { guestList: o.Guests2, guestAttach: o.GuestAttach, intent: o.Intent, + guestBinds: o.GuestBinds, host: o.HostReader, hostMetrics: o.HostMetrics, hostID: o.HostID,