Add SMART_DEVICES_SEPARATOR + allow drives with the same name to be added with different types (e.g. raid controllers) (#1655)

* Add SMART_DEVICES_SEPARATOR to override ,

* Allow composite keys in smart devices for raid controller support
This commit is contained in:
Julian Nadeau
2026-01-21 17:58:20 -05:00
committed by GitHub
parent 988de6de7b
commit 648a979a81
2 changed files with 129 additions and 10 deletions

View File

@@ -202,7 +202,11 @@ func (sm *SmartManager) ScanDevices(force bool) error {
}
func (sm *SmartManager) parseConfiguredDevices(config string) ([]*DeviceInfo, error) {
entries := strings.Split(config, ",")
splitChar := os.Getenv("SMART_DEVICES_SEPARATOR")
if splitChar == "" {
splitChar = ","
}
entries := strings.Split(config, splitChar)
devices := make([]*DeviceInfo, 0, len(entries))
for _, entry := range entries {
entry = strings.TrimSpace(entry)
@@ -326,6 +330,16 @@ func normalizeParserType(value string) string {
}
}
// deviceKey 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 deviceKey(name, deviceType string) string {
if deviceType == "" {
return name
}
return name + "|" + 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 {
@@ -586,7 +600,7 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
if dev == nil || dev.Name == "" {
continue
}
existingIndex[dev.Name] = dev
existingIndex[deviceKey(dev.Name, dev.Type)] = dev
}
finalDevices := make([]*DeviceInfo, 0, len(scanned)+len(configured))
@@ -602,12 +616,14 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
// Work on a copy so we can safely adjust metadata without mutating the
// input slices that may be reused elsewhere.
copyDev := *dev
if prev := existingIndex[copyDev.Name]; prev != nil {
key := deviceKey(copyDev.Name, copyDev.Type)
if prev := existingIndex[key]; prev != nil {
preserveVerifiedType(&copyDev, prev)
}
finalDevices = append(finalDevices, &copyDev)
deviceIndex[copyDev.Name] = finalDevices[len(finalDevices)-1]
copyKey := deviceKey(copyDev.Name, copyDev.Type)
deviceIndex[copyKey] = finalDevices[len(finalDevices)-1]
}
// Merge configured devices on top so users can override scan results (except
@@ -617,7 +633,8 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
continue
}
if existingDev, ok := deviceIndex[dev.Name]; ok {
key := deviceKey(dev.Name, dev.Type)
if existingDev, ok := deviceIndex[key]; ok {
// Only update the type if it has not been verified yet; otherwise we
// keep the existing verified metadata intact.
if dev.Type != "" && !existingDev.typeVerified {
@@ -636,14 +653,16 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
}
copyDev := *dev
if prev := existingIndex[copyDev.Name]; prev != nil {
key = deviceKey(copyDev.Name, copyDev.Type)
if prev := existingIndex[key]; prev != nil {
preserveVerifiedType(&copyDev, prev)
} else if copyDev.Type != "" {
copyDev.parserType = normalizeParserType(copyDev.Type)
}
finalDevices = append(finalDevices, &copyDev)
deviceIndex[copyDev.Name] = finalDevices[len(finalDevices)-1]
copyKey := deviceKey(copyDev.Name, copyDev.Type)
deviceIndex[copyKey] = finalDevices[len(finalDevices)-1]
}
return finalDevices
@@ -661,12 +680,12 @@ func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) {
return
}
validNames := make(map[string]struct{}, len(devices))
validKeys := make(map[string]struct{}, len(devices))
for _, device := range devices {
if device == nil || device.Name == "" {
continue
}
validNames[device.Name] = struct{}{}
validKeys[deviceKey(device.Name, device.Type)] = struct{}{}
}
for key, data := range sm.SmartDataMap {
@@ -675,7 +694,7 @@ func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) {
continue
}
if _, ok := validNames[data.DiskName]; ok {
if _, ok := validKeys[deviceKey(data.DiskName, data.DiskType)]; ok {
continue
}

View File

@@ -195,6 +195,24 @@ func TestDevicesSnapshotReturnsCopy(t *testing.T) {
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)
require.NoError(t, err)
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")
@@ -442,6 +460,88 @@ func TestMergeDeviceListsUpdatesTypeWhenUnverified(t *testing.T) {
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",
},
},
}
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")
}
func TestParseSmartOutputMarksVerified(t *testing.T) {
fixturePath := filepath.Join("test-data", "smart", "nvme0.json")
data, err := os.ReadFile(fixturePath)