From 50405af74397df5c217e8e83f17e33d73121da83 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 08:01:27 +0100 Subject: [PATCH] fix(alerts): skip backup-age alerts for PBS host backups (#2136) A PBS host/config backup (backup-type host) is keyed by the node name, not a guest VMID, so CheckBackupsWithInventory raised a guest backup-age alert under a node-named subject (for example 'nas backup via proxmox-pbs') that never advanced while the node's guests were backed up, notifying repeatedly. Skip backup-age alerts for PBS subjects with no guest VMID: they are host/config backups, not guest workloads, and the Backups overview already keeps them out of guest coverage. PVE host backups already produce no subject and no alert, restoring consistency. Genuine orphaned PBS guest backups (numeric VMID) and PMG node backups still alert. The recovery point and rollup are retained, so inventory and artifact display are unchanged; only the spurious alert is cleared. Includes the alerts subsystem contract record and the regression tests. Change-source: pulse-maintainer --- .../v6/internal/subsystems/alerts.md | 12 +++ internal/alerts/alerts_test.go | 90 +++++++++++++++++++ internal/alerts/backup_snapshot.go | 10 +++ 3 files changed, 112 insertions(+) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 4f53856f97..8c9ef7805c 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1620,6 +1620,18 @@ of a reporter installation or a migration that guesses operator intent. `TestMergeSnapshotOverrideInheritsZeroFields` in `internal/alerts/alerts_test.go` pin the merge, migration, and preservation behaviors. +Backup-age alerts are guest-workload scoped. A PBS recovery rollup whose subject +carries no guest VMID is a host/config backup (`backup-type host`, keyed by the +node name), not a guest workload, and must not raise a guest backup-age alert: +the Backups overview already excludes those entries from guest coverage, and a +stale host config backup otherwise notified forever under a node-named subject +while the node's guests were backed up. Evaluation skips PBS subjects with an +empty VMID, retaining their recovery point and rollup so inventory and artifact +display are unchanged. PVE host backups already produce no subject and no alert. +Genuinely orphaned PBS guest backups (numeric VMID) and PMG node backups still +alert. `TestCheckBackupsIgnoresPBSHostBackupSubject` and +`TestCheckBackupsStillAlertsOnOrphanedGuestPBSBackup` in +`internal/alerts/alerts_test.go` pin both sides of this boundary (#2136). Proxmox disk health alert evaluation now lives in `internal/alerts/disk_health.go`. That file owns Proxmox disk canonical identity, disk health assessment alerts, known-firmware health suppression, and diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 0cca818bb0..48c8f041bc 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -2322,6 +2322,96 @@ func TestCheckBackupsHandlesPmgBackups(t *testing.T) { } } +// TestCheckBackupsIgnoresPBSHostBackupSubject covers #2136: a PBS host config +// backup (backup-type "host") is keyed by the node name, not a VMID, and must +// not raise a guest backup-age alert. The node name subject ("nas backup") +// otherwise looks like a stale guest and notifies forever even though the +// node's guests are backed up. +func TestCheckBackupsIgnoresPBSHostBackupSubject(t *testing.T) { + m := newTestManager(t) + m.ClearActiveAlerts() + + m.mu.Lock() + m.config.Enabled = true + m.config.BackupDefaults = BackupAlertConfig{ + Enabled: true, + WarningDays: 7, + CriticalDays: 14, + } + m.mu.Unlock() + + now := time.Now() + rollups := []recovery.ProtectionRollup{ + { + RollupID: "ext:pbs-host-nas", + SubjectRef: &recovery.ExternalRef{ + Type: "proxmox-guest", + Namespace: "pbs", + Name: "nas", + ID: "nas", + }, + LastSuccessAt: ptrTime(now.Add(-43.5 * 24 * time.Hour)), + LastOutcome: recovery.OutcomeSuccess, + Providers: []recovery.Provider{recovery.ProviderProxmoxPBS}, + }, + } + + m.CheckBackups(rollups, map[string]GuestLookup{}, map[string][]GuestLookup{}) + + m.mu.RLock() + defer m.mu.RUnlock() + for storageKey, alert := range m.activeAlerts { + if alert != nil && alert.Type == "backup-age" { + t.Fatalf("expected PBS host config backup to be excluded from guest backup-age alerts, found %s", effectiveAlertID(alert, storageKey)) + } + } +} + +// TestCheckBackupsStillAlertsOnOrphanedGuestPBSBackup is the companion guard: +// a genuinely orphaned PBS guest backup (numeric VMID, no live guest) must keep +// alerting, so the host-backup exclusion above does not silence real staleness. +func TestCheckBackupsStillAlertsOnOrphanedGuestPBSBackup(t *testing.T) { + m := newTestManager(t) + m.ClearActiveAlerts() + + m.mu.Lock() + m.config.Enabled = true + m.config.BackupDefaults = BackupAlertConfig{ + Enabled: true, + WarningDays: 7, + CriticalDays: 14, + } + m.mu.Unlock() + + now := time.Now() + rollups := []recovery.ProtectionRollup{ + { + RollupID: "ext:pbs-orphan-102", + SubjectRef: &recovery.ExternalRef{ + Type: "proxmox-vm", + Namespace: "pbs", + Name: "102", + ID: "102", + }, + LastSuccessAt: ptrTime(now.Add(-43.5 * 24 * time.Hour)), + LastOutcome: recovery.OutcomeSuccess, + Providers: []recovery.Provider{recovery.ProviderProxmoxPBS}, + }, + } + + m.CheckBackups(rollups, map[string]GuestLookup{}, map[string][]GuestLookup{}) + + m.mu.RLock() + defer m.mu.RUnlock() + if !testHasActiveAlert(t, m, buildCanonicalStateID("backup-subject:ext-pbs-orphan-102", "backup-subject:ext-pbs-orphan-102-backup-age")) { + var keys []string + for storageKey, active := range m.activeAlerts { + keys = append(keys, effectiveAlertID(active, storageKey)) + } + t.Fatalf("expected orphaned PBS guest backup alert, found keys: %v", keys) + } +} + func TestCheckBackupsSkipsOrphanedWhenDisabled(t *testing.T) { m := newTestManager(t) m.ClearActiveAlerts() diff --git a/internal/alerts/backup_snapshot.go b/internal/alerts/backup_snapshot.go index 3fcd264df7..c43c9f5714 100644 --- a/internal/alerts/backup_snapshot.go +++ b/internal/alerts/backup_snapshot.go @@ -859,6 +859,16 @@ func (m *Manager) CheckBackupsWithInventory( } ageDaysRounded := math.Round(ageDays*10) / 10 + // A PBS subject without a guest VMID is a host/config backup + // (PBS backup-type "host", keyed by the node name), not a guest + // workload. The Backups overview already keeps these out of guest + // coverage, so do not raise a guest backup-age alert for them: a + // stale host config backup otherwise notifies forever under a + // node-named subject while the node's guests are backed up (#2136). + if record.source == "PBS" && strings.TrimSpace(record.vmID) == "" { + continue + } + // Determine thresholds for this backup currentBackupCfg := backupCfg guestContext := guestSnapshotFromLookup(record.lookup, record.fallbackName)