diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 549454680e..c3d2879a0f 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -279,6 +279,17 @@ own inline boundary. A wearout arm keyed on `> 0` silently exempts the single worst reading a disk can publish, which let a spent SSD read critical on the Physical Disks surface while raising no alert at all. +Proxmox wearout recovery requires three consecutive reported readings of at +least 10% remaining life. An absent reading does not prove recovery, and a +new low reading resets the recovery run; until recovery is confirmed the +existing alert retains its low-life value and occurrence. This bounds the +resolved-then-refired notification loop when endurance data briefly looks +healthy (#2112), while a sustained replacement/corrected reading can still +resolve. `TestCheckDiskHealthWearoutFlappingDoesNotRepeatNotifications` and +`TestCheckDiskHealthWearoutRecoveryAlertCleared` pin dispatch/history +deduplication and confirmed recovery. This is synthetic source proof, not a +claim that the reporter's installed image has been repaired. + Host SMART counter growth is an event boundary rather than a warning on every historical non-zero value. For an agent-only disk, the first reported UDMA CRC error count establishes an in-memory baseline and raises no alert. A later diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 9de3a49844..7e78fb8377 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -4322,6 +4322,12 @@ func TestCheckDiskHealthWearoutRecoveryAlertCleared(t *testing.T) { // Wearout recovers (replaced drive, or misread corrected) disk.Wearout = 95 + for range 2 { + m.CheckDiskHealth("test-instance", "pve-node1", disk) + if !testHasActiveAlert(t, m, wearoutAlertID) { + t.Fatal("wearout alert resolved on an unconfirmed recovery reading") + } + } m.CheckDiskHealth("test-instance", "pve-node1", disk) m.mu.RLock() diff --git a/internal/alerts/canonical_lifecycle.go b/internal/alerts/canonical_lifecycle.go index 3a36d3f60b..d1c8e738b5 100644 --- a/internal/alerts/canonical_lifecycle.go +++ b/internal/alerts/canonical_lifecycle.go @@ -40,6 +40,7 @@ type canonicalLifecycleAlertParams struct { type canonicalStatefulAlertParams struct { Spec alertspecs.ResourceAlertSpec Evidence alertspecs.AlertEvidence + RecoveryConfirmations int AlertID string AlertType string ResourceID string @@ -675,8 +676,9 @@ func (m *Manager) evaluateCanonicalStatefulAlert(params canonicalStatefulAlertPa Severity: severity, ObservedAt: params.Evidence.ObservedAt, }, reducer.DiscreteRule{ - Confirmations: specConfirmationsRequired(params.Spec), - Disabled: params.Spec.Disabled, + Confirmations: specConfirmationsRequired(params.Spec), + RecoveryConfirmations: params.RecoveryConfirmations, + Disabled: params.Spec.Disabled, }) primary := reducer.EventType("") if len(events) > 0 { @@ -722,6 +724,12 @@ func (m *Manager) evaluateCanonicalStatefulAlert(params canonicalStatefulAlertPa result.State.ConsecutiveMatches = incident.Confirmations result.State.FirstMatchedAt = incident.StartedAt result.State.ActiveSince = incident.StartedAt + // A recovery still inside its confirmation window is not new failing + // evidence. Keep the previous alert value and message rather than + // projecting a healthy reading onto a still-active warning. + if !matched { + return result, true + } switch { case existing == nil: result.Transition = transition(alertspecs.EvaluationTransitionActivated, alertspecs.AlertStatePending, alertspecs.AlertStateFiring) diff --git a/internal/alerts/canonical_stateful_test.go b/internal/alerts/canonical_stateful_test.go index 8bf50a00e3..8f7f41e9aa 100644 --- a/internal/alerts/canonical_stateful_test.go +++ b/internal/alerts/canonical_stateful_test.go @@ -9,6 +9,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) func TestCanonicalLifecycleCopiesValidatedAlertCorrelation(t *testing.T) { @@ -323,3 +324,72 @@ func TestHealthAssessmentEscalationDelivery(t *testing.T) { t.Fatalf("rate-limited incident failed to update: %v", a.Level) } } + +// A single optimistic endurance reading must not close a still-worn disk's +// occurrence. Otherwise the next physical-disk poll fires and notifies again, +// as reported in #2112 after the host-agent SMART merge correction shipped. +func TestCheckDiskHealthWearoutFlappingDoesNotRepeatNotifications(t *testing.T) { + m := newEventLogManager(t) + m.SetAlertCallback(func(*Alert) {}) + disk := proxmox.Disk{ + DevPath: "/dev/sda", Model: "KINGSTON SA400", Serial: "stable-serial", + Type: "ssd", Health: "PASSED", Wearout: 0, + } + check := func(wearout int) { + disk.Wearout = wearout + m.CheckDiskHealth("pve", "rocket", disk) + } + + check(0) + active := m.GetActiveAlerts() + if len(active) != 1 || active[0].Type != "disk-wearout" { + t.Fatalf("spent SSD did not open its wearout alert: %+v", active) + } + initialID := active[0].ID + for range 12 { + check(100) // intermittent healthy-looking reading + if held := m.GetActiveAlerts(); len(held) != 1 || held[0].Value != 0 || held[0].Message != active[0].Message { + t.Fatalf("unconfirmed recovery rewrote the worn-disk warning: %+v", held) + } + check(0) // unchanged worn drive on the next poll + } + active = m.GetActiveAlerts() + if len(active) != 1 || active[0].ID != initialID || active[0].Value != 0 { + t.Fatalf("flapping reading changed the worn disk occurrence: %+v", active) + } + if got := len(m.historyManager.GetAllHistory(100)); got != 1 { + t.Fatalf("flapping produced %d history entries, want one", got) + } + if got := len(queryAlertEvents(t, m, eventlog.Filter{Types: []string{eventlog.TypeNotificationDispatched}})); got != 1 { + t.Fatalf("flapping produced %d dispatches, want one", got) + } + if got := len(m.GetRecentlyResolved()); got != 0 { + t.Fatalf("flapping produced %d resolved occurrences, want none", got) + } + check(-1) + if got := len(m.GetActiveAlerts()); got != 1 { + t.Fatalf("unreported wearout incorrectly proved recovery: %d active", got) + } + + // A genuinely sustained high reading still resolves; this is a recovery + // confirmation, not a permanent wearout-alert latch. + check(100) + check(100) + if got := len(m.GetActiveAlerts()); got != 1 { + t.Fatalf("alert recovered before the third healthy reading: %d", got) + } + check(100) + if got := len(m.GetActiveAlerts()); got != 0 { + t.Fatalf("alert did not recover after three healthy readings: %d", got) + } + if got := len(m.GetRecentlyResolved()); got != 1 { + t.Fatalf("sustained recovery produced %d resolved occurrences, want one", got) + } + check(0) + if got := len(m.GetActiveAlerts()); got != 1 { + t.Fatalf("new low-life reading after confirmed recovery did not alert: %d", got) + } + if got := len(queryAlertEvents(t, m, eventlog.Filter{Types: []string{eventlog.TypeNotificationDispatched}})); got != 2 { + t.Fatalf("genuine recurrence produced %d total dispatches, want two", got) + } +} diff --git a/internal/alerts/disk_health.go b/internal/alerts/disk_health.go index eea484d24c..db41da7cb3 100644 --- a/internal/alerts/disk_health.go +++ b/internal/alerts/disk_health.go @@ -36,7 +36,6 @@ func (m *Manager) CheckDiskHealth(instance, node string, disk proxmox.Disk) { canonicalResourceID := proxmoxDiskCanonicalResourceID(instance, node, disk.DevPath) resourceType := unifiedresources.ResourceType("proxmox-disk") canonicalHealthAlertID := buildCanonicalStateID(canonicalResourceID, canonicalResourceID+"-health") - canonicalWearoutAlertID := buildCanonicalStateID(canonicalResourceID, canonicalResourceID+"-wearout") clearDiskAlert := func(ids ...string) { seen := make(map[string]struct{}, len(ids)) @@ -119,11 +118,10 @@ func (m *Manager) CheckDiskHealth(instance, node string, disk proxmox.Disk) { clearDiskAlert(alertID, canonicalHealthAlertID) } - // Check for low wearout (SSD life remaining). Gate on the same predicate - // the risk assessment uses so a disk cannot read critical in the UI while - // staying silent here: 0 from an SSD means no endurance left and must - // alert, while -1 and a rotational 0 are absent evidence. - if storagehealth.WearoutReported(disk.Wearout, disk.Type) && disk.Wearout < 10 { + // Check wearout only when the disk reports endurance. Unknown readings + // cannot prove recovery; 0 from an SSD is a real, fully spent reading, + // whereas -1 and rotational 0 are absent evidence. + if storagehealth.WearoutReported(disk.Wearout, disk.Type) { message := fmt.Sprintf("SSD has less than 10%% life remaining (%d%% wearout)", disk.Wearout) spec, err := buildCanonicalSeverityThresholdSpecWithDirection(canonicalResourceID+"-wearout", canonicalResourceID, resourceName, resourceType, "wearout-remaining", alertspecs.ThresholdDirectionBelow, 10, 0, false) if err != nil { @@ -138,6 +136,10 @@ func (m *Manager) CheckDiskHealth(instance, node string, disk proxmox.Disk) { _, _ = m.evaluateCanonicalStatefulAlert(canonicalStatefulAlertParams{ Spec: spec, + // Endurance cannot normally improve on the same disk. Require + // sustained healthy-looking polls before resolving a worn-disk + // occurrence; a new low reading resets the recovery run (#2112). + RecoveryConfirmations: 3, Evidence: alertspecs.AlertEvidence{ ObservedAt: time.Now(), SeverityThreshold: &alertspecs.SeverityThresholdEvidence{ @@ -160,15 +162,14 @@ func (m *Manager) CheckDiskHealth(instance, node string, disk proxmox.Disk) { AddToHistory: true, }) - log.Warn(). - Str("node", node). - Str("disk", disk.DevPath). - Str("model", disk.Model). - Int("wearout", disk.Wearout). - Msg("Disk wearout alert created") + if disk.Wearout < 10 { + log.Warn(). + Str("node", node). + Str("disk", disk.DevPath). + Str("model", disk.Model). + Int("wearout", disk.Wearout). + Msg("Disk wearout alert observed") + } } - } else if disk.Wearout >= 10 { - // Wearout is acceptable, clear alert if it exists - clearDiskAlert(wearoutAlertID, canonicalWearoutAlertID) } }