diff --git a/CHANGELOG.md b/CHANGELOG.md index ecae680..2229d43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,23 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.35.0 — intermediary mount: guest-reboot re-propagation (load-bearing) (2026-06-15) + +Fix for the guest-reboot gap (caught in the live demo migration). A guest's parent bind is +NON-RECURSIVE, so on a guest reboot it does NOT carry the pre-existing drive submount, and mount +propagation only delivers events created AFTER the bind exists — so an enrolled drive is bound on the +HOST but INVISIBLE in the fresh guest namespace until re-bound. Without this, every guest reboot left +the apps on empty dirs. + +- `AttachDrive` now takes `vmid` and checks GUEST visibility (`GuestSeesMount`, reading + `/proc//mountinfo`): if the host has the bind but the guest doesn't see it + (post-reboot), it FORCE re-binds (umount + mount) to re-fire propagation into the current guest ns. +- A periodic reconcile (20s ticker in main) re-runs `ReassertGuestBinds`, so a guest reboot self-heals + without an agent restart. `EnsureSharedParent` skips the unit re-install when already present (cheap + on repeat). +- `/disks` `BoundUnderParent` now reflects GUEST visibility (not the host mount) — the accurate signal + the controller's drive-absent gate keys on to stop/restart apps across a guest reboot. + ## v0.34.0 — intermediary mount model: shared-parent + host-side attach/detach + reconcile (2026-06-15) The drive hot-swap re-architecture (SPIKE-intermediary-mount). Replaces the per-drive `pct set -mpN` diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index ad7b9dd..29c3aa4 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -44,7 +44,7 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.34.0" +var version = "0.35.0" // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots @@ -510,6 +510,23 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { // 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) + // Intermediary-mount GUEST-REBOOT self-heal: re-run the reconcile periodically. A guest reboot + // (without an agent restart) leaves enrolled drives bound on the HOST but INVISIBLE in the fresh + // guest namespace (a non-recursive parent bind doesn't carry pre-existing submounts); the periodic + // reconcile detects the guest can't see the bind and re-fires propagation (force re-bind). Cheap + // when nothing changed (guest-visibility checks only). 20s ≪ the controller gate's 30s tick. + go func() { + t := time.NewTicker(20 * time.Second) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + localSrv.ReassertGuestBinds(ctx) + } + } + }() // F20-BUG3: complete a disk format that an agent restart interrupted (durable-id-bound, re-resolved). localSrv.RecoverFormatJob(ctx) go func() { errc <- localSrv.Run(ctx) }() diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go index 17779a3..4fe419f 100644 --- a/internal/localapi/disks.go +++ b/internal/localapi/disks.go @@ -69,11 +69,15 @@ type GuestLister interface { type GuestAttacher interface { // AttachDrive (intermediary model) binds the drive's felhom-data under the shared parent so it // appears live in the guest at the returned stable path — no pct, no reboot. `where` = raw /mnt/. - AttachDrive(ctx context.Context, where string) (guestPath string, err error) + // vmid is needed to verify GUEST visibility (a guest reboot needs a fresh re-bind to re-propagate). + AttachDrive(ctx context.Context, vmid int, where string) (guestPath string, err error) // DetachDrive unmounts the drive's felhom-data from the shared parent (live, fail-closed). DetachDrive(ctx context.Context, where string) error // EnsureSharedParent makes the host stable parent shared + installs the boot-persistence unit. EnsureSharedParent(ctx context.Context) error + // GuestSeesMount reports whether vmid's guest sees `path` as a mount in its own namespace (the + // guest-usable signal — distinct from the host having the bind). Backs BoundUnderParent. + GuestSeesMount(ctx context.Context, vmid int, path string) bool // AttachBind is the LEGACY per-drive `pct set -mpN` bind (pre-intermediary). Retained for the // transition; new attaches use AttachDrive. AttachBind(ctx context.Context, vmid int, mountKey, where string) error @@ -177,7 +181,7 @@ func (s *Server) handleDisks(w http.ResponseWriter, r *http.Request, vmid int) { if di.Role == string(storage.RoleUserData) { if gp := StablePathForRaw(t.MountPath); gp != "" { di.GuestPath = gp - di.BoundUnderParent = s.boundUnderParent(gp) + di.BoundUnderParent = s.boundUnderParent(r.Context(), vmid, gp) } } // Inspect the backing device for the UI's data-bearing hint (the authoritative check @@ -389,7 +393,7 @@ func (s *Server) handleDiskGuestAttach(w http.ResponseWriter, r *http.Request, v if err := s.guestAttach.EnsureSharedParent(r.Context()); err != nil { s.logger.Warn("local-api: guest-attach — shared parent ensure failed (continuing)", "vmid", vmid, "err", err) } - stable, err := s.guestAttach.AttachDrive(r.Context(), where) + stable, err := s.guestAttach.AttachDrive(r.Context(), vmid, where) if err != nil { s.logger.Error("local-api: guest-attach (intermediary)", "vmid", vmid, "where", where, "err", err) writeErr(w, http.StatusBadGateway, "guest-attach failed: "+err.Error()) @@ -685,13 +689,18 @@ func (s *Server) handleDiskFormat(w http.ResponseWriter, r *http.Request, vmid i "device is system/backup-protected — format requires an operator signature ("+dec.Reason+")") } -// boundUnderParent reports whether a drive's felhom-data is currently bound at its stable guest path -// (intermediary model). Injectable via s.boundCheck for tests; defaults to the host mount-table read. -func (s *Server) boundUnderParent(stablePath string) bool { +// boundUnderParent reports whether a drive's felhom-data is bound at its stable guest path AND visible +// inside the guest (the usable-in-guest signal the controller's gate keys on — a guest reboot leaves the +// host bind in place but invisible to the guest until re-propagated). Injectable via s.boundCheck for +// tests; defaults to the guest-namespace mount check. +func (s *Server) boundUnderParent(ctx context.Context, vmid int, stablePath string) bool { if s.boundCheck != nil { return s.boundCheck(stablePath) } - return isHostMountpoint(stablePath) + if s.guestAttach == nil { + return isHostMountpoint(stablePath) + } + return s.guestAttach.GuestSeesMount(ctx, vmid, stablePath) } // guestBoundPaths returns the set of guest mountpoint paths (the `mp=` of each entry in the guest's @@ -798,7 +807,7 @@ func (s *Server) ReassertGuestBinds(ctx context.Context) { s.logger.Warn("reconcile: enrolled drive not present (durable-id absent) — skipping", "vmid", vmid, "durable_id", id) continue } - stable, err := s.guestAttach.AttachDrive(ctx, where) + stable, err := s.guestAttach.AttachDrive(ctx, vmid, where) if err != nil { s.logger.Error("reconcile: AttachDrive failed", "vmid", vmid, "where", where, "err", err) continue diff --git a/internal/localapi/disks_test.go b/internal/localapi/disks_test.go index 099ecdd..021ab69 100644 --- a/internal/localapi/disks_test.go +++ b/internal/localapi/disks_test.go @@ -412,7 +412,7 @@ func (f *fakeGuestAttacher) AttachBind(_ context.Context, vmid int, mountKey, wh } func (f *fakeGuestAttacher) count() int { f.mu.Lock(); defer f.mu.Unlock(); return len(f.calls) } -func (f *fakeGuestAttacher) AttachDrive(_ context.Context, where string) (string, error) { +func (f *fakeGuestAttacher) AttachDrive(_ context.Context, _ int, where string) (string, error) { f.mu.Lock() defer f.mu.Unlock() if f.attachDriveFail { @@ -421,6 +421,7 @@ func (f *fakeGuestAttacher) AttachDrive(_ context.Context, where string) (string f.attachDrives = append(f.attachDrives, where) return StablePathForRaw(where), nil } +func (f *fakeGuestAttacher) GuestSeesMount(_ context.Context, _ int, _ string) bool { return true } func (f *fakeGuestAttacher) attachDriveCount() int { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/localapi/intermediary.go b/internal/localapi/intermediary.go index 0f357d4..b623a4a 100644 --- a/internal/localapi/intermediary.go +++ b/internal/localapi/intermediary.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" ) @@ -93,10 +94,13 @@ func (b *GuestBinder) EnsureSharedParent(ctx context.Context) error { if err := b.run(ctx, "mount", "--make-shared", StableParentDir); err != nil { return fmt.Errorf("shared-parent: make-shared: %w", err) } - if err := b.installSharedParentUnit(ctx); err != nil { - b.logger.Warn("shared-parent: boot-persistence unit install failed (live setup OK; survives until host reboot)", "err", err) + // Install the boot-persistence unit only if it's not already there — EnsureSharedParent runs on a + // periodic reconcile, and re-writing files + daemon-reload every tick would be wasteful. + if _, err := os.Stat(sharedParentUnitPath); err != nil { + if ierr := b.installSharedParentUnit(ctx); ierr != nil { + b.logger.Warn("shared-parent: boot-persistence unit install failed (live setup OK; survives until host reboot)", "err", ierr) + } } - b.logger.Info("shared-parent: host stable parent is shared", "dir", StableParentDir) return nil } @@ -133,8 +137,15 @@ func (b *GuestBinder) installSharedParentUnit(ctx context.Context) error { // host PVE mount (/mnt/); only `/felhom-data` crosses into the guest (confinement). The // stable per-drive dir is created HOST-ROOT-owned (fail-closed when nothing is mounted under it); the // felhom-data namespace is created+chowned to the guest base so the in-guest controller owns it. -// Idempotent: if the stable path is already a mountpoint, it's a no-op. -func (b *GuestBinder) AttachDrive(ctx context.Context, where string) (string, error) { +// +// GUEST-REBOOT SAFETY (the load-bearing subtlety): a guest's parent bind is NON-RECURSIVE, so on a guest +// reboot it does NOT carry the pre-existing drive submount, and mount propagation only delivers mount +// events created AFTER the guest's bind exists. So "the host already has the bind" is NOT sufficient — +// the GUEST may not see it. AttachDrive therefore checks whether vmid's guest actually sees the stable +// path; if the host has the bind but the guest does not (the post-guest-reboot case), it FORCE re-binds +// (umount + mount) to fire a fresh propagation event into the current guest namespace. Idempotent when +// the guest already sees it. +func (b *GuestBinder) AttachDrive(ctx context.Context, vmid int, where string) (string, error) { stable := StablePathForRaw(where) if stable == "" { return "", fmt.Errorf("guest-attach: %q is not a /mnt/ mount", where) @@ -151,17 +162,61 @@ func (b *GuestBinder) AttachDrive(ctx context.Context, where string) (string, er if err := b.run(ctx, "mkdir", "-p", stable); err != nil { return "", fmt.Errorf("guest-attach: stable dir %s: %w", stable, err) } - if isHostMountpoint(stable) { - b.logger.Info("guest-attach: already bound under parent (idempotent)", "where", where, "stable", stable) - return stable, nil + hostHas := isHostMountpoint(stable) + guestSees := b.GuestSeesMount(ctx, vmid, stable) + switch { + case hostHas && guestSees: + return stable, nil // fully live — no-op + case hostHas && !guestSees: + // Guest rebooted (or bound the parent before this drive's bind existed): re-fire propagation. + if err := b.run(ctx, "umount", stable); err != nil { + b.logger.Warn("guest-attach: re-bind umount failed (continuing to re-mount)", "stable", stable, "err", err) + } + if err := b.run(ctx, "mount", "--bind", src, stable); err != nil { + return "", fmt.Errorf("guest-attach: re-bind %s -> %s: %w", src, stable, err) + } + b.logger.Info("guest-attach: re-bound drive to re-propagate into guest (post-reboot)", "vmid", vmid, "where", where, "stable", stable) + default: // !hostHas + if err := b.run(ctx, "mount", "--bind", src, stable); err != nil { + return "", fmt.Errorf("guest-attach: bind %s -> %s: %w", src, stable, err) + } + b.logger.Info("guest-attach: drive bound under shared parent (live, no reboot)", "vmid", vmid, "where", where, "stable", stable) } - if err := b.run(ctx, "mount", "--bind", src, stable); err != nil { - return "", fmt.Errorf("guest-attach: bind %s -> %s: %w", src, stable, err) - } - b.logger.Info("guest-attach: drive bound under shared parent (live, no reboot)", "where", where, "stable", stable) return stable, nil } +// GuestSeesMount reports whether vmid's guest currently has `path` as a mount target in ITS mount +// namespace (read from /proc//mountinfo). This is the GUEST-side truth the host-side +// isHostMountpoint can't see — the signal that distinguishes "bound on the host" from "live in the +// guest" after a guest reboot. A resolution/read error → false (treat as not-seen → AttachDrive re-binds, +// which is safe). The controller's BoundUnderParent report keys on this. +func (b *GuestBinder) GuestSeesMount(ctx context.Context, vmid int, path string) bool { + pid := b.guestInitPID(ctx, vmid) + if pid == "" { + return false + } + data, err := os.ReadFile("/proc/" + pid + "/mountinfo") + if err != nil { + return false + } + for _, line := range strings.Split(string(data), "\n") { + f := strings.Fields(line) + if len(f) >= 5 && f[4] == path { + return true + } + } + return false +} + +// guestInitPID returns the guest's PID-1 host PID (`lxc-info -n -p -H`), or "" on error. +func (b *GuestBinder) guestInitPID(ctx context.Context, vmid int) string { + out, _, err := b.runner.Run(ctx, "lxc-info", "-n", strconv.Itoa(vmid), "-p", "-H") + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + // DetachDrive unmounts a drive's felhom-data from the stable parent (propagates OUT of the guest live), // leaving the bare HOST-ROOT-owned stable dir → fail-closed (the guest can't write to it even as root, // since host uid 0 is unmapped). No pct, no reboot. Idempotent: a non-mountpoint is a no-op.