From 281c7b7e3f176f49d68a9bc8db961897e4b12f2b Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 15 Jun 2026 20:06:23 +0200 Subject: [PATCH] agent v0.36.1: decommission keeps raw mounted (re-enrollable) The decommission unmounted the raw /mnt/, orphaning a non-removable drive so re-enroll bound an empty dir. Now DetachDrive only (bind under parent); raw stays mounted so re-enroll re-binds. Test: raw NOT unmounted + DetachDrive called. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 8 ++++++++ cmd/felhom-agent/main.go | 2 +- internal/localapi/decommission_test.go | 19 ++++++++++++++----- internal/localapi/disks.go | 11 +++++------ 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a3dc6b..074ed9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## 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 +mount, which orphaned a non-removable drive (no re-plug) so a one-click re-enroll bound an empty dir. On +the intermediary model decommission is now a LOGICAL retire — it DetachDrive`s the bind under the parent +(drive invisible to the guest) but LEAVES the raw mounted, so re-enroll re-binds cleanly. Physical +removal stays the separate "remove from system" action. Test updated. + ## v0.36.0 — guest boot-id on /disks (deterministic guest-reboot recreate) (2026-06-15) The agent now emits `guest_boot_id` on GET /disks: `-` — changes on diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 9961edd..b587e2c 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.0" +var version = "0.36.1" // 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/decommission_test.go b/internal/localapi/decommission_test.go index 4b6ed31..fe38b14 100644 --- a/internal/localapi/decommission_test.go +++ b/internal/localapi/decommission_test.go @@ -92,15 +92,21 @@ func TestDecommission_RoleGated(t *testing.T) { } // TestDecommission_Effects: a user-data decommission sets IntentDecommissioned, removes the -// GuestBindStore entry, and unmounts — assert ALL THREE (not just no-error). NEVER formats. +// GuestBindStore entry, and DETACHES the bind under the parent — but (intermediary model) does NOT +// unmount the RAW host mount, so a one-click re-enroll (H3) can re-bind it. NEVER formats. +// +// COMPANION GUARD: the pre-fix decommission unmounted the raw drive (`d.unmountCalls`), which orphaned a +// non-removable drive so re-enroll bound an empty dir. The "raw NOT unmounted" + "DetachDrive called" +// assertions below fail that impl. func TestDecommission_Effects(t *testing.T) { d := &fakeDiskOps{} intent := newFakeIntent() intent.SetEnrolled("uuid:usb-1") // currently enrolled gb := tempBindStore(t) _ = gb.Record(8200, "uuid:usb-1") + ga := &fakeGuestAttacher{} - srv := decommServer(t, d, userDataAndProtected(), fakeGuestList{}, intent, gb, &fakeGuestAttacher{}, nil) + srv := decommServer(t, d, userDataAndProtected(), fakeGuestList{}, intent, gb, ga, nil) w := do(t, srv.Handler(), "POST", "/disks/decommission", "A", `{"where":"/mnt/bulk"}`) if w.Code != http.StatusOK { t.Fatalf("decommission user-data: got %d want 200 (%s)", w.Code, w.Body.String()) @@ -113,11 +119,14 @@ func TestDecommission_Effects(t *testing.T) { if ids := gb.Guests()[8200]; len(ids) != 0 { t.Errorf("guest-bind not removed: %v", ids) } - // 3) unmounted, never formatted + // 3) the bind under the parent is DETACHED, the RAW is NOT unmounted, and never formatted. + if len(ga.detachDrives) != 1 || ga.detachDrives[0] != "/mnt/bulk" { + t.Errorf("DetachDrive calls = %v, want [/mnt/bulk]", ga.detachDrives) + } d.mu.Lock() defer d.mu.Unlock() - if len(d.unmountCalls) != 1 || d.unmountCalls[0] != "/mnt/bulk" { - t.Errorf("Unmount calls = %v, want [/mnt/bulk]", d.unmountCalls) + if len(d.unmountCalls) != 0 { + t.Errorf("raw drive must NOT be unmounted on decommission (re-enrollable); got %v", d.unmountCalls) } if len(d.formatCalls) != 0 { t.Errorf("decommission must NEVER format; formatCalls = %v", d.formatCalls) diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go index e72f72f..ca24ddb 100644 --- a/internal/localapi/disks.go +++ b/internal/localapi/disks.go @@ -360,12 +360,11 @@ func (s *Server) handleDiskDecommission(w http.ResponseWriter, r *http.Request, } } } - // Unmount (mirror eject) — benign, data preserved. NEVER format/mkfs here. - if err := s.disks.Unmount(r.Context(), req.Where); err != nil { - s.logger.Error("local-api: disk decommission", "vmid", vmid, "where", req.Where, "err", err) - writeErr(w, http.StatusBadRequest, "decommission failed: "+err.Error()) - return - } + // Intermediary model: decommission is a LOGICAL retire (data stays, re-enrollable). It DetachDrive'd + // the bind under the parent above (the drive is no longer visible to the guest); it does NOT unmount + // the RAW /mnt/ host mount — that would orphan the drive (a non-removable SATA drive doesn't get + // re-plugged), so a one-click re-enroll (H3) could not re-bind it. The soft decommission marker blocks + // scheduling; physical removal is the separate "remove from system" action. NEVER format/mkfs here. writeOK(w, map[string]any{"vmid": vmid, "decommissioned": req.Where, "dependent_guests": dependents}) }