diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 926607e3d..0ab492cf5 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1220,13 +1220,21 @@ 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, even when Unraid gives those slots -topology labels such as `disk6` or `parity2`. Assigned disks may use +topology labels such as `disk6` or `parity2`. Transport-only native identity +sentinels such as `ata-_` are empty-slot metadata, not assignment evidence; +the collector must discard them before native-inventory merge and report +projection. Conversely, provider status `DISK_NP_MISSING` is explicit evidence +of an assigned-but-absent member and must remain reportable even when its +identity fields are unavailable. 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. Monitoring ingest must apply the same evidence filter to reports from deployed older agents, then make the remaining structured member and parity statuses -authoritative over stale aggregate failure and protection counters. +authoritative over stale aggregate failure and protection counters. Focused +provider-boundary proof lives in `internal/hostagent/unraid_test.go`, +`internal/unraid/status_test.go`, and +`internal/monitoring/monitor_host_agents_test.go`. 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/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index da05d2110..7c50c14d3 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1574,11 +1574,17 @@ not reported as missing or disabled media. Unraid topology labels such as `disk6` and `parity2` are not assignment evidence by themselves: a `DISK_NP`/`DISK_NP_DSBL` member remains reportable as genuinely missing only when device, model/serial identity, filesystem, or size evidence shows that a -disk was assigned. After filtering, structured member statuses override stale +disk was assigned. Unraid's transport-only `ata-_` identity and the `ata -` +model artifact emitted by older Pulse parsers are also placeholders, not disk +identity. The distinct provider status `DISK_NP_MISSING` is authoritative +assigned-member evidence and must never be removed merely because those +identity fields are empty or placeholders. After filtering, structured member statuses override stale aggregate missing/disabled/invalid counters, and structured parity status overrides the aggregate protected-parity count. This normalization belongs at server ingest as well as agent collection so deployed older agents stop -creating false health alerts without waiting for an agent upgrade. An Unraid +creating false health alerts without waiting for an agent upgrade. The focused +compatibility proof lives in `internal/monitoring/monitor_host_agents_test.go` +and `internal/unraid/status_test.go`. An Unraid array with assigned data disks but no configured parity is an attention/warning posture with the machine-readable `unraid_no_parity` reason, while active parity check/sync state remains a separate `unraid_sync_active` reason. Realtime resource diff --git a/internal/hostagent/unraid.go b/internal/hostagent/unraid.go index f1d1a1e83..446074e7d 100644 --- a/internal/hostagent/unraid.go +++ b/internal/hostagent/unraid.go @@ -10,6 +10,7 @@ import ( "strings" "time" + unraidstatus "github.com/rcourtman/pulse-go-rewrite/internal/unraid" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" "github.com/rs/zerolog/log" ) @@ -187,11 +188,12 @@ func parseUnraidStatusOutput(output string) (*agentshost.UnraidStorage, error) { indexes := collectUnraidIndexes(fields) disks := make([]agentshost.UnraidDisk, 0, len(indexes)) for _, idx := range indexes { + nativeIdentity := unraidstatus.NormalizeNativeIdentity(firstNonEmpty(fields[fmt.Sprintf("rdevSerial.%d", idx)], fields[fmt.Sprintf("diskSerial.%d", idx)], fields[fmt.Sprintf("rdevId.%d", idx)], fields[fmt.Sprintf("diskId.%d", idx)])) disk := agentshost.UnraidDisk{ Name: strings.TrimSpace(fields[fmt.Sprintf("diskName.%d", idx)]), Device: normalizeBlockDevice(firstNonEmpty(fields[fmt.Sprintf("rdevName.%d", idx)], fields[fmt.Sprintf("diskDevice.%d", idx)])), RawStatus: strings.TrimSpace(firstNonEmpty(fields[fmt.Sprintf("rdevStatus.%d", idx)], fields[fmt.Sprintf("diskState.%d", idx)])), - Serial: strings.TrimSpace(firstNonEmpty(fields[fmt.Sprintf("rdevSerial.%d", idx)], fields[fmt.Sprintf("diskSerial.%d", idx)], fields[fmt.Sprintf("rdevId.%d", idx)], fields[fmt.Sprintf("diskId.%d", idx)])), + Serial: nativeIdentity, Filesystem: strings.TrimSpace(firstNonEmpty(fields[fmt.Sprintf("diskFsType.%d", idx)], fields[fmt.Sprintf("fsType.%d", idx)])), SizeBytes: parseUnraidKiBAsBytes(firstNonEmpty(fields[fmt.Sprintf("diskSize.%d", idx)], fields[fmt.Sprintf("rdevSize.%d", idx)])), Slot: idx, @@ -272,9 +274,10 @@ func parseUnraidDisksINI(input string) []agentshost.UnraidDisk { ErrorCount: parseFirstInt64(fields["numErrors"]), Slot: idx, } - disk.Model, disk.Serial = parseUnraidDiskIdentity(firstNonEmpty(fields["id"], fields["idSb"])) + nativeIdentity := unraidstatus.NormalizeNativeIdentity(firstNonEmpty(fields["id"], fields["idSb"])) + disk.Model, disk.Serial = parseUnraidDiskIdentity(nativeIdentity) if disk.Serial == "" { - disk.Serial = strings.TrimSpace(firstNonEmpty(fields["id"], fields["idSb"])) + disk.Serial = nativeIdentity } if isUnraidEmptySlot(disk) { continue @@ -510,6 +513,9 @@ func defaultUnraidDiskName(role string, idx int) string { func isUnraidEmptySlot(disk agentshost.UnraidDisk) bool { rawStatus := strings.ToUpper(strings.TrimSpace(disk.RawStatus)) status := strings.ToLower(strings.TrimSpace(disk.Status)) + if unraidstatus.IsExplicitMissingMember(rawStatus) { + return false + } if !strings.Contains(rawStatus, "DISK_NP") && status != "missing" { return false } @@ -518,8 +524,7 @@ func isUnraidEmptySlot(disk agentshost.UnraidDisk) bool { // 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) == "" && + !unraidstatus.HasMeaningfulIdentity(disk.Model, disk.Serial) && strings.TrimSpace(disk.Filesystem) == "" && disk.SizeBytes == 0 } diff --git a/internal/hostagent/unraid_test.go b/internal/hostagent/unraid_test.go index 951497978..2f5d92b57 100644 --- a/internal/hostagent/unraid_test.go +++ b/internal/hostagent/unraid_test.go @@ -153,17 +153,17 @@ rdevId.1=WDC_DATA diskNumber.5=5 diskName.5=disk5 diskSize.5=0 -diskId.5= +diskId.5=ata-_ rdevStatus.5=DISK_NP rdevName.5= -rdevId.5= +rdevId.5=ata-_ diskNumber.29=29 diskName.29=parity2 diskSize.29=0 -diskId.29= +diskId.29=ata-_ rdevStatus.29=DISK_NP_DSBL rdevName.29= -rdevId.29= +rdevId.29=ata-_ ` storage, err := parseUnraidStatusOutput(output) @@ -178,6 +178,28 @@ rdevId.29= } } +func TestParseUnraidStatusOutputPreservesExplicitMissingMember(t *testing.T) { + storage, err := parseUnraidStatusOutput(` +mdState=STARTED +mdNumMissing=1 +diskName.6=disk6 +diskSize.6=0 +diskId.6=ata-_ +rdevStatus.6=DISK_NP_MISSING +rdevName.6= +rdevId.6=ata-_ +`) + if err != nil { + t.Fatalf("parseUnraidStatusOutput() error = %v", err) + } + if len(storage.Disks) != 1 { + t.Fatalf("disk count = %d, want explicit missing member: %+v", len(storage.Disks), storage.Disks) + } + if got := storage.Disks[0]; got.Name != "disk6" || got.Status != "missing" { + t.Fatalf("unexpected missing member: %+v", got) + } +} + func TestReconcileUnraidDiskCountsIgnoresNamedEmptySlots(t *testing.T) { storage, err := parseUnraidStatusOutput(` mdState=STARTED @@ -260,7 +282,7 @@ fsUsed="166957852" idx="6" name="disk6" device="" -id="" +id="ata-_" size="0" status="DISK_NP" type="Data" @@ -268,7 +290,7 @@ type="Data" idx="29" name="parity2" device="" -id="" +id="ata-_" size="0" status="DISK_NP_DSBL" type="Parity" diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 569773f66..2b5180c8c 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -18,6 +18,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/remoteconfig" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + unraidstatus "github.com/rcourtman/pulse-go-rewrite/internal/unraid" agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" "github.com/rcourtman/pulse-go-rewrite/pkg/fsfilters" @@ -3654,12 +3655,14 @@ func normalizeLegacyUnraidDiskStatus(rawStatus, device string) string { func isLegacyUnraidEmptySlot(disk agentshost.UnraidDisk, normalizedStatus string) bool { rawStatus := strings.ToUpper(strings.TrimSpace(disk.RawStatus)) status := strings.ToLower(strings.TrimSpace(normalizedStatus)) + if unraidstatus.IsExplicitMissingMember(rawStatus) { + return false + } if !strings.Contains(rawStatus, "DISK_NP") && status != "missing" { return false } return strings.TrimSpace(disk.Device) == "" && - strings.TrimSpace(disk.Model) == "" && - strings.TrimSpace(disk.Serial) == "" && + !unraidstatus.HasMeaningfulIdentity(disk.Model, disk.Serial) && strings.TrimSpace(disk.Filesystem) == "" && disk.SizeBytes == 0 } diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index a0ccdfb4d..921e67f4e 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -2411,8 +2411,8 @@ func TestApplyHostReportFiltersLegacyUnraidEmptySlots(t *testing.T) { Disks: []agentshost.UnraidDisk{ {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}, + {Name: "disk6", Role: "data", RawStatus: "DISK_NP", Model: "ata -", Serial: "ata-_", Slot: 6}, + {Name: "parity2", Role: "parity", RawStatus: "DISK_NP_DSBL", Model: "ata -", Serial: "ata-_", Slot: 29}, }, }, Timestamp: time.Now().UTC(), @@ -2447,6 +2447,23 @@ func TestApplyHostReportFiltersLegacyUnraidEmptySlots(t *testing.T) { } } +func TestLegacyUnraidEmptySlotPreservesExplicitMissingMember(t *testing.T) { + t.Parallel() + + disk := agentshost.UnraidDisk{ + Name: "disk6", + Role: "data", + Status: "missing", + RawStatus: "DISK_NP_MISSING", + Model: "ata -", + Serial: "ata-_", + Slot: 6, + } + if isLegacyUnraidEmptySlot(disk, disk.Status) { + t.Fatal("explicit assigned-but-missing member was classified as an empty slot") + } +} + func TestApplyHostReportFiltersVendorManagedSystemRAIDArrays(t *testing.T) { t.Helper() diff --git a/internal/unraid/status.go b/internal/unraid/status.go new file mode 100644 index 000000000..2ea54e51e --- /dev/null +++ b/internal/unraid/status.go @@ -0,0 +1,39 @@ +package unraid + +import "strings" + +// NormalizeNativeIdentity removes transport-only sentinel values emitted by +// Unraid for slots that have never had a device assigned. These values are not +// disk identities and must not turn DISK_NP placeholders into missing members. +func NormalizeNativeIdentity(value string) string { + value = strings.TrimSpace(value) + if IsPlaceholderIdentity(value) { + return "" + } + return value +} + +// IsPlaceholderIdentity recognizes the empty identity forms exposed by mdcmd +// and the model-shaped value produced when an older agent parsed that identity. +func IsPlaceholderIdentity(value string) bool { + switch strings.ToLower(strings.TrimSpace(value)) { + case "_", "ata-_", "scsi-_", "sas-_", "usb-_", "nvme-_", + "ata -", "scsi -", "sas -", "usb -", "nvme -": + return true + default: + return false + } +} + +// HasMeaningfulIdentity reports whether either native identity field names a +// device rather than an unassigned-slot sentinel. +func HasMeaningfulIdentity(model, serial string) bool { + return NormalizeNativeIdentity(model) != "" || NormalizeNativeIdentity(serial) != "" +} + +// IsExplicitMissingMember reports Unraid's provider-owned status for a slot +// that was assigned but whose device is no longer present. Plain DISK_NP means +// no device is assigned and must not be treated as equivalent. +func IsExplicitMissingMember(rawStatus string) bool { + return strings.EqualFold(strings.TrimSpace(rawStatus), "DISK_NP_MISSING") +} diff --git a/internal/unraid/status_test.go b/internal/unraid/status_test.go new file mode 100644 index 000000000..8c5be0c36 --- /dev/null +++ b/internal/unraid/status_test.go @@ -0,0 +1,37 @@ +package unraid + +import "testing" + +func TestNormalizeNativeIdentity(t *testing.T) { + t.Parallel() + + tests := []struct { + input string + want string + }{ + {input: "ata-_", want: ""}, + {input: " ATA - ", want: ""}, + {input: "_", want: ""}, + {input: "ata-Samsung_SSD_S6BC", want: "ata-Samsung_SSD_S6BC"}, + {input: "S6BCNG0R213032E", want: "S6BCNG0R213032E"}, + } + + for _, test := range tests { + if got := NormalizeNativeIdentity(test.input); got != test.want { + t.Errorf("NormalizeNativeIdentity(%q) = %q, want %q", test.input, got, test.want) + } + } +} + +func TestIsExplicitMissingMember(t *testing.T) { + t.Parallel() + + if !IsExplicitMissingMember("DISK_NP_MISSING") { + t.Fatal("DISK_NP_MISSING must identify an assigned missing member") + } + for _, status := range []string{"DISK_NP", "DISK_NP_DSBL", "DISK_OK", ""} { + if IsExplicitMissingMember(status) { + t.Errorf("%q must not identify an assigned missing member", status) + } + } +}