From 038f0ecd5db5d4c6178b272999cbce70cad1c2e9 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 15 Jun 2026 20:12:44 +0200 Subject: [PATCH] agent v0.36.2: eject keeps raw mounted (reconnectable, like decommission) Eject now DetachDrive's the bind under the parent but leaves the raw /mnt/ mounted, so disconnect->reconnect re-binds on a non-removable drive. Tests updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 +++++++ cmd/felhom-agent/main.go | 2 +- internal/localapi/disks.go | 15 +++++++-------- internal/localapi/disks_test.go | 32 ++++++++++++++++++++++++-------- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 074ed9e..6ad5346 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.36.2 — eject also keeps the raw mounted (reconnectable) (2026-06-15) + +Extends v0.36.1 to EJECT: eject now DetachDrive`s the bind under the parent but LEAVES the raw +/mnt/ mounted (consistent with decommission), so the H1 disconnect→reconnect roundtrip re-binds +cleanly on a non-removable drive. Physical removal is the separate "remove from system" action. Test: +eject calls DetachDrive + does NOT unmount the raw. + ## v0.36.1 — decommission keeps the raw mounted (re-enrollable) (2026-06-15) Fix caught in the E10 acceptance test: the self-serve decommission unmounted the RAW /mnt/ host diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index b587e2c..82b4890 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.36.1" +var version = "0.36.2" // 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 diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go index ca24ddb..a024465 100644 --- a/internal/localapi/disks.go +++ b/internal/localapi/disks.go @@ -285,18 +285,17 @@ func (s *Server) handleDiskEject(w http.ResponseWriter, r *http.Request, vmid in // self-heal watchdog leaves it alone — an OFFICIAL eject is the only thing that sets this (P3); an // out-of-band unmount records nothing and is healed. s.recordIntent(r.Context(), req.Where, "ejected") - // Intermediary model: unmount the felhom-data bind from under the shared parent FIRST (live, fail- - // closed in the guest), so nothing references the raw mount before we unmount it. + // Intermediary model: eject = DETACH the felhom-data bind from under the shared parent (live, fail- + // closed in the guest). The RAW /mnt/ host mount is LEFT MOUNTED so a reconnect re-binds it + // cleanly (a non-removable drive isn't re-plugged); a raw unmount here would orphan it. Physical + // removal is the separate "remove from system" action. (Consistent with decommission.) if s.guestAttach != nil { if err := s.guestAttach.DetachDrive(r.Context(), req.Where); err != nil { - s.logger.Warn("local-api: eject guest-detach (intermediary) failed", "vmid", vmid, "where", req.Where, "err", err) + s.logger.Error("local-api: eject guest-detach (intermediary)", "vmid", vmid, "where", req.Where, "err", err) + writeErr(w, http.StatusBadRequest, "eject failed: "+err.Error()) + return } } - if err := s.disks.Unmount(r.Context(), req.Where); err != nil { - s.logger.Error("local-api: disk eject", "vmid", vmid, "where", req.Where, "err", err) - writeErr(w, http.StatusBadRequest, "eject failed: "+err.Error()) - return - } writeOK(w, map[string]any{"vmid": vmid, "ejected": req.Where, "dependent_guests": dependents}) } diff --git a/internal/localapi/disks_test.go b/internal/localapi/disks_test.go index 983ddd5..d36a715 100644 --- a/internal/localapi/disks_test.go +++ b/internal/localapi/disks_test.go @@ -288,20 +288,35 @@ func TestEject_UnmountAndDependents(t *testing.T) { sv := fakeStorage{targets: []hub.StorageTarget{{Name: "bulk", Type: hub.StorageTypeUSB, BackingDevice: "/dev/sdb1", MountPath: "/mnt/bulk"}}} // guest 8200 mounts storage "bulk"; guest 9300 does not gl := fakeGuestList{guests: []proxmox.Guest{{VMID: 8200}, {VMID: 9300}}} - h := newDiskServerWithGuestConfigs(t, d, sv, gl, map[int]map[string]string{ - 8200: {"mp0": "bulk:200,mp=/mnt/media,backup=0"}, - 9300: {"mp0": "local-lvm:8,mp=/var,backup=1"}, + ga := &fakeGuestAttacher{} + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", Guests: &fakeGuestsCfg{mounts: map[int]map[string]string{ + 8200: {"mp0": "bulk:200,mp=/mnt/media,backup=0"}, + 9300: {"mp0": "local-lvm:8,mp=/var,backup=1"}, + }}, Backups: &fakeBackups{}, Store: &fakeStore{}, Storage: sv, + Tokens: staticTokens{"A": 8200, "B": 9300}, Disks: d, DiskGate: &fakeGate{}, Guests2: gl, + GuestAttach: ga, HostReader: sysOnSDA(), Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), }) + if err != nil { + t.Fatal(err) + } + srv.baseCtx = context.Background() + h := srv.Handler() w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/bulk"}`) if w.Code != http.StatusOK { t.Fatalf("eject: got %d (%s)", w.Code, w.Body.String()) } + // Intermediary model: eject DETACHES the bind under the parent but does NOT unmount the raw drive + // (so a reconnect re-binds it). COMPANION GUARD: the pre-fix eject called Unmount → orphaned the drive. + if len(ga.detachDrives) != 1 || ga.detachDrives[0] != "/mnt/bulk" { + t.Fatalf("DetachDrive not called for /mnt/bulk: %v", ga.detachDrives) + } d.mu.Lock() unmounts := append([]string(nil), d.unmountCalls...) d.mu.Unlock() - if len(unmounts) != 1 || unmounts[0] != "/mnt/bulk" { - t.Fatalf("Unmount not called: %v", unmounts) + if len(unmounts) != 0 { + t.Fatalf("raw drive must NOT be unmounted on eject (reconnectable); got %v", unmounts) } var resp struct { Data struct { @@ -357,15 +372,16 @@ func TestEject_RoleGated(t *testing.T) { } d2.mu.Unlock() - // user-data mount → ejects (Unmount called once). + // user-data mount → PERMITTED (200, not 403). The bind-detach happens via DetachDrive (asserted in + // TestEject_UnmountAndDependents); the role-gate's job here is to NOT refuse it + NOT unmount the raw. d3 := &fakeDiskOps{} h3 := newDiskServer(t, d3, &fakeGate{}, sv, nil) if w := do(t, h3, "POST", "/disks/eject", "A", `{"where":"/mnt/bulk"}`); w.Code != http.StatusOK { t.Fatalf("eject user-data mount: got %d want 200 (%s)", w.Code, w.Body.String()) } d3.mu.Lock() - if len(d3.unmountCalls) != 1 || d3.unmountCalls[0] != "/mnt/bulk" { - t.Fatalf("user-data eject did not Unmount /mnt/bulk: %v", d3.unmountCalls) + if len(d3.unmountCalls) != 0 { + t.Fatalf("user-data eject must NOT unmount the raw drive (reconnectable): %v", d3.unmountCalls) } d3.mu.Unlock()