mirror of
https://github.com/henrygd/beszel.git
synced 2026-10-21 22:02:34 +00:00
fix(agent): preserve NVMe namespace fallbacks across rescans (#2597)
This commit is contained in:
spatiumstas
+54
-12
@@ -31,6 +31,7 @@ type SmartManager struct {
|
||||
lastScanTime time.Time
|
||||
smartctlPath string
|
||||
excludedDevices map[string]struct{}
|
||||
nvmeNamespacePaths map[string]string // controller to working namespace; guarded by Mutex
|
||||
darwinNvmeOnce sync.Once
|
||||
darwinNvmeCapacity map[string]uint64 // serial → bytes cache, written once via darwinNvmeOnce
|
||||
darwinNvmeProvider func() ([]byte, error) // overridable for testing
|
||||
@@ -216,6 +217,8 @@ func (sm *SmartManager) ScanDevices(force bool) error {
|
||||
hasValidScan = true
|
||||
}
|
||||
|
||||
scannedDevices = sm.resolveNvmeNamespacePaths(scannedDevices)
|
||||
configuredDevices = sm.resolveNvmeNamespacePaths(configuredDevices)
|
||||
finalDevices := mergeDeviceLists(currentDevices, scannedDevices, configuredDevices)
|
||||
finalDevices = sm.filterExcludedDevices(finalDevices)
|
||||
sm.updateSmartDevices(finalDevices)
|
||||
@@ -231,6 +234,39 @@ func (sm *SmartManager) ScanDevices(force bool) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
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)
|
||||
}
|
||||
return resolved
|
||||
}
|
||||
|
||||
func (sm *SmartManager) parseConfiguredDevices(config string) ([]*DeviceInfo, error) {
|
||||
splitChar, _ := utils.GetEnv("SMART_DEVICES_SEPARATOR")
|
||||
if splitChar == "" {
|
||||
@@ -535,24 +571,26 @@ func (sm *SmartManager) CollectSmart(deviceInfo *DeviceInfo) error {
|
||||
controllerPath := deviceInfo.Name
|
||||
namespacePath := controllerPath + "n1"
|
||||
if !sm.isExcludedDevice(namespacePath) {
|
||||
deviceInfo.Name = namespacePath
|
||||
namespaceDevice := *deviceInfo
|
||||
namespaceDevice.Name = namespacePath
|
||||
|
||||
ctx3, cancel3 := context.WithTimeout(context.Background(), 15*time.Second)
|
||||
defer cancel3()
|
||||
args = sm.smartctlArgs(deviceInfo, false)
|
||||
args = sm.smartctlArgs(&namespaceDevice, false)
|
||||
cmd = exec.CommandContext(ctx3, sm.smartctlPath, args...)
|
||||
output, err = cmd.CombinedOutput()
|
||||
hasValidData = sm.parseSmartOutput(deviceInfo, output)
|
||||
hasValidData = sm.parseSmartOutput(&namespaceDevice, output)
|
||||
|
||||
// Auto-exclude the controller path so future scans don't re-add it
|
||||
// Resolve future controller scan results to this working namespace.
|
||||
if hasValidData {
|
||||
sm.Lock()
|
||||
if sm.excludedDevices == nil {
|
||||
sm.excludedDevices = make(map[string]struct{})
|
||||
*deviceInfo = namespaceDevice
|
||||
if sm.nvmeNamespacePaths == nil {
|
||||
sm.nvmeNamespacePaths = make(map[string]string)
|
||||
}
|
||||
sm.excludedDevices[controllerPath] = struct{}{}
|
||||
sm.nvmeNamespacePaths[controllerPath] = namespacePath
|
||||
sm.Unlock()
|
||||
slog.Debug("auto-excluded NVMe controller path", "path", controllerPath)
|
||||
slog.Debug("using NVMe namespace path", "controller", controllerPath, "namespace", namespacePath)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -802,10 +840,6 @@ func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) {
|
||||
|
||||
sm.SmartDevices = devices
|
||||
|
||||
if len(sm.SmartDataMap) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
validKeys := make(map[deviceKey]struct{}, len(devices))
|
||||
nameCounts := make(map[string]int, len(devices))
|
||||
for _, device := range devices {
|
||||
@@ -816,6 +850,14 @@ func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) {
|
||||
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)
|
||||
|
||||
+114
-2
@@ -3,6 +3,7 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
@@ -10,6 +11,7 @@ import (
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/henrygd/beszel/internal/entities/smart"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -1516,7 +1518,15 @@ func init() {
|
||||
if fixture == "" {
|
||||
return
|
||||
}
|
||||
if isNvmeControllerPath(os.Args[len(os.Args)-1]) {
|
||||
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)
|
||||
}
|
||||
@@ -1545,7 +1555,109 @@ func TestCollectSmartNvmeNamespaceFallback(t *testing.T) {
|
||||
require.NoError(t, sm.CollectSmart(device))
|
||||
assert.Equal(t, "/dev/nvme0n1", device.Name)
|
||||
assert.Equal(t, "/dev/nvme0n1", sm.SmartDataMap["2024031600129"].DiskName)
|
||||
assert.True(t, sm.isExcludedDevice("/dev/nvme0"))
|
||||
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.SmartInfoForNvme
|
||||
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, 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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user