mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-07-09 16:00:59 +00:00
fix(disks): report authoritative disk size and namespace devpath from the host agent
On a Proxmox node, physical disks collected by the host agent were keyed by the NVMe controller (e.g. "nvme0 [nvme]") instead of the namespace, reported sizeBytes 0 (or a stale filesystem-usage value), and flickered as the agent reading intermittently replaced the authoritative Proxmox disks/list reading. Root causes: - smartctl --scan-open reports NVMe disks by their controller char device (/dev/nvme0), and that scan label became the reported devPath. - DiskSMART carried no capacity, so the server backfilled size by matching the SMART device against host filesystem-usage entries, which never match a whole partitioned/LVM/ZFS disk, leaving size 0. - The unified-resource merge let the agent's controller label overwrite the canonical Proxmox /dev/... devPath. Fixes: - The agent now reports the canonical block device (an NVMe controller resolves to its namespace) and the authoritative capacity from /sys/block, with the smartctl user_capacity / nvme_total_capacity as a cross-platform fallback. Disks behind multiplexing controllers (megaraid, cciss, areca) keep their disambiguating label and smartctl-reported size. - SizeBytes flows through the agent report, host model, and adapter; the filesystem-usage match is demoted to a legacy fallback. - The merge keeps a canonical /dev/<device> devPath and never downgrades it to a scan label, so an un-updated agent can no longer corrupt Proxmox data. Refs #1483.
This commit is contained in:
parent
42105ec6e5
commit
bd20069c60
10 changed files with 554 additions and 30 deletions
|
|
@ -1388,6 +1388,7 @@ func (a *Agent) collectSMARTData(ctx context.Context, diskExclude []string) []ag
|
|||
Serial: disk.Serial,
|
||||
WWN: disk.WWN,
|
||||
Type: disk.Type,
|
||||
SizeBytes: disk.SizeBytes,
|
||||
Temperature: disk.Temperature,
|
||||
Health: disk.Health,
|
||||
Standby: disk.Standby,
|
||||
|
|
|
|||
|
|
@ -42,14 +42,15 @@ var (
|
|||
|
||||
// DiskSMART represents S.M.A.R.T. data for a single disk.
|
||||
type DiskSMART struct {
|
||||
Device string `json:"device"` // Device path (e.g., /dev/sda)
|
||||
Model string `json:"model,omitempty"` // Disk model
|
||||
Serial string `json:"serial,omitempty"` // Serial number
|
||||
WWN string `json:"wwn,omitempty"` // World Wide Name
|
||||
Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme
|
||||
Temperature int `json:"temperature"` // Temperature in Celsius
|
||||
Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN
|
||||
Standby bool `json:"standby,omitempty"` // True if disk was in standby
|
||||
Device string `json:"device"` // Block device name (e.g., sda, nvme0n1)
|
||||
Model string `json:"model,omitempty"` // Disk model
|
||||
Serial string `json:"serial,omitempty"` // Serial number
|
||||
WWN string `json:"wwn,omitempty"` // World Wide Name
|
||||
Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme
|
||||
SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown)
|
||||
Temperature int `json:"temperature"` // Temperature in Celsius
|
||||
Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN
|
||||
Standby bool `json:"standby,omitempty"` // True if disk was in standby
|
||||
Attributes *SMARTAttributes `json:"attributes,omitempty"`
|
||||
LastUpdated time.Time `json:"lastUpdated"` // When this reading was taken
|
||||
}
|
||||
|
|
@ -157,7 +158,11 @@ type smartctlJSON struct {
|
|||
OUI uint64 `json:"oui"`
|
||||
ID uint64 `json:"id"`
|
||||
} `json:"wwn"`
|
||||
SmartStatus *struct {
|
||||
UserCapacity struct {
|
||||
Bytes int64 `json:"bytes"`
|
||||
} `json:"user_capacity"`
|
||||
NVMeTotalCapacity int64 `json:"nvme_total_capacity"`
|
||||
SmartStatus *struct {
|
||||
Passed bool `json:"passed"`
|
||||
} `json:"smart_status,omitempty"`
|
||||
Temperature struct {
|
||||
|
|
@ -250,6 +255,7 @@ func CollectSMARTLocal(ctx context.Context, diskExclude []string) ([]DiskSMART,
|
|||
continue
|
||||
}
|
||||
if smart != nil {
|
||||
refineLinuxBlockDeviceIdentity(smart, target)
|
||||
results = append(results, *smart)
|
||||
}
|
||||
}
|
||||
|
|
@ -710,6 +716,123 @@ func isFreeBSDDiskDeviceName(name string) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
// refineLinuxBlockDeviceIdentity rewrites a freshly collected SMART reading so
|
||||
// that its device identity and size reflect the underlying block device rather
|
||||
// than the smartctl scan target. smartctl --scan-open reports NVMe disks by their
|
||||
// controller char device (/dev/nvme0), but the stable, user-visible identity is
|
||||
// the namespace block device (/dev/nvme0n1) — the same name Proxmox's disks/list
|
||||
// and /sys/block expose. It also backfills the capacity from /sys/block, the
|
||||
// authoritative size source, so the agent no longer depends on a fragile
|
||||
// filesystem-usage match on the server side.
|
||||
func refineLinuxBlockDeviceIdentity(smart *DiskSMART, target smartctlTarget) {
|
||||
if smart == nil || runtimeGOOS != "linux" {
|
||||
return
|
||||
}
|
||||
// Disks addressed behind a multiplexing controller (megaraid,7; cciss,1;
|
||||
// areca,1/1; ...) all share a single /dev path, so the smartctl scan label is
|
||||
// the only thing that disambiguates them and /sys/block describes the array,
|
||||
// not the member. Leave those as-is and trust the smartctl-reported capacity.
|
||||
if isMultiplexedDeviceType(target.DeviceType) {
|
||||
return
|
||||
}
|
||||
block := canonicalBlockDeviceForScanPath(target.Path)
|
||||
if block == "" {
|
||||
return
|
||||
}
|
||||
smart.Device = block
|
||||
if size := blockDeviceSizeBytes(block); size > 0 {
|
||||
smart.SizeBytes = size
|
||||
}
|
||||
}
|
||||
|
||||
// isMultiplexedDeviceType reports whether a smartctl -d type addresses a member
|
||||
// disk behind a controller (e.g. "megaraid,7"), as opposed to a directly
|
||||
// attached device ("", "sat", "nvme", "scsi", "sat,auto").
|
||||
func isMultiplexedDeviceType(deviceType string) bool {
|
||||
idx := strings.IndexByte(deviceType, ',')
|
||||
if idx < 0 || idx+1 >= len(deviceType) {
|
||||
return false
|
||||
}
|
||||
next := deviceType[idx+1]
|
||||
return next >= '0' && next <= '9'
|
||||
}
|
||||
|
||||
// canonicalBlockDeviceForScanPath maps a smartctl scan target to its canonical
|
||||
// /sys/block device name. NVMe controllers (nvmeN) resolve to their first
|
||||
// namespace (nvmeNnM); every other device keeps its basename.
|
||||
func canonicalBlockDeviceForScanPath(scanPath string) string {
|
||||
name := filepath.Base(strings.TrimSpace(scanPath))
|
||||
if name == "" || name == "." || name == string(filepath.Separator) {
|
||||
return ""
|
||||
}
|
||||
if isNVMeControllerName(name) {
|
||||
if ns := firstNVMeNamespace(name); ns != "" {
|
||||
return ns
|
||||
}
|
||||
}
|
||||
return name
|
||||
}
|
||||
|
||||
// isNVMeControllerName reports whether name is an NVMe controller char device
|
||||
// (e.g. "nvme0") rather than a namespace block device (e.g. "nvme0n1").
|
||||
func isNVMeControllerName(name string) bool {
|
||||
return hasNumericSuffix(name, "nvme")
|
||||
}
|
||||
|
||||
// firstNVMeNamespace returns the lowest-numbered namespace block device for an
|
||||
// NVMe controller (e.g. "nvme0" -> "nvme0n1"), or "" when none is found.
|
||||
func firstNVMeNamespace(controller string) string {
|
||||
entries, err := readDir("/sys/block")
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
prefix := controller + "n"
|
||||
best := ""
|
||||
for _, entry := range entries {
|
||||
name := strings.TrimSpace(entry.Name())
|
||||
if !strings.HasPrefix(name, prefix) {
|
||||
continue
|
||||
}
|
||||
// Require a pure namespace (nvme0n1), not a partition (nvme0n1p1).
|
||||
if suffix := name[len(prefix):]; suffix == "" || !isAllDigits(suffix) {
|
||||
continue
|
||||
}
|
||||
if best == "" || name < best {
|
||||
best = name
|
||||
}
|
||||
}
|
||||
return best
|
||||
}
|
||||
|
||||
// blockDeviceSizeBytes reads /sys/block/<name>/size, which the kernel always
|
||||
// reports in 512-byte sectors regardless of the physical block size.
|
||||
func blockDeviceSizeBytes(name string) int64 {
|
||||
if name == "" {
|
||||
return 0
|
||||
}
|
||||
data, err := smartctlReadFile(filepath.Join("/sys/block", name, "size"))
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
sectors, err := strconv.ParseInt(strings.TrimSpace(string(data)), 10, 64)
|
||||
if err != nil || sectors <= 0 {
|
||||
return 0
|
||||
}
|
||||
return sectors * 512
|
||||
}
|
||||
|
||||
func isAllDigits(s string) bool {
|
||||
if s == "" {
|
||||
return false
|
||||
}
|
||||
for _, r := range s {
|
||||
if r < '0' || r > '9' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func hasNumericSuffix(name, prefix string) bool {
|
||||
if !strings.HasPrefix(name, prefix) || len(name) == len(prefix) {
|
||||
return false
|
||||
|
|
@ -1002,6 +1125,15 @@ func parseSMARTOutput(output []byte, target smartctlTarget) (*DiskSMART, error)
|
|||
result.WWN = formatWWN(smartData.WWN.NAA, smartData.WWN.OUI, smartData.WWN.ID)
|
||||
}
|
||||
|
||||
// Capacity straight from the device smartctl just queried. On Linux this is
|
||||
// refined to the authoritative /sys/block value in CollectSMARTLocal; here it
|
||||
// is the cross-platform fallback so non-Linux hosts still report a size.
|
||||
if smartData.NVMeTotalCapacity > 0 {
|
||||
result.SizeBytes = smartData.NVMeTotalCapacity
|
||||
} else if smartData.UserCapacity.Bytes > 0 {
|
||||
result.SizeBytes = smartData.UserCapacity.Bytes
|
||||
}
|
||||
|
||||
if smartData.Temperature.Current > 0 {
|
||||
result.Temperature = smartData.Temperature.Current
|
||||
} else if smartData.NVMeSmartHealthInformationLog != nil && smartData.NVMeSmartHealthInformationLog.Temperature > 0 {
|
||||
|
|
|
|||
211
internal/hostagent/smartctl_size_test.go
Normal file
211
internal/hostagent/smartctl_size_test.go
Normal file
|
|
@ -0,0 +1,211 @@
|
|||
package hostagent
|
||||
|
||||
import (
|
||||
"io/fs"
|
||||
"os"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestParseSMARTOutputParsesNVMeCapacity(t *testing.T) {
|
||||
output := []byte(`{
|
||||
"device": {"name": "/dev/nvme0", "type": "nvme", "protocol": "NVMe"},
|
||||
"model_name": "KINGSTON SNV3S2000G",
|
||||
"serial_number": "50026B7282A0FB69",
|
||||
"nvme_total_capacity": 2000398934016,
|
||||
"smart_status": {"passed": true},
|
||||
"temperature": {"current": 37}
|
||||
}`)
|
||||
|
||||
result, err := parseSMARTOutput(output, smartctlTarget{Path: "/dev/nvme0", DeviceType: "nvme"})
|
||||
if err != nil {
|
||||
t.Fatalf("parseSMARTOutput returned error: %v", err)
|
||||
}
|
||||
if result.SizeBytes != 2000398934016 {
|
||||
t.Errorf("SizeBytes = %d, want 2000398934016", result.SizeBytes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseSMARTOutputParsesUserCapacity(t *testing.T) {
|
||||
output := []byte(`{
|
||||
"device": {"name": "/dev/sda", "type": "sat", "protocol": "ATA"},
|
||||
"model_name": "INTEL SSDSC2BW240A4",
|
||||
"serial_number": "CVDA000000000000",
|
||||
"user_capacity": {"bytes": 240057409536},
|
||||
"smart_status": {"passed": true},
|
||||
"temperature": {"current": 30}
|
||||
}`)
|
||||
|
||||
result, err := parseSMARTOutput(output, smartctlTarget{Path: "/dev/sda", DeviceType: "sat"})
|
||||
if err != nil {
|
||||
t.Fatalf("parseSMARTOutput returned error: %v", err)
|
||||
}
|
||||
if result.SizeBytes != 240057409536 {
|
||||
t.Errorf("SizeBytes = %d, want 240057409536", result.SizeBytes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsNVMeControllerName(t *testing.T) {
|
||||
cases := map[string]bool{
|
||||
"nvme0": true,
|
||||
"nvme11": true,
|
||||
"nvme0n1": false, // namespace, not controller
|
||||
"nvme0n1p1": false, // partition
|
||||
"sda": false,
|
||||
"nvme": false,
|
||||
"": false,
|
||||
}
|
||||
for name, want := range cases {
|
||||
if got := isNVMeControllerName(name); got != want {
|
||||
t.Errorf("isNVMeControllerName(%q) = %v, want %v", name, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanonicalBlockDeviceForScanPath(t *testing.T) {
|
||||
origReadDir := readDir
|
||||
t.Cleanup(func() { readDir = origReadDir })
|
||||
|
||||
readDir = func(string) ([]os.DirEntry, error) {
|
||||
return []os.DirEntry{
|
||||
fakeDirEntry{name: "nvme0n1"},
|
||||
fakeDirEntry{name: "nvme0n1p1"}, // partition must be ignored
|
||||
fakeDirEntry{name: "nvme1n1"},
|
||||
fakeDirEntry{name: "sda"},
|
||||
}, nil
|
||||
}
|
||||
|
||||
cases := map[string]string{
|
||||
"/dev/nvme0": "nvme0n1", // controller resolves to its namespace
|
||||
"/dev/nvme1": "nvme1n1",
|
||||
"/dev/nvme0n1": "nvme0n1", // already a namespace
|
||||
"/dev/sda": "sda",
|
||||
}
|
||||
for scanPath, want := range cases {
|
||||
if got := canonicalBlockDeviceForScanPath(scanPath); got != want {
|
||||
t.Errorf("canonicalBlockDeviceForScanPath(%q) = %q, want %q", scanPath, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanonicalBlockDeviceControllerWithoutNamespaceKeepsName(t *testing.T) {
|
||||
origReadDir := readDir
|
||||
t.Cleanup(func() { readDir = origReadDir })
|
||||
|
||||
readDir = func(string) ([]os.DirEntry, error) {
|
||||
return []os.DirEntry{fakeDirEntry{name: "nvme0n1p1"}}, nil // only a partition exists
|
||||
}
|
||||
|
||||
if got := canonicalBlockDeviceForScanPath("/dev/nvme0"); got != "nvme0" {
|
||||
t.Errorf("canonicalBlockDeviceForScanPath(/dev/nvme0) = %q, want fallback nvme0", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBlockDeviceSizeBytes(t *testing.T) {
|
||||
origReadFile := smartctlReadFile
|
||||
t.Cleanup(func() { smartctlReadFile = origReadFile })
|
||||
|
||||
smartctlReadFile = func(path string) ([]byte, error) {
|
||||
switch path {
|
||||
case "/sys/block/nvme0n1/size":
|
||||
return []byte("3907029168\n"), nil
|
||||
case "/sys/block/sda/size":
|
||||
return []byte("468862128\n"), nil
|
||||
default:
|
||||
return nil, fs.ErrNotExist
|
||||
}
|
||||
}
|
||||
|
||||
if got := blockDeviceSizeBytes("nvme0n1"); got != 2000398934016 {
|
||||
t.Errorf("blockDeviceSizeBytes(nvme0n1) = %d, want 2000398934016", got)
|
||||
}
|
||||
if got := blockDeviceSizeBytes("sda"); got != 240057409536 {
|
||||
t.Errorf("blockDeviceSizeBytes(sda) = %d, want 240057409536", got)
|
||||
}
|
||||
if got := blockDeviceSizeBytes("missing"); got != 0 {
|
||||
t.Errorf("blockDeviceSizeBytes(missing) = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRefineLinuxBlockDeviceIdentity(t *testing.T) {
|
||||
origGOOS := runtimeGOOS
|
||||
origReadDir := readDir
|
||||
origReadFile := smartctlReadFile
|
||||
t.Cleanup(func() {
|
||||
runtimeGOOS = origGOOS
|
||||
readDir = origReadDir
|
||||
smartctlReadFile = origReadFile
|
||||
})
|
||||
|
||||
runtimeGOOS = "linux"
|
||||
readDir = func(string) ([]os.DirEntry, error) {
|
||||
return []os.DirEntry{fakeDirEntry{name: "nvme0n1"}, fakeDirEntry{name: "sda"}}, nil
|
||||
}
|
||||
smartctlReadFile = func(path string) ([]byte, error) {
|
||||
switch path {
|
||||
case "/sys/block/nvme0n1/size":
|
||||
return []byte("3907029168"), nil
|
||||
default:
|
||||
return nil, fs.ErrNotExist
|
||||
}
|
||||
}
|
||||
|
||||
// NVMe controller scan target rewrites to the namespace and gains the size.
|
||||
smart := &DiskSMART{Device: "nvme0 [nvme]"}
|
||||
refineLinuxBlockDeviceIdentity(smart, smartctlTarget{Path: "/dev/nvme0", DeviceType: "nvme"})
|
||||
if smart.Device != "nvme0n1" {
|
||||
t.Errorf("Device = %q, want nvme0n1", smart.Device)
|
||||
}
|
||||
if smart.SizeBytes != 2000398934016 {
|
||||
t.Errorf("SizeBytes = %d, want 2000398934016", smart.SizeBytes)
|
||||
}
|
||||
|
||||
// A smartctl-reported capacity is preserved when /sys/block has no size.
|
||||
smart = &DiskSMART{Device: "sda [sat]", SizeBytes: 240057409536}
|
||||
refineLinuxBlockDeviceIdentity(smart, smartctlTarget{Path: "/dev/sda", DeviceType: "sat"})
|
||||
if smart.Device != "sda" {
|
||||
t.Errorf("Device = %q, want sda", smart.Device)
|
||||
}
|
||||
if smart.SizeBytes != 240057409536 {
|
||||
t.Errorf("SizeBytes = %d, want fallback 240057409536", smart.SizeBytes)
|
||||
}
|
||||
|
||||
// Disks behind a multiplexing controller keep their disambiguating label.
|
||||
smart = &DiskSMART{Device: "sda [megaraid,7]", SizeBytes: 480103981056}
|
||||
refineLinuxBlockDeviceIdentity(smart, smartctlTarget{Path: "/dev/sda", DeviceType: "megaraid,7"})
|
||||
if smart.Device != "sda [megaraid,7]" {
|
||||
t.Errorf("Device = %q, want unchanged megaraid label", smart.Device)
|
||||
}
|
||||
if smart.SizeBytes != 480103981056 {
|
||||
t.Errorf("SizeBytes = %d, want preserved smartctl capacity", smart.SizeBytes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsMultiplexedDeviceType(t *testing.T) {
|
||||
cases := map[string]bool{
|
||||
"megaraid,7": true,
|
||||
"cciss,1": true,
|
||||
"areca,1/1": true,
|
||||
"aacraid,0": true,
|
||||
"sat": false,
|
||||
"nvme": false,
|
||||
"": false,
|
||||
"sat,auto": false,
|
||||
}
|
||||
for dt, want := range cases {
|
||||
if got := isMultiplexedDeviceType(dt); got != want {
|
||||
t.Errorf("isMultiplexedDeviceType(%q) = %v, want %v", dt, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRefineLinuxBlockDeviceIdentityNonLinuxNoOp(t *testing.T) {
|
||||
origGOOS := runtimeGOOS
|
||||
t.Cleanup(func() { runtimeGOOS = origGOOS })
|
||||
runtimeGOOS = "darwin"
|
||||
|
||||
smart := &DiskSMART{Device: "nvme0 [nvme]"}
|
||||
refineLinuxBlockDeviceIdentity(smart, smartctlTarget{Path: "/dev/nvme0", DeviceType: "nvme"})
|
||||
if smart.Device != "nvme0 [nvme]" {
|
||||
t.Errorf("Device = %q, want unchanged on non-linux", smart.Device)
|
||||
}
|
||||
}
|
||||
|
|
@ -360,15 +360,16 @@ func (s HostSensorSummary) NormalizeCollections() HostSensorSummary {
|
|||
|
||||
// HostDiskSMART represents S.M.A.R.T. data for a disk from a host agent.
|
||||
type HostDiskSMART struct {
|
||||
Device string `json:"device"` // Device name (e.g., sda)
|
||||
Model string `json:"model,omitempty"` // Disk model
|
||||
Serial string `json:"serial,omitempty"` // Serial number
|
||||
WWN string `json:"wwn,omitempty"` // World Wide Name
|
||||
Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme
|
||||
Temperature int `json:"temperature"` // Temperature in Celsius
|
||||
Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN
|
||||
Standby bool `json:"standby,omitempty"` // True if disk was in standby
|
||||
Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member)
|
||||
Device string `json:"device"` // Block device name (e.g., sda, nvme0n1)
|
||||
Model string `json:"model,omitempty"` // Disk model
|
||||
Serial string `json:"serial,omitempty"` // Serial number
|
||||
WWN string `json:"wwn,omitempty"` // World Wide Name
|
||||
Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme
|
||||
SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown)
|
||||
Temperature int `json:"temperature"` // Temperature in Celsius
|
||||
Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN
|
||||
Standby bool `json:"standby,omitempty"` // True if disk was in standby
|
||||
Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member)
|
||||
Attributes *SMARTAttributes `json:"attributes,omitempty"`
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -563,6 +563,7 @@ func convertAgentSMARTToModels(smart []agentshost.DiskSMART) []models.HostDiskSM
|
|||
Serial: disk.Serial,
|
||||
WWN: disk.WWN,
|
||||
Type: disk.Type,
|
||||
SizeBytes: disk.SizeBytes,
|
||||
Temperature: disk.Temperature,
|
||||
Health: disk.Health,
|
||||
Standby: disk.Standby,
|
||||
|
|
|
|||
|
|
@ -624,10 +624,16 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res
|
|||
}
|
||||
}
|
||||
|
||||
sizeBytes := int64(0)
|
||||
// The agent reports the authoritative disk capacity (from /sys/block); the
|
||||
// filesystem-usage match is only a last-resort fallback for older agents that
|
||||
// predate the SizeBytes field. matchedDisk.Total is a filesystem size, not a
|
||||
// whole-disk size, so it is never preferred over a real capacity.
|
||||
sizeBytes := disk.SizeBytes
|
||||
used := ""
|
||||
if matchedDisk != nil {
|
||||
sizeBytes = matchedDisk.Total
|
||||
if sizeBytes <= 0 {
|
||||
sizeBytes = matchedDisk.Total
|
||||
}
|
||||
used = strings.TrimSpace(matchedDisk.Mountpoint)
|
||||
}
|
||||
unraidDisk := matchUnraidDisk(host.Unraid, disk)
|
||||
|
|
|
|||
|
|
@ -662,6 +662,59 @@ func TestResourceFromHostSMARTDiskNormalizesLegacyUnraidKiBSize(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestResourceFromHostSMARTDiskPrefersReportedSize(t *testing.T) {
|
||||
const diskBytes = int64(2000398934016)
|
||||
host := models.Host{
|
||||
ID: "pve-host",
|
||||
Hostname: "pve",
|
||||
LastSeen: time.Now().UTC(),
|
||||
// A filesystem-usage entry whose Total is NOT the whole-disk size. The
|
||||
// adapter must not pick this up over the agent-reported capacity.
|
||||
Disks: []models.Disk{
|
||||
{Device: "/dev/nvme0n1p2", Mountpoint: "/", Total: 100_000_000_000},
|
||||
},
|
||||
Sensors: models.HostSensorSummary{
|
||||
SMART: []models.HostDiskSMART{
|
||||
{Device: "nvme0n1", Serial: "50026B72FB69", Type: "nvme", Health: "PASSED", SizeBytes: diskBytes},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
resource, _ := resourceFromHostSMARTDisk(host, host.Sensors.SMART[0])
|
||||
if resource.PhysicalDisk == nil {
|
||||
t.Fatal("expected physical disk metadata")
|
||||
}
|
||||
if resource.PhysicalDisk.SizeBytes != diskBytes {
|
||||
t.Fatalf("sizeBytes = %d, want reported capacity %d", resource.PhysicalDisk.SizeBytes, diskBytes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceFromHostSMARTDiskFallsBackToFilesystemSize(t *testing.T) {
|
||||
const fsBytes = int64(240057409536)
|
||||
host := models.Host{
|
||||
ID: "pve-host",
|
||||
Hostname: "pve",
|
||||
LastSeen: time.Now().UTC(),
|
||||
Disks: []models.Disk{
|
||||
{Device: "/dev/sda", Mountpoint: "/data", Total: fsBytes},
|
||||
},
|
||||
Sensors: models.HostSensorSummary{
|
||||
SMART: []models.HostDiskSMART{
|
||||
// Legacy agent with no SizeBytes: fall back to the filesystem match.
|
||||
{Device: "sda", Serial: "INTEL-1", Type: "sata", Health: "PASSED"},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
resource, _ := resourceFromHostSMARTDisk(host, host.Sensors.SMART[0])
|
||||
if resource.PhysicalDisk == nil {
|
||||
t.Fatal("expected physical disk metadata")
|
||||
}
|
||||
if resource.PhysicalDisk.SizeBytes != fsBytes {
|
||||
t.Fatalf("sizeBytes = %d, want filesystem fallback %d", resource.PhysicalDisk.SizeBytes, fsBytes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceFromKubernetesDeployment_PopulatesMetricsUnderMockMode(t *testing.T) {
|
||||
mockruntime.SetEnabled(true)
|
||||
t.Cleanup(func() { mockruntime.SetEnabled(false) })
|
||||
|
|
|
|||
|
|
@ -2353,7 +2353,7 @@ func mergePhysicalDiskData(existing *PhysicalDiskMeta, incoming *PhysicalDiskMet
|
|||
}
|
||||
|
||||
merged := *existing
|
||||
if incoming.DevPath != "" {
|
||||
if shouldReplacePhysicalDiskDevPath(merged.DevPath, incoming.DevPath) {
|
||||
merged.DevPath = incoming.DevPath
|
||||
}
|
||||
if incoming.Model != "" {
|
||||
|
|
@ -2423,6 +2423,37 @@ func mergePhysicalDiskData(existing *PhysicalDiskMeta, incoming *PhysicalDiskMet
|
|||
return &merged
|
||||
}
|
||||
|
||||
// shouldReplacePhysicalDiskDevPath decides whether an incoming devPath should
|
||||
// overwrite the existing one when two sources describe the same disk. A canonical
|
||||
// /dev/<device> path (as produced by the Proxmox disks/list poll) must never be
|
||||
// downgraded to a non-canonical scan label such as the host agent's
|
||||
// "nvme0 [nvme]" controller display name. When both are equally canonical the
|
||||
// incoming value wins, preserving the previous last-writer behaviour.
|
||||
func shouldReplacePhysicalDiskDevPath(existing, incoming string) bool {
|
||||
incoming = strings.TrimSpace(incoming)
|
||||
if incoming == "" {
|
||||
return false
|
||||
}
|
||||
existing = strings.TrimSpace(existing)
|
||||
if existing == "" {
|
||||
return true
|
||||
}
|
||||
if isCanonicalDevPath(existing) && !isCanonicalDevPath(incoming) {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// isCanonicalDevPath reports whether devPath is a clean /dev/<device> path with
|
||||
// no whitespace or scan-type suffix (e.g. "/dev/nvme0n1", not "nvme0 [nvme]").
|
||||
func isCanonicalDevPath(devPath string) bool {
|
||||
devPath = strings.TrimSpace(devPath)
|
||||
if !strings.HasPrefix(devPath, "/dev/") {
|
||||
return false
|
||||
}
|
||||
return !strings.ContainsAny(devPath, " \t[")
|
||||
}
|
||||
|
||||
func shouldReplacePhysicalDiskHealth(existing, incoming string) bool {
|
||||
incoming = strings.ToUpper(strings.TrimSpace(incoming))
|
||||
if incoming == "" {
|
||||
|
|
|
|||
|
|
@ -1891,6 +1891,93 @@ func TestResourceRegistry_IngestSnapshotMergesAgentAndProxmoxPhysicalDisksByIden
|
|||
}
|
||||
}
|
||||
|
||||
// Regression for issue #1483: a legacy host agent reports an NVMe disk keyed by
|
||||
// its controller ("nvme0 [nvme]") with no size, while the Proxmox disks/list poll
|
||||
// reports the canonical namespace ("/dev/nvme0n1") with the real capacity. The
|
||||
// merge must keep the authoritative Proxmox devPath and size, and only enrich
|
||||
// with the agent's SMART temperature.
|
||||
func TestResourceRegistry_AgentDiskDoesNotClobberProxmoxDevPath(t *testing.T) {
|
||||
rr := NewRegistry(nil)
|
||||
now := time.Date(2026, 5, 29, 12, 0, 0, 0, time.UTC)
|
||||
const nvmeSize = int64(2000398934016)
|
||||
|
||||
rr.IngestSnapshot(models.StateSnapshot{
|
||||
PhysicalDisks: []models.PhysicalDisk{
|
||||
{
|
||||
ID: "pve-disk-1",
|
||||
Node: "pve",
|
||||
Instance: "pve",
|
||||
DevPath: "/dev/nvme0n1",
|
||||
Model: "KINGSTON SNV3S2000G",
|
||||
Serial: "SERIAL-NVME-0",
|
||||
Type: "nvme",
|
||||
Size: nvmeSize,
|
||||
Health: "PASSED",
|
||||
LastChecked: now,
|
||||
},
|
||||
},
|
||||
Hosts: []models.Host{
|
||||
{
|
||||
ID: "host-pve",
|
||||
Hostname: "pve",
|
||||
Status: "online",
|
||||
LastSeen: now,
|
||||
Sensors: models.HostSensorSummary{
|
||||
SMART: []models.HostDiskSMART{
|
||||
{
|
||||
Device: "nvme0 [nvme]", // legacy controller label, no size
|
||||
Model: "KINGSTON SNV3S2000G",
|
||||
Serial: "SERIAL-NVME-0",
|
||||
Type: "nvme",
|
||||
Temperature: 37,
|
||||
Health: "PASSED",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
disks := rr.ListByType(ResourceTypePhysicalDisk)
|
||||
if len(disks) != 1 {
|
||||
t.Fatalf("expected 1 merged physical disk resource, got %d", len(disks))
|
||||
}
|
||||
disk := disks[0]
|
||||
if disk.PhysicalDisk == nil {
|
||||
t.Fatal("expected physical disk metadata")
|
||||
}
|
||||
if disk.PhysicalDisk.DevPath != "/dev/nvme0n1" {
|
||||
t.Fatalf("devPath = %q, want canonical /dev/nvme0n1 (agent label must not clobber)", disk.PhysicalDisk.DevPath)
|
||||
}
|
||||
if disk.PhysicalDisk.SizeBytes != nvmeSize {
|
||||
t.Fatalf("sizeBytes = %d, want authoritative Proxmox capacity %d", disk.PhysicalDisk.SizeBytes, nvmeSize)
|
||||
}
|
||||
if disk.PhysicalDisk.Temperature != 37 {
|
||||
t.Fatalf("temperature = %d, want enriched agent SMART value 37", disk.PhysicalDisk.Temperature)
|
||||
}
|
||||
}
|
||||
|
||||
func TestShouldReplacePhysicalDiskDevPath(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
existing string
|
||||
incoming string
|
||||
want bool
|
||||
}{
|
||||
{"canonical not downgraded by scan label", "/dev/nvme0n1", "nvme0 [nvme]", false},
|
||||
{"canonical not downgraded by bare token", "/dev/nvme0n1", "nvme0n1", false},
|
||||
{"scan label upgraded to canonical", "nvme0 [nvme]", "/dev/nvme0n1", true},
|
||||
{"empty existing always replaced", "", "nvme0 [nvme]", true},
|
||||
{"empty incoming never replaces", "/dev/nvme0n1", "", false},
|
||||
{"both canonical last writer wins", "/dev/sda", "/dev/sdb", true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := shouldReplacePhysicalDiskDevPath(tc.existing, tc.incoming); got != tc.want {
|
||||
t.Errorf("%s: shouldReplacePhysicalDiskDevPath(%q, %q) = %v, want %v", tc.name, tc.existing, tc.incoming, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceRegistry_IngestSnapshotPropagatesUnraidDiskRole(t *testing.T) {
|
||||
rr := NewRegistry(nil)
|
||||
now := time.Date(2026, 3, 7, 12, 0, 0, 0, time.UTC)
|
||||
|
|
|
|||
|
|
@ -121,15 +121,16 @@ type Sensors struct {
|
|||
|
||||
// DiskSMART represents S.M.A.R.T. data for a single disk.
|
||||
type DiskSMART struct {
|
||||
Device string `json:"device"` // Device path (e.g., sda)
|
||||
Model string `json:"model,omitempty"` // Disk model
|
||||
Serial string `json:"serial,omitempty"` // Serial number
|
||||
WWN string `json:"wwn,omitempty"` // World Wide Name
|
||||
Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme
|
||||
Temperature int `json:"temperature"` // Temperature in Celsius
|
||||
Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN
|
||||
Standby bool `json:"standby,omitempty"` // True if disk was in standby
|
||||
Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member)
|
||||
Device string `json:"device"` // Block device name (e.g., sda, nvme0n1)
|
||||
Model string `json:"model,omitempty"` // Disk model
|
||||
Serial string `json:"serial,omitempty"` // Serial number
|
||||
WWN string `json:"wwn,omitempty"` // World Wide Name
|
||||
Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme
|
||||
SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown)
|
||||
Temperature int `json:"temperature"` // Temperature in Celsius
|
||||
Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN
|
||||
Standby bool `json:"standby,omitempty"` // True if disk was in standby
|
||||
Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member)
|
||||
Attributes *SMARTAttributes `json:"attributes,omitempty"`
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue