diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index c0c2d693e..c294eabd8 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1219,10 +1219,11 @@ platform when `mdcmd` or array-topology collection is unavailable. Unraid array collection belongs to that same runtime-normalized agent path: `internal/hostagent/unraid.go` must treat empty `DISK_NP`/`DISK_NP_DSBL` slots with no device, id, filesystem, or size as unassigned topology -placeholders rather than failed disks. Assigned disks may use `diskId`/`rdevId` -as the serial fallback when Unraid does not expose a separate serial field, so -monitoring receives stable disk identity without inventing host-profile or -platform state from optional storage probe success. +placeholders rather than failed disks, even when Unraid gives those slots +topology labels such as `disk6` or `parity2`. Assigned disks may use +`diskId`/`rdevId` as the serial fallback when Unraid does not expose a separate +serial field, so monitoring receives stable disk identity without inventing +host-profile or platform state from optional storage probe success. That Unraid runtime path must also prefer native appliance topology over generic block-device inference. The Unified Agent should best-effort merge `/var/local/emhttp/disks.ini` into the `mdcmd status` view and carry disk diff --git a/internal/hostagent/unraid.go b/internal/hostagent/unraid.go index 49ceb26de..f1d1a1e83 100644 --- a/internal/hostagent/unraid.go +++ b/internal/hostagent/unraid.go @@ -105,12 +105,20 @@ func reconcileUnraidDiskCounts(storage *agentshost.UnraidStorage) *agentshost.Un } hasStructuredStatus := false - disabled, invalid, missing := 0, 0, 0 + hasStructuredParity := false + disabled, invalid, missing, protected := 0, 0, 0, 0 for _, disk := range storage.Disks { if isUnraidEmptySlot(disk) { continue } - switch strings.ToLower(strings.TrimSpace(disk.Status)) { + status := strings.ToLower(strings.TrimSpace(disk.Status)) + if strings.EqualFold(strings.TrimSpace(disk.Role), "parity") && status != "" { + hasStructuredParity = true + if status == "online" { + protected++ + } + } + switch status { case "": continue case "disabled": @@ -131,6 +139,9 @@ func reconcileUnraidDiskCounts(storage *agentshost.UnraidStorage) *agentshost.Un storage.NumInvalid = invalid storage.NumMissing = missing } + if hasStructuredParity { + storage.NumProtected = protected + } return storage } @@ -502,8 +513,12 @@ func isUnraidEmptySlot(disk agentshost.UnraidDisk) bool { if !strings.Contains(rawStatus, "DISK_NP") && status != "missing" { return false } - return strings.TrimSpace(disk.Name) == "" && - strings.TrimSpace(disk.Device) == "" && + // Unraid names every configured slot (for example disk6 or parity2), even + // when it has never been assigned. A slot label is therefore topology, not + // membership evidence. Preserve DISK_NP members only when native identity, + // device, filesystem, or size evidence shows that a disk was assigned. + return strings.TrimSpace(disk.Device) == "" && + strings.TrimSpace(disk.Model) == "" && strings.TrimSpace(disk.Serial) == "" && strings.TrimSpace(disk.Filesystem) == "" && disk.SizeBytes == 0 diff --git a/internal/hostagent/unraid_test.go b/internal/hostagent/unraid_test.go index db34659c3..951497978 100644 --- a/internal/hostagent/unraid_test.go +++ b/internal/hostagent/unraid_test.go @@ -151,14 +151,14 @@ rdevStatus.1=DISK_OK rdevName.1=sde rdevId.1=WDC_DATA diskNumber.5=5 -diskName.5= +diskName.5=disk5 diskSize.5=0 diskId.5= rdevStatus.5=DISK_NP rdevName.5= rdevId.5= diskNumber.29=29 -diskName.29= +diskName.29=parity2 diskSize.29=0 diskId.29= rdevStatus.29=DISK_NP_DSBL @@ -178,6 +178,43 @@ rdevId.29= } } +func TestReconcileUnraidDiskCountsIgnoresNamedEmptySlots(t *testing.T) { + storage, err := parseUnraidStatusOutput(` +mdState=STARTED +mdNumProtected=0 +mdNumDisabled=1 +mdNumInvalid=1 +mdNumMissing=2 +diskName.0=parity +diskSize.0=12000 +rdevName.0=sdb +rdevStatus.0=DISK_OK +diskName.1=disk1 +diskSize.1=12000 +rdevName.1=sdc +rdevStatus.1=DISK_OK +diskName.6=disk6 +diskSize.6=0 +rdevStatus.6=DISK_NP +diskName.29=parity2 +diskSize.29=0 +rdevStatus.29=DISK_NP_DSBL +`) + if err != nil { + t.Fatalf("parseUnraidStatusOutput() error = %v", err) + } + storage = reconcileUnraidDiskCounts(storage) + if len(storage.Disks) != 2 { + t.Fatalf("disk count = %d, want only assigned parity and data disks: %+v", len(storage.Disks), storage.Disks) + } + if storage.NumMissing != 0 || storage.NumDisabled != 0 || storage.NumInvalid != 0 { + t.Fatalf("empty slots affected failure counts: %+v", storage) + } + if storage.NumProtected != 1 { + t.Fatalf("NumProtected = %d, want one online parity disk", storage.NumProtected) + } +} + func TestParseUnraidDisksINIAddsNativeTopologyFields(t *testing.T) { input := ` ["disk1"] @@ -219,6 +256,22 @@ type="Cache" fsType="btrfs" fsFree="1783019588" fsUsed="166957852" +["disk6"] +idx="6" +name="disk6" +device="" +id="" +size="0" +status="DISK_NP" +type="Data" +["parity2"] +idx="29" +name="parity2" +device="" +id="" +size="0" +status="DISK_NP_DSBL" +type="Parity" ` disks := parseUnraidDisksINI(input) diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index fb670b63e..44d0c461a 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -2936,16 +2936,17 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config. if syncAction == "" { syncProgress = 0 } + numProtected, numDisabled, numInvalid, numMissing := reconcileLegacyUnraidCounts(report.Unraid, disks) unraidData = &models.HostUnraidStorage{ ArrayStarted: report.Unraid.ArrayStarted, ArrayState: strings.TrimSpace(report.Unraid.ArrayState), SyncAction: syncAction, SyncProgress: syncProgress, SyncErrors: report.Unraid.SyncErrors, - NumProtected: report.Unraid.NumProtected, - NumDisabled: report.Unraid.NumDisabled, - NumInvalid: report.Unraid.NumInvalid, - NumMissing: report.Unraid.NumMissing, + NumProtected: numProtected, + NumDisabled: numDisabled, + NumInvalid: numInvalid, + NumMissing: numMissing, Disks: disks, } } @@ -3667,17 +3668,54 @@ func isLegacyUnraidEmptySlot(disk agentshost.UnraidDisk, normalizedStatus string if !strings.Contains(rawStatus, "DISK_NP") && status != "missing" { return false } - name := strings.ToLower(strings.TrimSpace(disk.Name)) - role := strings.ToLower(strings.TrimSpace(disk.Role)) - if name != "" && role != "parity" && !strings.HasPrefix(name, "parity") { - return false - } return strings.TrimSpace(disk.Device) == "" && + strings.TrimSpace(disk.Model) == "" && strings.TrimSpace(disk.Serial) == "" && strings.TrimSpace(disk.Filesystem) == "" && disk.SizeBytes == 0 } +func reconcileLegacyUnraidCounts(storage *agentshost.UnraidStorage, disks []models.HostUnraidDisk) (protected, disabled, invalid, missing int) { + protected = storage.NumProtected + disabled = storage.NumDisabled + invalid = storage.NumInvalid + missing = storage.NumMissing + + hasStructuredStatus := false + hasStructuredParity := false + structuredProtected, structuredDisabled, structuredInvalid, structuredMissing := 0, 0, 0, 0 + for _, disk := range disks { + status := strings.ToLower(strings.TrimSpace(disk.Status)) + if status == "" { + continue + } + hasStructuredStatus = true + if strings.EqualFold(strings.TrimSpace(disk.Role), "parity") { + hasStructuredParity = true + if status == "online" { + structuredProtected++ + } + } + switch status { + case "disabled": + structuredDisabled++ + case "invalid": + structuredInvalid++ + case "missing": + structuredMissing++ + } + } + if hasStructuredStatus { + disabled = structuredDisabled + invalid = structuredInvalid + missing = structuredMissing + } + if hasStructuredParity { + protected = structuredProtected + } + return protected, disabled, invalid, missing +} + type proxmoxDiskMatch struct { device string metricID string diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 9a083a54e..eb4c9125a 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -2404,13 +2404,15 @@ func TestApplyHostReportFiltersLegacyUnraidEmptySlots(t *testing.T) { Unraid: &agentshost.UnraidStorage{ ArrayStarted: true, ArrayState: "STARTED", - NumDisabled: 2, - NumInvalid: 2, + NumProtected: 0, + NumDisabled: 1, + NumInvalid: 1, + NumMissing: 2, Disks: []agentshost.UnraidDisk{ - {Name: "parity", Role: "parity", RawStatus: "DISK_NP_DSBL"}, - {Name: "md1p1", Device: "/dev/sde", RawStatus: "DISK_OK", SizeBytes: 5860522532}, - {RawStatus: "DISK_NP", Slot: 5}, - {RawStatus: "DISK_NP_DSBL", Slot: 29}, + {Name: "parity", Device: "/dev/sdb", Role: "parity", RawStatus: "DISK_OK", SizeBytes: 5860522532}, + {Name: "disk1", Device: "/dev/sde", Role: "data", RawStatus: "DISK_OK", SizeBytes: 5860522532}, + {Name: "disk6", Role: "data", RawStatus: "DISK_NP", Slot: 6}, + {Name: "parity2", Role: "parity", RawStatus: "DISK_NP_DSBL", Slot: 29}, }, }, Timestamp: time.Now().UTC(), @@ -2423,11 +2425,19 @@ func TestApplyHostReportFiltersLegacyUnraidEmptySlots(t *testing.T) { if host.Unraid == nil { t.Fatal("expected unraid topology on host") } - if len(host.Unraid.Disks) != 1 { + if len(host.Unraid.Disks) != 2 { t.Fatalf("unraid disk count = %d, want only assigned disks: %+v", len(host.Unraid.Disks), host.Unraid.Disks) } - if got := host.Unraid.Disks[0]; got.Device != "/dev/sde" || got.Status != "online" { - t.Fatalf("unexpected assigned disk: %+v", got) + if host.Unraid.NumMissing != 0 || host.Unraid.NumDisabled != 0 || host.Unraid.NumInvalid != 0 { + t.Fatalf("legacy empty slots affected normalized failure counts: %+v", host.Unraid) + } + if host.Unraid.NumProtected != 1 { + t.Fatalf("NumProtected = %d, want one online parity disk", host.Unraid.NumProtected) + } + for _, disk := range host.Unraid.Disks { + if disk.Status != "online" { + t.Fatalf("unexpected assigned disk: %+v", disk) + } } } diff --git a/internal/storagehealth/topology.go b/internal/storagehealth/topology.go index 46e7a0f41..c8c6b3dde 100644 --- a/internal/storagehealth/topology.go +++ b/internal/storagehealth/topology.go @@ -349,12 +349,8 @@ func isUnraidEmptySlot(disk models.HostUnraidDisk) bool { if !strings.Contains(rawStatus, "DISK_NP") && status != "missing" { return false } - name := strings.ToLower(strings.TrimSpace(disk.Name)) - role := strings.ToLower(strings.TrimSpace(disk.Role)) - if name != "" && role != "parity" && !strings.HasPrefix(name, "parity") { - return false - } return strings.TrimSpace(disk.Device) == "" && + strings.TrimSpace(disk.Model) == "" && strings.TrimSpace(disk.Serial) == "" && strings.TrimSpace(disk.Filesystem) == "" && disk.SizeBytes == 0 diff --git a/internal/storagehealth/topology_test.go b/internal/storagehealth/topology_test.go index 5e2bc990f..9352106c1 100644 --- a/internal/storagehealth/topology_test.go +++ b/internal/storagehealth/topology_test.go @@ -236,8 +236,8 @@ func TestAssessUnraidStorageTreatsEmptyNoPresentSlotsAsUnprotected(t *testing.T) Disks: []models.HostUnraidDisk{ {Name: "parity", Role: "parity", Status: "missing", RawStatus: "DISK_NP_DSBL"}, {Name: "md1p1", Device: "/dev/sde", Status: "online", RawStatus: "DISK_OK", SizeBytes: 5860522532}, - {Status: "missing", RawStatus: "DISK_NP", Slot: 5}, - {Status: "missing", RawStatus: "DISK_NP_DSBL", Slot: 29}, + {Name: "disk5", Role: "data", Status: "missing", RawStatus: "DISK_NP", Slot: 5}, + {Name: "parity2", Role: "parity", Status: "missing", RawStatus: "DISK_NP_DSBL", Slot: 29}, }, })