agent v0.37.0: host-reboot remount re-resolves enrolled drives by fs-UUID
TASK A — close out the reboot story (agent half). Root cause (pinned live on felhom-pve): an enrolled .mount unit left `disabled` by a prior detach never auto-mounts at boot, and kernel re-enumeration can move a drive's node (/dev/sdb->sdc). Fix re-asserts every enrolled mount by filesystem UUID at startup + on the periodic tick. - ResolveStorageDevice: resolve uuid:<fs-uuid> -> current /dev node via /dev/disk/by-uuid (never a cached node); errors if absent. - parseFelhomMountUnit: pure inverse of renderMountUnit (marker-gated). - (*SudoHostOps).ReassertEnrolledMounts: re-run EnsureMount (enable --now) for any enrolled unit not in /proc/mounts; idempotent, skips mounted/absent. - main.go: runs before ReassertGuestBinds at startup + on the 20s tick. - tests (Linux, seam=device resolution): letter-move tolerated (sdb->sdc) + red-proof companion, absent/scheme rejection, render->parse round-trip. TASK A2 verdict: enrolling a NEW drive does NOT need an LXC restart — the path lands on the live AttachDrive (shared parent, named live slots), not RebootGuest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -73,6 +73,24 @@ func ResolveDurableDevice(durableID string) (string, error) {
|
||||
}
|
||||
}
|
||||
|
||||
// ResolveStorageDevice resolves an enrolled STORAGE durable-id (the `uuid:<fs-uuid>` scheme that
|
||||
// deriveDurableID emits for usb/local-dir drives) to its CURRENT backing /dev path by re-scanning
|
||||
// /dev/disk/by-uuid. This is the load-bearing host-reboot fix: kernel re-enumeration can move a drive
|
||||
// from /dev/sdb to /dev/sdc, so a remount MUST re-resolve by UUID (never trust a remembered node) — the
|
||||
// reshuffle is then a no-op. Errors if the UUID no longer resolves (the drive is genuinely absent), so a
|
||||
// caller can skip re-mounting a gone drive instead of failing on a stale node.
|
||||
func ResolveStorageDevice(durableID string) (string, error) {
|
||||
const scheme = "uuid:"
|
||||
if !strings.HasPrefix(durableID, scheme) {
|
||||
return "", fmt.Errorf("storage: durable id %q is not the uuid: scheme — cannot resolve by filesystem UUID", durableID)
|
||||
}
|
||||
uuid := strings.TrimPrefix(durableID, scheme)
|
||||
if !safeLinkName(uuid) {
|
||||
return "", fmt.Errorf("storage: unsafe durable id %q", durableID)
|
||||
}
|
||||
return filepath.EvalSymlinks(filepath.Join(devDiskRoot, "by-uuid", uuid))
|
||||
}
|
||||
|
||||
// bestByIDLink returns the highest-priority /dev/disk/by-id link name whose target is `device`
|
||||
// (already symlink-resolved), or "" if none.
|
||||
func bestByIDLink(device string) string {
|
||||
|
||||
@@ -46,6 +46,84 @@ func withDevDiskRoot(t *testing.T, root string) {
|
||||
t.Cleanup(func() { devDiskRoot = old })
|
||||
}
|
||||
|
||||
// TestResolveStorageDevice_ToleratesDeviceLetterMove (Task A): on a host reboot the kernel can move a
|
||||
// drive from /dev/sdb to /dev/sdc; ResolveStorageDevice re-resolves the enrolled durable-id by filesystem
|
||||
// UUID and returns the CURRENT node, so the reshuffle is a no-op. COMPANION: a node-based resolver (what
|
||||
// the broken remount effectively did — trust the enroll-time node) returns the STALE node → the WRONG
|
||||
// device once the letter moves.
|
||||
func TestResolveStorageDevice_ToleratesDeviceLetterMove(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("symlink-based device resolution runs on Linux (the agent's OS)")
|
||||
}
|
||||
base := t.TempDir()
|
||||
sdb := filepath.Join(base, "sdb") // the enroll-time node (now holds some OTHER drive after reboot)
|
||||
sdc := filepath.Join(base, "sdc") // where felhom-usb's UUID now lives after re-enumeration
|
||||
if err := os.WriteFile(sdb, []byte("x"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(sdc, []byte("x"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
root := filepath.Join(base, "disk")
|
||||
if err := os.MkdirAll(filepath.Join(root, "by-uuid"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
const uuid = "da9e7089-cf8e-4617-adcb-a377743fae00"
|
||||
if err := os.Symlink(sdc, filepath.Join(root, "by-uuid", uuid)); err != nil { // moved: UUID → sdc
|
||||
t.Fatal(err)
|
||||
}
|
||||
withDevDiskRoot(t, root)
|
||||
|
||||
// FIX: resolve by UUID → the CURRENT node (sdc), tolerating the move.
|
||||
got, err := ResolveStorageDevice("uuid:" + uuid)
|
||||
if err != nil {
|
||||
t.Fatalf("ResolveStorageDevice: %v", err)
|
||||
}
|
||||
wantSdc, _ := filepath.EvalSymlinks(sdc)
|
||||
if got != wantSdc {
|
||||
t.Fatalf("resolved %q, want the CURRENT node %q (the UUID moved sdb→sdc)", got, wantSdc)
|
||||
}
|
||||
|
||||
// COMPANION: a node-based remount that trusts the recorded enroll-time node (sdb) now targets the
|
||||
// WRONG device — sdb is no longer where this UUID lives.
|
||||
cachedNode, _ := filepath.EvalSymlinks(sdb)
|
||||
if cachedNode == got {
|
||||
t.Fatalf("companion: the cached enroll-time node must differ from the freshly-resolved device after a move (cached=%q resolved=%q)", cachedNode, got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveStorageDevice_AbsentAndScheme: a vanished UUID errors (so the re-assert skips a gone drive),
|
||||
// and only the uuid: scheme is resolvable (never a bare /dev node — the anti-node-binding invariant).
|
||||
func TestResolveStorageDevice_AbsentAndScheme(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("linux")
|
||||
}
|
||||
withDevDiskRoot(t, filepath.Join(t.TempDir(), "empty"))
|
||||
if _, err := ResolveStorageDevice("uuid:gone-uuid"); err == nil {
|
||||
t.Fatal("an absent UUID must error so the re-assert skips a gone drive instead of fail-mounting")
|
||||
}
|
||||
for _, id := range []string{"/dev/sdb1", "store:felhom-usb", "byid:wwn-x", ""} {
|
||||
if _, err := ResolveStorageDevice(id); err == nil {
|
||||
t.Errorf("ResolveStorageDevice(%q) must reject a non-uuid: scheme", id)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseFelhomMountUnit round-trips renderMountUnit → parseFelhomMountUnit and rejects foreign units.
|
||||
func TestParseFelhomMountUnit(t *testing.T) {
|
||||
spec := MountSpec{Name: "felhom-usb", UUID: "da9e7089-cf8e-4617-adcb-a377743fae00", Where: "/mnt/felhom-usb", FSType: "ext4"}
|
||||
got, ok := parseFelhomMountUnit(renderMountUnit(spec))
|
||||
if !ok {
|
||||
t.Fatal("our own rendered unit must parse")
|
||||
}
|
||||
if got.Name != spec.Name || got.UUID != spec.UUID || got.Where != spec.Where || got.FSType != spec.FSType {
|
||||
t.Fatalf("round-trip mismatch: got %+v want %+v", got, spec)
|
||||
}
|
||||
if _, ok := parseFelhomMountUnit("[Mount]\nWhat=/dev/sdb1\nWhere=/mnt/x\n"); ok {
|
||||
t.Fatal("a non-felhom unit (no marker, not by-uuid) must not parse")
|
||||
}
|
||||
}
|
||||
|
||||
// DeviceDurableID prefers a wwn- by-id link over ata-/serial links and over a uuid.
|
||||
func TestDeviceDurableID_PrefersWWN(t *testing.T) {
|
||||
root, device := fakeDevDisk(t,
|
||||
|
||||
@@ -226,6 +226,64 @@ func (h *SudoHostOps) EnsureMount(ctx context.Context, spec MountSpec) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// ReassertEnrolledMounts re-mounts every enrolled drive whose backing device is present (resolved FRESH
|
||||
// by filesystem UUID, never a cached /dev node) but whose systemd mount unit is not currently mounted —
|
||||
// the host-reboot remount fix. On a host reboot a unit left `disabled` by a prior detach never
|
||||
// auto-mounts, and kernel re-enumeration can move the device (/dev/sdb→sdc); re-running EnsureMount
|
||||
// (idempotent `enable --now`, What=/dev/disk/by-uuid/<UUID>) re-enables the unit AND mounts the CURRENT
|
||||
// device by UUID, so a letter reshuffle is a no-op. Idempotent + cheap on the steady state: an
|
||||
// already-mounted drive is skipped (no daemon-reload churn). A drive whose UUID no longer resolves
|
||||
// (genuinely absent) is skipped, not failed — its unit re-asserts on a later tick once it enumerates.
|
||||
func (h *SudoHostOps) ReassertEnrolledMounts(ctx context.Context) {
|
||||
entries, err := os.ReadDir(h.unitDir)
|
||||
if err != nil {
|
||||
h.logger.Warn("storage: reassert enrolled mounts — cannot read unit dir", "dir", h.unitDir, "err", err)
|
||||
return
|
||||
}
|
||||
mounted := h.mountedSet()
|
||||
for _, e := range entries {
|
||||
if !strings.HasSuffix(e.Name(), ".mount") {
|
||||
continue
|
||||
}
|
||||
data, rerr := os.ReadFile(filepath.Join(h.unitDir, e.Name()))
|
||||
if rerr != nil {
|
||||
continue
|
||||
}
|
||||
spec, ok := parseFelhomMountUnit(string(data))
|
||||
if !ok || mounted[spec.Where] {
|
||||
continue // not ours, or already mounted → nothing to do (no churn)
|
||||
}
|
||||
dev, derr := ResolveStorageDevice("uuid:" + spec.UUID)
|
||||
if derr != nil {
|
||||
h.logger.Info("storage: enrolled drive absent by UUID — not re-asserting (will retry when it enumerates)", "name", spec.Name, "where", spec.Where, "uuid", spec.UUID)
|
||||
continue
|
||||
}
|
||||
if err := h.EnsureMount(ctx, spec); err != nil {
|
||||
h.logger.Warn("storage: re-assert enrolled mount failed", "name", spec.Name, "where", spec.Where, "err", err)
|
||||
continue
|
||||
}
|
||||
h.logger.Info("storage: re-asserted enrolled mount by UUID (enable --now)", "name", spec.Name, "where", spec.Where, "uuid", spec.UUID, "device", dev)
|
||||
}
|
||||
}
|
||||
|
||||
// mountedSet returns the set of currently-active mountpoints from /proc/mounts (mountpoint is field 2).
|
||||
// Best-effort: a read failure yields an empty set (every enrolled drive is then considered for
|
||||
// re-assert, which EnsureMount makes idempotent).
|
||||
func (h *SudoHostOps) mountedSet() map[string]bool {
|
||||
out := map[string]bool{}
|
||||
data, err := os.ReadFile("/proc/mounts")
|
||||
if err != nil {
|
||||
return out
|
||||
}
|
||||
for _, line := range strings.Split(string(data), "\n") {
|
||||
f := strings.Fields(line)
|
||||
if len(f) >= 2 {
|
||||
out[f[1]] = true
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// Unmount stops + disables the unit (detach). The caller is responsible for authorization.
|
||||
func (h *SudoHostOps) Unmount(ctx context.Context, where string) error {
|
||||
unitName, err := UnitNameForMount(where)
|
||||
|
||||
@@ -5,6 +5,41 @@ import (
|
||||
"strings"
|
||||
)
|
||||
|
||||
// felhomUnitMarker is the header renderMountUnit writes; parseFelhomMountUnit uses it to tell our
|
||||
// units apart from any other .mount unit on the host.
|
||||
const felhomUnitMarker = "Managed by felhom-agent"
|
||||
|
||||
// parseFelhomMountUnit is the inverse of renderMountUnit for the fields the host-reboot re-assert needs.
|
||||
// It returns the MountSpec (Name from Description, UUID from What=/dev/disk/by-uuid/<UUID>, Where, Type,
|
||||
// Options), or ok=false when the content is not a felhom-rendered by-UUID mount unit. Pure → unit-tested.
|
||||
func parseFelhomMountUnit(content string) (MountSpec, bool) {
|
||||
if !strings.Contains(content, felhomUnitMarker) {
|
||||
return MountSpec{}, false
|
||||
}
|
||||
var spec MountSpec
|
||||
for _, line := range strings.Split(content, "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
switch {
|
||||
case strings.HasPrefix(line, "Description=Felhom storage mount "):
|
||||
spec.Name = strings.TrimPrefix(line, "Description=Felhom storage mount ")
|
||||
case strings.HasPrefix(line, "What="):
|
||||
if u, found := strings.CutPrefix(strings.TrimPrefix(line, "What="), byUUIDDir+"/"); found {
|
||||
spec.UUID = u
|
||||
}
|
||||
case strings.HasPrefix(line, "Where="):
|
||||
spec.Where = strings.TrimPrefix(line, "Where=")
|
||||
case strings.HasPrefix(line, "Type="):
|
||||
spec.FSType = strings.TrimPrefix(line, "Type=")
|
||||
case strings.HasPrefix(line, "Options="):
|
||||
spec.Options = strings.TrimPrefix(line, "Options=")
|
||||
}
|
||||
}
|
||||
if spec.UUID == "" || spec.Where == "" { // not a by-uuid mount we can re-resolve
|
||||
return MountSpec{}, false
|
||||
}
|
||||
return spec, true
|
||||
}
|
||||
|
||||
// renderMountUnit builds the systemd .mount unit content for a (already-validated) spec.
|
||||
// Keyed by fs-UUID via What=/dev/disk/by-uuid/<UUID> so it survives /dev/sdX renumbering;
|
||||
// WantedBy=multi-user.target so `enable` makes it persist across reboot.
|
||||
|
||||
Reference in New Issue
Block a user