v0.95.0: SMART coverage — union-path drives + LVM/dm root + device model
Implements SPIKE-smart-coverage-2026-07-25 fixes B+A (additive; MinAgent unchanged). Fix B: storage.SmartReader.SMARTForBacking wired into the /disks union path (localapi Smart seam) so registry/USB drives get a real SMART read (watchdog Known stays enrich-free). Fix A: smartDeviceFor resolves dm/LVM to the whole disk via /sys/block/<dm>/slaves (recursive; skips >1-disk); the builtin local dir on the LVM root gets a SMART-only device from its containing filesystem (never touches backing/durable_id). SmartSummary.ModelName captured from smartctl. Fix C (-d sat) stays rejected. Tests + red-proofs (dm multi-disk skip, enrich smartHint, union routing); Known-path-never-SMARTs asserted.
This commit is contained in:
+41
-10
@@ -58,6 +58,9 @@ type observed struct {
|
||||
known KnownTarget
|
||||
src proxmox.Storage
|
||||
cat storageCategory
|
||||
// smartHint (v0.95.0) is a SMART-ONLY whole-disk device for a dir-storage whose own backing is
|
||||
// empty (the builtin `local` on the shared LVM root). Never assigned to BackingDevice/durable_id.
|
||||
smartHint string
|
||||
}
|
||||
|
||||
// Observe builds the reported []hub.StorageTarget. A non-nil error means the Proxmox read
|
||||
@@ -84,13 +87,22 @@ func (o *Observer) enrich(ctx context.Context, ob observed) hub.StorageTarget {
|
||||
if o.ops == nil {
|
||||
return t
|
||||
}
|
||||
// SMART: only for dir-backed targets with a resolvable whole-disk device.
|
||||
if ob.cat == catDir && t.BackingDevice != "" {
|
||||
if dev, ok := smartDeviceFor(t.BackingDevice); ok {
|
||||
if sm, err := o.ops.SMART(ctx, dev); err != nil {
|
||||
o.logger.Warn("storage: SMART read failed; health UNKNOWN", "device", dev, "err", err)
|
||||
} else {
|
||||
t.Smart = sm
|
||||
// SMART: for dir-backed targets. The device is the target's own backing (a USB/local-dir exact
|
||||
// mount) OR, for a dir on a shared filesystem whose backing is deliberately empty (the builtin
|
||||
// `local` on the LVM root — removable-safety guard in build), the SMART-only hint build() resolved
|
||||
// from the containing filesystem. smartDeviceFor then resolves dm/LVM/partition to the whole disk.
|
||||
if ob.cat == catDir {
|
||||
smartDev := t.BackingDevice
|
||||
if smartDev == "" {
|
||||
smartDev = ob.smartHint
|
||||
}
|
||||
if smartDev != "" {
|
||||
if dev, ok := smartDeviceFor(smartDev); ok {
|
||||
if sm, err := o.ops.SMART(ctx, dev); err != nil {
|
||||
o.logger.Warn("storage: SMART read failed; health UNKNOWN", "device", dev, "err", err)
|
||||
} else {
|
||||
t.Smart = sm
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -238,10 +250,23 @@ func (o *Observer) build(s proxmox.Storage, mounts []Mount) observed {
|
||||
}
|
||||
}
|
||||
|
||||
// SMART-only device hint (v0.95.0): a dir-storage that lives INSIDE a shared filesystem (the
|
||||
// builtin `local` on the LVM root) has empty backing by design, yet we can still read the PHYSICAL
|
||||
// disk's SMART by resolving its containing filesystem. Gated to catDir + no own backing + reachable,
|
||||
// so an UNPLUGGED removable (disconnected) never reads root's SMART, and a mounted removable uses
|
||||
// its own backing instead.
|
||||
smartHint := ""
|
||||
if category == catDir && backingDevice == "" && reachable {
|
||||
if dev, ok := containingMountDevice(mounts, s.Path); ok {
|
||||
smartHint = dev
|
||||
}
|
||||
}
|
||||
|
||||
return observed{
|
||||
target: tgt,
|
||||
src: s,
|
||||
cat: category,
|
||||
target: tgt,
|
||||
src: s,
|
||||
cat: category,
|
||||
smartHint: smartHint,
|
||||
known: KnownTarget{
|
||||
Name: s.Storage,
|
||||
Type: typ,
|
||||
@@ -260,6 +285,12 @@ func (o *Observer) build(s proxmox.Storage, mounts []Mount) observed {
|
||||
// smartctl (which targets the disk, not the partition). Returns ok=false when the result
|
||||
// isn't a recognized raw disk (e.g. device-mapper / LVM), so SMART is simply skipped.
|
||||
func smartDeviceFor(device string) (string, bool) {
|
||||
// Device-mapper / LVM (Fix A, v0.95.0): resolve to the single backing whole disk via sysfs
|
||||
// slaves. This is what finally covers the system SSD under `pve-root`. The sysfs resolution IS
|
||||
// the existence check, so we do NOT re-run ValidateSMARTDevice on its result.
|
||||
if strings.HasPrefix(device, "/dev/dm-") || strings.HasPrefix(device, "/dev/mapper/") {
|
||||
return dmWholeDisk(device)
|
||||
}
|
||||
dev := device
|
||||
if m := reNVMePart.FindStringSubmatch(device); m != nil {
|
||||
dev = m[1] // /dev/nvme0n1p2 -> /dev/nvme0n1
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
package storage
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
||||
)
|
||||
|
||||
// Fix A (A2): the builtin `local` dir lives on the LVM root, so its backing is empty by design — but
|
||||
// enrich now resolves the containing filesystem (/ → /dev/mapper/pve-root) and, via the dm sysfs
|
||||
// slaves, the physical disk (/dev/sda). The system disk stops reading "Nincs adat".
|
||||
// Red-proof: drop the `smartDev = ob.smartHint` fallback in enrich → local stays UNKNOWN and SMART
|
||||
// is never called on /dev/sda.
|
||||
func TestObserve_SystemDirSMARTViaContainingFS(t *testing.T) {
|
||||
fixtureSysfs(t, map[string][]string{"dm-1": {"sda3"}}, map[string]string{"dm-1": "pve-root"})
|
||||
|
||||
api := &fakeStorageAPI{
|
||||
node: "demo-felhom",
|
||||
cluster: []proxmox.Storage{{Storage: "local", Type: "dir", Path: "/var/lib/vz"}},
|
||||
nodeSt: []proxmox.Storage{{Storage: "local", Type: "dir", Path: "/var/lib/vz", Active: 1}},
|
||||
}
|
||||
host := &fakeHostReader{
|
||||
mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}},
|
||||
}
|
||||
ops := &fakeHostOps{smartByDevice: map[string]hub.SmartSummary{"/dev/sda": {Health: hub.SmartPassed, ModelName: strptr("AirDisk 512GB SSD")}}}
|
||||
|
||||
got, err := NewObserver(api, host, ops, quietLogger()).Observe(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
local := byName(got)["local"]
|
||||
if local.Smart.Health != hub.SmartPassed {
|
||||
t.Errorf("system disk SMART not enriched via the containing fs: health=%q", local.Smart.Health)
|
||||
}
|
||||
if local.Smart.ModelName == nil || *local.Smart.ModelName != "AirDisk 512GB SSD" {
|
||||
t.Errorf("model not carried: %v", local.Smart.ModelName)
|
||||
}
|
||||
// SMART must have run on the resolved PHYSICAL disk, never the dm/mapper node.
|
||||
if len(ops.smartDevices) != 1 || ops.smartDevices[0] != "/dev/sda" {
|
||||
t.Errorf("SMART should target /dev/sda, got %v", ops.smartDevices)
|
||||
}
|
||||
// The backing device / durable_id must stay untouched by the SMART-only resolution.
|
||||
if local.BackingDevice != "" {
|
||||
t.Errorf("system-dir SMART resolution leaked into BackingDevice: %q", local.BackingDevice)
|
||||
}
|
||||
}
|
||||
|
||||
// The watchdog Known() path MUST remain enrich-free (its slow root-shelling reads are the reason it
|
||||
// exists as a separate fast path). Known must never invoke SMART.
|
||||
// Red-proof: route Known through enrich → smartDevices is non-empty and this fails.
|
||||
func TestKnown_NeverInvokesSMART(t *testing.T) {
|
||||
fixtureSysfs(t, map[string][]string{"dm-1": {"sda3"}}, map[string]string{"dm-1": "pve-root"})
|
||||
api := &fakeStorageAPI{
|
||||
node: "demo-felhom",
|
||||
cluster: []proxmox.Storage{{Storage: "local", Type: "dir", Path: "/var/lib/vz"}},
|
||||
nodeSt: []proxmox.Storage{{Storage: "local", Type: "dir", Path: "/var/lib/vz", Active: 1}},
|
||||
}
|
||||
host := &fakeHostReader{mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}}
|
||||
ops := &fakeHostOps{smartByDevice: map[string]hub.SmartSummary{"/dev/sda": {Health: hub.SmartPassed}}}
|
||||
|
||||
if _, err := NewObserver(api, host, ops, quietLogger()).Known(context.Background()); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(ops.smartDevices) != 0 {
|
||||
t.Errorf("Known() invoked SMART %d time(s) — it must stay enrich-free: %v", len(ops.smartDevices), ops.smartDevices)
|
||||
}
|
||||
}
|
||||
|
||||
func strptr(s string) *string { return &s }
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
// presence so an absent section (e.g. NVMe fields on a SATA disk, or no SMART at all on a
|
||||
// USB bridge) decodes cleanly to nil and we degrade to UNKNOWN.
|
||||
type smartctlJSON struct {
|
||||
ModelName *string `json:"model_name"`
|
||||
SmartStatus *struct {
|
||||
Passed bool `json:"passed"`
|
||||
} `json:"smart_status"`
|
||||
@@ -64,6 +65,9 @@ func parseSMART(raw []byte) hub.SmartSummary {
|
||||
s.Health = hub.SmartFailing
|
||||
}
|
||||
}
|
||||
if j.ModelName != nil && *j.ModelName != "" {
|
||||
s.ModelName = j.ModelName
|
||||
}
|
||||
if j.Temperature != nil && j.Temperature.Current != nil {
|
||||
s.TemperatureC = j.Temperature.Current
|
||||
}
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
package storage
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||
)
|
||||
|
||||
// sysBlockRoot is the sysfs block directory. A package var so tests can point it at a fixture tree.
|
||||
var sysBlockRoot = "/sys/block"
|
||||
|
||||
// dmWholeDisk resolves a device-mapper / LVM device to its SINGLE backing whole disk, recursing
|
||||
// through stacked dm layers via /sys/block/<dm>/slaves (Fix A, SPIKE-smart-coverage-2026-07-25).
|
||||
// Returns ok=false when the device is not dm, sysfs is missing, there are no slaves, or the slaves
|
||||
// span MORE THAN ONE physical disk — in that last case we deliberately skip rather than guess which
|
||||
// of two disks to SMART (e.g. a mirrored LV).
|
||||
func dmWholeDisk(device string) (string, bool) {
|
||||
name := dmName(device)
|
||||
if name == "" {
|
||||
return "", false
|
||||
}
|
||||
disks := map[string]bool{}
|
||||
if !collectSlaveDisks(name, disks, 0) {
|
||||
return "", false
|
||||
}
|
||||
if len(disks) != 1 {
|
||||
return "", false // no disk, or an ambiguous multi-disk dm — never guess
|
||||
}
|
||||
for d := range disks {
|
||||
return "/dev/" + d, true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
// dmName maps a dm device path to its sysfs name (dm-N). Handles /dev/dm-N (and a bare dm-N)
|
||||
// directly, and /dev/mapper/<name> by matching /sys/block/dm-*/dm/name.
|
||||
func dmName(device string) string {
|
||||
base := filepath.Base(device)
|
||||
if strings.HasPrefix(base, "dm-") {
|
||||
return base
|
||||
}
|
||||
if strings.HasPrefix(device, "/dev/mapper/") {
|
||||
entries, err := os.ReadDir(sysBlockRoot)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
for _, e := range entries {
|
||||
if !strings.HasPrefix(e.Name(), "dm-") {
|
||||
continue
|
||||
}
|
||||
b, err := os.ReadFile(filepath.Join(sysBlockRoot, e.Name(), "dm", "name"))
|
||||
if err == nil && strings.TrimSpace(string(b)) == base {
|
||||
return e.Name()
|
||||
}
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// collectSlaveDisks fills `disks` with the whole-disk names backing dm `name`, recursing through
|
||||
// nested dm. Returns false on missing sysfs, no slaves, or excessive nesting (loop guard).
|
||||
func collectSlaveDisks(name string, disks map[string]bool, depth int) bool {
|
||||
if depth > 8 {
|
||||
return false
|
||||
}
|
||||
entries, err := os.ReadDir(filepath.Join(sysBlockRoot, name, "slaves"))
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
if len(entries) == 0 {
|
||||
return false
|
||||
}
|
||||
for _, e := range entries {
|
||||
s := e.Name()
|
||||
if strings.HasPrefix(s, "dm-") {
|
||||
if !collectSlaveDisks(s, disks, depth+1) {
|
||||
return false
|
||||
}
|
||||
continue
|
||||
}
|
||||
disks[wholeDiskName(s)] = true
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// wholeDiskName strips a partition suffix to the whole-disk name (sda3→sda, nvme0n1p3→nvme0n1).
|
||||
func wholeDiskName(part string) string {
|
||||
if m := reNVMePartName.FindStringSubmatch(part); m != nil {
|
||||
return m[1]
|
||||
}
|
||||
if m := reSDPartName.FindStringSubmatch(part); m != nil {
|
||||
return m[1]
|
||||
}
|
||||
return part
|
||||
}
|
||||
|
||||
var (
|
||||
reNVMePartName = regexp.MustCompile(`^(nvme[0-9]+n[0-9]+)p[0-9]+$`)
|
||||
reSDPartName = regexp.MustCompile(`^((?:sd|hd|vd)[a-z]+)[0-9]+$`)
|
||||
)
|
||||
|
||||
// containingMountDevice returns the device of the mount whose mountpoint is the LONGEST prefix of
|
||||
// path — the filesystem that actually holds `path`. Used ONLY to pick a whole-disk device for a
|
||||
// SMART read of a dir-storage that lives inside a shared filesystem (the builtin `local` on the LVM
|
||||
// root); it never feeds durable_id / backing_device (which stay empty for such targets by design —
|
||||
// the removable-safety guard in build()).
|
||||
func containingMountDevice(mounts []Mount, path string) (string, bool) {
|
||||
clean := cleanMountPath(path)
|
||||
best, bestLen := "", -1
|
||||
for _, m := range mounts {
|
||||
if m.Device == "" {
|
||||
continue
|
||||
}
|
||||
mp := cleanMountPath(m.MountPoint)
|
||||
if mp == clean || mp == "/" || strings.HasPrefix(clean, strings.TrimRight(mp, "/")+"/") {
|
||||
if len(mp) > bestLen {
|
||||
best, bestLen = m.Device, len(mp)
|
||||
}
|
||||
}
|
||||
}
|
||||
return best, best != ""
|
||||
}
|
||||
|
||||
// SmartReader reads per-disk SMART for a backing device, resolving partition / dm / LVM down to the
|
||||
// whole disk. It exists so the localapi /disks UNION path (registry/USB drives that skip Observe's
|
||||
// enrich) gets the SAME SMART read the dir targets get, without duplicating smartDeviceFor (Fix B).
|
||||
// A zero-value summary (Health "") means "could not read" (nil ops, unresolvable device, or a read
|
||||
// error) — distinct from a read that returned UNKNOWN — so the caller can omit it exactly like the
|
||||
// dir path does.
|
||||
type SmartReader struct{ ops HostOps }
|
||||
|
||||
// NewSmartReader wraps a HostOps for the localapi union path.
|
||||
func NewSmartReader(ops HostOps) *SmartReader { return &SmartReader{ops: ops} }
|
||||
|
||||
// SMARTForBacking reads SMART for backingDevice (partition/dm/whole-disk). Never returns an error;
|
||||
// on any failure the summary's Health is "".
|
||||
func (r *SmartReader) SMARTForBacking(ctx context.Context, backingDevice string) hub.SmartSummary {
|
||||
if r == nil || r.ops == nil {
|
||||
return hub.SmartSummary{}
|
||||
}
|
||||
dev, ok := smartDeviceFor(backingDevice)
|
||||
if !ok {
|
||||
return hub.SmartSummary{}
|
||||
}
|
||||
sm, err := r.ops.SMART(ctx, dev)
|
||||
if err != nil {
|
||||
return hub.SmartSummary{}
|
||||
}
|
||||
return sm
|
||||
}
|
||||
@@ -0,0 +1,107 @@
|
||||
package storage
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||
)
|
||||
|
||||
// fixtureSysfs builds a /sys/block-shaped tree and points sysBlockRoot at it. `slaves` maps a dm
|
||||
// name to its slave entries; `dmNames` maps a dm name to its /dm/name content (for /dev/mapper/*).
|
||||
func fixtureSysfs(t *testing.T, slaves map[string][]string, dmNames map[string]string) {
|
||||
t.Helper()
|
||||
root := t.TempDir()
|
||||
for dm, sl := range slaves {
|
||||
for _, s := range sl {
|
||||
if err := os.MkdirAll(filepath.Join(root, dm, "slaves", s), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
for dm, name := range dmNames {
|
||||
dir := filepath.Join(root, dm, "dm")
|
||||
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(dir, "name"), []byte(name+"\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
old := sysBlockRoot
|
||||
sysBlockRoot = root
|
||||
t.Cleanup(func() { sysBlockRoot = old })
|
||||
}
|
||||
|
||||
// Fix A dm/LVM resolution. Red-proof: remove the `len(disks) != 1` all-same-disk guard in
|
||||
// dmWholeDisk → the "mirror over two disks" case resolves to one of them instead of skipping.
|
||||
func TestDMWholeDisk(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
slaves map[string][]string
|
||||
dmNames map[string]string
|
||||
in string
|
||||
want string
|
||||
ok bool
|
||||
}{
|
||||
{"single SATA slave", map[string][]string{"dm-1": {"sda3"}}, nil, "/dev/dm-1", "/dev/sda", true},
|
||||
{"single NVMe slave", map[string][]string{"dm-0": {"nvme0n1p3"}}, nil, "/dev/dm-0", "/dev/nvme0n1", true},
|
||||
{"stacked dm → one disk", map[string][]string{"dm-2": {"dm-1"}, "dm-1": {"sda3"}}, nil, "/dev/dm-2", "/dev/sda", true},
|
||||
{"mapper name → dm-1", map[string][]string{"dm-1": {"sda3"}}, map[string]string{"dm-1": "pve-root"}, "/dev/mapper/pve-root", "/dev/sda", true},
|
||||
{"mirror over two disks → skip", map[string][]string{"dm-1": {"sda3", "sdb3"}}, nil, "/dev/dm-1", "", false},
|
||||
{"no slaves → skip", map[string][]string{"dm-1": {}}, nil, "/dev/dm-1", "", false},
|
||||
}
|
||||
for _, c := range cases {
|
||||
fixtureSysfs(t, c.slaves, c.dmNames)
|
||||
got, ok := dmWholeDisk(c.in)
|
||||
if ok != c.ok || got != c.want {
|
||||
t.Errorf("%s: dmWholeDisk(%q) = (%q,%v), want (%q,%v)", c.name, c.in, got, ok, c.want, c.ok)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// smartDeviceFor routes dm/mapper devices through the resolver, and whole-disk/partition through the
|
||||
// regex path unchanged.
|
||||
func TestSmartDeviceFor_DMBranch(t *testing.T) {
|
||||
fixtureSysfs(t, map[string][]string{"dm-1": {"sda3"}}, map[string]string{"dm-1": "pve-root"})
|
||||
if dev, ok := smartDeviceFor("/dev/mapper/pve-root"); !ok || dev != "/dev/sda" {
|
||||
t.Errorf("smartDeviceFor(/dev/mapper/pve-root) = (%q,%v), want (/dev/sda,true)", dev, ok)
|
||||
}
|
||||
// missing sysfs → skip, not a guess
|
||||
fixtureSysfs(t, map[string][]string{}, nil)
|
||||
if _, ok := smartDeviceFor("/dev/dm-9"); ok {
|
||||
t.Error("smartDeviceFor should skip an unresolvable dm device")
|
||||
}
|
||||
}
|
||||
|
||||
func TestContainingMountDevice(t *testing.T) {
|
||||
mounts := []Mount{
|
||||
{Device: "/dev/mapper/pve-root", MountPoint: "/"},
|
||||
{Device: "/dev/sda2", MountPoint: "/boot/efi"},
|
||||
{Device: "/dev/sdb1", MountPoint: "/mnt/usb"},
|
||||
}
|
||||
// A dir inside root resolves to root's device (longest prefix wins over "/").
|
||||
if dev, ok := containingMountDevice(mounts, "/var/lib/vz"); !ok || dev != "/dev/mapper/pve-root" {
|
||||
t.Errorf("containing(/var/lib/vz) = (%q,%v), want /dev/mapper/pve-root", dev, ok)
|
||||
}
|
||||
// A path under a more-specific mount picks that mount, not root.
|
||||
if dev, ok := containingMountDevice(mounts, "/mnt/usb/data"); !ok || dev != "/dev/sdb1" {
|
||||
t.Errorf("containing(/mnt/usb/data) = (%q,%v), want /dev/sdb1", dev, ok)
|
||||
}
|
||||
}
|
||||
|
||||
// Model capture (v0.95.0) — smartctl's model_name flows into SmartSummary; absent → nil.
|
||||
func TestParseSMART_ModelName(t *testing.T) {
|
||||
withModel := parseSMART([]byte(`{"model_name":"TOSHIBA MQ04ABF100","smart_status":{"passed":true}}`))
|
||||
if withModel.ModelName == nil || *withModel.ModelName != "TOSHIBA MQ04ABF100" {
|
||||
t.Errorf("ModelName = %v, want TOSHIBA MQ04ABF100", withModel.ModelName)
|
||||
}
|
||||
if withModel.Health != hub.SmartPassed {
|
||||
t.Errorf("health = %q, want PASSED", withModel.Health)
|
||||
}
|
||||
noModel := parseSMART([]byte(`{"smart_status":{"passed":true}}`))
|
||||
if noModel.ModelName != nil {
|
||||
t.Errorf("absent model_name should be nil, got %v", noModel.ModelName)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user