Keep platform metric alerts through Proxmox node cleanup (#2672)

`CleanupAlertsForNodes` runs on every Proxmox node poll
(`monitor_pve_storage.go`) and on every mock alert tick. It removes
alerts whose `Node` is not a current Proxmox node or PBS instance, and
keeps the types `shouldPreserveAlertOutsideNodeCleanup` lists.

Kubernetes, TrueNAS and vSphere metric alerts carry a cluster or
platform host as `Node`, and a display label (`TrueNAS Disk`,
`Kubernetes Pod`, `vSphere VM`) in `metadata.resourceType`. The preserve
list only had the bare words `truenas`, `kubernetes`, `k8s` and
`vmware`, so none of these labels matched. In an estate with Proxmox
plus one of those platforms:

- every Proxmox poll deleted these alerts without resolving them;
- their reducer incident stayed firing, so the next unified evaluation
re-created each one and handed it to the alert callback again as a new
alert.

## Change

`shouldPreserveAlertOutsideNodeCleanup` keeps an alert that carries
`metadata.platformType` (set on every unified Kubernetes/TrueNAS/vSphere
metric alert), or whose label maps through `CanonicalAlertResourceType`
to one of those platform types. The label path covers alerts persisted
before `platformType` existed. A guest alert whose Proxmox node left the
estate is still removed.

## Evidence

- Unit probe on main: a TrueNAS disk temperature alert, a Kubernetes pod
CPU alert and a vSphere VM CPU alert were all deleted by
`CleanupAlertsForNodes({"pve1": true})`. Over three
cleanup-then-evaluate cycles the TrueNAS alert went to the alert
callback 3 times; with this change, 0 times and it stays active.
- Private mock stack: TrueNAS disk temperature alerts kept vanishing
between mock alert ticks, while the disks kept their readings in
`/api/resources`. With this change they stay open.

## Tests

`TestCleanupAlertsForNodesKeepsPlatformMetricAlerts` (in
`internal/alerts/alerts_test.go`) raises a TrueNAS disk, a TrueNAS pool,
a Kubernetes pod and a vSphere VM alert, then runs the cleanup three
ways: as raised, with only `platformType` (label unrecognised), and with
only the label (`platformType` removed).
`TestCleanupAlertsForNodesStillRemovesMissingProxmoxNodeAlerts` keeps
the removal of a guest alert on a removed Proxmox node. Each preserve
condition was mutation-checked: removing either one fails the first
test.
This commit is contained in:
rcourtman
2026-10-07 16:56:25 +01:00
committed by GitHub
3 changed files with 103 additions and 0 deletions
@@ -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
+10
View File
@@ -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),
+76
View File
@@ -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")