From 77b4f2145008314d1370d058e9cc5c2ef787a2e2 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 9 Jun 2026 11:01:40 +0200 Subject: [PATCH] =?UTF-8?q?v0.5.1:=20live-validation=20prep=20=E2=80=94=20?= =?UTF-8?q?fix=20unmounted-dir=20durable=5Fid=20mis-id=20+=20watchdog=20UU?= =?UTF-8?q?ID=20memory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaced preparing the live USB validation on demo-felhom: - observe.go: an unmounted removable dir-storage no longer falls through to the ROOT fs for its backing device/UUID — durable_id was becoming uuid: (a DR mis-id that would re-attach the wrong disk). Now derived only from the target's own mountpoint; unmounted → no device + stable store: durable_id. Removed containingMountDevice. - watchdog.go: remember the fs-UUID observed while attached and backfill it onto the re-mount target, so re-mount works even if the known-set cache refreshed mid-drop (doc 03 §7 "sourced from the existing definition"). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 24 ++++++++++++++++++++++++ cmd/felhom-agent/main.go | 2 +- internal/storage/observe.go | 30 ++++++++---------------------- internal/storage/observe_test.go | 10 ++++++++++ internal/storage/watchdog.go | 17 +++++++++++++++++ internal/storage/watchdog_test.go | 30 ++++++++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fab85a..43cb758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,30 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.5.1 — slice 5 live-validation prep: durable_id mis-id fix + re-mount UUID memory (2026-06-09) + +Two correctness fixes surfaced while preparing the live USB validation on `demo-felhom` +(a real 1TB USB HDD, sdb1, ext4). Both are DR-load-bearing — exactly the "false-id → +re-attach the wrong disk" failure mode the slice warned about. + +### Fixed +- **Unmounted dir-storage no longer inherits the ROOT filesystem's UUID** (`observe.go`). + Previously, when a removable dir-storage was unmounted, the observer fell through to the + *containing* mount (root) for the backing device, so its `durable_id` became + `uuid:` — a catastrophic DR mis-id (the hub would re-attach the wrong disk). + Now the backing device/UUID/`durable_id` are derived ONLY from the target's OWN + mountpoint; an unmounted target reports no device and a stable `store:` durable_id, + never another filesystem's UUID. (Removed the `containingMountDevice` root-fallthrough.) +- **Watchdog remembers the fs-UUID observed while attached** (`watchdog.go`) so a re-mount + works even after the known-set cache refreshes mid-drop (an unmounted target can't resolve + its own UUID). The re-mount key is backfilled from this memory — aligning with doc 03 §7's + "sourced from the existing definition, no hub manifest needed": the agent learns the UUID + while the target is attached, then re-mounts by it on return. + +### Tests +- Observer: an unmounted dir-storage asserts NO `uuid:` durable_id and no backing device. +- Watchdog: a drop where the cache lost the UUID still re-mounts using the remembered UUID. + ## v0.5.0 — slice 5 Phase B: the host-root surface (mounts + SMART + grow + destructive gate) (2026-06-09) The write surface — the agent's first step outside its Proxmox API token into OS-root. diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 5c9b447..509da76 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -31,7 +31,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.5.0" +var version = "0.5.1" func main() { var ( diff --git a/internal/storage/observe.go b/internal/storage/observe.go index 3349544..b6bfb78 100644 --- a/internal/storage/observe.go +++ b/internal/storage/observe.go @@ -161,14 +161,19 @@ func (o *Observer) snapshot(ctx context.Context) ([]observed, error) { func (o *Observer) build(s proxmox.Storage, mounts []Mount) observed { category := categorize(s.Type) - // Resolve the backing device + mount path for dir-like targets. + // Resolve the backing device + mount path for dir-like targets — ONLY from the + // target's OWN mountpoint. We deliberately do NOT fall through to the containing + // filesystem (e.g. root): an unmounted removable dir-storage's mountpoint reverts to a + // bare directory on root, and resolving its UUID/durable_id to ROOT's UUID would be a + // catastrophic DR mis-id (the hub would re-attach the wrong disk). When the target is + // not its own mount, we leave the device/UUID unknown and the durable_id falls back to a + // stable store id (never another fs's UUID). The authoritative UUID for re-attach comes + // from a prior attached observation (watchdog memory) or the hub manifest (slice 10). var backingDevice, mountPath string var exactMount bool if category == catDir { if dev, mp, ok := exactMountDevice(mounts, s.Path); ok { backingDevice, mountPath, exactMount = dev, mp, true - } else if dev, ok := containingMountDevice(mounts, s.Path); ok { - backingDevice = dev // for the class hint only; not its own mount } } @@ -396,25 +401,6 @@ func exactMountDevice(mounts []Mount, path string) (device, mountPoint string, o return "", "", false } -// containingMountDevice finds the device of the longest mountpoint that is a prefix of -// path (the filesystem that path lives on) — used only for the class-hint disk lookup. -func containingMountDevice(mounts []Mount, path string) (device string, ok bool) { - if path == "" { - return "", false - } - clean := cleanMountPath(path) - best := -1 - for _, m := range mounts { - mp := cleanMountPath(m.MountPoint) - if clean == mp || strings.HasPrefix(clean, mp+"/") || mp == "/" { - if len(mp) > best { - best, device, ok = len(mp), m.Device, true - } - } - } - return device, ok -} - func cleanMountPath(p string) string { p = strings.TrimRight(p, "/") if p == "" { diff --git a/internal/storage/observe_test.go b/internal/storage/observe_test.go index 4288c97..b349116 100644 --- a/internal/storage/observe_test.go +++ b/internal/storage/observe_test.go @@ -4,6 +4,7 @@ import ( "context" "io" "log/slog" + "strings" "testing" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" @@ -249,6 +250,15 @@ func TestObserve_USBUnpluggedIsDisconnected(t *testing.T) { if usb.DurableID == "" { t.Errorf("durable_id must never be empty (DR re-attach lookup)") } + // CRITICAL: an unmounted dir-storage must NOT inherit the ROOT filesystem's UUID/device + // (that would re-attach the WRONG disk on DR). With only root mounted, the USB target + // resolves to no device and a stable store-id durable_id — never "uuid:". + if strings.HasPrefix(usb.DurableID, "uuid:") { + t.Errorf("unmounted target must not carry a uuid durable_id (got %q — root-UUID mis-id risk)", usb.DurableID) + } + if usb.BackingDevice != "" { + t.Errorf("unmounted target must not resolve a backing device (got %q)", usb.BackingDevice) + } } func TestObserve_ProxmoxErrorIsFatalForStorage(t *testing.T) { diff --git a/internal/storage/watchdog.go b/internal/storage/watchdog.go index b5265ec..b59f376 100644 --- a/internal/storage/watchdog.go +++ b/internal/storage/watchdog.go @@ -86,6 +86,7 @@ type Watchdog struct { fired bool // lastFire is valid pending bool // a transition is awaiting the debounce window lastRemount map[string]time.Time // name -> last re-mount dispatch (rate-limit) + lastUUID map[string]string // name -> last fs-UUID observed while ATTACHED (re-mount key) } // WatchdogOptions configures a Watchdog. Targets, Liveness and Trigger are required; the @@ -131,6 +132,7 @@ func NewWatchdog(opts WatchdogOptions) *Watchdog { spawn: func(f func()) { go f() }, last: map[string]bool{}, lastRemount: map[string]time.Time{}, + lastUUID: map[string]string{}, } } @@ -169,6 +171,21 @@ func (w *Watchdog) tick(ctx context.Context) { return } + // Remember each target's fs-UUID while it is observable (attached), and backfill it + // onto a target the current observe couldn't resolve (the unmounted case loses it). The + // re-mount key is "sourced from the existing definition" (doc 03 §7): the agent learns + // the UUID while attached, so a drop+return cycle can re-mount by-UUID even after the + // known-set cache refreshed mid-drop. Single-goroutine (tick), guarded for -race. + w.mu.Lock() + for i := range known { + if known[i].UUID != "" { + w.lastUUID[known[i].Name] = known[i].UUID + } else if u := w.lastUUID[known[i].Name]; u != "" { + known[i].UUID = u + } + } + w.mu.Unlock() + // Probe outside the lock. type probe struct { t KnownTarget diff --git a/internal/storage/watchdog_test.go b/internal/storage/watchdog_test.go index a56554f..609edc7 100644 --- a/internal/storage/watchdog_test.go +++ b/internal/storage/watchdog_test.go @@ -147,12 +147,14 @@ func TestWatchdog_DebounceCoalescesFlaps(t *testing.T) { type fakeRemounter struct { mu sync.Mutex calls []string + uuids []string } func (r *fakeRemounter) Remount(_ context.Context, t KnownTarget) { r.mu.Lock() defer r.mu.Unlock() r.calls = append(r.calls, t.Name) + r.uuids = append(r.uuids, t.UUID) } func (r *fakeRemounter) count() int { r.mu.Lock() @@ -208,6 +210,34 @@ func TestWatchdog_ReMountOnDeviceReturn(t *testing.T) { } } +func TestWatchdog_ReMountUsesRememberedUUID(t *testing.T) { + // The re-mount key must survive the known-set cache losing the UUID mid-drop (an + // unmounted dir-storage can't resolve its own UUID). The watchdog remembers the UUID + // observed while attached and backfills it onto the re-mount target. + known := &staticKnown{targets: []KnownTarget{{Name: "usb", MountBacked: true, UUID: "277a2179", MountPath: "/mnt/usb"}}} + live := &mapLiveness{present: map[string]bool{"usb": true}, device: map[string]bool{"usb": true}} + rem := &fakeRemounter{} + w, _, _ := newTestWatchdog(known, live, 30*time.Second) + w.remounter = rem + w.spawn = func(f func()) { f() } + ctx := context.Background() + + w.tick(ctx) // baseline attached → remembers UUID 277a2179 + + // Cache refreshes during the drop and loses the UUID (the unmounted observe can't + // resolve it); device still present, unmounted. + known.targets = []KnownTarget{{Name: "usb", MountBacked: true, UUID: "", MountPath: "/mnt/usb"}} + live.set("usb", false) + w.tick(ctx) + + if rem.count() != 1 { + t.Fatalf("expected one re-mount, got %d", rem.count()) + } + if rem.uuids[0] != "277a2179" { + t.Fatalf("re-mount must use the remembered UUID, got %q", rem.uuids[0]) + } +} + func TestWatchdog_ReadErrorSkipsTick(t *testing.T) { known := &staticKnown{err: errors.New("proxmox blip")} live := &mapLiveness{present: map[string]bool{}}