From eccb64d155aeda3c146e8a222a1e4a6fc5cd6468 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:00:21 +0100 Subject: [PATCH] fix(alerts): honour PBS datastore capacity policy Keep topology capacity bands as resource risk rather than duplicate parent and datastore alerts. The existing live PBS capacity evaluator owns thresholds, aliases and hysteresis; retain datastore state failures and reconcile old duplicates normally. Reproduce the 97.9% report with real registry projection and exercise threshold changes through synthetic PBS HTTP polling. Release adaptations require the live-poll evaluator, not this filter alone. Change-source: pulse-maintainer --- .../v6/internal/subsystems/alerts.md | 24 ++++ internal/alerts/unified_incidents.go | 13 ++ internal/alerts/unified_incidents_test.go | 127 +++++++++++++++--- .../monitoring/monitor_pbs_coverage_test.go | 25 +++- 4 files changed, 169 insertions(+), 20 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 49dc1123c..fd7a30e86 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -2856,3 +2856,27 @@ This changes neither delivery scheduling nor the meaning of a successful test. The registered WebhookConfig regression covers alias test/save payload equality; the browser fixture exercises the real form at desktop and phone widths with synthetic callbacks, not a hosted Pushover destination or installed delivery. + +### PBS capacity has one alert policy owner + +PBS datastore capacity alerts are evaluated by `CheckStorageWithCapacityTrend` +from fresh PBS polls. Storage defaults, canonical datastore aliases, per-resource +overrides, hysteresis and predictive capacity policy govern that lifecycle. +The fixed 90/95% PBS topology assessment remains resource risk evidence; its +Pulse-generated `capacity_runway_low` incidents must not independently enter +active alerts on either the datastore or the parent backup server. Existing +copies retire through normal policy reconciliation, without deleting state or +claiming that capacity itself recovered. Other datastore state/error incidents +and native provider incidents retain their existing lifecycle. + +`TestPBSCapacityUsesStoragePolicyNotTopologyBands` in +`internal/alerts/unified_incidents_test.go` uses real registry projection to pin +both duplicate symptoms at 97.9%, canonical-alias 99% versus 90% policy, existing +alert retirement, threshold recovery, and preservation of datastore failure. +The posture and roll-up tests use datastore state failures, independent of +capacity. This relies on the live PBS poll evaluator; a release adaptation must +include that evaluator rather than remove the topology alerts in isolation. +`TestPBSPolledCapacityRequiresObservedRecovery` in +`internal/monitoring/monitor_pbs_coverage_test.go` additionally exercises the +97.9% policy transition through synthetic PBS HTTP polling, storage conversion +and unified alert synchronisation, including absence of duplicate parent alerts. diff --git a/internal/alerts/unified_incidents.go b/internal/alerts/unified_incidents.go index 4130235da..cb1239e78 100644 --- a/internal/alerts/unified_incidents.go +++ b/internal/alerts/unified_incidents.go @@ -94,6 +94,19 @@ func (m *Manager) SyncUnifiedResourceIncidents(resources []unifiedresources.Reso storageKey := canonicalTrackingKeyForSpec(spec, alert.ID) observedConditions[storageKey] = struct{}{} + // PBS capacity is evaluated by CheckStorageWithCapacityTrend using + // storage defaults, aliases, overrides and hysteresis. The topology's + // fixed risk bands are resource context, not a second alert policy. + // Keep the condition observed so pre-upgrade duplicates retire as a + // policy change without claiming that datastore health recovered. + if strings.EqualFold(strings.TrimSpace(incident.Provider), "pulse") && + incident.Code == "capacity_runway_low" && + (resource.Type == unifiedresources.ResourceTypePBS || + (resource.Type == unifiedresources.ResourceTypeStorage && resource.Storage != nil && + resource.Storage.Platform == "pbs" && resource.Storage.Type == "pbs-datastore")) { + continue + } + if alertType, ok := unifiedAlertResourceType(resource); ok { if disableAllKubernetes && isUnifiedKubernetesAlertType(alertType) { continue diff --git a/internal/alerts/unified_incidents_test.go b/internal/alerts/unified_incidents_test.go index 8bcac0c40..37976b396 100644 --- a/internal/alerts/unified_incidents_test.go +++ b/internal/alerts/unified_incidents_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" + "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/operationaltrust" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" "github.com/rcourtman/pulse-go-rewrite/internal/truenas" @@ -576,7 +578,7 @@ func TestSyncUnifiedResourceIncidentsMarksPBSBackupPosture(t *testing.T) { Hostname: "pbs-main.local", DatastoreCount: 2, Datastores: []unifiedresources.PBSDatastoreMeta{ - {Name: "fast", Status: "online", Total: 100, Used: 96}, + {Name: "fast", Status: "ERROR", Total: 100, Used: 96}, {Name: "archive", Status: "online", Total: 100, Used: 40}, }, ProtectedWorkloadCount: 2, @@ -585,22 +587,22 @@ func TestSyncUnifiedResourceIncidentsMarksPBSBackupPosture(t *testing.T) { StorageRisk: &unifiedresources.StorageRisk{ Level: storagehealth.RiskCritical, Reasons: []unifiedresources.StorageRiskReason{ - {Code: "capacity_runway_low", Severity: storagehealth.RiskCritical, Summary: "PBS datastore fast is 96% full"}, + {Code: "pbs_datastore_state", Severity: storagehealth.RiskCritical, Summary: "PBS datastore fast is ERROR"}, }, }, }, Incidents: []unifiedresources.ResourceIncident{{ Provider: "pulse", - NativeID: "pbs-instance:pbs-main:capacity_runway_low", - Code: "capacity_runway_low", + NativeID: "pbs-instance:pbs-main:pbs_datastore_state", + Code: "pbs_datastore_state", Severity: storagehealth.RiskCritical, - Summary: "PBS datastore fast is 96% full", + Summary: "PBS datastore fast is ERROR", }}, } m.SyncUnifiedResourceIncidents([]unifiedresources.Resource{resource}) - alertID := "unified-incident-pbs-main-pulse-pbs-instance-pbs-main-capacity-runway-low-capacity-runway-low" + alertID := "unified-incident-pbs-main-pulse-pbs-instance-pbs-main-pbs-datastore-state-pbs-datastore-state" assertAlertPresent(t, m, alertID) m.mu.RLock() @@ -610,7 +612,7 @@ func TestSyncUnifiedResourceIncidentsMarksPBSBackupPosture(t *testing.T) { if alert.Type != "backup-posture-incident" { t.Fatalf("alert type = %q, want backup-posture-incident", alert.Type) } - wantMessage := "Backup server pbs-main has datastore capacity risk. Affects 1 backup datastore: fast" + wantMessage := "Backup server pbs-main has degraded datastore availability. Affects 1 backup datastore: fast" if alert.Message != wantMessage { t.Fatalf("message = %q, want %q", alert.Message, wantMessage) } @@ -734,17 +736,17 @@ func TestSyncUnifiedResourceIncidentsSuppressesPBSDatastoreChildWhenParentRollsU PBS: &unifiedresources.PBSData{ DatastoreCount: 1, Datastores: []unifiedresources.PBSDatastoreMeta{ - {Name: "fast", Status: "online", Total: 100, Used: 96}, + {Name: "fast", Status: "ERROR", Total: 100, Used: 96}, }, ProtectedWorkloadCount: 2, ProtectedWorkloadNames: []string{"media01", "app01"}, }, Incidents: []unifiedresources.ResourceIncident{{ Provider: "pulse", - NativeID: "pbs-instance:pbs-main:capacity_runway_low", - Code: "capacity_runway_low", + NativeID: "pbs-instance:pbs-main:pbs_datastore_state", + Code: "pbs_datastore_state", Severity: storagehealth.RiskCritical, - Summary: "PBS datastore fast is 96% full", + Summary: "PBS datastore fast is ERROR", }}, }, { @@ -761,10 +763,10 @@ func TestSyncUnifiedResourceIncidentsSuppressesPBSDatastoreChildWhenParentRollsU }, Incidents: []unifiedresources.ResourceIncident{{ Provider: "pulse", - NativeID: "pbs-instance:pbs-main:capacity_runway_low", - Code: "capacity_runway_low", + NativeID: "pbs-instance:pbs-main:pbs_datastore_state", + Code: "pbs_datastore_state", Severity: storagehealth.RiskCritical, - Summary: "PBS datastore fast is 96% full", + Summary: "PBS datastore fast is ERROR", }}, }, } @@ -1003,17 +1005,17 @@ func TestGetActiveAlertsPrioritizesBackupPostureExposure(t *testing.T) { PBS: &unifiedresources.PBSData{ DatastoreCount: 1, Datastores: []unifiedresources.PBSDatastoreMeta{ - {Name: "fast", Status: "online", Total: 100, Used: 96}, + {Name: "fast", Status: "ERROR", Total: 100, Used: 96}, }, ProtectedWorkloadCount: 2, ProtectedWorkloadNames: []string{"media01", "app01"}, }, Incidents: []unifiedresources.ResourceIncident{{ Provider: "pulse", - NativeID: "pbs-instance:pbs-main:capacity_runway_low", - Code: "capacity_runway_low", + NativeID: "pbs-instance:pbs-main:pbs_datastore_state", + Code: "pbs_datastore_state", Severity: storagehealth.RiskCritical, - Summary: "PBS datastore fast is 96% full", + Summary: "PBS datastore fast is ERROR", }}, }, { @@ -1332,3 +1334,92 @@ func TestTrueNASNativeCriticalTransition(t *testing.T) { }) } } + +// PBS capacity has one policy owner: CheckStorage. Topology risk remains +// visible, but must not create threshold-independent child/parent alerts. +func TestPBSCapacityUsesStoragePolicyNotTopologyBands(t *testing.T) { + m := newTestManager(t) + config := unifiedEvalBaseConfig() + config.StorageDefault = HysteresisThreshold{Trigger: 90, Clear: 85} + config.Overrides = map[string]ThresholdConfig{"pbs-main/fast": {Usage: &HysteresisThreshold{Trigger: 99, Clear: 98}}} + configureUnifiedEvalManager(t, m, config) + disableTestTimeThresholds(m) + instance := models.PBSInstance{ID: "pbs-main", Name: "main", Status: "online", LastSeen: time.Now(), Datastores: []models.PBSDatastore{{Name: "fast", Status: "online", Total: 1000, Used: 979, Free: 21, Usage: 97.9}}} + registry := unifiedresources.NewRegistry(unifiedresources.NewMemoryStore()) + registry.IngestSnapshot(models.StateSnapshot{PBSInstances: []models.PBSInstance{instance}}) + resources := registry.List() + capacityResources := 0 + for _, r := range resources { + for _, i := range r.Incidents { + if i.Code == "capacity_runway_low" { + capacityResources++ + break + } + } + } + if capacityResources != 2 { + t.Fatalf("want real parent and child capacity evidence, got %d", capacityResources) + } + storage := models.Storage{ID: "pbs-main-fast", AliasIDs: []string{"pbs-main/fast"}, Name: "fast", Instance: "pbs-main", Type: "pbs", Status: "online", Total: 1000, Used: 979, Free: 21, Usage: 97.9} + observe := func() { + for range 5 { + m.CheckStorage(storage) + m.SyncUnifiedResourceIncidents(resources) + } + } + observe() + if active := m.GetActiveAlerts(); len(active) != 0 { + t.Fatalf("99%% policy bypassed by topology incidents: %+v", active) + } + // Seed both pre-upgrade canonical alerts, with unchanged risk evidence. + // The next sync must retire them without deleting resource observations. + m.mu.Lock() + for _, spec := range alertspecs.BuildUnifiedResourceAlertSpecs(resources) { + if spec.Kind != alertspecs.AlertSpecKindProviderIncident { + continue + } + for _, resource := range resources { + if resource.ID != spec.ResourceID { + continue + } + incident, ok := incidentForProviderSpec(resource, spec) + if !ok || incident.Code != "capacity_runway_low" { + continue + } + alert := unifiedIncidentAlert(resource, incident, AlertLevelCritical, time.Now()) + applyCanonicalIdentity(alert, spec.ID, string(spec.Kind)) + m.setActiveAlertNoLock(canonicalTrackingKeyForSpec(spec, alert.ID), alert) + } + } + m.mu.Unlock() + if active := m.GetActiveAlerts(); len(active) != 2 { + t.Fatalf("want two pre-upgrade duplicates, got %d", len(active)) + } + observe() + if active := m.GetActiveAlerts(); len(active) != 0 { + t.Fatalf("pre-upgrade duplicates retained: %+v", active) + } + config.Overrides["pbs-main/fast"] = ThresholdConfig{Usage: &HysteresisThreshold{Trigger: 90, Clear: 85}} + m.UpdateConfig(config) + disableTestTimeThresholds(m) + observe() + active := m.GetActiveAlerts() + if len(active) != 1 || active[0].ResourceID != storage.ID || active[0].CanonicalKind != "metric-threshold" { + t.Fatalf("want one policy-owned capacity alert, got %+v", active) + } + config.Overrides["pbs-main/fast"] = ThresholdConfig{Usage: &HysteresisThreshold{Trigger: 99, Clear: 98}} + m.UpdateConfig(config) + disableTestTimeThresholds(m) + observe() + if active := m.GetActiveAlerts(); len(active) != 0 { + t.Fatalf("raising policy did not clear capacity: %+v", active) + } + // The same nearly-full datastore failing is still actionable. + instance.Datastores[0].Status = "ERROR" + registry.IngestSnapshot(models.StateSnapshot{PBSInstances: []models.PBSInstance{instance}}) + resources = registry.List() + observe() + if active := m.GetActiveAlerts(); len(active) == 0 { + t.Fatal("capacity policy hid datastore failure") + } +} diff --git a/internal/monitoring/monitor_pbs_coverage_test.go b/internal/monitoring/monitor_pbs_coverage_test.go index 34db989b4..a18a4ec6f 100644 --- a/internal/monitoring/monitor_pbs_coverage_test.go +++ b/internal/monitoring/monitor_pbs_coverage_test.go @@ -697,12 +697,33 @@ func TestPBSPolledCapacityRequiresObservedRecovery(t *testing.T) { t.Fatalf("incorrect recovery: %+v", resolved) } // Alternate PBS counter names must feed the same policy and identity. - // Stay below the separate 90% backup-posture incident threshold; the - // configured minimum delta of one permits this immediate recurrence. + // The configured minimum delta of one permits this immediate recurrence. response.Store(`{"data":{"total-space":1000,"used-space":860,"avail-space":140}}`) poll() active = manager.GetActiveAlerts() if len(active) != 1 || active[0].ID != original.ID || active[0].Value != 86 || !active[0].StartTime.After(original.StartTime) { t.Fatalf("incorrect recurrent incident: %+v", active) } + // The reported 97.9% crosses both topology bands. Neither the datastore + // nor parent posture may bypass the UI's 99% capacity policy. + response.Store(`{"data":{"total":1000,"used":979,"avail":21}}`) + highPolicy := basePolicy + highPolicy.Overrides = map[string]alerts.ThresholdConfig{"pbs-pbs-capacity/backups": { + Usage: &alerts.HysteresisThreshold{Trigger: 99, Clear: 98}, + }} + manager.UpdateConfig(highPolicy) + poll() + if active := manager.GetActiveAlerts(); len(active) != 0 { + t.Fatalf("97.9%% poll bypassed 99%% policy with topology incidents: %+v", active) + } + manager.UpdateConfig(basePolicy) + poll() + if active := manager.GetActiveAlerts(); len(active) != 1 || active[0].Type != "usage" { + t.Fatalf("high usage must have one policy-owned alert: %+v", active) + } + manager.UpdateConfig(highPolicy) + poll() + if active := manager.GetActiveAlerts(); len(active) != 0 { + t.Fatalf("raised policy did not clear high-usage alert: %+v", active) + } }