diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 9d7946179e..d7ad6e1cce 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1867,6 +1867,23 @@ well as its alerts. `internal/alerts/host_unraid_lifecycle_test.go` pins the clear, the holds, the restarted count and the pending-run and count cleanup. +### Proxmox node cleanup keeps platform metric alerts + +`CleanupAlertsForNodes` runs on every Proxmox node poll (and every mock alert +tick) and removes alerts whose `Node` is not a current Proxmox node or PBS +instance. Kubernetes, TrueNAS and vSphere metric alerts carry a cluster or +platform host in `Node` and a display label such as `TrueNAS Disk` or +`Kubernetes Pod` in `metadata.resourceType`, which the preserve list's +platform words never matched. The cleanup used to delete them silently on +each poll while their reducer incident stayed firing, so the next evaluation +re-created each one and handed it to the alert callback again as a new +alert. `shouldPreserveAlertOutsideNodeCleanup` now keeps any alert carrying +`metadata.platformType`, or whose label maps through +`CanonicalAlertResourceType` to a Kubernetes, TrueNAS or vSphere type. +`TestCleanupAlertsForNodesKeepsPlatformMetricAlerts` in +`internal/alerts/alerts_test.go` pins both signals, and its +sibling pins that a guest alert on a removed Proxmox node is still removed. + ### Configured flapping thresholds remain reachable Every accepted positive `FlappingThreshold`, including values above ten, must diff --git a/internal/alerts/active_cleanup.go b/internal/alerts/active_cleanup.go index 4da24fa05d..2fdfcd34fc 100644 --- a/internal/alerts/active_cleanup.go +++ b/internal/alerts/active_cleanup.go @@ -4,6 +4,7 @@ import ( "strings" "time" + alertconfig "github.com/rcourtman/pulse-go-rewrite/internal/alerts/config" alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rs/zerolog/log" @@ -264,6 +265,15 @@ func shouldPreserveAlertOutsideNodeCleanup(alertID string, alert *Alert) bool { if alert.CanonicalKind == string(alertspecs.AlertSpecKindProviderIncident) { return true } + // Kubernetes, TrueNAS and vSphere metric alerts carry a display label such + // as "TrueNAS Disk" or "Kubernetes Pod" in resourceType, which the platform + // words below never matched, and a cluster or host in Node rather than a + // Proxmox node. Every Proxmox poll used to delete them silently, and the + // next evaluation raised them again as new alerts. + if metadataStringValue(alert.Metadata, alertPlatformTypeKey) != "" || + isUnifiedModernPlatformAlertType(alertconfig.CanonicalAlertResourceType(alertMetadataResourceType(alert))) { + return true + } switch alertMetadataResourceType(alert) { case string(unifiedresources.ResourceTypeAgent), diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index f4c4463c5e..f0ad060e0f 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -12628,6 +12628,82 @@ func TestCleanupAlertsForNodes(t *testing.T) { }) } +// TestCleanupAlertsForNodesKeepsPlatformMetricAlerts pins that the Proxmox +// node cleanup, which runs on every Proxmox poll, leaves Kubernetes, TrueNAS +// and vSphere metric alerts alone. Their Node is a cluster or platform host, +// never a Proxmox node, and their resourceType is a display label. +func TestCleanupAlertsForNodesKeepsPlatformMetricAlerts(t *testing.T) { + m := newTestManager(t) + configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig()) + + inputs := []*UnifiedResourceInput{ + {ID: "physical-disk:truenas-main/sda", Type: "truenas-disk", Name: "sda", Node: "truenas-main", Instance: "TrueNAS", Temperature: &UnifiedResourceMetric{Value: 80, Percent: 80}}, + {ID: "storage:truenas-main/pool:tank", Type: "truenas-pool", Name: "tank", Node: "truenas-main", Instance: "TrueNAS", Disk: &UnifiedResourceMetric{Percent: 95}}, + {ID: "k8s:prod:pod:api", Type: "pod", Name: "api", Node: "k8s-node-1", Instance: "prod", CPU: &UnifiedResourceMetric{Percent: 99}}, + {ID: "vmware:vc:vm:app-01", Type: "vmware-vm", Name: "app-01", Node: "esxi-01", Instance: "vc", CPU: &UnifiedResourceMetric{Percent: 99}}, + } + for _, input := range inputs { + m.CheckUnifiedResource(input) + } + platformAlerts := alertKeys(m) + if len(platformAlerts) != len(inputs) { + t.Fatalf("expected one alert per platform resource, got %v", platformAlerts) + } + + m.CleanupAlertsForNodes(map[string]bool{"pve1": true}) + + for _, alertID := range platformAlerts { + if _, exists := testLookupActiveAlert(t, m, alertID); !exists { + t.Fatalf("Proxmox node cleanup removed platform alert %q; remaining %v", alertID, alertKeys(m)) + } + } + + // The platform id alone keeps an alert whose label is not recognised. + m.mu.Lock() + labels := make(map[*Alert]interface{}, len(m.activeAlerts)) + for _, alert := range m.activeAlerts { + labels[alert] = alert.Metadata["resourceType"] + alert.Metadata["resourceType"] = "Unrecognised" + } + m.mu.Unlock() + m.CleanupAlertsForNodes(map[string]bool{"pve1": true}) + if got := alertKeys(m); len(got) != len(platformAlerts) { + t.Fatalf("platform alerts removed despite their platformType: %v", got) + } + m.mu.Lock() + for alert, label := range labels { + alert.Metadata["resourceType"] = label + } + m.mu.Unlock() + + // A legacy-shaped alert whose metadata predates platformType is still kept + // by its display label. + m.mu.Lock() + for _, alert := range m.activeAlerts { + delete(alert.Metadata, alertPlatformTypeKey) + } + m.mu.Unlock() + m.CleanupAlertsForNodes(map[string]bool{"pve1": true}) + if got := alertKeys(m); len(got) != len(platformAlerts) { + t.Fatalf("display-label platform alerts removed without platformType: %v", got) + } +} + +// The cleanup still removes a Proxmox guest alert whose node left the estate. +func TestCleanupAlertsForNodesStillRemovesMissingProxmoxNodeAlerts(t *testing.T) { + m := newTestManager(t) + configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig()) + + m.CheckUnifiedResource(&UnifiedResourceInput{ID: "pve-gone:node-gone:101", Type: "vm", Name: "vm-101", Node: "node-gone", Instance: "pve-gone", CPU: &UnifiedResourceMetric{Percent: 99}}) + if len(alertKeys(m)) != 1 { + t.Fatalf("expected the guest CPU alert, got %v", alertKeys(m)) + } + m.CleanupAlertsForNodes(map[string]bool{"pve1": true}) + if got := alertKeys(m); len(got) != 0 { + t.Fatalf("guest alert on a removed node survived cleanup: %v", got) + } +} + func TestCheckZFSPoolHealth(t *testing.T) { // t.Parallel() poolStateAlertID := buildCanonicalStateID("local-zfs/zfs-pool:rpool", "local-zfs/zfs-pool:rpool-state")