diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index a0752e604..3a4ab82f4 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -3050,9 +3050,10 @@ func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incomin continue } - identityMatch := - (incomingSerial != "" && strings.EqualFold(incomingSerial, existing.PhysicalDisk.Serial)) || - (incomingWWN != "" && strings.EqualFold(incomingWWN, existing.PhysicalDisk.WWN)) + identityMatch := diskinventory.HardwareIdentityMatch( + incomingSerial, incomingWWN, + existing.PhysicalDisk.Serial, existing.PhysicalDisk.WWN, + ) existingDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(existing.PhysicalDisk.DevPath)) agentReportsSAS := (source == SourceProxmox && strings.EqualFold(existing.PhysicalDisk.DiskType, "sas")) || (source == SourceAgent && strings.EqualFold(incoming.PhysicalDisk.DiskType, "sas")) diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index 38a5c1ef5..fbf1b9c10 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -6296,3 +6296,116 @@ func TestIssue1720AgentControllerMembersInheritOwningProxmoxScope(t *testing.T) t.Fatalf("controller members collapsed or disappeared: %v", targets) } } + +// Follow-up regression on issue #1720: the PVE disks/list poll and the agent +// SMART report both observe the same RAID array volume, but PVE surfaces the +// NAA identifier as a bare-hex serial while smartctl reports it as a +// naa.-prefixed WWN with no serial at all. The two observations must join into +// one resource instead of rendering the volume twice with split histories. +// Sibling volumes on one controller share their truncated udev ID_WWN +// (0x + leading 16 hex), so the join must never match on truncated forms. +func TestIssue1720ArrayVolumeMergesBareSerialWithPrefixedWWN(t *testing.T) { + now := time.Date(2026, 8, 27, 10, 0, 0, 0, time.UTC) + registry := NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{ + Nodes: []models.Node{{ + ID: "cluster-a-indus", + Name: "indus", + Instance: "cluster-a", + ClusterName: "cluster-a", + LinkedAgentID: "host-1", + Status: "online", + LastSeen: now, + }}, + PhysicalDisks: []models.PhysicalDisk{ + { + ID: "cluster-a-indus--dev-sda", + Node: "indus", + Instance: "cluster-a", + DevPath: "/dev/sda", + Model: "PERC_H730_Mini", + Serial: "61866da053481f002f58a43b22f964a7", + WWN: "0x61866da053481f00", + Type: "sata", + Health: "UNKNOWN", + Size: 479_559_942_144, + LastChecked: now, + }, + { + ID: "cluster-a-indus--dev-sdb", + Node: "indus", + Instance: "cluster-a", + DevPath: "/dev/sdb", + Model: "PERC_H730_Mini", + Serial: "61866da053481f0030543ecb1d3b4cca", + WWN: "0x61866da053481f00", + Type: "sata", + Health: "UNKNOWN", + Size: 3_000_034_656_256, + LastChecked: now, + }, + }, + Hosts: []models.Host{{ + ID: "host-1", + Hostname: "indus", + LinkedNodeID: "cluster-a-indus", + Status: "online", + LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{ + { + Device: "sda", + Model: "PERC H730 Mini", + WWN: "naa.61866da053481f002f58a43b22f964a7", + Type: "sata", + Controller: "0000:02:00.0", + Target: "0:2:0:0", + SizeBytes: 479_559_942_144, + Health: "UNKNOWN", + Standby: true, + }, + { + Device: "sdb", + Model: "PERC H730 Mini", + WWN: "naa.61866da053481f0030543ecb1d3b4cca", + Type: "sata", + Controller: "0000:02:00.0", + Target: "0:2:1:0", + SizeBytes: 3_000_034_656_256, + Health: "UNKNOWN", + Standby: true, + }, + }}, + }}, + }) + + disks := registry.ListByType(ResourceTypePhysicalDisk) + if len(disks) != 2 { + devs := make([]string, 0, len(disks)) + for _, disk := range disks { + if disk.PhysicalDisk != nil { + devs = append(devs, disk.PhysicalDisk.DevPath) + } + } + t.Fatalf("physical disk resources = %d (%v), want 2 merged volumes", len(disks), devs) + } + + byPath := make(map[string]Resource, len(disks)) + for _, disk := range disks { + if disk.PhysicalDisk == nil { + t.Fatalf("disk %q has no physical-disk facet", disk.ID) + } + byPath[disk.PhysicalDisk.DevPath] = disk + } + for _, devPath := range []string{"/dev/sda", "/dev/sdb"} { + disk, ok := byPath[devPath] + if !ok { + t.Fatalf("no merged resource kept canonical devPath %q: %v", devPath, byPath) + } + if !containsDataSource(disk.Sources, SourceAgent) || !containsDataSource(disk.Sources, SourceProxmox) { + t.Fatalf("disk %q sources = %v, want merged proxmox+agent", devPath, disk.Sources) + } + } + if byPath["/dev/sda"].PhysicalDisk.SizeBytes == byPath["/dev/sdb"].PhysicalDisk.SizeBytes { + t.Fatalf("sibling volumes collapsed into one identity: %+v", byPath) + } +} diff --git a/pkg/diskinventory/identity.go b/pkg/diskinventory/identity.go index 62d47ac42..36ef7cba6 100644 --- a/pkg/diskinventory/identity.go +++ b/pkg/diskinventory/identity.go @@ -66,6 +66,54 @@ func IsUsableHardwareID(value string) bool { return !allZero && !allF } +// normalizeHardwareID canonicalizes a serial or WWN for cross-source +// comparison. Reporters disagree on framing, not identity: smartctl emits +// naa./eui.-prefixed WWNs, udev emits wwn-0x tokens, and PVE surfaces bare +// hex. Values are only ever prefix-stripped and case-folded, never truncated: +// sibling volumes on one RAID controller share their leading WWN bytes, so a +// truncated form must stay unequal to the full identifier. +func normalizeHardwareID(value string) string { + value = strings.ToLower(strings.TrimSpace(value)) + if !IsUsableHardwareID(value) { + return "" + } + for { + trimmed := value + for _, prefix := range []string{"naa.", "eui.", "wwn-", "0x"} { + trimmed = strings.TrimPrefix(trimmed, prefix) + } + if trimmed == value { + break + } + value = trimmed + } + if !IsUsableHardwareID(value) { + return "" + } + return value +} + +// HardwareIdentityMatch reports whether two disk observations carry the same +// stable hardware identity. Serial and WWN are folded together because +// sources disagree on which field holds the durable identifier: PVE reports a +// RAID array volume's NAA identifier as its serial while smartctl reports the +// same value as a naa.-prefixed WWN with no serial at all. +func HardwareIdentityMatch(leftSerial, leftWWN, rightSerial, rightWWN string) bool { + left := [2]string{normalizeHardwareID(leftSerial), normalizeHardwareID(leftWWN)} + right := [2]string{normalizeHardwareID(rightSerial), normalizeHardwareID(rightWWN)} + for _, l := range left { + if l == "" { + continue + } + for _, r := range right { + if r != "" && l == r { + return true + } + } + } + return false +} + // IsControllerMemberTarget reports whether target addresses one member behind // a shared controller block path. Controller grammars vary after the numeric // member prefix (for example megaraid,7, areca,1/1, and sssraid,0,1). diff --git a/pkg/diskinventory/identity_test.go b/pkg/diskinventory/identity_test.go index b130141c7..f3477f738 100644 --- a/pkg/diskinventory/identity_test.go +++ b/pkg/diskinventory/identity_test.go @@ -54,3 +54,74 @@ func TestPreferredIDRejectsPlaceholderHardwareIdentity(t *testing.T) { t.Fatal("real disk serial/WWN was rejected") } } + +func TestHardwareIdentityMatch(t *testing.T) { + cases := []struct { + name string + aSerial, aWWN string + bSerial, bWWN string + want bool + }{ + { + name: "pve bare-hex serial matches smartctl naa wwn", + aSerial: "61866da053481f002f58a43b22f964a7", + aWWN: "0x61866da053481f00", + bWWN: "naa.61866da053481f002f58a43b22f964a7", + want: true, + }, + { + name: "same serial different case", + aSerial: "zr5dlayj", + bSerial: "ZR5DLAYJ", + want: true, + }, + { + name: "udev wwn-0x token matches naa wwn", + aWWN: "wwn-0x5000c500abcdef01", + bWWN: "naa.5000c500abcdef01", + want: true, + }, + { + name: "eui prefix matches bare nvme id", + aWWN: "eui.0025385b91501234", + bWWN: "0025385b91501234", + want: true, + }, + { + name: "truncated udev wwn never matches full sibling identifier", + aWWN: "0x61866da053481f00", + bSerial: "61866da053481f0030543ecb1d3b4cca", + bWWN: "naa.61866da053481f0030543ecb1d3b4cca", + want: false, + }, + { + name: "placeholder serials do not match each other", + aSerial: "UNKNOWN", + bSerial: "UNKNOWN", + want: false, + }, + { + name: "distinct disks stay distinct", + aSerial: "9410A0FWFVL9", + bSerial: "35C0A39YFVL9", + want: false, + }, + { + name: "empty observations never match", + want: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := HardwareIdentityMatch(tc.aSerial, tc.aWWN, tc.bSerial, tc.bWWN) + if got != tc.want { + t.Fatalf("HardwareIdentityMatch(%q,%q,%q,%q) = %v, want %v", + tc.aSerial, tc.aWWN, tc.bSerial, tc.bWWN, got, tc.want) + } + mirrored := HardwareIdentityMatch(tc.bSerial, tc.bWWN, tc.aSerial, tc.aWWN) + if mirrored != tc.want { + t.Fatalf("match is not symmetric: mirrored = %v, want %v", mirrored, tc.want) + } + }) + } +}