From 3334cccfd9f4a41a4c1f576ed7b9517e9af4c4b7 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 23:39:10 +0100 Subject: [PATCH] fix(unraid): suppress parity warning for explicit pool-only arrays Issue #1895 reports parity alerts when mdNumDisks=0 on a pool-only Unraid system. Array service state alone does not establish that a parity array exists. Preserve the optional disk count from collection through canonical runtime conversion and suppress only the no-parity warning for an explicit zero. Missing or malformed counts retain legacy behaviour, and disk failure reasons remain active. Validated focused Unraid tests in hostagent, storagehealth, monitoring, unifiedresources and alerts, including JSON zero preservation and canonical round trip. The new pool-only regression fails against the previous warning condition. Both agent and server need this change; no release or reporter retest is claimed. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 2 + .../v6/internal/subsystems/monitoring.md | 2 + .../internal/subsystems/storage-recovery.md | 2 + .../internal/subsystems/unified-resources.md | 2 + internal/hostagent/unraid.go | 5 ++ internal/hostagent/unraid_test.go | 52 +++++++++++++++++++ internal/models/models.go | 1 + internal/monitoring/monitor.go | 1 + internal/monitoring/monitor_agents.go | 1 + .../monitoring/monitor_host_agents_test.go | 31 +++++++++++ internal/storagehealth/topology.go | 4 +- internal/storagehealth/topology_test.go | 35 +++++++++++++ internal/unifiedresources/adapters.go | 2 + internal/unifiedresources/adapters_test.go | 11 ++++ .../canonical_ids_types_test.go | 34 ++++++++++++ internal/unifiedresources/types.go | 2 + pkg/agents/host/report.go | 1 + 17 files changed, 187 insertions(+), 1 deletion(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index edbc7f7ea..8dd94d00a 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,8 @@ ## Purpose +Unraid collection preserves optional nonnegative `mdNumDisks` as `numDisks` through the host report. Explicit zero survives JSON encoding; absent, negative, or malformed source counts remain unknown. This telemetry does not change enrollment or command authority; older agents retain unknown-count behaviour. + ### Portable installer lifecycle ownership The shared shell installer lifecycle directory (outside the least-privilege diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index d39d98483..5e641c9fd 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -17,6 +17,8 @@ ## Purpose +Unraid host ingestion and canonical read-state reconstruction preserve the optional array disk count, distinguishing explicit zero from unknown. Storage assessment suppresses only the no-parity warning for an explicit zero-disk array; unknown counts retain the prior warning and disabled, invalid, or missing member evidence remains effective. + Direct PBS backup polling correlates manifestless snapshots with current writer tasks before publishing guest backup-running state. The client queries running `backup` and `syncjob` task families separately with bounded pagination; diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 3ca096331..32508454e 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -21,6 +21,8 @@ ## Purpose +Unraid `StorageMeta.numDisks` is optional source evidence: zero denotes an explicitly empty parity array, not proof that pools are protected or that a recovery point exists. Suppressing the no-parity warning for zero must not erase disk-failure evidence or grant storage/recovery authority. + A manifestless PBS snapshot never becomes a successful recovery point and never advances backup age. Recovery mapping reports it as running while a live writer accounts for it or current-task visibility is unavailable; after a diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index af0f71be9..dbe212996 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -15,6 +15,8 @@ ## Purpose +Unraid adapters preserve optional `numDisks` in both host and storage metadata, including explicit zero in JSON and absence for unknown counts. Disk count is topology evidence only: changing it must not change canonical host/storage identity. The storage projection uses the monitoring-owned assessment so an explicit pool-only array does not acquire a no-parity warning. + Own canonical resource identity, type normalization, typed views, and cross-source deduplication. Storage metadata may expose source-authored alias IDs as compatibility evidence diff --git a/internal/hostagent/unraid.go b/internal/hostagent/unraid.go index 20c77ef3f..ed46a0a4e 100644 --- a/internal/hostagent/unraid.go +++ b/internal/hostagent/unraid.go @@ -185,6 +185,11 @@ func parseUnraidStatusOutput(output string) (*agentshost.UnraidStorage, error) { NumMissing: parseUnraidIntField(fields, "mdNumMissing"), } + // Zero is meaningful for pool-only systems; missing or invalid is unknown. + if count, err := strconv.Atoi(strings.TrimSpace(fields["mdNumDisks"])); err == nil && count >= 0 { + storage.NumDisks = &count + } + indexes := collectUnraidIndexes(fields) disks := make([]agentshost.UnraidDisk, 0, len(indexes)) for _, idx := range indexes { diff --git a/internal/hostagent/unraid_test.go b/internal/hostagent/unraid_test.go index a5fb58621..33a92b3f9 100644 --- a/internal/hostagent/unraid_test.go +++ b/internal/hostagent/unraid_test.go @@ -2,6 +2,7 @@ package hostagent import ( "context" + "encoding/json" "os" "path/filepath" "testing" @@ -384,3 +385,54 @@ func TestCollectUnraidStorageUsesResolvedMdcmd(t *testing.T) { t.Fatalf("CollectUnraidStorage() = %#v, want populated storage", storage) } } + +func TestParseUnraidStatusArrayDiskCount(t *testing.T) { + for _, tc := range []struct { + name, field string + want *int + }{ + {"pool-only", "mdNumDisks=0", newIntForArrayCount(0)}, + {"array", "mdNumDisks=3", newIntForArrayCount(3)}, + {"missing", "", nil}, + {"invalid", "mdNumDisks=unknown", nil}, + {"negative", "mdNumDisks=-1", nil}, + } { + t.Run(tc.name, func(t *testing.T) { + storage, err := parseUnraidStatusOutput("mdState=STARTED\n" + tc.field + "\n") + if err != nil { + t.Fatal(err) + } + if tc.want == nil { + if storage.NumDisks != nil { + t.Fatalf("unexpected count %d", *storage.NumDisks) + } + } else if storage.NumDisks == nil || *storage.NumDisks != *tc.want { + t.Fatalf("count = %v, want %d", storage.NumDisks, *tc.want) + } + if !storage.ArrayStarted { + t.Fatal("disk count must not change service state") + } + wire, err := json.Marshal(storage) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(wire, &fields); err != nil { + t.Fatal(err) + } + count, present := fields["numDisks"] + if present != (tc.want != nil) { + t.Fatalf("count presence in %s", wire) + } + if tc.want != nil { + var got int + if err := json.Unmarshal(count, &got); err != nil || got != *tc.want { + t.Fatalf("wire count = %s", count) + } + } + + }) + } +} + +func newIntForArrayCount(n int) *int { return &n } diff --git a/internal/models/models.go b/internal/models/models.go index 83dd00a44..318c8631a 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -739,6 +739,7 @@ type HostUnraidStorage struct { SyncAction string `json:"syncAction,omitempty"` SyncProgress float64 `json:"syncProgress,omitempty"` SyncErrors int64 `json:"syncErrors,omitempty"` + NumDisks *int `json:"numDisks,omitempty"` // nil means the agent did not report an array disk count. NumProtected int `json:"numProtected,omitempty"` NumDisabled int `json:"numDisabled,omitempty"` NumInvalid int `json:"numInvalid,omitempty"` diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index e5f1268f7..aa795b652 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -3896,6 +3896,7 @@ func hostUnraidFromReadStateView(unraid *unifiedresources.HostUnraidMeta) *model SyncAction: unraid.SyncAction, SyncProgress: unraid.SyncProgress, SyncErrors: unraid.SyncErrors, + NumDisks: unraid.NumDisks, NumProtected: unraid.NumProtected, NumDisabled: unraid.NumDisabled, NumInvalid: unraid.NumInvalid, diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 56a5cf1da..a4bf3c3af 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -3415,6 +3415,7 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config. SyncAction: syncAction, SyncProgress: syncProgress, SyncErrors: report.Unraid.SyncErrors, + NumDisks: report.Unraid.NumDisks, NumProtected: numProtected, NumDisabled: numDisabled, NumInvalid: numInvalid, diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 95c0d8c3b..ef5976097 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -5654,3 +5654,34 @@ func TestMonitorConstructionWiresNotificationDeliveryReconciliation(t *testing.T } } } + +func TestApplyHostReportPreservesPoolOnlyUnraidCount(t *testing.T) { + monitor := &Monitor{ + state: models.NewState(), alertManager: alerts.NewManager(), + hostTokenBindings: make(map[string]string), config: &config.Config{}, + rateTracker: NewRateTracker(), + } + t.Cleanup(func() { monitor.alertManager.Stop() }) + var report agentshost.Report + // Exercise the agent wire format: explicit zero must survive omitempty. + err := json.Unmarshal([]byte(`{"agent":{"id":"pool-only","version":"test"},"host":{"id":"pool-only","hostname":"pool-only"},"unraid":{"arrayStarted":true,"numDisks":0}}`), &report) + if err != nil { + t.Fatal(err) + } + report.Timestamp = time.Now().UTC() + host, err := monitor.ApplyHostReport(report, nil) + if err != nil { + t.Fatal(err) + } + if host.Unraid == nil || host.Unraid.NumDisks == nil || *host.Unraid.NumDisks != 0 { + t.Fatalf("lost explicit zero: %+v", host.Unraid) + } + record := unifiedresources.HostIngestRecord(host) + restored := hostUnraidFromReadStateView(record.Resource.Agent.Unraid) + if restored == nil || restored.NumDisks == nil || *restored.NumDisks != 0 { + t.Fatalf("canonical round trip lost explicit zero: %+v", restored) + } + if assessment := storagehealth.AssessUnraidStorage(*restored); assessment.Level != storagehealth.RiskHealthy { + t.Fatalf("pool-only host raised storage risk: %+v", assessment) + } +} diff --git a/internal/storagehealth/topology.go b/internal/storagehealth/topology.go index 25b61af35..d18f5a0b8 100644 --- a/internal/storagehealth/topology.go +++ b/internal/storagehealth/topology.go @@ -299,7 +299,9 @@ func AssessUnraidStorage(storage models.HostUnraidStorage) Assessment { } } - if storage.ArrayStarted && !parityConfigured { + // Unraid can start pool services with no array. Only an explicit zero + // suppresses this warning, preserving behaviour for older agents. + if storage.ArrayStarted && !parityConfigured && (storage.NumDisks == nil || *storage.NumDisks != 0) { addReason("unraid_no_parity", RiskWarning, "Unraid array is running without parity protection") } if storage.ArrayStarted && parityConfigured && !parityHealthy { diff --git a/internal/storagehealth/topology_test.go b/internal/storagehealth/topology_test.go index e61c095a1..01ea69283 100644 --- a/internal/storagehealth/topology_test.go +++ b/internal/storagehealth/topology_test.go @@ -382,3 +382,38 @@ func TestAssessPBSDatastoreHighUsage(t *testing.T) { t.Fatalf("unexpected reasons %+v", assessment.Reasons) } } + +func TestAssessUnraidStoragePoolOnlyParity(t *testing.T) { + zero, three := 0, 3 + for _, tc := range []struct { + name string + count *int + wantWarning bool + }{ + {"explicit pool-only", &zero, false}, + {"array without parity", &three, true}, + {"legacy unknown count", nil, true}, + } { + t.Run(tc.name, func(t *testing.T) { + assessment := AssessUnraidStorage(models.HostUnraidStorage{ + ArrayStarted: true, NumDisks: tc.count, + Disks: []models.HostUnraidDisk{{Name: "cache", Role: "cache", Status: "online"}}, + }) + found := false + for _, reason := range assessment.Reasons { + if reason.Code == "unraid_no_parity" { + found = true + } + } + if found != tc.wantWarning { + t.Fatalf("no-parity warning = %v; reasons %+v", found, assessment.Reasons) + } + }) + } + assessment := AssessUnraidStorage(models.HostUnraidStorage{ + ArrayStarted: true, NumDisks: &zero, NumDisabled: 1, + }) + if assessment.Level != RiskCritical { + t.Fatalf("zero count masked disk failure: %+v", assessment) + } +} diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index 64e429670..935baf82f 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -416,6 +416,7 @@ func resourceFromHost(host models.Host) (Resource, ResourceIdentity) { SyncAction: host.Unraid.SyncAction, SyncProgress: host.Unraid.SyncProgress, SyncErrors: host.Unraid.SyncErrors, + NumDisks: host.Unraid.NumDisks, NumProtected: host.Unraid.NumProtected, NumDisabled: host.Unraid.NumDisabled, NumInvalid: host.Unraid.NumInvalid, @@ -726,6 +727,7 @@ func resourceFromHostUnraidStorage(host models.Host) (Resource, ResourceIdentity ArrayState: host.Unraid.ArrayState, SyncAction: host.Unraid.SyncAction, SyncProgress: host.Unraid.SyncProgress, + NumDisks: host.Unraid.NumDisks, NumProtected: host.Unraid.NumProtected, NumDisabled: host.Unraid.NumDisabled, NumInvalid: host.Unraid.NumInvalid, diff --git a/internal/unifiedresources/adapters_test.go b/internal/unifiedresources/adapters_test.go index 09795546f..4768e5a8a 100644 --- a/internal/unifiedresources/adapters_test.go +++ b/internal/unifiedresources/adapters_test.go @@ -1969,3 +1969,14 @@ func TestResourceFromGuestSetsProxmoxGuestKey(t *testing.T) { t.Fatalf("keyless VM must not carry a guest key, got %q", keyless.ProxmoxGuestKey) } } + +func TestUnraidArrayDiskCountAdapters(t *testing.T) { + for _, count := range []*int{nil, new(int), func() *int { n := 3; return &n }()} { + host := models.Host{ID: "pool-only", Hostname: "pool-only", Unraid: &models.HostUnraidStorage{NumDisks: count}} + resource, _ := resourceFromHost(host) + storage, _ := resourceFromHostUnraidStorage(host) + if !reflect.DeepEqual(resource.Agent.Unraid.NumDisks, count) || !reflect.DeepEqual(storage.Storage.NumDisks, count) { + t.Fatalf("adapters lost disk count %v", count) + } + } +} diff --git a/internal/unifiedresources/canonical_ids_types_test.go b/internal/unifiedresources/canonical_ids_types_test.go index 5537d33b3..08c477179 100644 --- a/internal/unifiedresources/canonical_ids_types_test.go +++ b/internal/unifiedresources/canonical_ids_types_test.go @@ -554,3 +554,37 @@ func TestWearoutUnreportedSentinelIsNegativeOne(t *testing.T) { t.Fatal("the unreported sentinel must not fall inside the real 0-100 reporting range") } } + +func TestUnraidDiskCountJSONAndIdentity(t *testing.T) { + host := models.Host{ID: "pool-only", Hostname: "pool-only", Unraid: &models.HostUnraidStorage{}} + _, unknownIdentity := resourceFromHostUnraidStorage(host) + zero := 0 + host.Unraid.NumDisks = &zero + resource, zeroIdentity := resourceFromHostUnraidStorage(host) + if unknownIdentity.MachineID != zeroIdentity.MachineID { + t.Fatal("disk count changed storage identity") + } + data, err := json.Marshal(resource.Storage) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `"numDisks":0`) { + t.Fatalf("explicit zero lost: %s", data) + } + var restored StorageMeta + if err := json.Unmarshal(data, &restored); err != nil { + t.Fatal(err) + } + if restored.NumDisks == nil || *restored.NumDisks != 0 { + t.Fatal("zero lost in round trip") + } + host.Unraid.NumDisks = nil + resource, _ = resourceFromHostUnraidStorage(host) + data, err = json.Marshal(resource.Storage) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), `"numDisks"`) { + t.Fatalf("unknown count became known: %s", data) + } +} diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index 826c508e5..0eef1d26c 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -454,6 +454,7 @@ type StorageMeta struct { ArrayState string `json:"arrayState,omitempty"` SyncAction string `json:"syncAction,omitempty"` SyncProgress float64 `json:"syncProgress,omitempty"` + NumDisks *int `json:"numDisks,omitempty"` // nil means the agent did not report an array disk count. NumProtected int `json:"numProtected,omitempty"` NumDisabled int `json:"numDisabled,omitempty"` NumInvalid int `json:"numInvalid,omitempty"` @@ -748,6 +749,7 @@ type HostUnraidMeta struct { SyncAction string `json:"syncAction,omitempty"` SyncProgress float64 `json:"syncProgress,omitempty"` SyncErrors int64 `json:"syncErrors,omitempty"` + NumDisks *int `json:"numDisks,omitempty"` // nil means the agent did not report an array disk count. NumProtected int `json:"numProtected,omitempty"` NumDisabled int `json:"numDisabled,omitempty"` NumInvalid int `json:"numInvalid,omitempty"` diff --git a/pkg/agents/host/report.go b/pkg/agents/host/report.go index ae34b0aea..d484c4733 100644 --- a/pkg/agents/host/report.go +++ b/pkg/agents/host/report.go @@ -491,6 +491,7 @@ type UnraidStorage struct { SyncAction string `json:"syncAction,omitempty"` SyncProgress float64 `json:"syncProgress,omitempty"` SyncErrors int64 `json:"syncErrors,omitempty"` + NumDisks *int `json:"numDisks,omitempty"` // nil means the agent did not report an array disk count. NumProtected int `json:"numProtected,omitempty"` NumDisabled int `json:"numDisabled,omitempty"` NumInvalid int `json:"numInvalid,omitempty"`