From 7a8376c055a4ccd8da954a39cac631f3b1066427 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 14:54:59 +0100 Subject: [PATCH] Keep platform metric alerts through Proxmox node cleanup CleanupAlertsForNodes runs on every Proxmox node poll 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 as their node and a display label such as "TrueNAS Disk" in metadata.resourceType, which the preserve list's platform words never matched. So in an estate with Proxmox and one of those platforms, every poll deleted these alerts silently while their reducer incident stayed firing, and the next evaluation re-created each one and handed it to the alert callback again as a new alert. Keep alerts that carry metadata.platformType, or whose label maps to a Kubernetes, TrueNAS or vSphere type. A guest alert on a removed Proxmox node is still removed. --- .../v6/internal/subsystems/alerts.md | 17 +++++ internal/alerts/active_cleanup.go | 10 +++ internal/alerts/alerts_test.go | 76 +++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 2690c5f5ea..3509cfddf1 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1842,6 +1842,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")