6b5dade4dc
Live validation caught what the tests could not. On demo-felhom the recipe read namespace "root" with namespace_state "resolved" — confident and wrong, a worse shape than the original defect. mergeConfig overlays the cluster storage config onto the node entry through a hand-listed set of fields and Namespace was not among them. NodeStorage does not return the namespace at all, so PBSNamespace always read "" and latestPBSCoord correctly treated that as the root namespace. Every v0.118.0 test built StorageTarget values directly — including the two through Collector.Collect(), which inject a fakeObserver — so nothing crossed the merge. Two new tests drive the real Observe path with PVE's actual split returns and table the merge itself. Red-proof: dropping the added line fails both. Suite rc=0, 29 packages, 0 FAIL.
358 lines
15 KiB
Go
358 lines
15 KiB
Go
package storage
|
|
|
|
import (
|
|
"context"
|
|
"io"
|
|
"log/slog"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
|
)
|
|
|
|
func quietLogger() *slog.Logger {
|
|
return slog.New(slog.NewTextHandler(io.Discard, nil))
|
|
}
|
|
|
|
// fakeStorageAPI serves fixed cluster + node storage lists.
|
|
type fakeStorageAPI struct {
|
|
node string
|
|
cluster []proxmox.Storage
|
|
nodeSt []proxmox.Storage
|
|
listErr error
|
|
nodeErr error
|
|
}
|
|
|
|
func (f *fakeStorageAPI) Node() string { return f.node }
|
|
func (f *fakeStorageAPI) ListStorage(context.Context) ([]proxmox.Storage, error) {
|
|
return f.cluster, f.listErr
|
|
}
|
|
func (f *fakeStorageAPI) NodeStorage(context.Context) ([]proxmox.Storage, error) {
|
|
return f.nodeSt, f.nodeErr
|
|
}
|
|
|
|
// fakeHostReader is a fully synthetic HostReader — no real devices touched.
|
|
type fakeHostReader struct {
|
|
mounts []Mount
|
|
mountsErr error
|
|
uuids map[string]string // device -> uuid
|
|
exists map[string]bool // device -> present
|
|
rotational map[string]bool // device -> rotational (presence => known)
|
|
removable map[string]bool // device -> removable (presence => known)
|
|
slaves map[string][]string // block NAME -> sysfs slaves (presence => /sys/block dir exists)
|
|
}
|
|
|
|
func (h *fakeHostReader) Mounts() ([]Mount, error) { return h.mounts, h.mountsErr }
|
|
func (h *fakeHostReader) ResolveUUID(device string) (string, bool) {
|
|
u, ok := h.uuids[device]
|
|
return u, ok
|
|
}
|
|
func (h *fakeHostReader) DeviceExists(device string) bool { return h.exists[device] }
|
|
func (h *fakeHostReader) Rotational(device string) (bool, bool) {
|
|
v, ok := h.rotational[device]
|
|
return v, ok
|
|
}
|
|
func (h *fakeHostReader) Removable(device string) (bool, bool) {
|
|
v, ok := h.removable[device]
|
|
return v, ok
|
|
}
|
|
func (h *fakeHostReader) BlockSlaves(name string) ([]string, bool) {
|
|
s, ok := h.slaves[name]
|
|
return s, ok
|
|
}
|
|
|
|
// byName indexes observed targets for assertions.
|
|
func byName(targets []hub.StorageTarget) map[string]hub.StorageTarget {
|
|
m := make(map[string]hub.StorageTarget, len(targets))
|
|
for _, t := range targets {
|
|
m[t.Name] = t
|
|
}
|
|
return m
|
|
}
|
|
|
|
func TestObserve_BuildsTargetsFromProxmoxAndHostReads(t *testing.T) {
|
|
api := &fakeStorageAPI{
|
|
node: "demo-felhom",
|
|
cluster: []proxmox.Storage{
|
|
{Storage: "local", Type: "dir", Content: "vztmpl,backup", Path: "/var/lib/vz"},
|
|
{Storage: "local-lvm", Type: "lvmthin", Content: "rootdir,images", VGName: "pve", ThinPool: "data"},
|
|
{Storage: "usb-backup", Type: "dir", Content: "backup", Path: "/mnt/usb-backup"},
|
|
{Storage: "nfs-arch", Type: "nfs", Content: "backup", Server: "10.0.0.5", Export: "/export/backups"},
|
|
},
|
|
nodeSt: []proxmox.Storage{
|
|
{Storage: "local", Type: "dir", Content: "vztmpl,backup", Path: "/var/lib/vz", Total: 100, Used: 20, Avail: 80, Active: 1, UsedFraction: 0.2},
|
|
{Storage: "local-lvm", Type: "lvmthin", Content: "rootdir,images", Total: 1000, Used: 900, Avail: 100, Active: 1, UsedFraction: 0.9},
|
|
{Storage: "usb-backup", Type: "dir", Content: "backup", Path: "/mnt/usb-backup", Total: 2000, Used: 500, Avail: 1500, Active: 1, UsedFraction: 0.25},
|
|
{Storage: "nfs-arch", Type: "nfs", Content: "backup", Total: 5000, Used: 1000, Avail: 4000, Active: 1, UsedFraction: 0.2},
|
|
},
|
|
}
|
|
host := &fakeHostReader{
|
|
mounts: []Mount{
|
|
{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"},
|
|
{Device: "/dev/sdb1", MountPoint: "/mnt/usb-backup", FSType: "ext4"},
|
|
},
|
|
uuids: map[string]string{"/dev/sdb1": "1111-2222"},
|
|
exists: map[string]bool{"/dev/sdb1": true, "/dev/mapper/pve-root": true},
|
|
rotational: map[string]bool{"/dev/sdb1": true, "/dev/mapper/pve-root": false},
|
|
removable: map[string]bool{"/dev/sdb1": true, "/dev/mapper/pve-root": false},
|
|
}
|
|
|
|
got, err := NewObserver(api, host, nil, quietLogger()).Observe(context.Background())
|
|
if err != nil {
|
|
t.Fatalf("Observe: %v", err)
|
|
}
|
|
if len(got) != 4 {
|
|
t.Fatalf("got %d targets, want 4", len(got))
|
|
}
|
|
m := byName(got)
|
|
|
|
// builtin local: dir within root → type "local", attached via active flag.
|
|
if local := m["local"]; local.Type != hub.StorageTypeLocal || local.State != hub.StorageStateAttached {
|
|
t.Errorf("local = %+v, want type=local state=attached", local)
|
|
}
|
|
|
|
// lvmthin: thin-pool DATA fill surfaced; durable_id = vg/pool; no mount/device.
|
|
lvm := m["local-lvm"]
|
|
if lvm.Type != hub.StorageTypeLVMThin || lvm.DurableID != "pve/data" {
|
|
t.Errorf("local-lvm type/durable = %q/%q", lvm.Type, lvm.DurableID)
|
|
}
|
|
if lvm.ThinPool == nil || lvm.ThinPool.DataUsedFraction != 0.9 {
|
|
t.Errorf("local-lvm thin_pool = %+v, want data_used_fraction=0.9", lvm.ThinPool)
|
|
}
|
|
if lvm.ThinPool.MetadataUsedFraction != nil {
|
|
t.Errorf("metadata fill must be nil in Phase A (lvs is Phase B)")
|
|
}
|
|
|
|
// usb: removable dir, mounted → type usb, durable_id from UUID, class_hint slow (rotational).
|
|
usb := m["usb-backup"]
|
|
if usb.Type != hub.StorageTypeUSB {
|
|
t.Errorf("usb-backup type = %q, want usb", usb.Type)
|
|
}
|
|
if usb.DurableID != "uuid:1111-2222" {
|
|
t.Errorf("usb-backup durable_id = %q, want uuid:1111-2222", usb.DurableID)
|
|
}
|
|
if usb.ClassHint != "slow" {
|
|
t.Errorf("usb-backup class_hint = %q, want slow (rotational)", usb.ClassHint)
|
|
}
|
|
if usb.MountPath != "/mnt/usb-backup" || usb.BackingDevice != "/dev/sdb1" {
|
|
t.Errorf("usb-backup mount/device = %q/%q", usb.MountPath, usb.BackingDevice)
|
|
}
|
|
if usb.State != hub.StorageStateAttached || !usb.Reachable {
|
|
t.Errorf("usb-backup should be attached+reachable: %+v", usb)
|
|
}
|
|
if usb.ThinPool != nil {
|
|
t.Errorf("non-lvmthin must omit thin_pool")
|
|
}
|
|
|
|
// nfs: durable_id = server:export; attached via active flag; no class hint.
|
|
nfs := m["nfs-arch"]
|
|
if nfs.Type != hub.StorageTypeNFS || nfs.DurableID != "10.0.0.5:/export/backups" {
|
|
t.Errorf("nfs-arch type/durable = %q/%q", nfs.Type, nfs.DurableID)
|
|
}
|
|
if nfs.ClassHint != "" {
|
|
t.Errorf("network target must have no class hint, got %q", nfs.ClassHint)
|
|
}
|
|
|
|
// SMART is UNKNOWN in Phase A for every target.
|
|
for _, tgt := range got {
|
|
if tgt.Smart.Health != hub.SmartUnknown {
|
|
t.Errorf("%s smart health = %q, want UNKNOWN in Phase A", tgt.Name, tgt.Smart.Health)
|
|
}
|
|
}
|
|
}
|
|
|
|
// fakeHostOps fills SMART + thin-pool metadata for the enrichment test.
|
|
type fakeHostOps struct {
|
|
smartByDevice map[string]hub.SmartSummary
|
|
metaByPool map[string]float64 // "vg/pool" -> fraction
|
|
smartDevices []string // records which devices SMART was called on
|
|
}
|
|
|
|
func (f *fakeHostOps) EnsureMount(context.Context, MountSpec) error { return nil }
|
|
func (f *fakeHostOps) Unmount(context.Context, string) error { return nil }
|
|
func (f *fakeHostOps) SMART(_ context.Context, device string) (hub.SmartSummary, error) {
|
|
f.smartDevices = append(f.smartDevices, device)
|
|
if s, ok := f.smartByDevice[device]; ok {
|
|
return s, nil
|
|
}
|
|
return hub.SmartSummary{Health: hub.SmartUnknown}, nil
|
|
}
|
|
func (f *fakeHostOps) ThinPoolMetadata(_ context.Context, vg, pool string) (float64, bool) {
|
|
v, ok := f.metaByPool[vg+"/"+pool]
|
|
return v, ok
|
|
}
|
|
func (f *fakeHostOps) InspectDevice(_ context.Context, device string) (DeviceProbe, error) {
|
|
return DeviceProbe{Device: device, Probed: true}, nil
|
|
}
|
|
func (f *fakeHostOps) Format(context.Context, string, string) error { return nil }
|
|
|
|
func TestObserve_EnrichesSMARTAndThinPoolMetadata(t *testing.T) {
|
|
api := &fakeStorageAPI{
|
|
node: "demo-felhom",
|
|
cluster: []proxmox.Storage{
|
|
{Storage: "local-lvm", Type: "lvmthin", VGName: "pve", ThinPool: "data"},
|
|
{Storage: "usb-backup", Type: "dir", Path: "/mnt/usb-backup"},
|
|
},
|
|
nodeSt: []proxmox.Storage{
|
|
{Storage: "local-lvm", Type: "lvmthin", Active: 1, UsedFraction: 0.4},
|
|
{Storage: "usb-backup", Type: "dir", Path: "/mnt/usb-backup", Active: 1},
|
|
},
|
|
}
|
|
host := &fakeHostReader{
|
|
mounts: []Mount{{Device: "/dev/sdb1", MountPoint: "/mnt/usb-backup", FSType: "ext4"}},
|
|
uuids: map[string]string{"/dev/sdb1": "1111-2222"},
|
|
exists: map[string]bool{"/dev/sdb1": true},
|
|
removable: map[string]bool{"/dev/sdb1": true},
|
|
}
|
|
ops := &fakeHostOps{
|
|
smartByDevice: map[string]hub.SmartSummary{"/dev/sdb": {Health: hub.SmartPassed}},
|
|
metaByPool: map[string]float64{"pve/data": 0.12},
|
|
}
|
|
got, err := NewObserver(api, host, ops, quietLogger()).Observe(context.Background())
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
m := byName(got)
|
|
|
|
// SMART runs on the WHOLE disk (/dev/sdb), not the partition (/dev/sdb1).
|
|
if len(ops.smartDevices) != 1 || ops.smartDevices[0] != "/dev/sdb" {
|
|
t.Errorf("SMART should target the whole disk /dev/sdb, got %v", ops.smartDevices)
|
|
}
|
|
if m["usb-backup"].Smart.Health != hub.SmartPassed {
|
|
t.Errorf("usb SMART not enriched: %+v", m["usb-backup"].Smart)
|
|
}
|
|
// lvmthin metadata fill (Phase B) is now populated.
|
|
lvm := m["local-lvm"]
|
|
if lvm.ThinPool == nil || lvm.ThinPool.MetadataUsedFraction == nil {
|
|
t.Fatalf("lvmthin metadata fill not enriched: %+v", lvm.ThinPool)
|
|
}
|
|
if *lvm.ThinPool.MetadataUsedFraction != 0.12 {
|
|
t.Errorf("metadata fraction = %v, want 0.12", *lvm.ThinPool.MetadataUsedFraction)
|
|
}
|
|
}
|
|
|
|
func TestObserve_USBUnpluggedIsDisconnected(t *testing.T) {
|
|
api := &fakeStorageAPI{
|
|
node: "demo-felhom",
|
|
cluster: []proxmox.Storage{
|
|
{Storage: "usb-backup", Type: "dir", Content: "backup", Path: "/mnt/usb-backup"},
|
|
},
|
|
nodeSt: []proxmox.Storage{
|
|
// PVE may still show the storage entry (active possibly 1) but the mount is gone.
|
|
{Storage: "usb-backup", Type: "dir", Content: "backup", Path: "/mnt/usb-backup", Active: 1},
|
|
},
|
|
}
|
|
host := &fakeHostReader{
|
|
mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, // no /mnt/usb-backup
|
|
}
|
|
got, err := NewObserver(api, host, nil, quietLogger()).Observe(context.Background())
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
usb := byName(got)["usb-backup"]
|
|
// Without its own mount, a USB target is unplugged regardless of PVE's stale active flag.
|
|
if usb.State != hub.StorageStateDisconnected || usb.Reachable {
|
|
t.Errorf("unplugged usb should be disconnected/unreachable, got state=%q reachable=%v", usb.State, usb.Reachable)
|
|
}
|
|
// Its durable_id falls back to a stable form (no UUID resolvable while detached).
|
|
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:<root-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) {
|
|
api := &fakeStorageAPI{node: "n", listErr: context.DeadlineExceeded}
|
|
if _, err := NewObserver(api, &fakeHostReader{}, nil, quietLogger()).Observe(context.Background()); err == nil {
|
|
t.Fatal("a Proxmox read error must surface (the collector then omits storage this cycle)")
|
|
}
|
|
}
|
|
|
|
func TestObserve_MountReadFailureDegradesNotFatal(t *testing.T) {
|
|
api := &fakeStorageAPI{
|
|
node: "n",
|
|
cluster: []proxmox.Storage{{Storage: "local-lvm", Type: "lvmthin", VGName: "pve", ThinPool: "data"}},
|
|
nodeSt: []proxmox.Storage{{Storage: "local-lvm", Type: "lvmthin", Active: 1, UsedFraction: 0.1}},
|
|
}
|
|
host := &fakeHostReader{mountsErr: io.ErrUnexpectedEOF}
|
|
got, err := NewObserver(api, host, nil, quietLogger()).Observe(context.Background())
|
|
if err != nil {
|
|
t.Fatalf("a host mount-read failure must degrade, not fail: %v", err)
|
|
}
|
|
if len(got) != 1 || got[0].DurableID != "pve/data" {
|
|
t.Errorf("lvmthin still derivable without mounts: %+v", got)
|
|
}
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------------------------
|
|
// R-106 — the pbs namespace must survive the cluster/node merge.
|
|
// ---------------------------------------------------------------------------------------------
|
|
|
|
// TestObserve_CarriesPBSNamespaceThroughMerge pins the gap that shipped in agent v0.118.0 and was caught
|
|
// by LIVE VALIDATION rather than by tests: `mergeConfig` copies a hand-listed set of type-specific fields
|
|
// from the CLUSTER config onto the NODE entry, and `Namespace` was not on that list. `NodeStorage` does
|
|
// not return the namespace at all — it is cluster-config only — so `StorageTarget.PBSNamespace` was
|
|
// always empty and the DR recipe reported the root namespace on every per-customer box, exactly the
|
|
// R-106 symptom the fix was supposed to remove.
|
|
//
|
|
// The DR-recipe tests could not catch it: they construct StorageTarget values directly, so nothing
|
|
// crossed this merge. This test drives the REAL Observe path with the split PVE returns reproduced —
|
|
// namespace present in the cluster list, absent from the node list, which is what PVE actually does.
|
|
func TestObserve_CarriesPBSNamespaceThroughMerge(t *testing.T) {
|
|
api := &fakeStorageAPI{
|
|
node: "demo-felhom",
|
|
// Cluster config: carries the type-specific fields, as /storage does.
|
|
cluster: []proxmox.Storage{{
|
|
Storage: "felhom-pbs", Type: "pbs", Content: "backup",
|
|
Server: "10.77.0.1", Datastore: "felhom-offsite", Namespace: "demo-felhom",
|
|
}},
|
|
// Node entry: live usage + active flag, and NO namespace — the shape that made the bug invisible.
|
|
nodeSt: []proxmox.Storage{{
|
|
Storage: "felhom-pbs", Type: "pbs", Content: "backup",
|
|
Total: 100, Used: 10, Avail: 90, Active: 1, Enabled: 1,
|
|
}},
|
|
}
|
|
o := NewObserver(api, &fakeHostReader{}, nil, quietLogger())
|
|
|
|
targets, err := o.Observe(context.Background())
|
|
if err != nil {
|
|
t.Fatalf("Observe: %v", err)
|
|
}
|
|
if len(targets) != 1 {
|
|
t.Fatalf("want 1 target, got %d", len(targets))
|
|
}
|
|
if got := targets[0].PBSNamespace; got != "demo-felhom" {
|
|
t.Errorf("PBSNamespace=%q, want %q — the namespace was lost in mergeConfig, so the DR recipe "+
|
|
"reports the root namespace on a per-customer box (R-106)", got, "demo-felhom")
|
|
}
|
|
}
|
|
|
|
// TestMergeConfig_CarriesPBSNamespace is the direct table over the merge itself: a node entry that omits
|
|
// a field takes the cluster's value, and a node entry that HAS one keeps its own (never clobbered).
|
|
func TestMergeConfig_CarriesPBSNamespace(t *testing.T) {
|
|
cluster := proxmox.Storage{Storage: "felhom-pbs", Type: "pbs", Namespace: "demo-hp", Datastore: "felhom-offsite"}
|
|
|
|
// Node omits the namespace (the real PVE shape) → it must be filled from the cluster config.
|
|
if got := mergeConfig(proxmox.Storage{Storage: "felhom-pbs"}, cluster).Namespace; got != "demo-hp" {
|
|
t.Errorf("namespace absent on the node entry: got %q, want it merged from the cluster config", got)
|
|
}
|
|
// Node already has one → keep it (the merge is fill-if-empty, never overwrite).
|
|
nodeOwn := proxmox.Storage{Storage: "felhom-pbs", Namespace: "node-wins"}
|
|
if got := mergeConfig(nodeOwn, cluster).Namespace; got != "node-wins" {
|
|
t.Errorf("merge clobbered the node's own namespace: got %q", got)
|
|
}
|
|
// No cluster row at all → the node entry passes through untouched.
|
|
if got := mergeConfig(proxmox.Storage{Storage: "x", Namespace: "keep"}, proxmox.Storage{}).Namespace; got != "keep" {
|
|
t.Errorf("empty cluster row altered the node entry: got %q", got)
|
|
}
|
|
}
|