From 9c914ff6ae281769dfcbbdf6e09f73a46f10f70e Mon Sep 17 00:00:00 2001 From: Pulse Test Date: Sat, 29 Aug 2026 16:54:18 +0100 Subject: [PATCH] fix(alerts): hold SMART incidents on unknown evidence --- docs/release-control/v6/internal/status.json | 16 ++- .../v6/internal/subsystems/alerts.md | 11 ++ internal/alerts/alerts_test.go | 67 +++++++++ internal/alerts/host.go | 130 +++++++++++++++++- 4 files changed, 221 insertions(+), 3 deletions(-) diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index 4438b39bd..d88e824aa 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -9766,7 +9766,21 @@ } ], "candidate_lanes": [], - "work_claims": [], + "work_claims": [ + { + "id": "codex-lane-l6", + "agent_id": "codex", + "summary": "Make SMART alert recovery hold on standby evidence", + "target_id": "v6-product-lane-expansion", + "claimed_at": "2026-08-29T15:45:01Z", + "heartbeat_at": "2026-08-29T15:45:01Z", + "expires_at": "2026-08-29T17:45:01Z", + "work_item": { + "kind": "lane", + "id": "L6" + } + } + ], "open_decisions": [], "source_of_truth_file": "docs/release-control/v6/internal/SOURCE_OF_TRUTH.md", "resolved_decisions": [ diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index ec284e513..0f3a5adcb 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -139,6 +139,17 @@ round-trip every rule without changing these defaults. Linked Proxmox host agents continue to defer risk alert ownership even when their resolved SMART policy differs. +Host SMART recovery is field-evidence-aware. A standby report preserves disk +identity but carries no health authority, so it must not open or clear either +the canonical `disk-health` or `disk-wearout` assessment. For an active SMART +alert, every still-enabled reason that raised the occurrence must be observed +again before recovery evaluation; an omitted counter or endurance field is +unknown rather than a healthy zero. Explicitly disabling the owning SMART rule +remains authoritative and may clear the alert without waking the disk. +`TestCheckHostSMARTDiskAlertRequiresRelevantRecoveryEvidence` in +`internal/alerts/alerts_test.go` pins the fire, standby hold, partial-evidence +hold, and authoritative recovery sequence. + Threshold sections are keyed by override identity, not by resource type. The Virtualization Hosts section reads and writes overrides on the bare resource id, while the Machines section resolves through the agent-derived identity diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 60f8ec2f5..a7167f81b 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -4369,6 +4369,73 @@ func TestCheckHostSMARTDiskHealthAnnotatesCanonicalSpecMetadata(t *testing.T) { } } +func TestCheckHostSMARTDiskAlertRequiresRelevantRecoveryEvidence(t *testing.T) { + m := newTestManager(t) + m.ClearActiveAlerts() + + reallocated := int64(16) + host := models.Host{ + ID: "tower-host", + Hostname: "tower", + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sdc", + Model: "Seagate IronWolf", + Serial: "SERIAL-TOWER-SDC", + Health: "PASSED", + Attributes: &models.SMARTAttributes{ + ReallocatedSectors: &reallocated, + }, + }}}, + } + alertID := buildCanonicalStateID("agent:tower-host/disk:sdc", "agent:tower-host/disk:sdc-disk-health") + + m.CheckHost(host) + fired := testRequireActiveAlert(t, m, alertID) + originalStart := fired.StartTime + if codes, ok := fired.Metadata["riskCodes"].([]string); !ok || !reflect.DeepEqual(codes, []string{"reallocated_sectors"}) { + t.Fatalf("riskCodes = %#v, want reallocated_sectors", fired.Metadata["riskCodes"]) + } + if got := len(m.historyManager.GetAllHistory(1000)); got != 1 { + t.Fatalf("history entries after fire = %d, want 1", got) + } + + host.Sensors.SMART[0].Standby = true + host.Sensors.SMART[0].Health = "UNKNOWN" + host.Sensors.SMART[0].Attributes = nil + m.CheckHost(host) + + held := testRequireActiveAlert(t, m, alertID) + if !held.StartTime.Equal(originalStart) { + t.Fatalf("standby report changed alert start from %v to %v", originalStart, held.StartTime) + } + if _, resolved := testLookupResolvedAlert(t, m, alertID); resolved { + t.Fatal("standby report must not resolve the SMART alert") + } + + // Waking the disk but omitting the counter that raised the alert is still + // insufficient recovery evidence for that specific rule. + host.Sensors.SMART[0].Standby = false + host.Sensors.SMART[0].Health = "PASSED" + host.Sensors.SMART[0].Attributes = &models.SMARTAttributes{} + m.CheckHost(host) + if !testHasActiveAlert(t, m, alertID) { + t.Fatal("missing reallocated-sector evidence must hold the SMART alert") + } + + zero := int64(0) + host.Sensors.SMART[0].Attributes.ReallocatedSectors = &zero + m.CheckHost(host) + if testHasActiveAlert(t, m, alertID) { + t.Fatal("authoritative zero reallocated sectors should resolve the SMART alert") + } + if _, resolved := testLookupResolvedAlert(t, m, alertID); !resolved { + t.Fatal("expected resolved SMART alert after authoritative healthy evidence") + } + if got := len(m.historyManager.GetAllHistory(1000)); got != 1 { + t.Fatalf("history entries after recovery = %d, want one occurrence", got) + } +} + func TestCheckHostSkipsSMARTDiskRiskAlertsWhenLinkedToProxmoxNode(t *testing.T) { m := newTestManager(t) m.ClearActiveAlerts() diff --git a/internal/alerts/host.go b/internal/alerts/host.go index b0dda1293..1b3794557 100644 --- a/internal/alerts/host.go +++ b/internal/alerts/host.go @@ -1157,8 +1157,134 @@ func (m *Manager) syncHostSMARTDiskRiskAlerts(host models.Host, disk models.Host assessment.Reasons = append(assessment.Reasons, m.hostSMARTCounterGrowthReasons(resourceID, disk, int64Value(thresholds.SMARTCRCErrorDelta))...) healthReasons, wearReasons := splitSMARTAlertReasons(assessment.Reasons) - m.syncHostSMARTDiskAlert(host, disk, resourceID, resourceName, nodeName, instanceName, baseMetadata, "disk-health", healthReasons) - m.syncHostSMARTDiskAlert(host, disk, resourceID, resourceName, nodeName, instanceName, baseMetadata, "disk-wearout", wearReasons) + if m.hostSMARTDiskAlertEvidenceKnown(resourceID, "disk-health", disk, healthReasons, smartThresholds, int64Value(thresholds.SMARTCRCErrorDelta)) { + m.syncHostSMARTDiskAlert(host, disk, resourceID, resourceName, nodeName, instanceName, baseMetadata, "disk-health", healthReasons) + } + if m.hostSMARTDiskAlertEvidenceKnown(resourceID, "disk-wearout", disk, wearReasons, smartThresholds, int64Value(thresholds.SMARTCRCErrorDelta)) { + m.syncHostSMARTDiskAlert(host, disk, resourceID, resourceName, nodeName, instanceName, baseMetadata, "disk-wearout", wearReasons) + } +} + +// hostSMARTDiskAlertEvidenceKnown prevents an unavailable SMART field from +// being interpreted as a healthy zero. A standby report deliberately carries +// stable disk identity without waking the device to collect health evidence. +// Existing reasons therefore remain active until every still-enabled signal +// that raised the alert is observed again. Explicitly disabling the relevant +// rule remains authoritative and can clear the alert without a disk reading. +func (m *Manager) hostSMARTDiskAlertEvidenceKnown(resourceID, alertType string, disk models.HostDiskSMART, currentReasons []storagehealth.Reason, thresholds storagehealth.SMARTThresholds, crcMinimumDelta int64) bool { + stateID := buildCanonicalStateID(resourceID, resourceID+"-"+alertType) + + m.mu.RLock() + activeAlert, active := m.getActiveAlertNoLock(stateID) + var activeCodes []string + if active && activeAlert != nil { + activeCodes = hostSMARTRiskCodes(activeAlert.Metadata["riskCodes"]) + } + m.mu.RUnlock() + + if active && len(activeCodes) == 0 { + // A restored pre-metadata alert may be reaffirmed by current bad + // evidence, but absence of reasons is not sufficient recovery proof. + return len(currentReasons) > 0 + } + + remainingEnabledReasons := 0 + for _, code := range activeCodes { + if !hostSMARTRiskRuleEnabled(code, thresholds, crcMinimumDelta) { + continue + } + remainingEnabledReasons++ + if disk.Standby || !hostSMARTRiskReasonObserved(code, disk) { + return false + } + } + if active && remainingEnabledReasons == 0 { + return true + } + if disk.Standby { + return false + } + + return hostSMARTAlertFamilyObserved(alertType, disk) +} + +func hostSMARTRiskCodes(value interface{}) []string { + switch codes := value.(type) { + case []string: + return append([]string(nil), codes...) + case []interface{}: + result := make([]string, 0, len(codes)) + for _, code := range codes { + if normalized := strings.TrimSpace(fmt.Sprint(code)); normalized != "" { + result = append(result, normalized) + } + } + return result + default: + return nil + } +} + +func hostSMARTRiskRuleEnabled(code string, thresholds storagehealth.SMARTThresholds, crcMinimumDelta int64) bool { + switch code { + case "health_status": + return thresholds.HealthFailure + case "reallocated_sectors": + return thresholds.ReallocatedSectors > 0 + case "pending_sectors": + return thresholds.PendingSectors > 0 + case "offline_uncorrectable": + return thresholds.OfflineUncorrectable > 0 + case "media_errors": + return thresholds.MediaErrors > 0 + case "crc_errors_increased": + return crcMinimumDelta > 0 + case "wearout_low", "nvme_percentage_used_high": + return thresholds.LifeWarning > 0 || thresholds.LifeCritical > 0 + case "nvme_available_spare_low": + return thresholds.AvailableSpareWarn > 0 || thresholds.AvailableSpareCrit > 0 + default: + return true + } +} + +func hostSMARTRiskReasonObserved(code string, disk models.HostDiskSMART) bool { + attrs := disk.Attributes + switch code { + case "health_status": + health := strings.ToUpper(strings.TrimSpace(disk.Health)) + return health != "" && health != "UNKNOWN" + case "reallocated_sectors": + return attrs != nil && attrs.ReallocatedSectors != nil + case "pending_sectors": + return attrs != nil && attrs.PendingSectors != nil + case "offline_uncorrectable": + return attrs != nil && attrs.OfflineUncorrectable != nil + case "media_errors": + return attrs != nil && attrs.MediaErrors != nil + case "crc_errors_increased": + return attrs != nil && attrs.UDMACRCErrors != nil + case "wearout_low", "nvme_percentage_used_high": + return attrs != nil && attrs.PercentageUsed != nil + case "nvme_available_spare_low": + return attrs != nil && attrs.AvailableSpare != nil + default: + return false + } +} + +func hostSMARTAlertFamilyObserved(alertType string, disk models.HostDiskSMART) bool { + if alertType == "disk-wearout" { + return hostSMARTRiskReasonObserved("nvme_percentage_used_high", disk) || + hostSMARTRiskReasonObserved("nvme_available_spare_low", disk) + } + + return hostSMARTRiskReasonObserved("health_status", disk) || + hostSMARTRiskReasonObserved("reallocated_sectors", disk) || + hostSMARTRiskReasonObserved("pending_sectors", disk) || + hostSMARTRiskReasonObserved("offline_uncorrectable", disk) || + hostSMARTRiskReasonObserved("media_errors", disk) || + hostSMARTRiskReasonObserved("crc_errors_increased", disk) } // hostSMARTCounterGrowthReasons turns a newly increased SMART counter into an