From e233d663c31bab74e1b824f49ee569a91a47042c Mon Sep 17 00:00:00 2001 From: Sven van Ginkel Date: Mon, 19 Oct 2026 16:57:25 +0200 Subject: [PATCH] refactor(agent): simplify SMART collection with per-device smartctl attempts (#2620) Rebuild the SMART device list on every refresh instead of keeping per-device type and parser state across rescans. Each device gets a short ordered list of smartctl queries, and the three parsers are merged into one that rejects output without health data so the next query runs. - scsi devices try auto-detect, then -d sat (#2295) - NVMe controllers fall back to their n1 namespace - disks in standby reuse their last reading; a disk not yet collected is woken once per agent run - the last good reading survives temporary read failures - failed or empty rescans keep the previous device list - a missing SMART status is UNKNOWN instead of FAILED - the reported disk type is the type that worked (e.g. sat) - disks seen through two paths are reported once --------- Co-authored-by: hank --- agent/emmc_linux.go | 106 +- agent/emmc_linux_test.go | 27 +- agent/emmc_stub.go | 9 +- agent/mdraid_linux.go | 67 +- agent/mdraid_linux_test.go | 27 +- agent/mdraid_stub.go | 8 +- agent/smart.go | 1444 ++++-------- agent/smart_parse.go | 333 +++ agent/smart_test.go | 1949 +++++++---------- .../smart/nvme_permission_denied.json | 228 ++ agent/test-data/smart/sat.json | 26 + .../test-data/smart/sat_sleep_permission.json | 40 + agent/test-data/smart/scsi_identity.json | 20 + internal/entities/smart/smart.go | 2 +- 14 files changed, 1966 insertions(+), 2320 deletions(-) create mode 100644 agent/smart_parse.go create mode 100644 agent/test-data/smart/nvme_permission_denied.json create mode 100644 agent/test-data/smart/sat.json create mode 100644 agent/test-data/smart/sat_sleep_permission.json create mode 100644 agent/test-data/smart/scsi_identity.json diff --git a/agent/emmc_linux.go b/agent/emmc_linux.go index 147630276..6060c2dd0 100644 --- a/agent/emmc_linux.go +++ b/agent/emmc_linux.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "strconv" - "strings" "github.com/henrygd/beszel/agent/utils" "github.com/henrygd/beszel/internal/entities/smart" @@ -25,99 +24,70 @@ type emmcHealth struct { lifeB uint8 } -func scanEmmcDevices() []*DeviceInfo { +// scanEmmcDevices returns the /dev paths of eMMC devices that expose health +// fields in sysfs. +func scanEmmcDevices() []string { blockDir := filepath.Join(emmcSysfsRoot, "class", "block") entries, err := os.ReadDir(blockDir) if err != nil { return nil } - devices := make([]*DeviceInfo, 0, 2) + var devices []string for _, ent := range entries { name := ent.Name() if !isEmmcBlockName(name) { continue } - - deviceDir := filepath.Join(blockDir, name, "device") - if !hasEmmcHealthFiles(deviceDir) { + if !hasEmmcHealthFiles(filepath.Join(blockDir, name, "device")) { continue } - - devPath := filepath.Join("/dev", name) - devices = append(devices, &DeviceInfo{ - Name: devPath, - Type: "emmc", - InfoName: devPath + " [eMMC]", - Protocol: "MMC", - }) + devices = append(devices, filepath.Join("/dev", name)) } - return devices } -func (sm *SmartManager) collectEmmcHealth(deviceInfo *DeviceInfo) (bool, error) { - if deviceInfo == nil || deviceInfo.Name == "" { - return false, nil - } - - base := filepath.Base(deviceInfo.Name) - if !isEmmcBlockName(base) && !strings.EqualFold(deviceInfo.Type, "emmc") && !strings.EqualFold(deviceInfo.Type, "mmc") { - return false, nil - } - +// readEmmcSmartData reads eMMC wear / EOL indicators from sysfs. It returns the +// data map key (serial, or the device path when there is none). +func readEmmcSmartData(path string) (string, *smart.SmartData, bool) { + base := filepath.Base(path) health, ok := readEmmcHealth(base) if !ok { - return false, nil + return "", nil, false } - // Normalize the device type to keep pruning logic stable across refreshes. - deviceInfo.Type = "emmc" - + devPath := filepath.Join("/dev", base) key := health.serial if key == "" { - key = filepath.Join("/dev", base) + key = devPath } - status := emmcSmartStatus(health.preEOL) - - attrs := []*smart.SmartAttribute{ - { - Name: "PreEOLInfo", - RawValue: uint64(health.preEOL), - RawString: emmcPreEOLString(health.preEOL), + return key, &smart.SmartData{ + ModelName: health.model, + SerialNumber: health.serial, + FirmwareVersion: health.revision, + Capacity: health.capacity, + SmartStatus: emmcSmartStatus(health.preEOL), + DiskName: devPath, + DiskType: "emmc", + Attributes: []*smart.SmartAttribute{ + { + Name: "PreEOLInfo", + RawValue: uint64(health.preEOL), + RawString: emmcPreEOLString(health.preEOL), + }, + { + Name: "DeviceLifeTimeEstA", + RawValue: uint64(health.lifeA), + RawString: emmcLifeTimeString(health.lifeA), + }, + { + Name: "DeviceLifeTimeEstB", + RawValue: uint64(health.lifeB), + RawString: emmcLifeTimeString(health.lifeB), + }, }, - { - Name: "DeviceLifeTimeEstA", - RawValue: uint64(health.lifeA), - RawString: emmcLifeTimeString(health.lifeA), - }, - { - Name: "DeviceLifeTimeEstB", - RawValue: uint64(health.lifeB), - RawString: emmcLifeTimeString(health.lifeB), - }, - } - - sm.Lock() - defer sm.Unlock() - - if _, exists := sm.SmartDataMap[key]; !exists { - sm.SmartDataMap[key] = &smart.SmartData{} - } - - data := sm.SmartDataMap[key] - data.ModelName = health.model - data.SerialNumber = health.serial - data.FirmwareVersion = health.revision - data.Capacity = health.capacity - data.Temperature = 0 - data.SmartStatus = status - data.DiskName = filepath.Join("/dev", base) - data.DiskType = "emmc" - data.Attributes = attrs - - return true, nil + }, true } func readEmmcHealth(blockName string) (emmcHealth, bool) { diff --git a/agent/emmc_linux_test.go b/agent/emmc_linux_test.go index 626abada1..c53478df0 100644 --- a/agent/emmc_linux_test.go +++ b/agent/emmc_linux_test.go @@ -6,8 +6,6 @@ import ( "os" "path/filepath" "testing" - - "github.com/henrygd/beszel/internal/entities/smart" ) func TestEmmcMockSysfsScanAndCollect(t *testing.T) { @@ -45,26 +43,19 @@ func TestEmmcMockSysfsScanAndCollect(t *testing.T) { if len(devs) != 1 { t.Fatalf("scanEmmcDevices() = %d devices, want 1", len(devs)) } - if devs[0].Name != "/dev/mmcblk0" || devs[0].Type != "emmc" { - t.Fatalf("scanEmmcDevices()[0] = %+v, want Name=/dev/mmcblk0 Type=emmc", devs[0]) + if devs[0] != "/dev/mmcblk0" { + t.Fatalf("scanEmmcDevices()[0] = %q, want /dev/mmcblk0", devs[0]) } - sm := &SmartManager{SmartDataMap: map[string]*smart.SmartData{}} - ok, err := sm.collectEmmcHealth(devs[0]) - if err != nil || !ok { - t.Fatalf("collectEmmcHealth() = (ok=%v, err=%v), want (true,nil)", ok, err) + // sysfs targets are read directly instead of through smartctl. + result, err := (&SmartManager{}).collectTarget(smartTarget{name: devs[0], dtype: "emmc"}) + if err != nil { + t.Fatalf("collectTarget() err = %v", err) } - if len(sm.SmartDataMap) != 1 { - t.Fatalf("SmartDataMap len=%d, want 1", len(sm.SmartDataMap)) - } - var got *smart.SmartData - for _, v := range sm.SmartDataMap { - got = v - break - } - if got == nil { - t.Fatalf("SmartDataMap value nil") + if result.key != "01234567" { + t.Fatalf("key = %q, want 01234567", result.key) } + got := result.data if got.DiskType != "emmc" || got.DiskName != "/dev/mmcblk0" { t.Fatalf("disk fields = (type=%q name=%q), want (emmc,/dev/mmcblk0)", got.DiskType, got.DiskName) } diff --git a/agent/emmc_stub.go b/agent/emmc_stub.go index 9886f757a..859b80ff4 100644 --- a/agent/emmc_stub.go +++ b/agent/emmc_stub.go @@ -2,13 +2,14 @@ package agent +import "github.com/henrygd/beszel/internal/entities/smart" + // Non-Linux builds: eMMC health via sysfs is not available. -func scanEmmcDevices() []*DeviceInfo { +func scanEmmcDevices() []string { return nil } -func (sm *SmartManager) collectEmmcHealth(deviceInfo *DeviceInfo) (bool, error) { - return false, nil +func readEmmcSmartData(path string) (string, *smart.SmartData, bool) { + return "", nil, false } - diff --git a/agent/mdraid_linux.go b/agent/mdraid_linux.go index 2376a3371..b56ec6221 100644 --- a/agent/mdraid_linux.go +++ b/agent/mdraid_linux.go @@ -30,57 +30,37 @@ type mdraidHealth struct { capacity uint64 } -// scanMdraidDevices discovers Linux md arrays exposed in sysfs. -func scanMdraidDevices() []*DeviceInfo { +// scanMdraidDevices returns the /dev paths of Linux md arrays exposed in sysfs. +func scanMdraidDevices() []string { blockDir := filepath.Join(mdraidSysfsRoot, "block") entries, err := os.ReadDir(blockDir) if err != nil { return nil } - devices := make([]*DeviceInfo, 0, 2) + var devices []string for _, ent := range entries { name := ent.Name() if !isMdraidBlockName(name) { continue } - mdDir := filepath.Join(blockDir, name, "md") - if !utils.FileExists(filepath.Join(mdDir, "array_state")) { + if !utils.FileExists(filepath.Join(blockDir, name, "md", "array_state")) { continue } - - devPath := filepath.Join("/dev", name) - devices = append(devices, &DeviceInfo{ - Name: devPath, - Type: "mdraid", - InfoName: devPath + " [mdraid]", - Protocol: "MD", - }) + devices = append(devices, filepath.Join("/dev", name)) } - return devices } -// collectMdraidHealth reads mdraid health and stores it in SmartDataMap. -func (sm *SmartManager) collectMdraidHealth(deviceInfo *DeviceInfo) (bool, error) { - if deviceInfo == nil || deviceInfo.Name == "" { - return false, nil - } - - base := filepath.Base(deviceInfo.Name) - if !isMdraidBlockName(base) && !strings.EqualFold(deviceInfo.Type, "mdraid") { - return false, nil - } - +// readMdraidSmartData reads mdraid health from sysfs. It returns the data map +// key ("mdraid:"). +func readMdraidSmartData(path string) (string, *smart.SmartData, bool) { + base := filepath.Base(path) health, ok := readMdraidHealth(base) if !ok { - return false, nil + return "", nil, false } - deviceInfo.Type = "mdraid" - key := fmt.Sprintf("mdraid:%s", base) - status := mdraidSmartStatus(health) - attrs := make([]*smart.SmartAttribute, 0, 10) if health.arrayState != "" { attrs = append(attrs, &smart.SmartAttribute{Name: "ArrayState", RawString: health.arrayState}) @@ -110,25 +90,18 @@ func (sm *SmartManager) collectMdraidHealth(deviceInfo *DeviceInfo) (bool, error attrs = append(attrs, &smart.SmartAttribute{Name: "MismatchCount", RawValue: health.mismatchCnt}) } - sm.Lock() - defer sm.Unlock() - - if _, exists := sm.SmartDataMap[key]; !exists { - sm.SmartDataMap[key] = &smart.SmartData{} - } - - data := sm.SmartDataMap[key] - data.ModelName = "Linux MD RAID" + model := "Linux MD RAID" if health.level != "" { - data.ModelName = "Linux MD RAID (" + health.level + ")" + model = "Linux MD RAID (" + health.level + ")" } - data.Capacity = health.capacity - data.SmartStatus = status - data.DiskName = filepath.Join("/dev", base) - data.DiskType = "mdraid" - data.Attributes = attrs - - return true, nil + return fmt.Sprintf("mdraid:%s", base), &smart.SmartData{ + ModelName: model, + Capacity: health.capacity, + SmartStatus: mdraidSmartStatus(health), + DiskName: filepath.Join("/dev", base), + DiskType: "mdraid", + Attributes: attrs, + }, true } // readMdraidHealth reads md array health fields from sysfs. diff --git a/agent/mdraid_linux_test.go b/agent/mdraid_linux_test.go index c6c93e3ee..93572be19 100644 --- a/agent/mdraid_linux_test.go +++ b/agent/mdraid_linux_test.go @@ -6,8 +6,6 @@ import ( "os" "path/filepath" "testing" - - "github.com/henrygd/beszel/internal/entities/smart" ) func TestMdraidMockSysfsScanAndCollect(t *testing.T) { @@ -56,26 +54,19 @@ func TestMdraidMockSysfsScanAndCollect(t *testing.T) { if len(devs) != 1 { t.Fatalf("scanMdraidDevices() = %d devices, want 1", len(devs)) } - if devs[0].Name != "/dev/md0" || devs[0].Type != "mdraid" { - t.Fatalf("scanMdraidDevices()[0] = %+v, want Name=/dev/md0 Type=mdraid", devs[0]) + if devs[0] != "/dev/md0" { + t.Fatalf("scanMdraidDevices()[0] = %q, want /dev/md0", devs[0]) } - sm := &SmartManager{SmartDataMap: map[string]*smart.SmartData{}} - ok, err := sm.collectMdraidHealth(devs[0]) - if err != nil || !ok { - t.Fatalf("collectMdraidHealth() = (ok=%v, err=%v), want (true,nil)", ok, err) + // sysfs targets are read directly instead of through smartctl. + result, err := (&SmartManager{}).collectTarget(smartTarget{name: devs[0], dtype: "mdraid"}) + if err != nil { + t.Fatalf("collectTarget() err = %v", err) } - if len(sm.SmartDataMap) != 1 { - t.Fatalf("SmartDataMap len=%d, want 1", len(sm.SmartDataMap)) - } - var got *smart.SmartData - for _, v := range sm.SmartDataMap { - got = v - break - } - if got == nil { - t.Fatalf("SmartDataMap value nil") + if result.key != "mdraid:md0" { + t.Fatalf("key = %q, want mdraid:md0", result.key) } + got := result.data if got.DiskType != "mdraid" || got.DiskName != "/dev/md0" { t.Fatalf("disk fields = (type=%q name=%q), want (mdraid,/dev/md0)", got.DiskType, got.DiskName) } diff --git a/agent/mdraid_stub.go b/agent/mdraid_stub.go index 4c95d20b0..d6298b7a0 100644 --- a/agent/mdraid_stub.go +++ b/agent/mdraid_stub.go @@ -2,10 +2,12 @@ package agent -func scanMdraidDevices() []*DeviceInfo { +import "github.com/henrygd/beszel/internal/entities/smart" + +func scanMdraidDevices() []string { return nil } -func (sm *SmartManager) collectMdraidHealth(deviceInfo *DeviceInfo) (bool, error) { - return false, nil +func readMdraidSmartData(path string) (string, *smart.SmartData, bool) { + return "", nil, false } diff --git a/agent/smart.go b/agent/smart.go index 57dae3a8a..c7152c97f 100644 --- a/agent/smart.go +++ b/agent/smart.go @@ -4,6 +4,7 @@ package agent import ( + "bytes" "context" "encoding/json" "errors" @@ -13,7 +14,7 @@ import ( "os/exec" "path/filepath" "runtime" - "strconv" + "slices" "strings" "sync" "time" @@ -22,124 +23,129 @@ import ( "github.com/henrygd/beszel/internal/entities/smart" ) -// SmartManager manages data collection for SMART devices +// SmartManager collects SMART data. Every refresh builds the list of targets +// (smartctl scan, SMART_DEVICES, eMMC and mdraid), queries each one and +// replaces the collected data, so no per-device state has to be kept in sync. type SmartManager struct { - sync.Mutex - SmartDataMap map[string]*smart.SmartData - SmartDevices []*DeviceInfo - refreshMutex sync.Mutex - lastScanTime time.Time - smartctlPath string - excludedDevices map[string]struct{} - nvmeNamespacePaths map[string]string // controller to working namespace; guarded by Mutex + sync.Mutex // guards data + data map[string]*smart.SmartData // collected data keyed by serial (or sysfs key) + refreshMutex sync.Mutex // serializes refreshes; guards the fields below + last map[string]smartResult // last success per target key + woken map[string]struct{} // target keys already queried once without -n standby; never cleared + scanned []smartDevice // cached smartctl --scan result + scanErr error // error of the cached scan + lastScan time.Time + smartctlPath string + excludedDevices map[string]struct{} + // run executes smartctl; nil means exec smartctlPath. Overridable for testing. + run smartctlRunner darwinNvmeOnce sync.Once darwinNvmeCapacity map[string]uint64 // serial → bytes cache, written once via darwinNvmeOnce darwinNvmeProvider func() ([]byte, error) // overridable for testing } -type scanOutput struct { - Devices []struct { - Name string `json:"name"` - Type string `json:"type"` - InfoName string `json:"info_name"` - Protocol string `json:"protocol"` - } `json:"devices"` +// smartctlRunner runs smartctl and returns its stdout and exit code. err is set +// only when smartctl could not be run or timed out. +type smartctlRunner func(timeout time.Duration, args ...string) (output []byte, exitCode int, err error) + +// smartDevice is a device from the smartctl scan, SMART_DEVICES or sysfs. +type smartDevice struct { + name string + dtype string + // explicit marks a type set in SMART_DEVICES. It is a deliberate override, + // so it is always passed to smartctl and never second-guessed (#2102). + explicit bool } -type DeviceInfo struct { - Name string `json:"name"` - Type string `json:"type"` - InfoName string `json:"info_name"` - Protocol string `json:"protocol"` - // typeVerified reports whether we have already parsed SMART data for this device - // with the stored parserType. When true we can skip re-running the detection logic. - typeVerified bool - // parserType holds the parser type (nvme, sat, scsi) that last succeeded. - parserType string - // explicitType reports whether Type came from an explicit ":type" hint in - // SMART_DEVICES. Such a type is a deliberate user override and must always be - // passed to smartctl via -d, even for scsi/ata where a scan-detected type is - // otherwise left off (see smartctlArgs and issue #1345). - explicitType bool +// smartAttempt is one way to query a device: a path and a -d type. An empty +// type lets smartctl auto-detect. +type smartAttempt struct { + path string + dtype string } -// deviceKey is a composite key for a device, used to identify a device uniquely. -type deviceKey struct { - name string - deviceType string +type smartTarget struct { + key string // name and type; stable across refreshes + dtype string // emmc and mdraid are read from sysfs instead of smartctl + name string + attempts []smartAttempt +} + +type smartResult struct { + key string // data map key + data *smart.SmartData + attempt int // index of the attempt that succeeded, tried first next time + ata bool // device speaks ATA, so -l devstat is requested } var errNoValidSmartData = fmt.Errorf("no valid SMART data found") // Error for missing data -// Refresh updates SMART data for all known devices and reports whether every -// discovered device was collected successfully. +const smartScanInterval = 30 * time.Minute + +// Refresh updates SMART data for all devices and reports whether every device +// was collected, which lets the hub prune records of removed devices. func (sm *SmartManager) Refresh(forceScan bool) (bool, error) { sm.refreshMutex.Lock() defer sm.refreshMutex.Unlock() - scanErr := sm.ScanDevices(false) + targets, scanErr := sm.buildTargets(forceScan) if scanErr != nil { slog.Debug("smartctl scan failed", "err", scanErr) } - devices := sm.devicesSnapshot() + data := make(map[string]*smart.SmartData, len(targets)) + last := make(map[string]smartResult, len(targets)) + complete := scanErr == nil var collectErr error - for _, deviceInfo := range devices { - if deviceInfo == nil { + for _, target := range targets { + result, err := sm.collectTarget(target) + if err != nil { + // Keep the last successful reading for future standby reuse. Only + // targets still present are retained, so removed disks are forgotten. + if prev, ok := sm.last[target.key]; ok { + last[target.key] = prev + } + complete = false + if !errors.Is(err, errStandby) { + slog.Info("smartctl failed", "device", target.name, "err", err) + collectErr = err + } continue } - if err := sm.CollectSmart(deviceInfo); err != nil { - slog.Debug("smartctl collect failed", "device", deviceInfo.Name, "err", err) - collectErr = err + if existing, ok := data[result.key]; ok { + slog.Debug("skipping duplicate SMART device", "device", target.name, "serial", result.key, "first", existing.DiskName) + continue } + data[result.key] = result.data + last[target.key] = result } + sm.last = last - return scanErr == nil && collectErr == nil, sm.resolveRefreshError(scanErr, collectErr) -} - -// devicesSnapshot returns a copy of the current device slice to avoid iterating -// while holding the primary mutex for longer than necessary. -func (sm *SmartManager) devicesSnapshot() []*DeviceInfo { sm.Lock() - defer sm.Unlock() - - devices := make([]*DeviceInfo, len(sm.SmartDevices)) - copy(devices, sm.SmartDevices) - return devices -} - -// resolveRefreshError determines the proper error to return after a refresh. -func (sm *SmartManager) resolveRefreshError(scanErr, collectErr error) error { - sm.Lock() - noDevices := len(sm.SmartDevices) == 0 - noData := len(sm.SmartDataMap) == 0 + sm.data = data sm.Unlock() - if noDevices { - if scanErr != nil { - return scanErr - } + if len(data) > 0 { + return complete, nil } - - if !noData { - return nil + if len(targets) == 0 && scanErr != nil { + return false, scanErr } - - if collectErr != nil { - return collectErr + switch { + case collectErr != nil: + return false, collectErr + case scanErr != nil: + return false, scanErr } - if scanErr != nil { - return scanErr - } - return errNoValidSmartData + return false, errNoValidSmartData } // GetCurrentData returns the current SMART data func (sm *SmartManager) GetCurrentData() map[string]smart.SmartData { sm.Lock() defer sm.Unlock() - result := make(map[string]smart.SmartData, len(sm.SmartDataMap)) - for key, value := range sm.SmartDataMap { + result := make(map[string]smart.SmartData, len(sm.data)) + for key, value := range sm.data { if value != nil { result[key] = *value } @@ -147,157 +153,346 @@ func (sm *SmartManager) GetCurrentData() map[string]smart.SmartData { return result } -// ScanDevices scans for SMART devices -// Scan devices using `smartctl --scan -j` -// If scan fails, return error -// If scan succeeds, parse the output and update the SmartDevices slice -func (sm *SmartManager) ScanDevices(force bool) error { - if !force && time.Since(sm.lastScanTime) < 30*time.Minute { - return nil - } - sm.lastScanTime = time.Now() - currentDevices := sm.devicesSnapshot() - - var configuredDevices []*DeviceInfo - if configuredRaw, ok := utils.GetEnv("SMART_DEVICES"); ok { - slog.Info("SMART_DEVICES", "value", configuredRaw) - config := strings.TrimSpace(configuredRaw) - if config == "" { - return errNoValidSmartData +// buildTargets merges the smartctl scan, SMART_DEVICES and sysfs devices into +// the list of targets to query. +func (sm *SmartManager) buildTargets(forceScan bool) ([]smartTarget, error) { + var configured []smartDevice + if raw, ok := utils.GetEnv("SMART_DEVICES"); ok { + slog.Debug("SMART_DEVICES", "value", raw) + // An empty SMART_DEVICES disables SMART entirely. + if strings.TrimSpace(raw) == "" { + return nil, errNoValidSmartData } - - parsedDevices, err := sm.parseConfiguredDevices(config) - if err != nil { - return err + var err error + if configured, err = parseConfiguredDevices(raw); err != nil { + return nil, err } - configuredDevices = parsedDevices } - var ( - scanErr error - scannedDevices []*DeviceInfo - hasValidScan bool - ) + scanned, scanErr := sm.scanSmartctl(forceScan) + scanned = append(slices.Clone(scanned), scanSysfsDevices()...) - if sm.smartctlPath != "" { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + devices := mergeDevices(scanned, configured) + targets := make([]smartTarget, 0, len(devices)) + for _, device := range devices { + if sm.isExcludedDevice(device.name) { + continue + } + targets = append(targets, smartTarget{ + key: device.name + "|" + device.dtype, + dtype: device.dtype, + name: device.name, + attempts: sm.attemptsFor(device), + }) + } + return targets, scanErr +} - cmd := exec.CommandContext(ctx, sm.smartctlPath, "--scan", "-j") - output, err := cmd.Output() - if err != nil { - scanErr = err - } else { - scannedDevices, hasValidScan = sm.parseScan(output) - if !hasValidScan { - scanErr = errNoValidSmartData +// scanSysfsDevices returns the eMMC and mdraid devices read from sysfs instead +// of smartctl. It is a variable so tests can isolate themselves from the host. +var scanSysfsDevices = func() []smartDevice { + var devices []smartDevice + for _, path := range scanEmmcDevices() { + devices = append(devices, smartDevice{name: path, dtype: "emmc"}) + } + for _, path := range scanMdraidDevices() { + devices = append(devices, smartDevice{name: path, dtype: "mdraid"}) + } + return devices +} + +// mergeDevices adds SMART_DEVICES entries to the scanned devices. A configured +// entry with a type replaces the scanned entry for the same path when that path +// was scanned exactly once; otherwise it is added (e.g. extra RAID members that +// share a controller path). Duplicates are dropped. +func mergeDevices(scanned, configured []smartDevice) []smartDevice { + devices := slices.Clone(scanned) + nameCounts := make(map[string]int, len(scanned)) + for _, device := range scanned { + nameCounts[device.name]++ + } + for _, device := range configured { + if i := slices.IndexFunc(devices, func(d smartDevice) bool { return d.name == device.name && d.dtype == device.dtype }); i >= 0 { + devices[i].explicit = devices[i].explicit || device.explicit + continue + } + if nameCounts[device.name] == 1 { + if device.dtype != "" { + i := slices.IndexFunc(devices, func(d smartDevice) bool { return d.name == device.name }) + devices[i] = device + nameCounts[device.name] = 0 // further types for this path are added + } + continue + } + devices = append(devices, device) + } + + seen := make(map[smartDevice]struct{}, len(devices)) + return slices.DeleteFunc(devices, func(d smartDevice) bool { + d.explicit = false + if _, ok := seen[d]; ok || d.name == "" { + return true + } + seen[d] = struct{}{} + return false + }) +} + +// attemptsFor returns the smartctl queries to try for a device, in order. All +// fallback behavior lives here: +// - an explicit SMART_DEVICES type is used as-is (#2102) +// - scan types ata and scsi are often wrong (e.g. SATA disks behind a SAS +// HBA or in Docker), so smartctl auto-detects first (#1345); scsi then +// falls back to sat for SATA disks that auto-detection misses (#2295) +// - NVMe controller paths fall back to their first namespace (#1504) +func (sm *SmartManager) attemptsFor(device smartDevice) []smartAttempt { + dtype := strings.ToLower(device.dtype) + var attempts []smartAttempt + switch { + case device.explicit: + attempts = []smartAttempt{{device.name, device.dtype}} + case dtype == "scsi": + attempts = []smartAttempt{{device.name, ""}, {device.name, "sat"}} + case dtype == "ata": + attempts = []smartAttempt{{device.name, ""}} + default: + attempts = []smartAttempt{{device.name, device.dtype}} + } + if isNvmeControllerPath(device.name) && (dtype == "" || dtype == "nvme") { + if namespace := device.name + "n1"; !sm.isExcludedDevice(namespace) { + attempts = append(attempts, smartAttempt{namespace, device.dtype}) + } + } + return attempts +} + +// scanSmartctl returns the devices found by `smartctl --scan -j`. The result, +// including a failure, is cached for smartScanInterval. +func (sm *SmartManager) scanSmartctl(force bool) ([]smartDevice, error) { + if sm.smartctlPath == "" && sm.run == nil { + return nil, nil + } + if !force && !sm.lastScan.IsZero() && time.Since(sm.lastScan) < smartScanInterval { + return sm.scanned, sm.scanErr + } + sm.lastScan = time.Now() + + output, exitCode, err := sm.runSmartctl(10*time.Second, "--scan", "-j") + if err == nil && exitCode != 0 { + err = fmt.Errorf("smartctl scan failed (exit status %d)", exitCode) + } + if err == nil { + var devices []smartDevice + if devices, err = parseScan(output); err == nil { + // An empty scan after devices were found is more likely a transient + // failure than every disk being removed. Accepting it would let the + // hub prune all smartctl disks while sysfs devices still collect. + if len(devices) == 0 && len(sm.scanned) > 0 { + err = errors.New("smartctl scan found no devices") + } else { + sm.scanned = devices } } } - - // Add eMMC devices (Linux only) by reading sysfs health fields. This does not - // require smartctl and does not scan the whole device. - if emmcDevices := scanEmmcDevices(); len(emmcDevices) > 0 { - scannedDevices = append(scannedDevices, emmcDevices...) - hasValidScan = true - } - - // Add Linux mdraid arrays by reading sysfs health fields. This does not - // require smartctl and does not scan the whole device. - if raidDevices := scanMdraidDevices(); len(raidDevices) > 0 { - scannedDevices = append(scannedDevices, raidDevices...) - hasValidScan = true - } - - scannedDevices = sm.resolveNvmeNamespacePaths(scannedDevices) - configuredDevices = sm.resolveNvmeNamespacePaths(configuredDevices) - finalDevices := mergeDeviceLists(currentDevices, scannedDevices, configuredDevices) - finalDevices = sm.filterExcludedDevices(finalDevices) - sm.updateSmartDevices(finalDevices) - - if len(finalDevices) == 0 { - if scanErr != nil { - slog.Debug("smartctl scan failed", "err", scanErr) - return scanErr - } - return errNoValidSmartData - } - - return nil + // Keep the previous scan on failure so devices don't disappear. + sm.scanErr = err + return sm.scanned, err } -// resolveNvmeNamespacePaths keeps successful namespace fallbacks when a later -// scan (or SMART_DEVICES) reports the original controller path again. Preserve -// the controller's presence in the scan so removed drives can still be pruned. -func (sm *SmartManager) resolveNvmeNamespacePaths(devices []*DeviceInfo) []*DeviceInfo { - sm.Lock() - defer sm.Unlock() - - if len(sm.nvmeNamespacePaths) == 0 { - return devices +// parseScan parses the output of smartctl --scan -j. +func parseScan(output []byte) ([]smartDevice, error) { + var scan struct { + Devices []struct { + Name string `json:"name"` + Type string `json:"type"` + Protocol string `json:"protocol"` + } `json:"devices"` } - resolved := make([]*DeviceInfo, 0, len(devices)) - seen := make(map[deviceKey]struct{}, len(devices)) - for _, device := range devices { - if device == nil { - continue - } - // Keep excluded controller entries unchanged until filterExcludedDevices - // runs, so an empty input cannot make mergeDeviceLists retain old devices. - if namespace := sm.nvmeNamespacePaths[device.Name]; namespace != "" && !sm.isExcludedDevice(device.Name) { - copyDevice := *device - copyDevice.Name = namespace - device = ©Device - } - key := makeDeviceKey(device.Name, device.Type) - if _, ok := seen[key]; ok { - continue - } - seen[key] = struct{}{} - resolved = append(resolved, device) + if err := json.Unmarshal(output, &scan); err != nil { + return nil, fmt.Errorf("invalid smartctl scan output: %w", err) } - return resolved + devices := make([]smartDevice, 0, len(scan.Devices)) + for _, device := range scan.Devices { + slog.Debug("smartctl scan", "name", device.Name, "type", device.Type, "protocol", device.Protocol) + devices = append(devices, smartDevice{name: device.Name, dtype: device.Type}) + } + return devices, nil } -func (sm *SmartManager) parseConfiguredDevices(config string) ([]*DeviceInfo, error) { +// parseConfiguredDevices parses SMART_DEVICES entries of the form path[:type]. +func parseConfiguredDevices(config string) ([]smartDevice, error) { splitChar, _ := utils.GetEnv("SMART_DEVICES_SEPARATOR") if splitChar == "" { splitChar = "," } - entries := strings.Split(config, splitChar) - devices := make([]*DeviceInfo, 0, len(entries)) - for _, entry := range entries { + var devices []smartDevice + for entry := range strings.SplitSeq(config, splitChar) { entry = strings.TrimSpace(entry) if entry == "" { continue } - - parts := strings.SplitN(entry, ":", 2) - - name := strings.TrimSpace(parts[0]) + name, dtype, _ := strings.Cut(entry, ":") + name = strings.TrimSpace(name) if name == "" { return nil, fmt.Errorf("invalid SMART_DEVICES entry %q", entry) } - - devType := "" - if len(parts) == 2 { - devType = strings.ToLower(strings.TrimSpace(parts[1])) - } - - devices = append(devices, &DeviceInfo{ - Name: name, - Type: devType, - explicitType: devType != "", - }) + dtype = strings.ToLower(strings.TrimSpace(dtype)) + devices = append(devices, smartDevice{name: name, dtype: dtype, explicit: dtype != ""}) } - if len(devices) == 0 { return nil, errNoValidSmartData } - return devices, nil } +var errStandby = errors.New("device is in standby") + +// collectTarget reads one target. smartctl targets try each attempt in turn, +// starting with the one that worked last time. +func (sm *SmartManager) collectTarget(target smartTarget) (smartResult, error) { + switch target.dtype { + case "emmc", "mmc": + if key, data, ok := readEmmcSmartData(target.name); ok { + return smartResult{key: key, data: data}, nil + } + return smartResult{}, errNoValidSmartData + case "mdraid": + if key, data, ok := readMdraidSmartData(target.name); ok { + return smartResult{key: key, data: data}, nil + } + return smartResult{}, errNoValidSmartData + } + if sm.smartctlPath == "" && sm.run == nil { + return smartResult{}, errNoValidSmartData + } + + prev, hasPrev := sm.last[target.key] + order := make([]int, 0, len(target.attempts)) + if hasPrev && prev.attempt < len(target.attempts) { + order = append(order, prev.attempt) + } + for i := range target.attempts { + if !hasPrev || i != prev.attempt { + order = append(order, i) + } + } + + var err error + for _, i := range order { + attempt := target.attempts[i] + // A scanned ata type requests devstat until a reading confirms the protocol. + ata := prev.ata || !hasPrev && strings.EqualFold(target.dtype, "ata") + args := smartctlArgs(attempt, ata, true) + output, exitCode, runErr := sm.runSmartctl(15*time.Second, args...) + if runErr != nil { + err = runErr + slog.Debug("smartctl attempt failed", "device", attempt.path, "type", attempt.dtype, "err", runErr) + continue + } + data, protocol, parseErr := sm.parseSmartctl(output, attempt.dtype) + if parseErr == nil { + return smartResult{key: data.SerialNumber, data: data, attempt: i, ata: protocol == "ata"}, nil + } + // Don't wake a sleeping disk that was collected before: reuse its last data. + if isStandby(output, exitCode) { + if hasPrev { + slog.Debug("smartctl device in standby", "device", attempt.path) + return prev, nil + } + // Wake a disk once per agent run so it is collected at least once. This + // is tracked apart from last, so a disk that fails after waking is not + // woken again on every refresh. + if _, ok := sm.woken[target.key]; !ok { + if sm.woken == nil { + sm.woken = make(map[string]struct{}) + } + sm.woken[target.key] = struct{}{} + slog.Info("smartctl waking device in standby for first collection", "device", attempt.path) + result, wakeErr := sm.wakeTarget(attempt, i, ata) + if wakeErr == nil { + return result, nil + } + // The disk is awake now, so the remaining attempts can run with + // -n standby (e.g. sat after scsi auto-detection, #2295). + err = wakeErr + continue + } + // Already woken once: skip it until it wakes, so the refresh is + // incomplete and the hub keeps its record. smartctl also reports SLEEP + // when it lacks permission to check the power mode, so say so. + if bytes.Contains(bytes.ToUpper(output), []byte("SLEEP MODE")) { + slog.Info("smartctl reports device asleep (or lacks permission), skipping", "device", attempt.path) + } else { + slog.Info("smartctl reports device in standby, skipping", "device", attempt.path) + } + return smartResult{}, errStandby + } + err = fmt.Errorf("%w (exit status %d)", parseErr, exitCode) + slog.Debug("smartctl attempt failed", "device", attempt.path, "type", attempt.dtype, "args", args, "exit", exitCode, "err", parseErr) + } + return smartResult{}, err +} + +// wakeTarget queries an attempt without -n standby, spinning up the disk. +// A failure is returned as a real error rather than errStandby, which also +// surfaces the permission problem that smartctl otherwise reports as SLEEP. +func (sm *SmartManager) wakeTarget(attempt smartAttempt, i int, ata bool) (smartResult, error) { + args := smartctlArgs(attempt, ata, false) + output, exitCode, err := sm.runSmartctl(15*time.Second, args...) + if err != nil { + return smartResult{}, err + } + data, protocol, err := sm.parseSmartctl(output, attempt.dtype) + if err != nil { + slog.Debug("smartctl wake attempt failed", "device", attempt.path, "type", attempt.dtype, "args", args, "exit", exitCode, "err", err) + return smartResult{}, fmt.Errorf("%w (exit status %d)", err, exitCode) + } + return smartResult{key: data.SerialNumber, data: data, attempt: i, ata: protocol == "ata"}, nil +} + +// smartctlArgs returns the smartctl arguments for one attempt. standby adds +// -n standby so smartctl skips a sleeping disk instead of waking it. +func smartctlArgs(attempt smartAttempt, ata, standby bool) []string { + args := make([]string, 0, 9) + if attempt.dtype != "" { + args = append(args, "-d", attempt.dtype) + } + args = append(args, "-a", "--json=c") + if dtype := strings.ToLower(attempt.dtype); ata || dtype == "sat" || dtype == "ata" { + args = append(args, "-l", "devstat") + } + if standby { + args = append(args, "-n", "standby") + } + return append(args, attempt.path) +} + +// isStandby reports whether smartctl skipped the device because -n standby +// found it in a low-power mode: exit bit 1 plus its "Device is in X mode" message. +func isStandby(output []byte, exitCode int) bool { + if exitCode&2 == 0 { + return false + } + upper := bytes.ToUpper(output) + return bytes.Contains(upper, []byte("DEVICE IS IN ")) && bytes.Contains(upper, []byte(" MODE")) +} + +// runSmartctl runs smartctl with a timeout. +func (sm *SmartManager) runSmartctl(timeout time.Duration, args ...string) ([]byte, int, error) { + if sm.run != nil { + return sm.run(timeout, args...) + } + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + output, err := exec.CommandContext(ctx, sm.smartctlPath, args...).Output() + // smartctl reports device problems through its exit status bitmask while + // still printing JSON, so a non-zero exit is not an error here. + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && ctx.Err() == nil { + return output, exitErr.ExitCode(), nil + } + return output, 0, err +} + func (sm *SmartManager) refreshExcludedDevices() { rawValue, _ := utils.GetEnv("EXCLUDE_SMART") sm.excludedDevices = make(map[string]struct{}) @@ -316,859 +511,6 @@ func (sm *SmartManager) isExcludedDevice(deviceName string) bool { return exists } -func (sm *SmartManager) filterExcludedDevices(devices []*DeviceInfo) []*DeviceInfo { - if devices == nil { - return []*DeviceInfo{} - } - - excluded := sm.excludedDevices - if len(excluded) == 0 { - return devices - } - - filtered := make([]*DeviceInfo, 0, len(devices)) - for _, device := range devices { - if device == nil || device.Name == "" { - continue - } - if _, skip := excluded[device.Name]; skip { - continue - } - filtered = append(filtered, device) - } - return filtered -} - -// detectSmartOutputType inspects protocol-specific sections and the reported -// device type to choose a parser, including when the NVMe health log is missing. -func detectSmartOutputType(output []byte) string { - var hints struct { - Device struct { - Type string `json:"type"` - } `json:"device"` - AtaSmartAttributes json.RawMessage `json:"ata_smart_attributes"` - NVMeSmartHealthInformationLog json.RawMessage `json:"nvme_smart_health_information_log"` - ScsiErrorCounterLog json.RawMessage `json:"scsi_error_counter_log"` - } - - if err := json.Unmarshal(output, &hints); err != nil { - return "" - } - - switch { - case hasJSONValue(hints.NVMeSmartHealthInformationLog), normalizeParserType(hints.Device.Type) == "nvme": - return "nvme" - case hasJSONValue(hints.AtaSmartAttributes): - return "sat" - case hasJSONValue(hints.ScsiErrorCounterLog): - return "scsi" - default: - return "sat" - } -} - -// hasJSONValue reports whether a JSON payload contains a concrete value. The -// smartctl output often emits "null" for sections that do not apply, so we -// only treat non-null content as a hint. -func hasJSONValue(raw json.RawMessage) bool { - if len(raw) == 0 { - return false - } - trimmed := strings.TrimSpace(string(raw)) - return trimmed != "" && trimmed != "null" -} - -func normalizeParserType(value string) string { - switch strings.ToLower(strings.TrimSpace(value)) { - case "nvme", "sntasmedia", "sntrealtek": - return "nvme" - case "sat", "ata": - return "sat" - case "scsi": - return "scsi" - default: - return strings.ToLower(strings.TrimSpace(value)) - } -} - -// makeDeviceKey creates a composite key from device name and type. -// This allows multiple drives under the same device path (e.g., RAID controllers) -// to be tracked separately. -func makeDeviceKey(name, deviceType string) deviceKey { - return deviceKey{name: name, deviceType: deviceType} -} - -// parseSmartOutput attempts each SMART parser, optionally detecting the type when -// it is not provided, and updates the device info when a parser succeeds. -func (sm *SmartManager) parseSmartOutput(deviceInfo *DeviceInfo, output []byte) bool { - parsers := []struct { - Type string - Parse func([]byte) (bool, int) - }{ - {Type: "nvme", Parse: func(output []byte) (bool, int) { - return sm.parseSmartForNvme(output, deviceInfo.Type) - }}, - {Type: "sat", Parse: func(output []byte) (bool, int) { - return sm.parseSmartForSata(output, deviceInfo.Type) - }}, - {Type: "scsi", Parse: func(output []byte) (bool, int) { - return sm.parseSmartForScsi(output, deviceInfo.Type) - }}, - } - - deviceType := normalizeParserType(deviceInfo.parserType) - if deviceType == "" { - deviceType = normalizeParserType(deviceInfo.Type) - } - if deviceInfo.parserType == "" { - switch deviceType { - case "nvme", "sat", "scsi": - deviceInfo.parserType = deviceType - } - } - - // Inspect every response so a failed NVMe query cannot reach other parsers. - structureType := detectSmartOutputType(output) - // Update the stored parser only when it is not yet verified. - needsDetection := deviceType == "" || !deviceInfo.typeVerified - if needsDetection { - if deviceType != structureType { - deviceType = structureType - deviceInfo.parserType = structureType - deviceInfo.typeVerified = false - } - if deviceInfo.Type == "" || strings.EqualFold(deviceInfo.Type, structureType) { - deviceInfo.Type = structureType - } - } - - // Try the most likely parser first, but keep the remaining parsers in reserve - // so an incorrect hint never leaves the device unparsed. - selectedParsers := make([]struct { - Type string - Parse func([]byte) (bool, int) - }, 0, len(parsers)) - if deviceType != "" { - for _, parser := range parsers { - if parser.Type == deviceType { - selectedParsers = append(selectedParsers, parser) - break - } - } - } - for _, parser := range parsers { - alreadySelected := false - for _, selected := range selectedParsers { - if selected.Type == parser.Type { - alreadySelected = true - break - } - } - if alreadySelected { - continue - } - selectedParsers = append(selectedParsers, parser) - } - - // Try the selected parsers in order until we find one that succeeds. - for _, parser := range selectedParsers { - // A failed NVMe response may still contain a serial number, which is - // enough for the SATA and SCSI parsers to accept incorrect zero values. - if structureType == "nvme" && parser.Type != "nvme" { - continue - } - hasData, _ := parser.Parse(output) - if hasData { - deviceInfo.parserType = parser.Type - if deviceInfo.Type == "" || strings.EqualFold(deviceInfo.Type, parser.Type) { - deviceInfo.Type = parser.Type - } - // Remember that this parser is valid so future refreshes can bypass - // detection entirely. - deviceInfo.typeVerified = true - return true - } - slog.Debug("parser failed", "device", deviceInfo.Name, "parser", parser.Type) - } - - // Leave verification false so the next pass will attempt detection again. - deviceInfo.typeVerified = false - slog.Debug("parsing failed", "device", deviceInfo.Name) - return false -} - -// CollectSmart collects SMART data for a device -// Collect data using `smartctl -d -aj /dev/` when device type is known -// Always attempts to parse output even if command fails, as some data may still be available -// If collect fails, return error -// If collect succeeds, parse the output and update the SmartDataMap -// Uses -n standby to avoid waking up sleeping disks, but bypasses standby mode -// for initial data collection when no cached data exists -func (sm *SmartManager) CollectSmart(deviceInfo *DeviceInfo) error { - if deviceInfo != nil && sm.isExcludedDevice(deviceInfo.Name) { - return errNoValidSmartData - } - - // mdraid health is not exposed via SMART; Linux exposes array state in sysfs. - if deviceInfo != nil { - if ok, err := sm.collectMdraidHealth(deviceInfo); ok { - return err - } - } - // eMMC health is not exposed via SMART on Linux, but the kernel provides - // wear / EOL indicators via sysfs. Prefer that path when available. - if deviceInfo != nil { - if ok, err := sm.collectEmmcHealth(deviceInfo); ok { - return err - } - } - - if sm.smartctlPath == "" { - return errNoValidSmartData - } - - // Check if we have existing data for this exact device identity. Multiple - // bridge slots can share a path, so a name-only match is not sufficient. - hasExistingData := sm.hasDataForDevice(deviceInfo) - - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - - // Try with -n standby first if we have existing data - args := sm.smartctlArgs(deviceInfo, hasExistingData) - cmd := exec.CommandContext(ctx, sm.smartctlPath, args...) - output, err := cmd.CombinedOutput() - - // NVMe has no standby check; exit status 2 may mean the controller is inaccessible. - if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && exitErr.ExitCode() == 2 && !isNvmeControllerPath(deviceInfo.Name) { - if hasExistingData { - // Device is in standby and we have cached data, keep using cache - return nil - } - // No cached data, need to collect initial data by bypassing standby - ctx2, cancel2 := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel2() - args = sm.smartctlArgs(deviceInfo, false) - cmd = exec.CommandContext(ctx2, sm.smartctlPath, args...) - output, err = cmd.CombinedOutput() - } - - hasValidData := sm.parseSmartOutput(deviceInfo, output) - - // If NVMe controller path failed, try namespace path as fallback. - // NVMe controllers (/dev/nvme0) don't always support SMART queries. See github.com/henrygd/beszel/issues/1504 - if !hasValidData && err != nil && isNvmeControllerPath(deviceInfo.Name) { - controllerPath := deviceInfo.Name - namespacePath := controllerPath + "n1" - if !sm.isExcludedDevice(namespacePath) { - namespaceDevice := *deviceInfo - namespaceDevice.Name = namespacePath - - ctx3, cancel3 := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel3() - args = sm.smartctlArgs(&namespaceDevice, false) - cmd = exec.CommandContext(ctx3, sm.smartctlPath, args...) - output, err = cmd.CombinedOutput() - hasValidData = sm.parseSmartOutput(&namespaceDevice, output) - - // Resolve future controller scan results to this working namespace. - if hasValidData { - sm.Lock() - *deviceInfo = namespaceDevice - if sm.nvmeNamespacePaths == nil { - sm.nvmeNamespacePaths = make(map[string]string) - } - sm.nvmeNamespacePaths[controllerPath] = namespacePath - sm.Unlock() - slog.Debug("using NVMe namespace path", "controller", controllerPath, "namespace", namespacePath) - } - } - } - - if !hasValidData { - if err != nil { - slog.Info("smartctl failed", "device", deviceInfo.Name, "err", err) - return err - } - slog.Info("no valid SMART data found", "device", deviceInfo.Name) - return errNoValidSmartData - } - - return nil -} - -// smartctlArgs returns the arguments for the smartctl command -// based on the device type and whether to include standby mode -func (sm *SmartManager) smartctlArgs(deviceInfo *DeviceInfo, includeStandby bool) []string { - args := make([]string, 0, 9) - var deviceType, parserType string - - if deviceInfo != nil { - deviceType = strings.ToLower(deviceInfo.Type) - parserType = strings.ToLower(deviceInfo.parserType) - // types sometimes misidentified in scan; see github.com/henrygd/beszel/issues/1345 - // An explicit SMART_DEVICES ":type" hint is a deliberate override, so always - // pass it through; otherwise scsi/ata are left off so smartctl can auto-detect. - if deviceType != "" && (deviceInfo.explicitType || (deviceType != "scsi" && deviceType != "ata")) { - args = append(args, "-d", deviceInfo.Type) - } - } - - args = append(args, "-a", "--json=c") - effectiveType := parserType - if effectiveType == "" { - effectiveType = deviceType - } - if effectiveType == "sat" || effectiveType == "ata" { - args = append(args, "-l", "devstat") - } - - if includeStandby { - args = append(args, "-n", "standby") - } - - if deviceInfo != nil { - args = append(args, deviceInfo.Name) - } - - return args -} - -// hasDataForDevice checks if we have cached SMART data for a specific device identity. -func (sm *SmartManager) hasDataForDevice(deviceInfo *DeviceInfo) bool { - if deviceInfo == nil { - return false - } - - sm.Lock() - defer sm.Unlock() - - deviceKey := makeDeviceKey(deviceInfo.Name, deviceInfo.Type) - for _, data := range sm.SmartDataMap { - if data != nil && makeDeviceKey(data.DiskName, data.DiskType) == deviceKey { - return true - } - } - return false -} - -// parseScan parses the output of smartctl --scan -j and returns the discovered devices. -func (sm *SmartManager) parseScan(output []byte) ([]*DeviceInfo, bool) { - scan := &scanOutput{} - - if err := json.Unmarshal(output, scan); err != nil { - return nil, false - } - - if len(scan.Devices) == 0 { - slog.Debug("no devices found in smartctl scan") - return nil, false - } - - devices := make([]*DeviceInfo, 0, len(scan.Devices)) - for _, device := range scan.Devices { - slog.Debug("smartctl scan", "name", device.Name, "type", device.Type, "protocol", device.Protocol) - devices = append(devices, &DeviceInfo{ - Name: device.Name, - Type: device.Type, - InfoName: device.InfoName, - Protocol: device.Protocol, - }) - } - - return devices, true -} - -// mergeDeviceLists combines scanned and configured SMART devices, preferring -// configured SMART_DEVICES when both sources reference the same device. -func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo { - if len(scanned) == 0 && len(configured) == 0 { - return existing - } - - // buildUniqueNameIndex returns devices that appear exactly once by name. - // It is used to safely apply name-only fallbacks without RAID ambiguity. - buildUniqueNameIndex := func(devices []*DeviceInfo) map[string]*DeviceInfo { - counts := make(map[string]int, len(devices)) - for _, dev := range devices { - if dev == nil || dev.Name == "" { - continue - } - counts[dev.Name]++ - } - unique := make(map[string]*DeviceInfo, len(counts)) - for _, dev := range devices { - if dev == nil || dev.Name == "" { - continue - } - if counts[dev.Name] == 1 { - unique[dev.Name] = dev - } - } - return unique - } - - // preserveVerifiedType copies the verified type/parser metadata from an existing - // device record so that subsequent scans/config updates never downgrade a - // previously verified device. - preserveVerifiedType := func(target, prev *DeviceInfo) { - if prev == nil || !prev.typeVerified { - return - } - target.Type = prev.Type - target.typeVerified = true - target.parserType = prev.parserType - if prev.explicitType { - target.explicitType = true - } - } - - // applyConfiguredMetadata updates a matched device with any configured - // overrides, preserving verified type data when present. - applyConfiguredMetadata := func(existingDev, configuredDev *DeviceInfo) { - // Only update the type if it has not been verified yet; otherwise we - // keep the existing verified metadata intact. - if configuredDev.Type != "" && !existingDev.typeVerified { - newType := strings.TrimSpace(configuredDev.Type) - existingDev.Type = newType - existingDev.typeVerified = false - existingDev.parserType = normalizeParserType(newType) - } - if configuredDev.explicitType { - existingDev.explicitType = true - } - if configuredDev.InfoName != "" { - existingDev.InfoName = configuredDev.InfoName - } - if configuredDev.Protocol != "" { - existingDev.Protocol = configuredDev.Protocol - } - } - - existingIndex := make(map[deviceKey]*DeviceInfo, len(existing)) - for _, dev := range existing { - if dev == nil || dev.Name == "" { - continue - } - existingIndex[makeDeviceKey(dev.Name, dev.Type)] = dev - } - existingByName := buildUniqueNameIndex(existing) - - finalDevices := make([]*DeviceInfo, 0, len(scanned)+len(configured)) - deviceIndex := make(map[deviceKey]*DeviceInfo, len(scanned)+len(configured)) - - // Start with the newly scanned devices so we always surface fresh metadata, - // but ensure we retain any previously verified parser assignment. - for _, scannedDevice := range scanned { - if scannedDevice == nil || scannedDevice.Name == "" { - continue - } - - // Work on a copy so we can safely adjust metadata without mutating the - // input slices that may be reused elsewhere. - copyDev := *scannedDevice - key := makeDeviceKey(copyDev.Name, copyDev.Type) - if prev := existingIndex[key]; prev != nil { - preserveVerifiedType(©Dev, prev) - } else if prev := existingByName[copyDev.Name]; prev != nil { - preserveVerifiedType(©Dev, prev) - } - - finalDevices = append(finalDevices, ©Dev) - copyKey := makeDeviceKey(copyDev.Name, copyDev.Type) - deviceIndex[copyKey] = finalDevices[len(finalDevices)-1] - } - deviceIndexByName := buildUniqueNameIndex(finalDevices) - - // Merge configured devices on top so users can override scan results (except - // for verified type information). - for _, configuredDevice := range configured { - if configuredDevice == nil || configuredDevice.Name == "" { - continue - } - - key := makeDeviceKey(configuredDevice.Name, configuredDevice.Type) - if existingDev, ok := deviceIndex[key]; ok { - applyConfiguredMetadata(existingDev, configuredDevice) - continue - } - if existingDev := deviceIndexByName[configuredDevice.Name]; existingDev != nil { - oldKey := makeDeviceKey(existingDev.Name, existingDev.Type) - if prev := existingIndex[key]; prev != nil { - preserveVerifiedType(existingDev, prev) - } - applyConfiguredMetadata(existingDev, configuredDevice) - delete(deviceIndex, oldKey) - deviceIndex[makeDeviceKey(existingDev.Name, existingDev.Type)] = existingDev - delete(deviceIndexByName, configuredDevice.Name) - continue - } - - copyDev := *configuredDevice - key = makeDeviceKey(copyDev.Name, copyDev.Type) - if prev := existingIndex[key]; prev != nil { - preserveVerifiedType(©Dev, prev) - } else if prev := existingByName[copyDev.Name]; prev != nil { - preserveVerifiedType(©Dev, prev) - } else if copyDev.Type != "" { - copyDev.parserType = normalizeParserType(copyDev.Type) - } - - finalDevices = append(finalDevices, ©Dev) - copyKey := makeDeviceKey(copyDev.Name, copyDev.Type) - deviceIndex[copyKey] = finalDevices[len(finalDevices)-1] - } - - return finalDevices -} - -// updateSmartDevices replaces the cached device list and prunes SMART data -// entries whose backing device no longer exists. -func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) { - sm.Lock() - defer sm.Unlock() - - sm.SmartDevices = devices - - validKeys := make(map[deviceKey]struct{}, len(devices)) - nameCounts := make(map[string]int, len(devices)) - for _, device := range devices { - if device == nil || device.Name == "" { - continue - } - validKeys[makeDeviceKey(device.Name, device.Type)] = struct{}{} - nameCounts[device.Name]++ - } - - // Forget aliases when their device disappears, so a replacement controller - // can be queried normally instead of inheriting an old namespace fallback. - for controller, namespace := range sm.nvmeNamespacePaths { - if nameCounts[controller] == 0 && nameCounts[namespace] == 0 { - delete(sm.nvmeNamespacePaths, controller) - } - } - - for key, data := range sm.SmartDataMap { - if data == nil { - delete(sm.SmartDataMap, key) - continue - } - - if data.DiskType == "" { - if nameCounts[data.DiskName] == 1 { - continue - } - } else if _, ok := validKeys[makeDeviceKey(data.DiskName, data.DiskType)]; ok { - continue - } - - delete(sm.SmartDataMap, key) - } -} - -// isVirtualDevice checks if a device is a virtual disk that should be filtered out -// (e.g., Kubernetes PVCs, QEMU, VirtualBox, etc.) -func isVirtualDevice(data *smart.SmartctlOutput) bool { - for _, field := range []string{data.ScsiVendor, data.ScsiProduct, data.ModelName, data.ScsiModelName} { - field = strings.ToUpper(field) - switch { - case strings.Contains(field, "IET"), // iSCSI Enterprise Target - strings.Contains(field, "VIRTUAL"), - strings.Contains(field, "QEMU"), - strings.Contains(field, "VBOX"), - strings.Contains(field, "VMWARE"), - strings.Contains(field, "MSFT"): // Microsoft Hyper-V - return true - } - } - return false -} - -// decodeSmartctlOutput unmarshals smartctl JSON output and rejects devices -// without a serial number and virtual devices. It also returns the smartctl exit status. -func decodeSmartctlOutput(output []byte) (*smart.SmartctlOutput, int, bool) { - var data smart.SmartctlOutput - if err := json.Unmarshal(output, &data); err != nil { - slog.Debug("failed to parse smartctl output", "err", err) - return nil, 0, false - } - exitStatus := data.Smartctl.ExitStatus - - if data.SerialNumber == "" { - slog.Debug("no serial number", "device", data.Device.Name) - return nil, exitStatus, false - } - if isVirtualDevice(&data) { - slog.Debug("skipping smart", "device", data.Device.Name, "model", data.ModelName) - return nil, exitStatus, false - } - return &data, exitStatus, true -} - -// newSmartData fills the fields shared by all smartctl protocols. deviceType is -// the exact type used to identify and query the device; when set, it takes -// precedence over the generic type reported by smartctl. -func newSmartData(data *smart.SmartctlOutput, deviceType, model, firmware string, temperature int64) *smart.SmartData { - temp := uint8(min(max(temperature, 0), 255)) - smartData := &smart.SmartData{ - ModelName: model, - SerialNumber: data.SerialNumber, - FirmwareVersion: firmware, - Capacity: data.UserCapacity.Bytes, - Temperature: temp, - SmartStatus: getSmartStatus(temp, data.SmartStatus.Passed), - DiskName: data.Device.Name, - DiskType: data.Device.Type, - } - if deviceType != "" { - smartData.DiskType = deviceType - } - return smartData -} - -// storeSmartData saves parsed smartctl data, keyed by serial number. -func (sm *SmartManager) storeSmartData(smartData *smart.SmartData) { - sm.Lock() - defer sm.Unlock() - sm.SmartDataMap[smartData.SerialNumber] = smartData -} - -// parseSmartForSata parses the output of smartctl --all -j for SATA/ATA devices and updates the SmartDataMap. -// Returns hasValidData and exitStatus -func (sm *SmartManager) parseSmartForSata(output []byte, deviceType string) (bool, int) { - data, exitStatus, ok := decodeSmartctlOutput(output) - if !ok { - return false, exitStatus - } - - smartData := newSmartData(data, deviceType, data.ModelName, data.FirmwareVersion, data.Temperature.Current) - - // get values from ata_device_statistics if necessary - var ataDeviceStats smart.AtaDeviceStatistics - if smartData.Temperature == 0 { - if temp := findAtaDeviceStatisticsValue(data, &ataDeviceStats, 5, "Current Temperature", 0, 255); temp != nil { - smartData.Temperature = uint8(*temp) - } - } - - var table []smart.AtaSmartAttribute - if data.AtaSmartAttributes != nil { - table = data.AtaSmartAttributes.Table - } - smartData.Attributes = make([]*smart.SmartAttribute, 0, len(table)) - for _, attr := range table { - smartData.Attributes = append(smartData.Attributes, &smart.SmartAttribute{ - ID: attr.ID, - Name: attr.Name, - Value: attr.Value, - Worst: attr.Worst, - Threshold: attr.Thresh, - RawValue: uint64(attr.Raw.Value), // already parsed from raw.string when possible - RawString: attr.Raw.String, - WhenFailed: attr.WhenFailed, - }) - } - - sm.storeSmartData(smartData) - return true, exitStatus -} - -func getSmartStatus(temperature uint8, passed bool) string { - if passed { - return "PASSED" - } else if temperature > 0 { - return "FAILED" - } else { - return "UNKNOWN" - } -} - -// findAtaDeviceStatisticsEntry centralizes ATA devstat lookups so additional -// metrics can be pulled from the same structure in the future. -func findAtaDeviceStatisticsValue(data *smart.SmartctlOutput, ataDeviceStats *smart.AtaDeviceStatistics, entryNumber uint8, entryName string, minValue, maxValue int64) *int64 { - if len(ataDeviceStats.Pages) == 0 { - if len(data.AtaDeviceStatistics) == 0 { - return nil - } - if err := json.Unmarshal(data.AtaDeviceStatistics, ataDeviceStats); err != nil { - return nil - } - } - for pageIdx := range ataDeviceStats.Pages { - page := &ataDeviceStats.Pages[pageIdx] - if page.Number != entryNumber { - continue - } - for entryIdx := range page.Table { - entry := &page.Table[entryIdx] - if !strings.EqualFold(entry.Name, entryName) { - continue - } - if entry.Value == nil || *entry.Value < minValue || *entry.Value > maxValue { - return nil - } - return entry.Value - } - } - return nil -} - -// parseSmartForScsi parses the output of smartctl --all -j for SCSI devices and updates the SmartDataMap. -// Returns hasValidData and exitStatus -func (sm *SmartManager) parseSmartForScsi(output []byte, deviceType string) (bool, int) { - data, exitStatus, ok := decodeSmartctlOutput(output) - if !ok { - return false, exitStatus - } - - smartData := newSmartData(data, deviceType, data.ScsiModelName, data.ScsiRevision, data.Temperature.Current) - - cycles := data.ScsiStartStopCycleCounter - attributes := []*smart.SmartAttribute{ - {Name: "PowerOnHours", RawValue: data.PowerOnTime.Hours}, - {Name: "PowerOnMinutes", RawValue: data.PowerOnTime.Minutes}, - {Name: "GrownDefectList", RawValue: data.ScsiGrownDefectList}, - {Name: "StartStopCycles", RawValue: cycles.AccumulatedStartStopCycles}, - {Name: "LoadUnloadCycles", RawValue: cycles.AccumulatedLoadUnloadCycles}, - {Name: "StartStopSpecified", RawValue: cycles.SpecifiedCycleCountOverDeviceLifetime}, - {Name: "LoadUnloadSpecified", RawValue: cycles.SpecifiedLoadUnloadCountOverDeviceLifetime}, - } - - var errorLog smart.ScsiErrorCounterLog - if data.ScsiErrorCounterLog != nil { - errorLog = *data.ScsiErrorCounterLog - } - for _, counter := range []struct { - prefix string - stats smart.ScsiErrorCounter - }{{"Read", errorLog.Read}, {"Write", errorLog.Write}, {"Verify", errorLog.Verify}} { - attributes = append(attributes, - &smart.SmartAttribute{Name: counter.prefix + "TotalErrorsCorrected", RawValue: counter.stats.TotalErrorsCorrected}, - &smart.SmartAttribute{Name: counter.prefix + "TotalUncorrectedErrors", RawValue: counter.stats.TotalUncorrectedErrors}, - &smart.SmartAttribute{Name: counter.prefix + "CorrectionAlgorithmInvocations", RawValue: counter.stats.CorrectionAlgorithmInvocations}, - ) - if val := parseScsiGigabytesProcessed(counter.stats.GigabytesProcessed); val >= 0 { - attributes = append(attributes, &smart.SmartAttribute{Name: counter.prefix + "GigabytesProcessed", RawValue: uint64(val)}) - } - } - smartData.Attributes = attributes - - sm.storeSmartData(smartData) - return true, exitStatus -} - -func parseScsiGigabytesProcessed(value string) int64 { - if value == "" { - return -1 - } - normalized := strings.ReplaceAll(value, ",", "") - parsed, err := strconv.ParseInt(normalized, 10, 64) - if err != nil { - return -1 - } - return parsed -} - -// lookupDarwinNvmeCapacity returns the capacity in bytes for a given NVMe serial number on Darwin. -// It uses system_profiler SPNVMeDataType to get capacity since Apple SSDs don't report user_capacity -// via smartctl. Results are cached after the first call via sync.Once. -func (sm *SmartManager) lookupDarwinNvmeCapacity(serial string) uint64 { - sm.darwinNvmeOnce.Do(func() { - sm.darwinNvmeCapacity = make(map[string]uint64) - - provider := sm.darwinNvmeProvider - if provider == nil { - provider = func() ([]byte, error) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - return exec.CommandContext(ctx, "system_profiler", "SPNVMeDataType", "-json").Output() - } - } - - out, err := provider() - if err != nil { - slog.Debug("system_profiler NVMe lookup failed", "err", err) - return - } - - var result struct { - SPNVMeDataType []struct { - Items []struct { - DeviceSerial string `json:"device_serial"` - SizeInBytes uint64 `json:"size_in_bytes"` - } `json:"_items"` - } `json:"SPNVMeDataType"` - } - if err := json.Unmarshal(out, &result); err != nil { - slog.Debug("system_profiler NVMe parse failed", "err", err) - return - } - - for _, controller := range result.SPNVMeDataType { - for _, item := range controller.Items { - if item.DeviceSerial != "" && item.SizeInBytes > 0 { - sm.darwinNvmeCapacity[item.DeviceSerial] = item.SizeInBytes - } - } - } - }) - return sm.darwinNvmeCapacity[serial] -} - -// parseSmartForNvme parses the output of smartctl --all -j /dev/nvmeX and updates the SmartDataMap. -// deviceType is the exact type used to identify and query the device; when set, -// it takes precedence over the generic type reported by smartctl. -// Returns hasValidData and exitStatus -func (sm *SmartManager) parseSmartForNvme(output []byte, deviceType string) (bool, int) { - data, exitStatus, ok := decodeSmartctlOutput(output) - if !ok { - return false, exitStatus - } - - // smartctl may return device identity fields before failing to read the NVMe - // health log (for example, on an unsupported controller path or with insufficient - // permissions). Do not accept that partial response as valid SMART data: doing - // so stores incorrect zero values and prevents the namespace-path fallback. - log := data.NVMeSmartHealthInformationLog - if log == nil { - slog.Debug("no NVMe SMART health information", "device", data.Device.Name) - return false, exitStatus - } - - smartData := newSmartData(data, deviceType, data.ModelName, data.FirmwareVersion, log.Temperature) - if smartData.Capacity == 0 { - smartData.Capacity = data.NVMeTotalCapacity - } - if smartData.Capacity == 0 && (runtime.GOOS == "darwin" || sm.darwinNvmeProvider != nil) { - smartData.Capacity = sm.lookupDarwinNvmeCapacity(data.SerialNumber) - } - - // nvme attributes does not follow the same format as ata attributes, - // so we manually map each field to SmartAttributes - smartData.Attributes = []*smart.SmartAttribute{ - {Name: "CriticalWarning", RawValue: log.CriticalWarning}, - {Name: "Temperature", RawValue: uint64(smartData.Temperature)}, - {Name: "AvailableSpare", RawValue: log.AvailableSpare}, - {Name: "AvailableSpareThreshold", RawValue: log.AvailableSpareThreshold}, - {Name: "PercentageUsed", RawValue: log.PercentageUsed}, - {Name: "DataUnitsRead", RawValue: log.DataUnitsRead}, - {Name: "DataUnitsWritten", RawValue: log.DataUnitsWritten}, - {Name: "HostReads", RawValue: log.HostReads}, - {Name: "HostWrites", RawValue: log.HostWrites}, - {Name: "ControllerBusyTime", RawValue: log.ControllerBusyTime}, - {Name: "PowerCycles", RawValue: log.PowerCycles}, - {Name: "PowerOnHours", RawValue: log.PowerOnHours}, - {Name: "UnsafeShutdowns", RawValue: log.UnsafeShutdowns}, - {Name: "MediaErrors", RawValue: log.MediaErrors}, - {Name: "NumErrLogEntries", RawValue: log.NumErrLogEntries}, - {Name: "WarningTempTime", RawValue: log.WarningTempTime}, - {Name: "CriticalCompTime", RawValue: log.CriticalCompTime}, - } - - sm.storeSmartData(smartData) - return true, exitStatus -} - // detectSmartctl checks if smartctl is installed, returns an error if not func (sm *SmartManager) detectSmartctl() (string, error) { if runtime.GOOS == "windows" { @@ -1212,7 +554,7 @@ func isNvmeControllerPath(path string) bool { // NewSmartManager creates and initializes a new SmartManager func NewSmartManager() (*SmartManager, error) { sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), + data: make(map[string]*smart.SmartData), } sm.refreshExcludedDevices() path, err := sm.detectSmartctl() diff --git a/agent/smart_parse.go b/agent/smart_parse.go new file mode 100644 index 000000000..9fa06b825 --- /dev/null +++ b/agent/smart_parse.go @@ -0,0 +1,333 @@ +package agent + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "os/exec" + "runtime" + "strconv" + "strings" + "time" + + "github.com/henrygd/beszel/internal/entities/smart" +) + +var errNoHealthData = errors.New("no SMART health data in smartctl output") + +// parseSmartctl turns `smartctl -a --json` output into SmartData and returns +// the protocol it was parsed as ("nvme", "scsi" or "ata"). dtype is the -d +// value used for the query; when set it is reported as the disk type. +// +// The protocol comes from smartctl's device.protocol, falling back to the +// sections present. Output that identifies a disk but holds no health data for +// its protocol is rejected, so the next smartctl attempt can run instead of a +// record of zeros being stored (#2295, #1504). +func (sm *SmartManager) parseSmartctl(output []byte, dtype string) (*smart.SmartData, string, error) { + data, err := decodeSmartctlOutput(output) + if err != nil { + return nil, "", err + } + + protocol := smartProtocol(data) + switch protocol { + case "nvme": + log := data.NVMeSmartHealthInformationLog + if log == nil { + return nil, protocol, errNoHealthData + } + smartData := newSmartData(data, dtype, data.ModelName, data.FirmwareVersion, log.Temperature) + if smartData.Capacity == 0 { + smartData.Capacity = data.NVMeTotalCapacity + } + if smartData.Capacity == 0 && (runtime.GOOS == "darwin" || sm.darwinNvmeProvider != nil) { + smartData.Capacity = sm.lookupDarwinNvmeCapacity(data.SerialNumber) + } + smartData.Attributes = nvmeAttributes(log, smartData.Temperature) + return smartData, protocol, nil + + case "scsi": + if data.SmartStatus.Passed == nil && data.ScsiErrorCounterLog == nil && data.Temperature.Current == 0 { + return nil, protocol, errNoHealthData + } + smartData := newSmartData(data, dtype, data.ScsiModelName, data.ScsiRevision, data.Temperature.Current) + smartData.Attributes = scsiAttributes(data) + return smartData, protocol, nil + + default: // ata + var table []smart.AtaSmartAttribute + if data.AtaSmartAttributes != nil { + table = data.AtaSmartAttributes.Table + } + if len(table) == 0 && data.SmartStatus.Passed == nil && data.Temperature.Current == 0 && !hasJSONValue(data.AtaDeviceStatistics) { + return nil, protocol, errNoHealthData + } + smartData := newSmartData(data, dtype, data.ModelName, data.FirmwareVersion, data.Temperature.Current) + if smartData.Temperature == 0 { + var ataDeviceStats smart.AtaDeviceStatistics + if temp := findAtaDeviceStatisticsValue(data, &ataDeviceStats, 5, "Current Temperature", 0, 255); temp != nil { + smartData.Temperature = uint8(*temp) + } + } + smartData.Attributes = ataAttributes(table) + return smartData, protocol, nil + } +} + +// smartProtocol returns "nvme", "scsi" or "ata" for a smartctl response. +func smartProtocol(data *smart.SmartctlOutput) string { + switch strings.ToUpper(data.Device.Protocol) { + case "NVME": + return "nvme" + case "SCSI": + return "scsi" + case "ATA": + return "ata" + } + switch { + case data.NVMeSmartHealthInformationLog != nil, strings.EqualFold(data.Device.Type, "nvme"): + return "nvme" + case data.AtaSmartAttributes != nil: + return "ata" + case data.ScsiErrorCounterLog != nil, data.ScsiModelName != "": + return "scsi" + default: + return "ata" + } +} + +// hasJSONValue reports whether a raw JSON value is present and not null. +func hasJSONValue(raw json.RawMessage) bool { + trimmed := strings.TrimSpace(string(raw)) + return trimmed != "" && trimmed != "null" +} + +// isVirtualDevice checks if a device is a virtual disk that should be filtered out +// (e.g., Kubernetes PVCs, QEMU, VirtualBox, etc.) +func isVirtualDevice(data *smart.SmartctlOutput) bool { + for _, field := range []string{data.ScsiVendor, data.ScsiProduct, data.ModelName, data.ScsiModelName} { + field = strings.ToUpper(field) + switch { + case strings.Contains(field, "IET"), // iSCSI Enterprise Target + strings.Contains(field, "VIRTUAL"), + strings.Contains(field, "QEMU"), + strings.Contains(field, "VBOX"), + strings.Contains(field, "VMWARE"), + strings.Contains(field, "MSFT"): // Microsoft Hyper-V + return true + } + } + return false +} + +// decodeSmartctlOutput unmarshals smartctl JSON output and rejects devices +// without a serial number and virtual devices. +func decodeSmartctlOutput(output []byte) (*smart.SmartctlOutput, error) { + var data smart.SmartctlOutput + if err := json.Unmarshal(output, &data); err != nil { + return nil, fmt.Errorf("invalid smartctl JSON: %w", err) + } + if data.SerialNumber == "" { + return nil, errors.New("no serial number") + } + if isVirtualDevice(&data) { + return nil, fmt.Errorf("virtual device %q", data.ModelName) + } + return &data, nil +} + +// newSmartData fills the fields shared by all smartctl protocols. deviceType is +// the exact type used to identify and query the device; when set, it takes +// precedence over the generic type reported by smartctl. +func newSmartData(data *smart.SmartctlOutput, deviceType, model, firmware string, temperature int64) *smart.SmartData { + smartData := &smart.SmartData{ + ModelName: model, + SerialNumber: data.SerialNumber, + FirmwareVersion: firmware, + Capacity: data.UserCapacity.Bytes, + Temperature: uint8(min(max(temperature, 0), 255)), + SmartStatus: getSmartStatus(data.SmartStatus.Passed), + DiskName: data.Device.Name, + DiskType: data.Device.Type, + } + if deviceType != "" { + smartData.DiskType = deviceType + } + return smartData +} + +// getSmartStatus maps smartctl's health check result. A missing result is +// UNKNOWN rather than FAILED, so an unreadable status never raises an alert. +func getSmartStatus(passed *bool) string { + switch { + case passed == nil: + return "UNKNOWN" + case *passed: + return "PASSED" + default: + return "FAILED" + } +} + +func ataAttributes(table []smart.AtaSmartAttribute) []*smart.SmartAttribute { + attributes := make([]*smart.SmartAttribute, 0, len(table)) + for _, attr := range table { + attributes = append(attributes, &smart.SmartAttribute{ + ID: attr.ID, + Name: attr.Name, + Value: attr.Value, + Worst: attr.Worst, + Threshold: attr.Thresh, + RawValue: uint64(attr.Raw.Value), // already parsed from raw.string when possible + RawString: attr.Raw.String, + WhenFailed: attr.WhenFailed, + }) + } + return attributes +} + +// findAtaDeviceStatisticsEntry centralizes ATA devstat lookups so additional +// metrics can be pulled from the same structure in the future. +func findAtaDeviceStatisticsValue(data *smart.SmartctlOutput, ataDeviceStats *smart.AtaDeviceStatistics, entryNumber uint8, entryName string, minValue, maxValue int64) *int64 { + if len(ataDeviceStats.Pages) == 0 { + if len(data.AtaDeviceStatistics) == 0 { + return nil + } + if err := json.Unmarshal(data.AtaDeviceStatistics, ataDeviceStats); err != nil { + return nil + } + } + for pageIdx := range ataDeviceStats.Pages { + page := &ataDeviceStats.Pages[pageIdx] + if page.Number != entryNumber { + continue + } + for entryIdx := range page.Table { + entry := &page.Table[entryIdx] + if !strings.EqualFold(entry.Name, entryName) { + continue + } + if entry.Value == nil || *entry.Value < minValue || *entry.Value > maxValue { + return nil + } + return entry.Value + } + } + return nil +} + +func scsiAttributes(data *smart.SmartctlOutput) []*smart.SmartAttribute { + cycles := data.ScsiStartStopCycleCounter + attributes := []*smart.SmartAttribute{ + {Name: "PowerOnHours", RawValue: data.PowerOnTime.Hours}, + {Name: "PowerOnMinutes", RawValue: data.PowerOnTime.Minutes}, + {Name: "GrownDefectList", RawValue: data.ScsiGrownDefectList}, + {Name: "StartStopCycles", RawValue: cycles.AccumulatedStartStopCycles}, + {Name: "LoadUnloadCycles", RawValue: cycles.AccumulatedLoadUnloadCycles}, + {Name: "StartStopSpecified", RawValue: cycles.SpecifiedCycleCountOverDeviceLifetime}, + {Name: "LoadUnloadSpecified", RawValue: cycles.SpecifiedLoadUnloadCountOverDeviceLifetime}, + } + + var errorLog smart.ScsiErrorCounterLog + if data.ScsiErrorCounterLog != nil { + errorLog = *data.ScsiErrorCounterLog + } + for _, counter := range []struct { + prefix string + stats smart.ScsiErrorCounter + }{{"Read", errorLog.Read}, {"Write", errorLog.Write}, {"Verify", errorLog.Verify}} { + attributes = append(attributes, + &smart.SmartAttribute{Name: counter.prefix + "TotalErrorsCorrected", RawValue: counter.stats.TotalErrorsCorrected}, + &smart.SmartAttribute{Name: counter.prefix + "TotalUncorrectedErrors", RawValue: counter.stats.TotalUncorrectedErrors}, + &smart.SmartAttribute{Name: counter.prefix + "CorrectionAlgorithmInvocations", RawValue: counter.stats.CorrectionAlgorithmInvocations}, + ) + if val := parseScsiGigabytesProcessed(counter.stats.GigabytesProcessed); val >= 0 { + attributes = append(attributes, &smart.SmartAttribute{Name: counter.prefix + "GigabytesProcessed", RawValue: uint64(val)}) + } + } + return attributes +} + +func parseScsiGigabytesProcessed(value string) int64 { + if value == "" { + return -1 + } + normalized := strings.ReplaceAll(value, ",", "") + parsed, err := strconv.ParseInt(normalized, 10, 64) + if err != nil { + return -1 + } + return parsed +} + +// nvmeAttributes maps the NVMe health log, which does not follow the ATA +// attribute format, to SmartAttributes. +func nvmeAttributes(log *smart.NVMeSmartHealthInformationLog, temperature uint8) []*smart.SmartAttribute { + return []*smart.SmartAttribute{ + {Name: "CriticalWarning", RawValue: log.CriticalWarning}, + {Name: "Temperature", RawValue: uint64(temperature)}, + {Name: "AvailableSpare", RawValue: log.AvailableSpare}, + {Name: "AvailableSpareThreshold", RawValue: log.AvailableSpareThreshold}, + {Name: "PercentageUsed", RawValue: log.PercentageUsed}, + {Name: "DataUnitsRead", RawValue: log.DataUnitsRead}, + {Name: "DataUnitsWritten", RawValue: log.DataUnitsWritten}, + {Name: "HostReads", RawValue: log.HostReads}, + {Name: "HostWrites", RawValue: log.HostWrites}, + {Name: "ControllerBusyTime", RawValue: log.ControllerBusyTime}, + {Name: "PowerCycles", RawValue: log.PowerCycles}, + {Name: "PowerOnHours", RawValue: log.PowerOnHours}, + {Name: "UnsafeShutdowns", RawValue: log.UnsafeShutdowns}, + {Name: "MediaErrors", RawValue: log.MediaErrors}, + {Name: "NumErrLogEntries", RawValue: log.NumErrLogEntries}, + {Name: "WarningTempTime", RawValue: log.WarningTempTime}, + {Name: "CriticalCompTime", RawValue: log.CriticalCompTime}, + } +} + +// lookupDarwinNvmeCapacity returns the capacity in bytes for a given NVMe serial number on Darwin. +// It uses system_profiler SPNVMeDataType to get capacity since Apple SSDs don't report user_capacity +// via smartctl. Results are cached after the first call via sync.Once. +func (sm *SmartManager) lookupDarwinNvmeCapacity(serial string) uint64 { + sm.darwinNvmeOnce.Do(func() { + sm.darwinNvmeCapacity = make(map[string]uint64) + + provider := sm.darwinNvmeProvider + if provider == nil { + provider = func() ([]byte, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + return exec.CommandContext(ctx, "system_profiler", "SPNVMeDataType", "-json").Output() + } + } + + out, err := provider() + if err != nil { + slog.Debug("system_profiler NVMe lookup failed", "err", err) + return + } + + var result struct { + SPNVMeDataType []struct { + Items []struct { + DeviceSerial string `json:"device_serial"` + SizeInBytes uint64 `json:"size_in_bytes"` + } `json:"_items"` + } `json:"SPNVMeDataType"` + } + if err := json.Unmarshal(out, &result); err != nil { + slog.Debug("system_profiler NVMe parse failed", "err", err) + return + } + + for _, controller := range result.SPNVMeDataType { + for _, item := range controller.Items { + if item.DeviceSerial != "" && item.SizeInBytes > 0 { + sm.darwinNvmeCapacity[item.DeviceSerial] = item.SizeInBytes + } + } + } + }) + return sm.darwinNvmeCapacity[serial] +} diff --git a/agent/smart_test.go b/agent/smart_test.go index c7b7ee960..0bc5c213c 100644 --- a/agent/smart_test.go +++ b/agent/smart_test.go @@ -3,11 +3,12 @@ package agent import ( - "encoding/json" "errors" "fmt" + "maps" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -17,42 +18,33 @@ import ( "github.com/stretchr/testify/require" ) -func TestParseSmartForScsi(t *testing.T) { - fixturePath := filepath.Join("test-data", "smart", "scsi.json") - data, err := os.ReadFile(fixturePath) - if err != nil { - t.Fatalf("failed reading fixture: %v", err) - } +func readSmartFixture(t *testing.T, name string) []byte { + t.Helper() + data, err := os.ReadFile(filepath.Join("test-data", "smart", name)) + require.NoError(t, err) + return data +} - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } +// parseSmart runs parseSmartctl and requires it to succeed. +func parseSmart(t *testing.T, output []byte, dtype string) *smart.SmartData { + t.Helper() + sm := &SmartManager{} + data, _, err := sm.parseSmartctl(output, dtype) + require.NoError(t, err) + return data +} - hasData, exitStatus := sm.parseSmartForScsi(data, "") - if !hasData { - t.Fatalf("expected SCSI data to parse successfully") - } - if exitStatus != 0 { - t.Fatalf("expected exit status 0, got %d", exitStatus) - } +func TestParseSmartctlScsi(t *testing.T) { + deviceData := parseSmart(t, readSmartFixture(t, "scsi.json"), "") - deviceData, ok := sm.SmartDataMap["9YHSDH9B"] - if !ok { - t.Fatalf("expected smart data entry for serial 9YHSDH9B") - } - - assert.Equal(t, deviceData.ModelName, "YADRO WUH721414AL4204") - assert.Equal(t, deviceData.SerialNumber, "9YHSDH9B") - assert.Equal(t, deviceData.FirmwareVersion, "C240") - assert.Equal(t, deviceData.DiskName, "/dev/sde") - assert.Equal(t, deviceData.DiskType, "scsi") - assert.EqualValues(t, deviceData.Temperature, 34) - assert.Equal(t, deviceData.SmartStatus, "PASSED") - assert.EqualValues(t, deviceData.Capacity, 14000519643136) - - if len(deviceData.Attributes) == 0 { - t.Fatalf("expected attributes to be populated") - } + assert.Equal(t, "YADRO WUH721414AL4204", deviceData.ModelName) + assert.Equal(t, "9YHSDH9B", deviceData.SerialNumber) + assert.Equal(t, "C240", deviceData.FirmwareVersion) + assert.Equal(t, "/dev/sde", deviceData.DiskName) + assert.Equal(t, "scsi", deviceData.DiskType) + assert.EqualValues(t, 34, deviceData.Temperature) + assert.Equal(t, "PASSED", deviceData.SmartStatus) + assert.EqualValues(t, 14000519643136, deviceData.Capacity) assertAttrValue(t, deviceData.Attributes, "PowerOnHours", 458) assertAttrValue(t, deviceData.Attributes, "PowerOnMinutes", 25) @@ -69,22 +61,13 @@ func TestParseSmartForScsi(t *testing.T) { assertAttrValue(t, deviceData.Attributes, "VerifyGigabytesProcessed", 0) } -func TestParseSmartForSata(t *testing.T) { - fixturePath := filepath.Join("test-data", "smart", "sda.json") - data, err := os.ReadFile(fixturePath) +func TestParseSmartctlSata(t *testing.T) { + sm := &SmartManager{} + deviceData, protocol, err := sm.parseSmartctl(readSmartFixture(t, "sda.json"), "") require.NoError(t, err) + assert.Equal(t, "ata", protocol) - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } - - hasData, exitStatus := sm.parseSmartForSata(data, "") - require.True(t, hasData) - assert.Equal(t, 64, exitStatus) - - deviceData, ok := sm.SmartDataMap["9C40918040082"] - require.True(t, ok, "expected smart data entry for serial 9C40918040082") - + assert.Equal(t, "9C40918040082", deviceData.SerialNumber) assert.Equal(t, "P3-2TB", deviceData.ModelName) assert.Equal(t, "X0104A0", deviceData.FirmwareVersion) assert.Equal(t, "/dev/sda", deviceData.DiskName) @@ -92,200 +75,13 @@ func TestParseSmartForSata(t *testing.T) { assert.Equal(t, uint8(31), deviceData.Temperature) assert.Equal(t, "PASSED", deviceData.SmartStatus) assert.Equal(t, uint64(2048408248320), deviceData.Capacity) - if assert.NotEmpty(t, deviceData.Attributes) { - assertAttrValue(t, deviceData.Attributes, "Temperature_Celsius", 31) - } + assertAttrValue(t, deviceData.Attributes, "Temperature_Celsius", 31) } -func TestParseSmartForSataPreservesFailedAndUnknownStatus(t *testing.T) { - for _, test := range []struct { - name string - temperature int - want string - }{ - {name: "failed", temperature: 30, want: "FAILED"}, - {name: "unknown", want: "UNKNOWN"}, - } { - t.Run(test.name, func(t *testing.T) { - jsonPayload := []byte(fmt.Sprintf(`{ - "device": {"name": "/dev/sda", "type": "sat"}, - "serial_number": "PRESERVE%s", - "temperature": {"current": %d}, - "ata_smart_attributes": {"table": [{"id": 197, "raw": {"value": 1, "string": "1"}}]} - }`, test.name, test.temperature)) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - hasData, _ := sm.parseSmartForSata(jsonPayload, "") - require.True(t, hasData) - assert.Equal(t, test.want, sm.SmartDataMap["PRESERVE"+test.name].SmartStatus) - }) - } -} - -func TestParseSmartForSataDeviceStatisticsTemperature(t *testing.T) { - jsonPayload := []byte(`{ - "smartctl": {"exit_status": 0}, - "device": {"name": "/dev/sdb", "type": "sat"}, - "model_name": "SanDisk SSD U110 16GB", - "serial_number": "DEVSTAT123", - "firmware_version": "U21B001", - "user_capacity": {"bytes": 16013942784}, - "smart_status": {"passed": true}, - "ata_smart_attributes": {"table": []}, - "ata_device_statistics": { - "pages": [ - { - "number": 5, - "name": "Temperature Statistics", - "table": [ - {"name": "Current Temperature", "value": 22, "flags": {"valid": true}} - ] - } - ] - } - }`) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "") - require.True(t, hasData) - assert.Equal(t, 0, exitStatus) - - deviceData, ok := sm.SmartDataMap["DEVSTAT123"] - require.True(t, ok, "expected smart data entry for serial DEVSTAT123") - assert.Equal(t, uint8(22), deviceData.Temperature) -} - -func TestParseSmartForSataAtaDeviceStatistics(t *testing.T) { - // tests that ata_device_statistics values are parsed correctly - jsonPayload := []byte(`{ - "smartctl": {"exit_status": 0}, - "device": {"name": "/dev/sdb", "type": "sat"}, - "model_name": "SanDisk SSD U110 16GB", - "serial_number": "lksjfh23lhj", - "firmware_version": "U21B001", - "user_capacity": {"bytes": 16013942784}, - "smart_status": {"passed": true}, - "ata_smart_attributes": {"table": []}, - "ata_device_statistics": { - "pages": [ - { - "number": 5, - "name": "Temperature Statistics", - "table": [ - {"name": "Current Temperature", "value": 43, "flags": {"valid": true}}, - {"name": "Specified Minimum Operating Temperature", "value": -20, "flags": {"valid": true}} - ] - } - ] - } - }`) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "") - require.True(t, hasData) - assert.Equal(t, 0, exitStatus) - - deviceData, ok := sm.SmartDataMap["lksjfh23lhj"] - require.True(t, ok, "expected smart data entry for serial lksjfh23lhj") - assert.Equal(t, uint8(43), deviceData.Temperature) -} - -func TestParseSmartForSataNegativeDeviceStatistics(t *testing.T) { - // Tests that negative values in ata_device_statistics (e.g. min operating temp) - // do not cause the entire SAT parser to fail. - jsonPayload := []byte(`{ - "smartctl": {"exit_status": 0}, - "device": {"name": "/dev/sdb", "type": "sat"}, - "model_name": "SanDisk SSD U110 16GB", - "serial_number": "NEGATIVE123", - "firmware_version": "U21B001", - "user_capacity": {"bytes": 16013942784}, - "smart_status": {"passed": true}, - "temperature": {"current": 38}, - "ata_smart_attributes": {"table": []}, - "ata_device_statistics": { - "pages": [ - { - "number": 5, - "name": "Temperature Statistics", - "table": [ - {"name": "Current Temperature", "value": 38, "flags": {"valid": true}}, - {"name": "Specified Minimum Operating Temperature", "value": -20, "flags": {"valid": true}} - ] - } - ] - } - }`) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "") - require.True(t, hasData) - assert.Equal(t, 0, exitStatus) - - deviceData, ok := sm.SmartDataMap["NEGATIVE123"] - require.True(t, ok, "expected smart data entry for serial NEGATIVE123") - assert.Equal(t, uint8(38), deviceData.Temperature) -} - -func TestParseSmartForSataParentheticalRawValue(t *testing.T) { - jsonPayload := []byte(`{ - "smartctl": {"exit_status": 0}, - "device": {"name": "/dev/sdz", "type": "sat"}, - "model_name": "Example", - "serial_number": "PARENTHESES123", - "firmware_version": "1.0", - "user_capacity": {"bytes": 1024}, - "smart_status": {"passed": true}, - "temperature": {"current": 25}, - "ata_smart_attributes": { - "table": [ - { - "id": 9, - "name": "Power_On_Hours", - "value": 93, - "worst": 55, - "thresh": 0, - "when_failed": "", - "raw": { - "value": 57891864217128, - "string": "39925 (212 206 0)" - } - } - ] - } - }`) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - - hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "") - require.True(t, hasData) - assert.Equal(t, 0, exitStatus) - - data, ok := sm.SmartDataMap["PARENTHESES123"] - require.True(t, ok) - require.Len(t, data.Attributes, 1) - - attr := data.Attributes[0] - assert.Equal(t, uint64(39925), attr.RawValue) - assert.Equal(t, "39925 (212 206 0)", attr.RawString) -} - -func TestParseSmartForNvme(t *testing.T) { - fixturePath := filepath.Join("test-data", "smart", "nvme0.json") - data, err := os.ReadFile(fixturePath) - require.NoError(t, err) - - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } - - hasData, exitStatus := sm.parseSmartForNvme(data, "") - require.True(t, hasData) - assert.Equal(t, 0, exitStatus) - - deviceData, ok := sm.SmartDataMap["2024031600129"] - require.True(t, ok, "expected smart data entry for serial 2024031600129") +func TestParseSmartctlNvme(t *testing.T) { + deviceData := parseSmart(t, readSmartFixture(t, "nvme0.json"), "") + assert.Equal(t, "2024031600129", deviceData.SerialNumber) assert.Equal(t, "PELADN 512GB", deviceData.ModelName) assert.Equal(t, "VC2S038E", deviceData.FirmwareVersion) assert.Equal(t, "/dev/nvme0", deviceData.DiskName) @@ -293,19 +89,169 @@ func TestParseSmartForNvme(t *testing.T) { assert.Equal(t, uint8(61), deviceData.Temperature) assert.Equal(t, "PASSED", deviceData.SmartStatus) assert.Equal(t, uint64(512110190592), deviceData.Capacity) - if assert.NotEmpty(t, deviceData.Attributes) { - assertAttrValue(t, deviceData.Attributes, "PercentageUsed", 0) - assertAttrValue(t, deviceData.Attributes, "DataUnitsWritten", 16040567) + assertAttrValue(t, deviceData.Attributes, "PercentageUsed", 0) + assertAttrValue(t, deviceData.Attributes, "DataUnitsWritten", 16040567) +} + +// The -d type used for the query is reported as the disk type, so RAID and +// bridge types stay visible in the hub. +func TestParseSmartctlReportsQueryType(t *testing.T) { + assert.Equal(t, "sat+megaraid", parseSmart(t, readSmartFixture(t, "sda.json"), "sat+megaraid").DiskType) + assert.Equal(t, "ata", parseSmart(t, readSmartFixture(t, "sda.json"), "ata").DiskType) +} + +func TestParseSmartctlStatus(t *testing.T) { + for _, test := range []struct { + name, status string + want string + }{ + {"passed", `,"smart_status":{"passed":true}`, "PASSED"}, + {"failed", `,"smart_status":{"passed":false}`, "FAILED"}, + // A missing status must not read as a failure, even with a temperature. + {"missing", "", "UNKNOWN"}, + {"null", `,"smart_status":{"passed":null}`, "UNKNOWN"}, + } { + t.Run(test.name, func(t *testing.T) { + output := fmt.Sprintf(`{ + "device": {"name": "/dev/sda", "type": "sat", "protocol": "ATA"}, + "serial_number": "STATUS", + "temperature": {"current": 30}, + "ata_smart_attributes": {"table": [{"id": 197, "raw": {"value": 1, "string": "1"}}]} + %s + }`, test.status) + assert.Equal(t, test.want, parseSmart(t, []byte(output), "").SmartStatus) + }) } } +// Output that identifies a disk but holds no health data for its protocol is +// rejected so the next attempt runs, instead of storing zeros (#2295, #1504). +func TestParseSmartctlRequiresHealthData(t *testing.T) { + for _, test := range []struct { + name, protocol, health string + valid bool + }{ + {"scsi failed status", "SCSI", `,"smart_status":{"passed":false}`, true}, + {"scsi passed status", "SCSI", `,"smart_status":{"passed":true}`, true}, + {"scsi error log", "SCSI", `,"scsi_error_counter_log":{"read":{}}`, true}, + {"scsi null status", "SCSI", `,"smart_status":null`, false}, + {"scsi nothing", "SCSI", "", false}, + {"nvme zero log", "NVMe", `,"nvme_smart_health_information_log":{"temperature":0}`, true}, + {"nvme null log", "NVMe", `,"nvme_smart_health_information_log":null`, false}, + {"nvme nothing", "NVMe", "", false}, + {"ata attributes", "ATA", `,"ata_smart_attributes":{"table":[{"id":5}]}`, true}, + {"ata status only", "ATA", `,"smart_status":{"passed":true}`, true}, + {"ata devstat only", "ATA", `,"ata_device_statistics":{"pages":[]}`, true}, + {"ata empty table", "ATA", `,"ata_smart_attributes":{"table":[]}`, false}, + } { + t.Run(test.name, func(t *testing.T) { + output := fmt.Sprintf(`{"device":{"name":"/dev/sda","protocol":%q},"serial_number":"HEALTH"%s}`, test.protocol, test.health) + _, _, err := (&SmartManager{}).parseSmartctl([]byte(output), "") + if test.valid { + assert.NoError(t, err) + } else { + assert.ErrorIs(t, err, errNoHealthData) + } + }) + } + + _, _, err := (&SmartManager{}).parseSmartctl(readSmartFixture(t, "scsi_identity.json"), "") + assert.ErrorIs(t, err, errNoHealthData) + + // An NVMe controller can return identity fields but fail the health log. + _, _, err = (&SmartManager{}).parseSmartctl([]byte(`{ + "smartctl": {"exit_status": 2}, + "device": {"name": "/dev/nvme0", "type": "nvme"}, + "model_name": "Netac NVMe SSD 500GB", + "serial_number": "IDENTITY-ONLY", + "user_capacity": {"bytes": 500107862016} + }`), "") + assert.ErrorIs(t, err, errNoHealthData) + + // Real smartctl output when the agent lacks CAP_SYS_ADMIN for NVMe. + _, _, err = (&SmartManager{}).parseSmartctl(readSmartFixture(t, "nvme_permission_denied.json"), "nvme") + assert.ErrorIs(t, err, errNoHealthData) +} + +func TestParseSmartctlRejectsInvalidOutput(t *testing.T) { + for name, output := range map[string]string{ + "not json": "not json", + "no serial": `{"device":{"name":"/dev/sda"},"smart_status":{"passed":true}}`, + "virtual": `{"serial_number":"V1","model_name":"QEMU HARDDISK","smart_status":{"passed":true}}`, + } { + t.Run(name, func(t *testing.T) { + _, _, err := (&SmartManager{}).parseSmartctl([]byte(output), "") + assert.Error(t, err) + }) + } +} + +// Without device.protocol (older smartctl, hand-written JSON) the protocol is +// inferred from the sections present. +func TestSmartProtocolInference(t *testing.T) { + for _, test := range []struct { + data smart.SmartctlOutput + want string + }{ + {smart.SmartctlOutput{Device: smart.DeviceInfo{Protocol: "NVMe"}}, "nvme"}, + {smart.SmartctlOutput{Device: smart.DeviceInfo{Type: "nvme"}}, "nvme"}, + {smart.SmartctlOutput{NVMeSmartHealthInformationLog: &smart.NVMeSmartHealthInformationLog{}}, "nvme"}, + {smart.SmartctlOutput{AtaSmartAttributes: &smart.AtaSmartAttributes{}}, "ata"}, + {smart.SmartctlOutput{ScsiErrorCounterLog: &smart.ScsiErrorCounterLog{}}, "scsi"}, + {smart.SmartctlOutput{ScsiModelName: "SEAGATE"}, "scsi"}, + {smart.SmartctlOutput{}, "ata"}, + } { + assert.Equal(t, test.want, smartProtocol(&test.data), "%+v", test.data) + } +} + +func TestParseSmartctlDeviceStatisticsTemperature(t *testing.T) { + for _, test := range []struct { + name string + current string + want uint8 + }{ + {"fallback when temperature missing", "", 22}, + // Negative entries (e.g. minimum operating temperature) must not fail the parse. + {"reported temperature wins", `"temperature": {"current": 38},`, 38}, + } { + t.Run(test.name, func(t *testing.T) { + output := fmt.Sprintf(`{ + "device": {"name": "/dev/sdb", "type": "sat"}, + "serial_number": "DEVSTAT123", + "smart_status": {"passed": true}, + %s + "ata_smart_attributes": {"table": []}, + "ata_device_statistics": {"pages": [{"number": 5, "table": [ + {"name": "Current Temperature", "value": 22}, + {"name": "Specified Minimum Operating Temperature", "value": -20} + ]}]} + }`, test.current) + assert.Equal(t, test.want, parseSmart(t, []byte(output), "").Temperature) + }) + } +} + +func TestParseSmartctlParentheticalRawValue(t *testing.T) { + deviceData := parseSmart(t, []byte(`{ + "device": {"name": "/dev/sdz", "type": "sat"}, + "serial_number": "PARENTHESES123", + "smart_status": {"passed": true}, + "ata_smart_attributes": {"table": [{ + "id": 9, "name": "Power_On_Hours", "value": 93, "worst": 55, "thresh": 0, + "raw": {"value": 57891864217128, "string": "39925 (212 206 0)"} + }]} + }`), "") + require.Len(t, deviceData.Attributes, 1) + assert.Equal(t, uint64(39925), deviceData.Attributes[0].RawValue) + assert.Equal(t, "39925 (212 206 0)", deviceData.Attributes[0].RawString) +} + // Counters larger than their old narrow Go types used to fail json.Unmarshal and // drop the whole device (#2484, #2582). -func TestParseSmartForNvmeLargeCounters(t *testing.T) { - jsonPayload := []byte(`{ - "smartctl": {"exit_status": 0}, +func TestParseSmartctlNvmeLargeCounters(t *testing.T) { + deviceData := parseSmart(t, []byte(`{ "device": {"name": "/dev/nvme0", "type": "nvme"}, - "model_name": "Test NVMe", "serial_number": "NVME-LARGE", "smart_status": {"passed": true}, "nvme_smart_health_information_log": { @@ -315,14 +261,7 @@ func TestParseSmartForNvmeLargeCounters(t *testing.T) { "power_on_hours": 5000000000, "unsafe_shutdowns": 159754 } - }`) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - hasData, _ := sm.parseSmartForNvme(jsonPayload, "") - require.True(t, hasData) - - deviceData := sm.SmartDataMap["NVME-LARGE"] - require.NotNil(t, deviceData) + }`), "") assertAttrValue(t, deviceData.Attributes, "UnsafeShutdowns", 159754) assertAttrValue(t, deviceData.Attributes, "PowerCycles", 70000) assertAttrValue(t, deviceData.Attributes, "PowerOnHours", 5000000000) @@ -330,34 +269,28 @@ func TestParseSmartForNvmeLargeCounters(t *testing.T) { } // Out-of-range temperatures are clamped instead of failing the whole parse. -func TestParseSmartTemperatureOutOfRange(t *testing.T) { - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - - hasData, _ := sm.parseSmartForSata([]byte(`{ +func TestParseSmartctlTemperatureOutOfRange(t *testing.T) { + sata := parseSmart(t, []byte(`{ "device": {"name": "/dev/sda", "type": "sat"}, "serial_number": "SATA-NEG", "smart_status": {"passed": true}, "temperature": {"current": -5}, "ata_smart_attributes": {"table": []} }`), "") - require.True(t, hasData) - assert.Equal(t, uint8(0), sm.SmartDataMap["SATA-NEG"].Temperature) + assert.Equal(t, uint8(0), sata.Temperature) - hasData, _ = sm.parseSmartForNvme([]byte(`{ + nvme := parseSmart(t, []byte(`{ "device": {"name": "/dev/nvme0", "type": "nvme"}, "serial_number": "NVME-HOT", "smart_status": {"passed": true}, "nvme_smart_health_information_log": {"temperature": 300} }`), "") - require.True(t, hasData) - assert.Equal(t, uint8(255), sm.SmartDataMap["NVME-HOT"].Temperature) - assertAttrValue(t, sm.SmartDataMap["NVME-HOT"].Attributes, "Temperature", 255) + assert.Equal(t, uint8(255), nvme.Temperature) + assertAttrValue(t, nvme.Attributes, "Temperature", 255) } -func TestParseSmartForScsiWithoutErrorCounterLog(t *testing.T) { - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - - hasData, _ := sm.parseSmartForScsi([]byte(`{ +func TestParseSmartctlScsiWithoutErrorCounterLog(t *testing.T) { + deviceData := parseSmart(t, []byte(`{ "device": {"name": "/dev/sdb", "type": "scsi"}, "scsi_model_name": "SEAGATE ST4000NM0023", "serial_number": "SCSI-NOLOG", @@ -365,617 +298,698 @@ func TestParseSmartForScsiWithoutErrorCounterLog(t *testing.T) { "temperature": {"current": 30}, "power_on_time": {"hours": 1234} }`), "") - require.True(t, hasData) - - deviceData := sm.SmartDataMap["SCSI-NOLOG"] - require.NotNil(t, deviceData) assertAttrValue(t, deviceData.Attributes, "PowerOnHours", 1234) assertAttrValue(t, deviceData.Attributes, "ReadTotalUncorrectedErrors", 0) assert.Nil(t, findAttr(deviceData.Attributes, "ReadGigabytesProcessed")) } -func TestHasDataForDevice(t *testing.T) { - sm := &SmartManager{ - SmartDataMap: map[string]*smart.SmartData{ - "serial-1": {DiskName: "/dev/sda", DiskType: "jms56x,0"}, - "serial-2": nil, - }, - } +func TestParseSmartctlNvmeAppleSSD(t *testing.T) { + // Apple SSDs don't report user_capacity via smartctl; capacity should be fetched + // from system_profiler via the darwinNvmeProvider fallback. + providerCalls := 0 + sm := &SmartManager{darwinNvmeProvider: func() ([]byte, error) { + providerCalls++ + return []byte(`{"SPNVMeDataType": [{"_items": [{"device_serial": "0ba0147940253c15", "size_in_bytes": 251000193024}]}]}`), nil + }} - assert.True(t, sm.hasDataForDevice(&DeviceInfo{Name: "/dev/sda", Type: "jms56x,0"})) - assert.False(t, sm.hasDataForDevice(&DeviceInfo{Name: "/dev/sda", Type: "jms56x,1"})) - assert.False(t, sm.hasDataForDevice(&DeviceInfo{Name: "/dev/sdb", Type: "jms56x,0"})) - assert.False(t, sm.hasDataForDevice(nil)) + output := readSmartFixture(t, "apple_nvme.json") + deviceData, _, err := sm.parseSmartctl(output, "") + require.NoError(t, err) + assert.Equal(t, "APPLE SSD AP0256Q", deviceData.ModelName) + assert.Equal(t, uint64(251000193024), deviceData.Capacity) + assert.Equal(t, uint8(42), deviceData.Temperature) + assert.Equal(t, "PASSED", deviceData.SmartStatus) + + _, _, err = sm.parseSmartctl(output, "") + require.NoError(t, err) + assert.Equal(t, 1, providerCalls, "system_profiler should be called once") } -func TestDevicesSnapshotReturnsCopy(t *testing.T) { - originalDevice := &DeviceInfo{Name: "/dev/sda"} - sm := &SmartManager{ - SmartDevices: []*DeviceInfo{ - originalDevice, - {Name: "/dev/sdb"}, - }, - } +func TestParseConfiguredDevices(t *testing.T) { + devices, err := parseConfiguredDevices("/dev/sda:SAT, /dev/nvme0:nvme,/dev/sdb") + require.NoError(t, err) + assert.Equal(t, []smartDevice{ + {name: "/dev/sda", dtype: "sat", explicit: true}, + {name: "/dev/nvme0", dtype: "nvme", explicit: true}, + {name: "/dev/sdb"}, + }, devices) - snapshot := sm.devicesSnapshot() - require.Len(t, snapshot, 2) - - sm.SmartDevices[0] = &DeviceInfo{Name: "/dev/sdz"} - assert.Equal(t, "/dev/sda", snapshot[0].Name) - - snapshot[1] = &DeviceInfo{Name: "/dev/nvme0"} - assert.Equal(t, "/dev/sdb", sm.SmartDevices[1].Name) - - sm.SmartDevices = append(sm.SmartDevices, &DeviceInfo{Name: "/dev/nvme1"}) - assert.Len(t, snapshot, 2) -} - -func TestScanDevicesWithEnvOverrideAndSeparator(t *testing.T) { t.Setenv("SMART_DEVICES_SEPARATOR", "|") - t.Setenv("SMART_DEVICES", "/dev/sda:jmb39x-q,0|/dev/nvme0:nvme") - - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } - - err := sm.ScanDevices(true) + devices, err = parseConfiguredDevices("/dev/sda:jmb39x-q,0|/dev/bus/0:megaraid,1") require.NoError(t, err) + assert.Equal(t, []smartDevice{ + {name: "/dev/sda", dtype: "jmb39x-q,0", explicit: true}, + {name: "/dev/bus/0", dtype: "megaraid,1", explicit: true}, + }, devices) - require.Len(t, sm.SmartDevices, 2) - assert.Equal(t, "/dev/sda", sm.SmartDevices[0].Name) - assert.Equal(t, "jmb39x-q,0", sm.SmartDevices[0].Type) - assert.Equal(t, "/dev/nvme0", sm.SmartDevices[1].Name) - assert.Equal(t, "nvme", sm.SmartDevices[1].Type) -} - -func TestScanDevicesWithEnvOverride(t *testing.T) { - t.Setenv("SMART_DEVICES", "/dev/sda:sat, /dev/nvme0:nvme") - - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } - - err := sm.ScanDevices(true) - require.NoError(t, err) - - require.Len(t, sm.SmartDevices, 2) - assert.Equal(t, "/dev/sda", sm.SmartDevices[0].Name) - assert.Equal(t, "sat", sm.SmartDevices[0].Type) - assert.Equal(t, "/dev/nvme0", sm.SmartDevices[1].Name) - assert.Equal(t, "nvme", sm.SmartDevices[1].Type) -} - -func TestScanDevicesWithEnvOverrideInvalid(t *testing.T) { - t.Setenv("SMART_DEVICES", ":sat") - - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } - - err := sm.ScanDevices(true) - require.Error(t, err) -} - -func TestScanDevicesWithEnvOverrideEmpty(t *testing.T) { - t.Setenv("SMART_DEVICES", " ") - - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - } - - err := sm.ScanDevices(true) + _, err = parseConfiguredDevices(":sat") + assert.Error(t, err) + _, err = parseConfiguredDevices(" | ") assert.ErrorIs(t, err, errNoValidSmartData) - assert.Empty(t, sm.SmartDevices) } -func TestSmartctlArgsWithoutType(t *testing.T) { - device := &DeviceInfo{Name: "/dev/sda"} - - sm := &SmartManager{} - - args := sm.smartctlArgs(device, true) - assert.Equal(t, []string{"-a", "--json=c", "-n", "standby", "/dev/sda"}, args) -} - -func TestSmartctlArgs(t *testing.T) { - sm := &SmartManager{} - - sataDevice := &DeviceInfo{Name: "/dev/sda", Type: "sat"} - assert.Equal(t, - []string{"-d", "sat", "-a", "--json=c", "-l", "devstat", "-n", "standby", "/dev/sda"}, - sm.smartctlArgs(sataDevice, true), - ) - - assert.Equal(t, - []string{"-d", "sat", "-a", "--json=c", "-l", "devstat", "/dev/sda"}, - sm.smartctlArgs(sataDevice, false), - ) - - nvmeDevice := &DeviceInfo{Name: "/dev/nvme0", Type: "nvme"} - assert.Equal(t, - []string{"-d", "nvme", "-a", "--json=c", "-n", "standby", "/dev/nvme0"}, - sm.smartctlArgs(nvmeDevice, true), - ) - - assert.Equal(t, - []string{"-a", "--json=c", "-n", "standby"}, - sm.smartctlArgs(nil, true), - ) -} - -// TestSmartctlArgsExplicitType verifies that an explicit SMART_DEVICES type hint -// is always passed to smartctl via -d, while a scan-detected scsi/ata type is -// still left off so smartctl can auto-detect it (see issue #1345). -func TestSmartctlArgsExplicitType(t *testing.T) { - sm := &SmartManager{} - - // Scan-detected scsi: -d is intentionally omitted. - scanScsi := &DeviceInfo{Name: "/dev/sda", Type: "scsi"} - assert.Equal(t, - []string{"-a", "--json=c", "/dev/sda"}, - sm.smartctlArgs(scanScsi, false), - ) - - // Explicit scsi from SMART_DEVICES: -d scsi must be passed. - explicitScsi := &DeviceInfo{Name: "/dev/sda", Type: "scsi", explicitType: true} - assert.Equal(t, - []string{"-d", "scsi", "-a", "--json=c", "/dev/sda"}, - sm.smartctlArgs(explicitScsi, false), - ) - - // Explicit ata from SMART_DEVICES: -d ata must be passed (devstat still added). - explicitAta := &DeviceInfo{Name: "/dev/sdb", Type: "ata", explicitType: true} - assert.Equal(t, - []string{"-d", "ata", "-a", "--json=c", "-l", "devstat", "/dev/sdb"}, - sm.smartctlArgs(explicitAta, false), - ) -} - -// TestSmartDevicesExplicitTypeFlowsToSmartctlArgs is a regression test for -// issue #2072: an explicit SMART_DEVICES type (e.g. /dev/sda:scsi) must win over -// a wrong scan-detected type (sat) and be handed to smartctl as -d scsi. -func TestSmartDevicesExplicitTypeFlowsToSmartctlArgs(t *testing.T) { - sm := &SmartManager{} - - configured, err := sm.parseConfiguredDevices("/dev/sda:scsi") - require.NoError(t, err) - require.Len(t, configured, 1) - assert.True(t, configured[0].explicitType) - - // smartctl --scan misreports this USB drive as sat, which fails on it. - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "sat", Protocol: "ATA"}, - } - - merged := mergeDeviceLists(nil, scanned, configured) - require.Len(t, merged, 1) - - device := merged[0] - assert.Equal(t, "scsi", device.Type, "configured type should win over scan-detected sat") - assert.True(t, device.explicitType, "explicit hint must survive the merge") - - assert.Equal(t, - []string{"-d", "scsi", "-a", "--json=c", "/dev/sda"}, - sm.smartctlArgs(device, false), - "explicit scsi type must be passed to smartctl, not dropped", - ) -} - -// TestMergeDeviceListsPreservesExplicitTypeAcrossRescan ensures a verified, -// explicitly-typed device keeps its explicit flag when a later scan re-reports -// it with a different auto-detected type. -func TestMergeDeviceListsPreservesExplicitTypeAcrossRescan(t *testing.T) { - existing := []*DeviceInfo{ - {Name: "/dev/sda", Type: "scsi", parserType: "scsi", typeVerified: true, explicitType: true}, - } - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "sat"}, - } - - merged := mergeDeviceLists(existing, scanned, nil) - require.Len(t, merged, 1) - assert.Equal(t, "scsi", merged[0].Type) - assert.True(t, merged[0].explicitType, "explicit type flag should survive a rescan") -} - -func TestResolveRefreshError(t *testing.T) { - scanErr := errors.New("scan failed") - collectErr := errors.New("collect failed") - - tests := []struct { - name string - devices []*DeviceInfo - data map[string]*smart.SmartData - scanErr error - collectErr error - expectedErr error - expectNoErr bool +func TestMergeDevices(t *testing.T) { + for _, test := range []struct { + name string + scanned []smartDevice + configured []smartDevice + want []smartDevice }{ { - name: "no devices returns scan error", - devices: nil, - data: make(map[string]*smart.SmartData), - scanErr: scanErr, - expectedErr: scanErr, + name: "configured type replaces scanned type", + scanned: []smartDevice{{name: "/dev/sda", dtype: "scsi"}, {name: "/dev/nvme0", dtype: "nvme"}}, + configured: []smartDevice{{name: "/dev/sda", dtype: "sat", explicit: true}}, + want: []smartDevice{{name: "/dev/sda", dtype: "sat", explicit: true}, {name: "/dev/nvme0", dtype: "nvme"}}, }, { - name: "has data ignores errors", - devices: []*DeviceInfo{{Name: "/dev/sda"}}, - data: map[string]*smart.SmartData{"serial": {}}, - scanErr: scanErr, - collectErr: collectErr, - expectNoErr: true, + name: "configured path without type keeps scanned type", + scanned: []smartDevice{{name: "/dev/sda", dtype: "scsi"}}, + configured: []smartDevice{{name: "/dev/sda"}}, + want: []smartDevice{{name: "/dev/sda", dtype: "scsi"}}, }, { - name: "collect error preferred", - devices: []*DeviceInfo{{Name: "/dev/sda"}}, - data: make(map[string]*smart.SmartData), - collectErr: collectErr, - expectedErr: collectErr, + name: "same path and type marks scanned device explicit", + scanned: []smartDevice{{name: "/dev/sda", dtype: "scsi"}}, + configured: []smartDevice{{name: "/dev/sda", dtype: "scsi", explicit: true}}, + want: []smartDevice{{name: "/dev/sda", dtype: "scsi", explicit: true}}, }, { - name: "scan error returned when no data", - devices: []*DeviceInfo{{Name: "/dev/sda"}}, - data: make(map[string]*smart.SmartData), - scanErr: scanErr, - expectedErr: scanErr, + name: "unscanned devices are added", + scanned: []smartDevice{{name: "/dev/sda", dtype: "sat"}}, + configured: []smartDevice{{name: "/dev/sdb", dtype: "sat", explicit: true}}, + want: []smartDevice{{name: "/dev/sda", dtype: "sat"}, {name: "/dev/sdb", dtype: "sat", explicit: true}}, }, { - name: "no errors returns sentinel", - devices: []*DeviceInfo{{Name: "/dev/sda"}}, - data: make(map[string]*smart.SmartData), - expectedErr: errNoValidSmartData, + name: "RAID members sharing a path are added", + scanned: []smartDevice{ + {name: "/dev/bus/0", dtype: "megaraid,0"}, + {name: "/dev/bus/0", dtype: "megaraid,1"}, + }, + configured: []smartDevice{{name: "/dev/bus/0", dtype: "megaraid,2", explicit: true}}, + want: []smartDevice{ + {name: "/dev/bus/0", dtype: "megaraid,0"}, + {name: "/dev/bus/0", dtype: "megaraid,1"}, + {name: "/dev/bus/0", dtype: "megaraid,2", explicit: true}, + }, }, { - name: "no devices collect error", - devices: nil, - data: make(map[string]*smart.SmartData), - collectErr: collectErr, - expectedErr: collectErr, + name: "several configured types for a path scanned once", + scanned: []smartDevice{{name: "/dev/sda", dtype: "scsi"}}, + configured: []smartDevice{ + {name: "/dev/sda", dtype: "cciss,0", explicit: true}, + {name: "/dev/sda", dtype: "cciss,1", explicit: true}, + }, + want: []smartDevice{ + {name: "/dev/sda", dtype: "cciss,0", explicit: true}, + {name: "/dev/sda", dtype: "cciss,1", explicit: true}, + }, }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - sm := &SmartManager{ - SmartDevices: tt.devices, - SmartDataMap: tt.data, - } - - err := sm.resolveRefreshError(tt.scanErr, tt.collectErr) - if tt.expectNoErr { - assert.NoError(t, err) - return - } - - if tt.expectedErr == nil { - assert.NoError(t, err) - } else { - assert.Equal(t, tt.expectedErr, err) - } + { + name: "duplicates are dropped", + scanned: []smartDevice{{name: "/dev/sda", dtype: "sat"}, {name: "/dev/sda", dtype: "sat"}}, + configured: []smartDevice{{name: "/dev/sdb"}, {name: "/dev/sdb"}}, + want: []smartDevice{{name: "/dev/sda", dtype: "sat"}, {name: "/dev/sdb"}}, + }, + } { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, mergeDevices(test.scanned, test.configured)) }) } } -func TestParseScan(t *testing.T) { - sm := &SmartManager{ - SmartDataMap: map[string]*smart.SmartData{ - "serial-active": {DiskName: "/dev/sda"}, - "serial-stale": {DiskName: "/dev/sdb"}, +func TestAttemptsFor(t *testing.T) { + sm := &SmartManager{excludedDevices: map[string]struct{}{"/dev/nvme1n1": {}}} + for _, test := range []struct { + device smartDevice + want []smartAttempt + }{ + {smartDevice{name: "/dev/sda", dtype: "sat"}, []smartAttempt{{"/dev/sda", "sat"}}}, + {smartDevice{name: "/dev/sda"}, []smartAttempt{{"/dev/sda", ""}}}, + {smartDevice{name: "/dev/sda", dtype: "ata"}, []smartAttempt{{"/dev/sda", ""}}}, + {smartDevice{name: "/dev/sda", dtype: "scsi"}, []smartAttempt{{"/dev/sda", ""}, {"/dev/sda", "sat"}}}, + {smartDevice{name: "/dev/sda", dtype: "scsi", explicit: true}, []smartAttempt{{"/dev/sda", "scsi"}}}, + {smartDevice{name: "/dev/sda", dtype: "ata", explicit: true}, []smartAttempt{{"/dev/sda", "ata"}}}, + {smartDevice{name: "/dev/bus/0", dtype: "megaraid,1"}, []smartAttempt{{"/dev/bus/0", "megaraid,1"}}}, + {smartDevice{name: "/dev/nvme0", dtype: "nvme"}, []smartAttempt{{"/dev/nvme0", "nvme"}, {"/dev/nvme0n1", "nvme"}}}, + {smartDevice{name: "/dev/nvme0", dtype: "nvme", explicit: true}, []smartAttempt{{"/dev/nvme0", "nvme"}, {"/dev/nvme0n1", "nvme"}}}, + {smartDevice{name: "/dev/nvme0"}, []smartAttempt{{"/dev/nvme0", ""}, {"/dev/nvme0n1", ""}}}, + {smartDevice{name: "/dev/nvme0", dtype: "sntasmedia"}, []smartAttempt{{"/dev/nvme0", "sntasmedia"}}}, + {smartDevice{name: "/dev/nvme0n1", dtype: "nvme"}, []smartAttempt{{"/dev/nvme0n1", "nvme"}}}, + {smartDevice{name: "/dev/nvme1", dtype: "nvme"}, []smartAttempt{{"/dev/nvme1", "nvme"}}}, + } { + assert.Equal(t, test.want, sm.attemptsFor(test.device), "%+v", test.device) + } +} + +func TestSmartctlArgs(t *testing.T) { + for _, test := range []struct { + attempt smartAttempt + ata bool + standby bool + want string + }{ + {smartAttempt{"/dev/sda", ""}, false, true, "-a --json=c -n standby /dev/sda"}, + {smartAttempt{"/dev/sda", ""}, true, true, "-a --json=c -l devstat -n standby /dev/sda"}, + {smartAttempt{"/dev/sda", "sat"}, false, true, "-d sat -a --json=c -l devstat -n standby /dev/sda"}, + {smartAttempt{"/dev/sda", "ata"}, false, true, "-d ata -a --json=c -l devstat -n standby /dev/sda"}, + {smartAttempt{"/dev/nvme0", "nvme"}, false, true, "-d nvme -a --json=c -n standby /dev/nvme0"}, + {smartAttempt{"/dev/bus/0", "megaraid,1"}, true, true, "-d megaraid,1 -a --json=c -l devstat -n standby /dev/bus/0"}, + {smartAttempt{"/dev/sda", "sat"}, false, false, "-d sat -a --json=c -l devstat /dev/sda"}, + {smartAttempt{"/dev/nvme0", "nvme"}, false, false, "-d nvme -a --json=c /dev/nvme0"}, + } { + assert.Equal(t, test.want, strings.Join(smartctlArgs(test.attempt, test.ata, test.standby), " ")) + } +} + +func TestIsStandby(t *testing.T) { + assert.True(t, isStandby([]byte(standbyOutput), 2)) + assert.True(t, isStandby([]byte(`{"smartctl":{"messages":[{"string":"Device is in SLEEP mode, exit(2)"}]}}`), 2)) + assert.False(t, isStandby([]byte(standbyOutput), 0), "needs exit bit 1") + assert.False(t, isStandby([]byte(`{"smartctl":{"messages":[{"string":"Permission denied"}]}}`), 2)) + // smartctl reports SLEEP (power_mode.ata_value -1) when it lacks permission + // to check the power mode; a really sleeping disk looks the same. + assert.True(t, isStandby(readSmartFixture(t, "sat_sleep_permission.json"), 2)) +} + +const standbyOutput = `{"smartctl":{"exit_status":2,"messages":[{"string":"Device is in STANDBY mode, exit(2)","severity":"information"}]},"device":{"name":"/dev/sda","type":"sat"}}` + +// fakeSmartctl answers smartctl calls from a table keyed by "<-d type> " +// (an empty type for auto-detection, e.g. " /dev/sda") and records each call. +// Calls without -n standby are answered from wake first, if it has the key. +type fakeSmartctl struct { + scan string + scanExit int + scanErr error + responses map[string]fakeResponse + wake map[string]fakeResponse + calls []string +} + +type fakeResponse struct { + output string + exit int +} + +func (f *fakeSmartctl) run(_ time.Duration, args ...string) ([]byte, int, error) { + call := strings.Join(args, " ") + f.calls = append(f.calls, call) + if call == "--scan -j" { + return []byte(f.scan), f.scanExit, f.scanErr + } + dtype := "" + if i := slices.Index(args, "-d"); i >= 0 { + dtype = args[i+1] + } + key := dtype + " " + args[len(args)-1] + response, ok := f.wake[key] + if !ok || slices.Contains(args, "-n") { + response, ok = f.responses[key] + } + if !ok { + return []byte(`{"smartctl":{"exit_status":2,"messages":[{"string":"No such device"}]}}`), 2, nil + } + return []byte(response.output), response.exit, nil +} + +// deviceCalls returns the recorded calls other than the scan. +func (f *fakeSmartctl) deviceCalls() []string { + return slices.DeleteFunc(slices.Clone(f.calls), func(call string) bool { return call == "--scan -j" }) +} + +func newFakeSmartManager(t *testing.T, fake *fakeSmartctl) *SmartManager { + t.Helper() + original := scanSysfsDevices + scanSysfsDevices = func() []smartDevice { return nil } + t.Cleanup(func() { scanSysfsDevices = original }) + return &SmartManager{data: map[string]*smart.SmartData{}, run: fake.run} +} + +// nvmeAt returns the nvme0.json fixture as if smartctl had queried path. +func nvmeAt(t *testing.T, path string) fakeResponse { + return fakeResponse{output: strings.ReplaceAll(string(readSmartFixture(t, "nvme0.json")), `"/dev/nvme0"`, `"`+path+`"`)} +} + +func scanOf(devices ...string) string { + entries := make([]string, 0, len(devices)) + for _, device := range devices { + name, dtype, _ := strings.Cut(device, ":") + entries = append(entries, fmt.Sprintf(`{"name":%q,"type":%q}`, name, dtype)) + } + return `{"devices":[` + strings.Join(entries, ",") + `]}` +} + +// A SATA disk that the scan reports as scsi returns identity-only output when +// auto-detected; the sat attempt recovers its data and is tried first next time (#2295). +func TestRefreshScsiFallsBackToSat(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:scsi"), + responses: map[string]fakeResponse{ + " /dev/sda": {output: string(readSmartFixture(t, "scsi_identity.json"))}, + "sat /dev/sda": {output: string(readSmartFixture(t, "sat.json"))}, }, } + sm := newFakeSmartManager(t, fake) - scanJSON := []byte(`{ - "devices": [ - {"name": "/dev/sda", "type": "sat", "info_name": "/dev/sda [SAT]", "protocol": "ATA"}, - {"name": "/dev/nvme0", "type": "nvme", "info_name": "/dev/nvme0", "protocol": "NVMe"} - ] - }`) + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + data := sm.GetCurrentData()["Z4K2A01FS"] + assert.Equal(t, "TOSHIBA MG03ACA200", data.ModelName) + assert.Equal(t, "sat", data.DiskType) + assert.Equal(t, uint8(38), data.Temperature) + assert.Equal(t, "PASSED", data.SmartStatus) + assert.Equal(t, []string{ + "-a --json=c -n standby /dev/sda", + "-d sat -a --json=c -l devstat -n standby /dev/sda", + }, fake.deviceCalls()) - devices, hasData := sm.parseScan(scanJSON) - assert.True(t, hasData) - - sm.updateSmartDevices(devices) - - require.Len(t, sm.SmartDevices, 2) - assert.Equal(t, "/dev/sda", sm.SmartDevices[0].Name) - assert.Equal(t, "sat", sm.SmartDevices[0].Type) - assert.Equal(t, "/dev/nvme0", sm.SmartDevices[1].Name) - assert.Equal(t, "nvme", sm.SmartDevices[1].Type) - - _, activeExists := sm.SmartDataMap["serial-active"] - assert.True(t, activeExists, "active smart data should be preserved when device path remains") - - _, staleExists := sm.SmartDataMap["serial-stale"] - assert.False(t, staleExists, "stale smart data entry should be removed when device path disappears") + fake.calls = nil + _, err = sm.Refresh(false) + require.NoError(t, err) + assert.Equal(t, []string{"-d sat -a --json=c -l devstat -n standby /dev/sda"}, fake.deviceCalls()) } -func TestMergeDeviceListsPrefersConfigured(t *testing.T) { - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "sat", InfoName: "scan-info", Protocol: "ATA"}, - {Name: "/dev/nvme0", Type: "nvme"}, +// A disk scanned as ata is auto-detected but requests devstat from the first +// query, including a wake query; after that the protocol of the reading decides. +func TestRefreshScannedAtaRequestsDevstat(t *testing.T) { + for _, test := range []struct { + fixture, next string + asleep bool + }{ + {"sda.json", "-a --json=c -l devstat -n standby /dev/sda", false}, + {"scsi.json", "-a --json=c -n standby /dev/sda", false}, + {"sda.json", "-a --json=c -l devstat -n standby /dev/sda", true}, + {"scsi.json", "-a --json=c -n standby /dev/sda", true}, + } { + t.Run(fmt.Sprintf("%s/asleep=%t", test.fixture, test.asleep), func(t *testing.T) { + reading := fakeResponse{output: string(readSmartFixture(t, test.fixture))} + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:ata"), + responses: map[string]fakeResponse{" /dev/sda": reading}, + } + wantCalls := []string{"-a --json=c -l devstat -n standby /dev/sda"} + if test.asleep { + fake.responses[" /dev/sda"] = fakeResponse{output: standbyOutput, exit: 2} + fake.wake = map[string]fakeResponse{" /dev/sda": reading} + wantCalls = append(wantCalls, "-a --json=c -l devstat /dev/sda") + } + sm := newFakeSmartManager(t, fake) + + _, err := sm.Refresh(false) + require.NoError(t, err) + assert.Equal(t, wantCalls, fake.deviceCalls()) + + fake.calls = nil + fake.responses[" /dev/sda"] = reading + _, err = sm.Refresh(false) + require.NoError(t, err) + assert.Equal(t, []string{test.next}, fake.deviceCalls()) + }) } - - configured := []*DeviceInfo{ - {Name: "/dev/sda", Type: "sat-override"}, - {Name: "/dev/sdb", Type: "sat"}, - } - - merged := mergeDeviceLists(nil, scanned, configured) - require.Len(t, merged, 3) - - byName := make(map[string]*DeviceInfo, len(merged)) - for _, dev := range merged { - byName[dev.Name] = dev - } - - require.Contains(t, byName, "/dev/sda") - assert.Equal(t, "sat-override", byName["/dev/sda"].Type, "configured type should override scanned type") - assert.Equal(t, "scan-info", byName["/dev/sda"].InfoName, "scan metadata should be preserved when config does not provide it") - - require.Contains(t, byName, "/dev/nvme0") - assert.Equal(t, "nvme", byName["/dev/nvme0"].Type) - - require.Contains(t, byName, "/dev/sdb") - assert.Equal(t, "sat", byName["/dev/sdb"].Type) } -func TestMergeDeviceListsExpandsConfiguredDevicesWithSamePath(t *testing.T) { - scanned := []*DeviceInfo{ - {Name: "/dev/sdb", Type: "sat", InfoName: "scan-info", Protocol: "ATA"}, - } - configured := []*DeviceInfo{ - {Name: "/dev/sdb", Type: "jms56x,0", explicitType: true}, - {Name: "/dev/sdb", Type: "jms56x,1", explicitType: true}, - } - - merged := mergeDeviceLists(nil, scanned, configured) - require.Len(t, merged, 2) - - byKey := make(map[deviceKey]*DeviceInfo, len(merged)) - for _, device := range merged { - byKey[makeDeviceKey(device.Name, device.Type)] = device - } - - first := byKey[makeDeviceKey("/dev/sdb", "jms56x,0")] - require.NotNil(t, first) - assert.Equal(t, "scan-info", first.InfoName) - assert.Equal(t, "ATA", first.Protocol) - assert.True(t, first.explicitType) - - second := byKey[makeDeviceKey("/dev/sdb", "jms56x,1")] - require.NotNil(t, second) - assert.True(t, second.explicitType) - assert.NotContains(t, byKey, makeDeviceKey("/dev/sdb", "sat")) -} - -func TestMergeDeviceListsPreservesSamePathVerificationAcrossRescan(t *testing.T) { - existing := []*DeviceInfo{ - {Name: "/dev/sdb", Type: "jms56x,0", parserType: "sat", typeVerified: true, explicitType: true}, - {Name: "/dev/sdb", Type: "jms56x,1", parserType: "sat", typeVerified: true, explicitType: true}, - } - scanned := []*DeviceInfo{ - {Name: "/dev/sdb", Type: "sat", Protocol: "ATA"}, - } - configured := []*DeviceInfo{ - {Name: "/dev/sdb", Type: "jms56x,0", explicitType: true}, - {Name: "/dev/sdb", Type: "jms56x,1", explicitType: true}, - } - - merged := mergeDeviceLists(existing, scanned, configured) - require.Len(t, merged, 2) - byKey := make(map[deviceKey]*DeviceInfo, len(merged)) - for _, device := range merged { - byKey[makeDeviceKey(device.Name, device.Type)] = device - assert.True(t, device.typeVerified, device.Type) - assert.Equal(t, "sat", device.parserType, device.Type) - assert.True(t, device.explicitType, device.Type) - } - assert.Contains(t, byKey, makeDeviceKey("/dev/sdb", "jms56x,0")) - assert.Contains(t, byKey, makeDeviceKey("/dev/sdb", "jms56x,1")) -} - -func TestMergeDeviceListsDeduplicatesConfiguredIdentityAfterRekey(t *testing.T) { - scanned := []*DeviceInfo{{Name: "/dev/sdb", Type: "sat"}} - configured := []*DeviceInfo{ - {Name: "/dev/sdb", Type: "jms56x,0", explicitType: true}, - {Name: "/dev/sdb", Type: "jms56x,0", explicitType: true}, - } - - merged := mergeDeviceLists(nil, scanned, configured) - require.Len(t, merged, 1) - assert.Equal(t, "/dev/sdb", merged[0].Name) - assert.Equal(t, "jms56x,0", merged[0].Type) -} - -func TestMergeDeviceListsPreservesVerification(t *testing.T) { - existing := []*DeviceInfo{ - {Name: "/dev/sda", Type: "sat+megaraid", parserType: "sat", typeVerified: true}, - } - - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "nvme"}, - } - - merged := mergeDeviceLists(existing, scanned, nil) - require.Len(t, merged, 1) - - device := merged[0] - assert.True(t, device.typeVerified) - assert.Equal(t, "sat", device.parserType) - assert.Equal(t, "sat+megaraid", device.Type) -} - -func TestMergeDeviceListsUpdatesTypeWhenUnverified(t *testing.T) { - existing := []*DeviceInfo{ - {Name: "/dev/sda", Type: "sat", parserType: "sat", typeVerified: false}, - } - - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "nvme"}, - } - - merged := mergeDeviceLists(existing, scanned, nil) - require.Len(t, merged, 1) - - device := merged[0] - assert.False(t, device.typeVerified) - assert.Equal(t, "nvme", device.Type) - assert.Equal(t, "", device.parserType) -} - -func TestMergeDeviceListsHandlesDevicesWithSameNameAndDifferentTypes(t *testing.T) { - // There are use cases where the same device name is re-used, - // for example, a RAID controller with multiple drives. - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "megaraid,0"}, - {Name: "/dev/sda", Type: "megaraid,1"}, - {Name: "/dev/sda", Type: "megaraid,2"}, - } - - merged := mergeDeviceLists(nil, scanned, nil) - require.Len(t, merged, 3, "should have 3 separate devices for RAID controller") - - byKey := make(map[string]*DeviceInfo, len(merged)) - for _, dev := range merged { - key := dev.Name + "|" + dev.Type - byKey[key] = dev - } - - assert.Contains(t, byKey, "/dev/sda|megaraid,0") - assert.Contains(t, byKey, "/dev/sda|megaraid,1") - assert.Contains(t, byKey, "/dev/sda|megaraid,2") -} - -func TestMergeDeviceListsHandlesMixedRAIDAndRegular(t *testing.T) { - // Test mixing RAID drives with regular devices - scanned := []*DeviceInfo{ - {Name: "/dev/sda", Type: "megaraid,0"}, - {Name: "/dev/sda", Type: "megaraid,1"}, - {Name: "/dev/sdb", Type: "sat"}, - {Name: "/dev/nvme0", Type: "nvme"}, - } - - merged := mergeDeviceLists(nil, scanned, nil) - require.Len(t, merged, 4, "should have 4 separate devices") - - byKey := make(map[string]*DeviceInfo, len(merged)) - for _, dev := range merged { - key := dev.Name + "|" + dev.Type - byKey[key] = dev - } - - assert.Contains(t, byKey, "/dev/sda|megaraid,0") - assert.Contains(t, byKey, "/dev/sda|megaraid,1") - assert.Contains(t, byKey, "/dev/sdb|sat") - assert.Contains(t, byKey, "/dev/nvme0|nvme") -} - -func TestUpdateSmartDevicesPreservesRAIDDrives(t *testing.T) { - // Test that updateSmartDevices correctly validates RAID drives using composite keys - sm := &SmartManager{ - SmartDevices: []*DeviceInfo{ - {Name: "/dev/sda", Type: "megaraid,0"}, - {Name: "/dev/sda", Type: "megaraid,1"}, - }, - SmartDataMap: map[string]*smart.SmartData{ - "serial-0": { - DiskName: "/dev/sda", - DiskType: "megaraid,0", - SerialNumber: "serial-0", - }, - "serial-1": { - DiskName: "/dev/sda", - DiskType: "megaraid,1", - SerialNumber: "serial-1", - }, - "serial-stale": { - DiskName: "/dev/sda", - DiskType: "megaraid,2", - SerialNumber: "serial-stale", - }, +// An explicit SMART_DEVICES type is a deliberate override and is not retried. +func TestRefreshExplicitTypeIsNotRetried(t *testing.T) { + t.Setenv("SMART_DEVICES", "/dev/sda:scsi") + fake := &fakeSmartctl{ + scan: scanOf(), + responses: map[string]fakeResponse{ + "scsi /dev/sda": {output: string(readSmartFixture(t, "scsi_identity.json"))}, + "sat /dev/sda": {output: string(readSmartFixture(t, "sat.json"))}, }, } + sm := newFakeSmartManager(t, fake) - sm.updateSmartDevices(sm.SmartDevices) - - // serial-0 and serial-1 should be preserved (matching devices exist) - assert.Contains(t, sm.SmartDataMap, "serial-0") - assert.Contains(t, sm.SmartDataMap, "serial-1") - // serial-stale should be removed (no matching device) - assert.NotContains(t, sm.SmartDataMap, "serial-stale") + complete, err := sm.Refresh(false) + assert.ErrorIs(t, err, errNoHealthData) + assert.False(t, complete) + assert.Empty(t, sm.GetCurrentData()) + assert.Equal(t, []string{"-d scsi -a --json=c -n standby /dev/sda"}, fake.deviceCalls()) } -func TestParseSmartOutputMarksVerified(t *testing.T) { - fixturePath := filepath.Join("test-data", "smart", "nvme0.json") - data, err := os.ReadFile(fixturePath) +// NVMe controllers that reject SMART queries fall back to their namespace, +// which is then queried directly on later refreshes (#1504, #2597). +func TestRefreshNvmeNamespaceFallback(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/nvme0:nvme"), + responses: map[string]fakeResponse{"nvme /dev/nvme0n1": nvmeAt(t, "/dev/nvme0n1")}, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, "/dev/nvme0n1", sm.GetCurrentData()["2024031600129"].DiskName) + assert.Equal(t, []string{ + "-d nvme -a --json=c -n standby /dev/nvme0", + "-d nvme -a --json=c -n standby /dev/nvme0n1", + }, fake.deviceCalls()) - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - device := &DeviceInfo{Name: "/dev/nvme0"} - - require.True(t, sm.parseSmartOutput(device, data)) - assert.Equal(t, "nvme", device.Type) - assert.Equal(t, "nvme", device.parserType) - assert.True(t, device.typeVerified) -} - -func TestParseSmartForNvmeRejectsIdentityOnlyResponse(t *testing.T) { - jsonPayload := []byte(`{ - "smartctl": {"exit_status": 2}, - "device": {"name": "/dev/nvme0", "type": "nvme"}, - "model_name": "Netac NVMe SSD 500GB", - "serial_number": "IDENTITY-ONLY", - "user_capacity": {"bytes": 500107862016} - }`) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - hasData, exitStatus := sm.parseSmartForNvme(jsonPayload, "") - - assert.False(t, hasData) - assert.Equal(t, 2, exitStatus) - assert.NotContains(t, sm.SmartDataMap, "IDENTITY-ONLY") - - device := &DeviceInfo{Name: "/dev/nvme0", Type: "nvme"} - assert.False(t, sm.parseSmartOutput(device, jsonPayload)) - assert.NotContains(t, sm.SmartDataMap, "IDENTITY-ONLY") -} - -func TestParseSmartOutputKeepsCustomType(t *testing.T) { - fixturePath := filepath.Join("test-data", "smart", "sda.json") - data, err := os.ReadFile(fixturePath) + // The next refresh, after the scan cache expired, goes straight to the namespace. + fake.calls = nil + sm.lastScan = time.Now().Add(-smartScanInterval - time.Minute) + complete, err = sm.Refresh(false) require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, []string{"-d nvme -a --json=c -n standby /dev/nvme0n1"}, fake.deviceCalls()) - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - device := &DeviceInfo{Name: "/dev/sda", Type: "sat+megaraid"} - - require.True(t, sm.parseSmartOutput(device, data)) - assert.Equal(t, "sat+megaraid", device.Type) - assert.Equal(t, "sat", device.parserType) - assert.True(t, device.typeVerified) - assert.Equal(t, "sat+megaraid", sm.SmartDataMap["9C40918040082"].DiskType) -} - -func TestParseSmartOutputDoesNotNormalizeDeviceIdentity(t *testing.T) { - fixturePath := filepath.Join("test-data", "smart", "sda.json") - data, err := os.ReadFile(fixturePath) + // A scan that reports both aliases does not duplicate the drive. + fake.scan = scanOf("/dev/nvme0:nvme", "/dev/nvme0n1:nvme") + complete, err = sm.Refresh(true) require.NoError(t, err) - - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - device := &DeviceInfo{Name: "/dev/sda", Type: "ata", explicitType: true} - - require.True(t, sm.parseSmartOutput(device, data)) - assert.Equal(t, "sat", device.parserType) - assert.Equal(t, "ata", sm.SmartDataMap["9C40918040082"].DiskType) + assert.True(t, complete) + assert.Len(t, sm.GetCurrentData(), 1) } -func TestParseSmartOutputResetsVerificationOnFailure(t *testing.T) { - sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} - device := &DeviceInfo{Name: "/dev/sda", Type: "sat", parserType: "sat", typeVerified: true} +func TestRefreshNvmeNamespaceFailure(t *testing.T) { + denied := fakeResponse{output: string(readSmartFixture(t, "nvme_permission_denied.json")), exit: 4} + fake := &fakeSmartctl{ + scan: scanOf("/dev/nvme0:nvme"), + responses: map[string]fakeResponse{"nvme /dev/nvme0": denied, "nvme /dev/nvme0n1": denied}, + } + sm := newFakeSmartManager(t, fake) - assert.False(t, sm.parseSmartOutput(device, []byte("not json"))) - assert.False(t, device.typeVerified) - assert.Equal(t, "sat", device.parserType) + complete, err := sm.Refresh(false) + assert.ErrorIs(t, err, errNoHealthData) + assert.False(t, complete) + assert.Empty(t, sm.GetCurrentData()) + assert.Len(t, fake.deviceCalls(), 2) +} + +func TestRefreshHonorsExclusions(t *testing.T) { + for _, excluded := range []string{"/dev/nvme0", "/dev/nvme0n1"} { + t.Run(excluded, func(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/nvme0:nvme"), + responses: map[string]fakeResponse{"nvme /dev/nvme0n1": nvmeAt(t, "/dev/nvme0n1")}, + } + sm := newFakeSmartManager(t, fake) + sm.excludedDevices = map[string]struct{}{excluded: {}} + + _, err := sm.Refresh(false) + assert.Error(t, err) + assert.Empty(t, sm.GetCurrentData()) + assert.NotContains(t, fake.deviceCalls(), "-d nvme -a --json=c -n standby /dev/nvme0n1") + }) + } +} + +// A sleeping disk that was collected before is never woken: its last data is reused. +func TestRefreshStandby(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat"), + responses: map[string]fakeResponse{"sat /dev/sda": {output: string(readSmartFixture(t, "sda.json")), exit: 64}}, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + + fake.responses["sat /dev/sda"] = fakeResponse{output: standbyOutput, exit: 2} + complete, err = sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete, "reusing the last data counts as collected") + assert.Equal(t, "P3-2TB", sm.GetCurrentData()["9C40918040082"].ModelName) + + for _, call := range fake.deviceCalls() { + assert.Contains(t, call, "-n standby") + } +} + +// A temporary read failure must not forget the last successful reading used +// when the disk subsequently enters standby, even if it was already woken once. +func TestRefreshStandbyAfterCollectionFailure(t *testing.T) { + for _, initiallyAsleep := range []bool{false, true} { + t.Run(fmt.Sprintf("initiallyAsleep=%t", initiallyAsleep), func(t *testing.T) { + reading := fakeResponse{output: string(readSmartFixture(t, "sda.json"))} + standby := fakeResponse{output: standbyOutput, exit: 2} + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat"), + responses: map[string]fakeResponse{"sat /dev/sda": reading}, + wake: map[string]fakeResponse{"sat /dev/sda": reading}, + } + if initiallyAsleep { + fake.responses["sat /dev/sda"] = standby + } + sm := newFakeSmartManager(t, fake) + _, err := sm.Refresh(false) + require.NoError(t, err) + previous := sm.GetCurrentData() + + fake.responses["sat /dev/sda"] = fakeResponse{output: `{"smartctl":{"exit_status":4}}`, exit: 4} + for range 2 { + complete, err := sm.Refresh(false) + assert.Error(t, err) + assert.False(t, complete) + assert.Empty(t, sm.GetCurrentData(), "failed reads must not be reported as fresh data") + } + + fake.responses["sat /dev/sda"] = standby + fake.calls = nil + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, previous, sm.GetCurrentData()) + assert.Equal(t, []string{"-d sat -a --json=c -l devstat -n standby /dev/sda"}, fake.deviceCalls()) + }) + } +} + +// A sleeping disk with no earlier data is woken once so it shows up. +func TestRefreshWakesUncollectedStandbyDisk(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat"), + responses: map[string]fakeResponse{"sat /dev/sda": {output: standbyOutput, exit: 2}}, + wake: map[string]fakeResponse{"sat /dev/sda": {output: string(readSmartFixture(t, "sda.json")), exit: 64}}, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, "P3-2TB", sm.GetCurrentData()["9C40918040082"].ModelName) + assert.Equal(t, []string{ + "-d sat -a --json=c -l devstat -n standby /dev/sda", + "-d sat -a --json=c -l devstat /dev/sda", + }, fake.deviceCalls()) + + // Collected now, so later standby reuses its data instead of waking it. + fake.calls = nil + complete, err = sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, []string{"-d sat -a --json=c -l devstat -n standby /dev/sda"}, fake.deviceCalls()) +} + +// A disk whose wake query fails is not woken again on later refreshes, and is +// skipped with the refresh reported incomplete so the hub keeps its record. +func TestRefreshWakeFailureNotRetried(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat"), + responses: map[string]fakeResponse{"sat /dev/sda": {output: string(readSmartFixture(t, "sat_sleep_permission.json")), exit: 2}}, + wake: map[string]fakeResponse{"sat /dev/sda": {output: `{"smartctl":{"exit_status":2,"messages":[{"string":"Permission denied"}]}}`, exit: 2}}, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + assert.False(t, complete) + assert.ErrorContains(t, err, "exit status 2", "the wake query's own error is reported, not standby") + assert.Empty(t, sm.GetCurrentData()) + assert.Equal(t, []string{ + "-d sat -a --json=c -l devstat -n standby /dev/sda", + "-d sat -a --json=c -l devstat /dev/sda", + }, fake.deviceCalls()) + + for range 2 { + fake.calls = nil + complete, err = sm.Refresh(false) + assert.False(t, complete, "a skipped sleeping disk makes the refresh incomplete") + assert.ErrorIs(t, err, errNoValidSmartData) + assert.Equal(t, []string{"-d sat -a --json=c -l devstat -n standby /dev/sda"}, fake.deviceCalls()) + } +} + +// A failed wake query moves on to the next attempt, which runs with -n standby +// against the now-awake disk (#2295). +func TestRefreshWakeFallsBackToSat(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:scsi"), + responses: map[string]fakeResponse{ + " /dev/sda": {output: standbyOutput, exit: 2}, + "sat /dev/sda": {output: string(readSmartFixture(t, "sat.json"))}, + }, + wake: map[string]fakeResponse{" /dev/sda": {output: string(readSmartFixture(t, "scsi_identity.json"))}}, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, "sat", sm.GetCurrentData()["Z4K2A01FS"].DiskType) + assert.Equal(t, []string{ + "-a --json=c -n standby /dev/sda", + "-a --json=c /dev/sda", + "-d sat -a --json=c -l devstat -n standby /dev/sda", + }, fake.deviceCalls()) +} + +// Data is rebuilt on each refresh, so a removed disk disappears. +func TestRefreshPrunesRemovedDevices(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat", "/dev/nvme0:nvme"), + responses: map[string]fakeResponse{ + "sat /dev/sda": {output: string(readSmartFixture(t, "sda.json"))}, + "nvme /dev/nvme0": nvmeAt(t, "/dev/nvme0"), + }, + } + sm := newFakeSmartManager(t, fake) + + _, err := sm.Refresh(false) + require.NoError(t, err) + assert.Len(t, sm.GetCurrentData(), 2) + + // A failed read retains standby history, but a subsequent scan that removes + // the disk must still discard that history. + delete(fake.responses, "sat /dev/sda") + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.False(t, complete) + + fake.scan = scanOf("/dev/nvme0:nvme") + complete, err = sm.Refresh(true) + require.NoError(t, err) + assert.True(t, complete) + assert.Equal(t, []string{"2024031600129"}, slices.Collect(maps.Keys(sm.GetCurrentData()))) + assert.NotContains(t, sm.last, "/dev/sda|sat") +} + +// One failing disk makes the refresh incomplete without hiding the others. +func TestRefreshPartialFailure(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat", "/dev/sdb:sat"), + responses: map[string]fakeResponse{"sat /dev/sda": {output: string(readSmartFixture(t, "sda.json"))}}, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.False(t, complete) + assert.Len(t, sm.GetCurrentData(), 1) +} + +// The same disk seen through two paths is reported once, by its first path (#1907). +func TestRefreshDuplicateSerial(t *testing.T) { + sda := readSmartFixture(t, "sda.json") + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat", "/dev/sdb:sat"), + responses: map[string]fakeResponse{ + "sat /dev/sda": {output: string(sda)}, + "sat /dev/sdb": {output: strings.ReplaceAll(string(sda), `"/dev/sda"`, `"/dev/sdb"`)}, + }, + } + sm := newFakeSmartManager(t, fake) + + for range 2 { + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + require.Len(t, sm.GetCurrentData(), 1) + assert.Equal(t, "/dev/sda", sm.GetCurrentData()["9C40918040082"].DiskName) + } +} + +func TestRefreshScanFailureKeepsPreviousScan(t *testing.T) { + for _, test := range []struct { + name, output string + exit int + err error + }{ + {name: "timeout", err: errors.New("scan timed out")}, + {name: "invalid JSON", output: "invalid JSON"}, + {name: "nonzero exit without devices", output: `{"smartctl":{"exit_status":1}}`, exit: 1}, + {name: "nonzero exit with partial devices", output: scanOf("/dev/sdb:sat"), exit: 1}, + {name: "empty device list", output: scanOf()}, + } { + t.Run(test.name, func(t *testing.T) { + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:sat"), + responses: map[string]fakeResponse{"sat /dev/sda": {output: string(readSmartFixture(t, "sda.json"))}}, + } + sm := newFakeSmartManager(t, fake) + _, err := sm.Refresh(false) + require.NoError(t, err) + + fake.scan, fake.scanExit, fake.scanErr = test.output, test.exit, test.err + for _, forceScan := range []bool{true, false} { + fake.calls = nil + complete, err := sm.Refresh(forceScan) + require.NoError(t, err) + assert.False(t, complete, "a failed scan must not let the hub prune devices") + assert.Contains(t, sm.GetCurrentData(), "9C40918040082") + assert.Error(t, sm.scanErr) + if test.exit != 0 { + assert.ErrorContains(t, sm.scanErr, "exit status 1") + } + assert.Equal(t, []string{"-d sat -a --json=c -l devstat -n standby /dev/sda"}, fake.deviceCalls()) + assert.Equal(t, forceScan, slices.Contains(fake.calls, "--scan -j")) + } + + fake.scan, fake.scanExit, fake.scanErr = scanOf("/dev/sda:sat"), 0, nil + complete, err := sm.Refresh(true) + require.NoError(t, err) + assert.True(t, complete) + assert.NoError(t, sm.scanErr) + }) + } +} + +func TestRefreshWithSmartDevices(t *testing.T) { + t.Setenv("SMART_DEVICES", "/dev/sda:sat,/dev/nvme0:nvme") + fake := &fakeSmartctl{ + scan: scanOf("/dev/sda:scsi"), + responses: map[string]fakeResponse{ + "sat /dev/sda": {output: string(readSmartFixture(t, "sda.json"))}, + "nvme /dev/nvme0": nvmeAt(t, "/dev/nvme0"), + }, + } + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + require.NoError(t, err) + assert.True(t, complete) + assert.Len(t, sm.GetCurrentData(), 2) + assert.Equal(t, []string{ + "-d sat -a --json=c -l devstat -n standby /dev/sda", + "-d nvme -a --json=c -n standby /dev/nvme0", + }, fake.deviceCalls()) +} + +func TestRefreshSmartDevicesEmptyDisablesSmart(t *testing.T) { + t.Setenv("SMART_DEVICES", " ") + fake := &fakeSmartctl{scan: scanOf("/dev/sda:sat")} + sm := newFakeSmartManager(t, fake) + + complete, err := sm.Refresh(false) + assert.ErrorIs(t, err, errNoValidSmartData) + assert.False(t, complete) + assert.Empty(t, fake.calls) +} + +func TestRefreshSmartDevicesInvalid(t *testing.T) { + t.Setenv("SMART_DEVICES", ":sat") + sm := newFakeSmartManager(t, &fakeSmartctl{scan: scanOf()}) + + _, err := sm.Refresh(false) + assert.Error(t, err) +} + +func TestRefreshNoDevices(t *testing.T) { + sm := newFakeSmartManager(t, &fakeSmartctl{scan: scanOf()}) + + complete, err := sm.Refresh(false) + assert.ErrorIs(t, err, errNoValidSmartData) + assert.False(t, complete) } func assertAttrValue(t *testing.T, attributes []*smart.SmartAttribute, name string, expected uint64) { @@ -1301,99 +1315,6 @@ func TestIsExcludedDevice(t *testing.T) { } } -func TestFilterExcludedDevices(t *testing.T) { - tests := []struct { - name string - excludedDevs map[string]struct{} - inputDevices []*DeviceInfo - expectedDevs []*DeviceInfo - expectedLength int - }{ - { - name: "no exclusions", - excludedDevs: map[string]struct{}{}, - inputDevices: []*DeviceInfo{ - {Name: "/dev/sda"}, - {Name: "/dev/sdb"}, - {Name: "/dev/nvme0"}, - }, - expectedDevs: []*DeviceInfo{ - {Name: "/dev/sda"}, - {Name: "/dev/sdb"}, - {Name: "/dev/nvme0"}, - }, - expectedLength: 3, - }, - { - name: "some devices excluded", - excludedDevs: map[string]struct{}{ - "/dev/sda": {}, - "/dev/nvme0": {}, - }, - inputDevices: []*DeviceInfo{ - {Name: "/dev/sda"}, - {Name: "/dev/sdb"}, - {Name: "/dev/nvme0"}, - {Name: "/dev/nvme1"}, - }, - expectedDevs: []*DeviceInfo{ - {Name: "/dev/sdb"}, - {Name: "/dev/nvme1"}, - }, - expectedLength: 2, - }, - { - name: "all devices excluded", - excludedDevs: map[string]struct{}{ - "/dev/sda": {}, - "/dev/sdb": {}, - }, - inputDevices: []*DeviceInfo{ - {Name: "/dev/sda"}, - {Name: "/dev/sdb"}, - }, - expectedDevs: []*DeviceInfo{}, - expectedLength: 0, - }, - { - name: "nil devices", - excludedDevs: map[string]struct{}{}, - inputDevices: nil, - expectedDevs: []*DeviceInfo{}, - expectedLength: 0, - }, - { - name: "filter nil and empty name devices", - excludedDevs: map[string]struct{}{ - "/dev/sda": {}, - }, - inputDevices: []*DeviceInfo{ - {Name: "/dev/sda"}, - nil, - {Name: ""}, - {Name: "/dev/sdb"}, - }, - expectedDevs: []*DeviceInfo{ - {Name: "/dev/sdb"}, - }, - expectedLength: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - sm := &SmartManager{ - excludedDevices: tt.excludedDevs, - } - - result := sm.filterExcludedDevices(tt.inputDevices) - - assert.Len(t, result, tt.expectedLength) - assert.Equal(t, tt.expectedDevs, result) - }) - } -} - func TestIsNvmeControllerPath(t *testing.T) { tests := []struct { path string @@ -1427,47 +1348,6 @@ func TestIsNvmeControllerPath(t *testing.T) { } } -func TestParseSmartForNvmeAppleSSD(t *testing.T) { - // Apple SSDs don't report user_capacity via smartctl; capacity should be fetched - // from system_profiler via the darwinNvmeProvider fallback. - fixturePath := filepath.Join("test-data", "smart", "apple_nvme.json") - data, err := os.ReadFile(fixturePath) - require.NoError(t, err) - - providerCalls := 0 - fakeProvider := func() ([]byte, error) { - providerCalls++ - return []byte(`{ - "SPNVMeDataType": [{ - "_items": [{ - "device_serial": "0ba0147940253c15", - "size_in_bytes": 251000193024 - }] - }] - }`), nil - } - - sm := &SmartManager{ - SmartDataMap: make(map[string]*smart.SmartData), - darwinNvmeProvider: fakeProvider, - } - - hasData, _ := sm.parseSmartForNvme(data, "") - require.True(t, hasData) - - deviceData, ok := sm.SmartDataMap["0ba0147940253c15"] - require.True(t, ok) - assert.Equal(t, "APPLE SSD AP0256Q", deviceData.ModelName) - assert.Equal(t, uint64(251000193024), deviceData.Capacity) - assert.Equal(t, uint8(42), deviceData.Temperature) - assert.Equal(t, "PASSED", deviceData.SmartStatus) - assert.Equal(t, 1, providerCalls, "system_profiler should be called once") - - // Second parse: provider should NOT be called again (cache hit) - _, _ = sm.parseSmartForNvme(data, "") - assert.Equal(t, 1, providerCalls, "system_profiler should not be called again after caching") -} - func TestLookupDarwinNvmeCapacityMultipleDisks(t *testing.T) { fakeProvider := func() ([]byte, error) { return []byte(`{ @@ -1504,154 +1384,3 @@ func TestLookupDarwinNvmeCapacityProviderError(t *testing.T) { // Cache should be initialized even on error so we don't retry (Once already fired) assert.NotNil(t, sm.darwinNvmeCapacity) } - -// A child test process acts as smartctl: the controller is inaccessible, -// while its namespace returns valid NVMe data. -func init() { - fixture := os.Getenv("BESZEL_SMART_TEST_FIXTURE") - if fixture == "" { - return - } - if os.Args[1] == "--scan" { - if scan := os.Getenv("BESZEL_SMART_TEST_SCAN"); scan != "" { - fmt.Print(scan) - } else { - fmt.Print(`{"devices":[{"name":"/dev/nvme0","type":"nvme","protocol":"NVMe"}]}`) - } - os.Exit(0) - } - if isNvmeControllerPath(os.Args[len(os.Args)-1]) || os.Getenv("BESZEL_SMART_TEST_NAMESPACE_FAIL") == "true" { - fmt.Print(`{"smartctl":{"exit_status":2,"messages":[{"string":"Permission denied","severity":"error"}]}}`) - os.Exit(2) - } - data, err := os.ReadFile(fixture) - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - fmt.Print(strings.ReplaceAll(string(data), "/dev/nvme0", os.Args[len(os.Args)-1])) - os.Exit(0) -} - -func TestCollectSmartNvmeNamespaceFallback(t *testing.T) { - fixture, err := filepath.Abs(filepath.Join("test-data", "smart", "nvme0.json")) - require.NoError(t, err) - t.Setenv("BESZEL_SMART_TEST_FIXTURE", fixture) - executable, err := os.Executable() - require.NoError(t, err) - for _, cached := range []bool{false, true} { - t.Run(fmt.Sprintf("cached=%t", cached), func(t *testing.T) { - device := &DeviceInfo{Name: "/dev/nvme0", Type: "nvme"} - sm := &SmartManager{smartctlPath: executable, SmartDataMap: make(map[string]*smart.SmartData)} - if cached { - sm.SmartDataMap["2024031600129"] = &smart.SmartData{DiskName: device.Name, DiskType: device.Type} - } - require.NoError(t, sm.CollectSmart(device)) - assert.Equal(t, "/dev/nvme0n1", device.Name) - assert.Equal(t, "/dev/nvme0n1", sm.SmartDataMap["2024031600129"].DiskName) - assert.Equal(t, "/dev/nvme0n1", sm.nvmeNamespacePaths["/dev/nvme0"]) - assert.False(t, sm.isExcludedDevice("/dev/nvme0")) - }) - } -} - -func TestRefreshNvmeNamespaceFallbackSurvivesRescan(t *testing.T) { - fixture, err := filepath.Abs(filepath.Join("test-data", "smart", "nvme0.json")) - require.NoError(t, err) - t.Setenv("BESZEL_SMART_TEST_FIXTURE", fixture) - executable, err := os.Executable() - require.NoError(t, err) - sm := &SmartManager{smartctlPath: executable, SmartDataMap: make(map[string]*smart.SmartData)} - - complete, err := sm.Refresh(false) - require.NoError(t, err) - require.True(t, complete) - require.Len(t, sm.SmartDevices, 1) - require.Equal(t, "/dev/nvme0n1", sm.SmartDevices[0].Name) - - // Return a newer health reading to prove the next refresh queries the disk, - // rather than only preserving the previous cached data. - fixtureData, err := os.ReadFile(fixture) - require.NoError(t, err) - var updatedData smart.SmartctlOutput - require.NoError(t, json.Unmarshal(fixtureData, &updatedData)) - require.NotNil(t, updatedData.NVMeSmartHealthInformationLog) - updatedData.NVMeSmartHealthInformationLog.Temperature++ - fixtureData, err = json.Marshal(updatedData) - require.NoError(t, err) - updatedFixture := filepath.Join(t.TempDir(), "nvme.json") - require.NoError(t, os.WriteFile(updatedFixture, fixtureData, 0o600)) - t.Setenv("BESZEL_SMART_TEST_FIXTURE", updatedFixture) - - // Simulate the next refresh after the 30-minute scan cache has expired. - sm.lastScanTime = time.Now().Add(-31 * time.Minute) - complete, err = sm.Refresh(false) - require.NoError(t, err) - require.True(t, complete) - require.Len(t, sm.SmartDevices, 1) - assert.Equal(t, "/dev/nvme0n1", sm.SmartDevices[0].Name) - require.Contains(t, sm.SmartDataMap, "2024031600129") - assert.Equal(t, "/dev/nvme0n1", sm.SmartDataMap["2024031600129"].DiskName) - assert.Equal(t, uint8(updatedData.NVMeSmartHealthInformationLog.Temperature), sm.SmartDataMap["2024031600129"].Temperature) - assert.True(t, sm.SmartDevices[0].typeVerified) - assert.Equal(t, "nvme", sm.SmartDevices[0].parserType) - - // Explicit controller configuration must keep resolving to the namespace. - t.Setenv("SMART_DEVICES", "/dev/nvme0:nvme") - require.NoError(t, sm.ScanDevices(true)) - require.Len(t, sm.SmartDevices, 1) - assert.Equal(t, "/dev/nvme0n1", sm.SmartDevices[0].Name) - require.Contains(t, sm.SmartDataMap, "2024031600129") - - // A scan that reports both aliases must not duplicate the drive. - t.Setenv("SMART_DEVICES", "") - require.NoError(t, os.Unsetenv("SMART_DEVICES")) - t.Setenv("BESZEL_SMART_TEST_SCAN", `{"devices":[{"name":"/dev/nvme0","type":"nvme"},{"name":"/dev/nvme0n1","type":"nvme"}]}`) - require.NoError(t, sm.ScanDevices(true)) - require.Len(t, sm.SmartDevices, 1) - - // Retaining a fallback must not retain a drive absent from a valid scan. - t.Setenv("BESZEL_SMART_TEST_SCAN", `{"devices":[{"name":"/dev/sda","type":"sat"}]}`) - require.NoError(t, sm.ScanDevices(true)) - require.Len(t, sm.SmartDevices, 1) - assert.Equal(t, "/dev/sda", sm.SmartDevices[0].Name) - assert.Empty(t, sm.SmartDataMap) - assert.Empty(t, sm.nvmeNamespacePaths) -} - -func TestCollectSmartNvmeNamespaceFailureKeepsController(t *testing.T) { - t.Setenv("BESZEL_SMART_TEST_FIXTURE", "unused") - t.Setenv("BESZEL_SMART_TEST_NAMESPACE_FAIL", "true") - executable, err := os.Executable() - require.NoError(t, err) - device := &DeviceInfo{Name: "/dev/nvme0", Type: "nvme"} - sm := &SmartManager{smartctlPath: executable, SmartDataMap: make(map[string]*smart.SmartData)} - - require.Error(t, sm.CollectSmart(device)) - assert.Equal(t, "/dev/nvme0", device.Name) - assert.Empty(t, sm.nvmeNamespacePaths) - assert.Empty(t, sm.SmartDataMap) -} - -func TestNvmeNamespaceFallbackHonorsExclusions(t *testing.T) { - fixture, err := filepath.Abs(filepath.Join("test-data", "smart", "nvme0.json")) - require.NoError(t, err) - t.Setenv("BESZEL_SMART_TEST_FIXTURE", fixture) - executable, err := os.Executable() - require.NoError(t, err) - for _, excluded := range []string{"/dev/nvme0", "/dev/nvme0n1"} { - t.Run(excluded, func(t *testing.T) { - sm := &SmartManager{ - smartctlPath: executable, - SmartDevices: []*DeviceInfo{{Name: "/dev/nvme0n1", Type: "nvme"}}, - SmartDataMap: map[string]*smart.SmartData{"2024031600129": {DiskName: "/dev/nvme0n1", DiskType: "nvme"}}, - nvmeNamespacePaths: map[string]string{"/dev/nvme0": "/dev/nvme0n1"}, - excludedDevices: map[string]struct{}{excluded: {}}, - } - require.ErrorIs(t, sm.ScanDevices(true), errNoValidSmartData) - assert.Empty(t, sm.SmartDevices) - assert.Empty(t, sm.SmartDataMap) - assert.Empty(t, sm.nvmeNamespacePaths) - }) - } -} diff --git a/agent/test-data/smart/nvme_permission_denied.json b/agent/test-data/smart/nvme_permission_denied.json new file mode 100644 index 000000000..f75270309 --- /dev/null +++ b/agent/test-data/smart/nvme_permission_denied.json @@ -0,0 +1,228 @@ +{ + "json_format_version": [ + 1, + 0 + ], + "smartctl": { + "version": [ + 7, + 5 + ], + "pre_release": false, + "argv": [ + "smartctl", + "-d", + "nvme", + "-a", + "--json=c", + "-n", + "standby", + "/dev/nvme0" + ], + "messages": [ + { + "string": "Read NVMe SMART/Health Information (NSID 0xffffffff) failed: NVME_IOCTL_ADMIN_CMD: Permission denied", + "severity": "error" + } + ], + "exit_status": 4 + }, + "device": { + "name": "/dev/nvme0", + "info_name": "/dev/nvme0", + "type": "nvme", + "protocol": "NVMe" + }, + "model_name": "HighRel 512GB SSD", + "serial_number": "MP51W00000001", + "firmware_version": "SN14665", + "nvme_pci_vendor": { + "id": 7755, + "subsystem_id": 7755 + }, + "nvme_ieee_oui_identifier": 3824167, + "nvme_total_capacity": 512110190592, + "nvme_unallocated_capacity": 0, + "nvme_controller_id": 0, + "nvme_version": { + "string": "1.4", + "value": 66560 + }, + "nvme_number_of_namespaces": 1, + "nvme_namespaces": [ + { + "id": 1, + "size": { + "blocks": 1000215216, + "bytes": 512110190592 + }, + "capacity": { + "blocks": 1000215216, + "bytes": 512110190592 + }, + "utilization": { + "blocks": 1000215216, + "bytes": 512110190592 + }, + "formatted_lba_size": 512, + "eui64": { + "oui": 3824167, + "ext_id": 1234567890 + }, + "features": { + "value": 0, + "thin_provisioning": false, + "na_fields": false, + "dealloc_or_unwritten_block_error": false, + "uid_reuse": false, + "np_fields": false, + "other": 0 + }, + "lba_formats": [ + { + "formatted": true, + "data_bytes": 512, + "metadata_bytes": 0, + "relative_performance": 0 + } + ] + } + ], + "user_capacity": { + "blocks": 1000215216, + "bytes": 512110190592 + }, + "logical_block_size": 512, + "smart_support": { + "available": true, + "enabled": true + }, + "nvme_firmware_update_capabilities": { + "value": 26, + "slots": 5, + "first_slot_is_read_only": false, + "activiation_without_reset": true, + "multiple_update_detection": false, + "other": 0 + }, + "nvme_optional_admin_commands": { + "value": 23, + "security_send_receive": true, + "format_nvm": true, + "firmware_download": true, + "namespace_management": false, + "self_test": true, + "directives": false, + "mi_send_receive": false, + "virtualization_management": false, + "doorbell_buffer_config": false, + "get_lba_status": false, + "command_and_feature_lockdown": false, + "other": 0 + }, + "nvme_optional_nvm_commands": { + "value": 31, + "compare": true, + "write_uncorrectable": true, + "dataset_management": true, + "write_zeroes": true, + "save_select_feature_nonzero": true, + "reservations": false, + "timestamp": false, + "verify": false, + "copy": false, + "other": 0 + }, + "nvme_log_page_attributes": { + "value": 6, + "smart_health_per_namespace": false, + "commands_effects_log": true, + "extended_get_log_page_cmd": true, + "telemetry_log": false, + "persistent_event_log": false, + "supported_log_pages_log": false, + "telemetry_data_area_4": false, + "other": 0 + }, + "nvme_maximum_data_transfer_pages": 128, + "nvme_composite_temperature_threshold": { + "warning": 90, + "critical": 95 + }, + "temperature": { + "op_limit_max": 90, + "critical_limit_max": 95 + }, + "nvme_power_states": [ + { + "non_operational_state": false, + "relative_read_latency": 0, + "relative_read_throughput": 0, + "relative_write_latency": 0, + "relative_write_throughput": 0, + "entry_latency_us": 0, + "exit_latency_us": 0, + "max_power": { + "value": 650, + "scale": 2, + "units_per_watt": 100 + } + }, + { + "non_operational_state": false, + "relative_read_latency": 1, + "relative_read_throughput": 1, + "relative_write_latency": 1, + "relative_write_throughput": 1, + "entry_latency_us": 0, + "exit_latency_us": 0, + "max_power": { + "value": 580, + "scale": 2, + "units_per_watt": 100 + } + }, + { + "non_operational_state": false, + "relative_read_latency": 2, + "relative_read_throughput": 2, + "relative_write_latency": 2, + "relative_write_throughput": 2, + "entry_latency_us": 0, + "exit_latency_us": 0, + "max_power": { + "value": 360, + "scale": 2, + "units_per_watt": 100 + } + }, + { + "non_operational_state": true, + "relative_read_latency": 3, + "relative_read_throughput": 3, + "relative_write_latency": 3, + "relative_write_throughput": 3, + "entry_latency_us": 5000, + "exit_latency_us": 10000, + "max_power": { + "value": 500, + "scale": 1, + "units_per_watt": 10000 + } + }, + { + "non_operational_state": true, + "relative_read_latency": 4, + "relative_read_throughput": 4, + "relative_write_latency": 4, + "relative_write_throughput": 4, + "entry_latency_us": 8000, + "exit_latency_us": 45000, + "max_power": { + "value": 25, + "scale": 1, + "units_per_watt": 10000 + } + } + ] +} diff --git a/agent/test-data/smart/sat.json b/agent/test-data/smart/sat.json new file mode 100644 index 000000000..f31e157d1 --- /dev/null +++ b/agent/test-data/smart/sat.json @@ -0,0 +1,26 @@ +{ + "smartctl": { + "exit_status": 0 + }, + "device": { + "name": "/dev/sda", + "info_name": "/dev/sda [SAT]", + "type": "sat", + "protocol": "ATA" + }, + "model_name": "TOSHIBA MG03ACA200", + "serial_number": "Z4K2A01FS", + "firmware_version": "5701", + "user_capacity": { "blocks": 5860533168, "bytes": 3000592982016 }, + "smart_status": { "passed": true }, + "temperature": { "current": 38 }, + "ata_smart_attributes": { + "revision": 16, + "table": [ + { "id": 1, "name": "Raw_Read_Error_Rate", "value": 100, "worst": 100, "thresh": 16, "when_failed": "", "raw": { "value": 0, "string": "0" } }, + { "id": 5, "name": "Reallocated_Sector_Ct", "value": 100, "worst": 100, "thresh": 5, "when_failed": "", "raw": { "value": 0, "string": "0" } }, + { "id": 9, "name": "Power_On_Hours", "value": 1, "worst": 1, "thresh": 0, "when_failed": "", "raw": { "value": 43953, "string": "43953" } }, + { "id": 194, "name": "Temperature_Celsius", "value": 62, "worst": 47, "thresh": 0, "when_failed": "", "raw": { "value": 38, "string": "38" } } + ] + } +} diff --git a/agent/test-data/smart/sat_sleep_permission.json b/agent/test-data/smart/sat_sleep_permission.json new file mode 100644 index 000000000..6c7b6ce2f --- /dev/null +++ b/agent/test-data/smart/sat_sleep_permission.json @@ -0,0 +1,40 @@ +{ + "json_format_version": [ + 1, + 0 + ], + "smartctl": { + "version": [ + 7, + 5 + ], + "pre_release": false, + "argv": [ + "smartctl", + "-d", + "sat", + "-a", + "--json=c", + "-n", + "standby", + "/dev/sda" + ], + "messages": [ + { + "string": "Device is in SLEEP mode, exit(2)", + "severity": "information" + } + ], + "exit_status": 2 + }, + "device": { + "name": "/dev/sda", + "info_name": "/dev/sda [SAT]", + "type": "sat", + "protocol": "ATA" + }, + "power_mode": { + "ata_value": -1, + "name": "SLEEP" + } +} diff --git a/agent/test-data/smart/scsi_identity.json b/agent/test-data/smart/scsi_identity.json new file mode 100644 index 000000000..12d0322a8 --- /dev/null +++ b/agent/test-data/smart/scsi_identity.json @@ -0,0 +1,20 @@ +{ + "smartctl": { + "exit_status": 0 + }, + "device": { + "name": "/dev/sda", + "info_name": "/dev/sda", + "type": "scsi", + "protocol": "SCSI" + }, + "scsi_vendor": "TOSHIBA", + "scsi_product": "MG03ACA200", + "scsi_model_name": "TOSHIBA MG03ACA200", + "scsi_revision": "5701", + "serial_number": "Z4K2A01FS", + "user_capacity": { "blocks": 5860533168, "bytes": 3000592982016 }, + "smart_status": { "passed": null }, + "temperature": { "current": 0, "drive_trip": 0 }, + "power_on_time": { "hours": 0, "minutes": 0 } +} diff --git a/internal/entities/smart/smart.go b/internal/entities/smart/smart.go index 0f961e42f..87b760654 100644 --- a/internal/entities/smart/smart.go +++ b/internal/entities/smart/smart.go @@ -21,7 +21,7 @@ type SmartctlOutput struct { FirmwareVersion string `json:"firmware_version"` UserCapacity UserCapacity `json:"user_capacity"` SmartStatus struct { - Passed bool `json:"passed"` + Passed *bool `json:"passed"` // nil when smartctl could not read the health status } `json:"smart_status"` Temperature struct { Current int64 `json:"current"`