Fix stale backup alert attribution

Change-source: pulse-maintainer

docs/release-control/v6/internal/subsystems/alerts.md

@@ -894,6 +894,19 @@ evaluation, backup rollup age evaluation, backup inventory readiness, PVE
 template subject matching, namespace disambiguation, and snapshot/backup active
 alert cleanup; future backup or snapshot alert behavior should extend that
 owner rather than expanding the central Manager file.
+
+### Backup-age attribution follows live identity
+
+Backup rollup subject refs are historical hints, not unconditional proof of a
+guest's current placement. An exact `instance:node:vmid` ref is authoritative
+only while it resolves to a live guest of the same type. Otherwise backup-age
+evaluation retries typed VMID attribution and prefers live candidates carrying
+a resource ID over last-known display metadata. A PVE-owned ref can still name
+its authoritative orphan after the PVE inventory is ready; a stale PBS ref that
+cannot be resolved uniquely remains unattributed instead of pinning an alert to
+an old or wrong PVE node. `TestCheckBackupsRemapsStaleSubjectRefToUniqueLiveGuest`
+and the PVE orphan/inventory-readiness tests in `internal/alerts/alerts_test.go`
+pin both sides of this boundary.
 Per-guest backup and snapshot overrides are sparse, not frozen copies. A
 zero-valued threshold field in an override (`warningDays`, `criticalDays`,
 `freshHours`, `staleHours`, and the snapshot size pair) inherits the current

internal/alerts/alerts_test.go

@@ -1764,6 +1764,79 @@ func TestCheckBackupsDisambiguatesWithNamespace(t *testing.T) {
 	}
 }
 
+func TestCheckBackupsRemapsStaleSubjectRefToUniqueLiveGuest(t *testing.T) {
+	m := newTestManager(t)
+	m.ClearActiveAlerts()
+
+	m.mu.Lock()
+	m.config.Enabled = true
+	m.config.BackupDefaults = BackupAlertConfig{
+		Enabled:      true,
+		WarningDays:  3,
+		CriticalDays: 5,
+	}
+	m.mu.Unlock()
+
+	now := time.Now()
+	current := GuestLookup{
+		ResourceID: "pve-current:node-a:100",
+		Name:       "web-current",
+		Instance:   "pve-current",
+		Node:       "node-a",
+		Type:       "qemu",
+		VMID:       100,
+	}
+	staleMetadata := GuestLookup{
+		Name:     "web-old",
+		Instance: "pve-retired",
+		Node:     "node-b",
+		Type:     "qemu",
+		VMID:     100,
+	}
+
+	rollups := []recovery.ProtectionRollup{
+		{
+			RollupID: "res:vm-web",
+			SubjectRef: &recovery.ExternalRef{
+				Type:      "proxmox-vm",
+				Namespace: "pve-retired",
+				Name:      "web-old",
+				ID:        "pve-retired:node-b:100",
+				Class:     "node-b",
+			},
+			LastSuccessAt: ptrTime(now.Add(-6 * 24 * time.Hour)),
+			LastOutcome:   recovery.OutcomeSuccess,
+			Providers:     []recovery.Provider{recovery.ProviderProxmoxPBS},
+		},
+	}
+	guestsByKey := map[string]GuestLookup{
+		BuildGuestKey(current.Instance, current.Node, current.VMID): current,
+	}
+	guestsByVMID := map[string][]GuestLookup{
+		"100": {current, staleMetadata},
+	}
+
+	m.CheckBackups(rollups, guestsByKey, guestsByVMID)
+
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+	expectedKey := "backup-age-" + sanitizeAlertKey(BuildGuestKey(current.Instance, current.Node, current.VMID))
+	alert, exists := testLookupActiveAlert(t, m, expectedKey)
+	if !exists {
+		var keys []string
+		for storageKey, active := range m.activeAlerts {
+			keys = append(keys, effectiveAlertID(active, storageKey))
+		}
+		t.Fatalf("expected alert remapped to current guest %q, found %v", expectedKey, keys)
+	}
+	if alert.Instance != current.Instance || alert.Node != current.Node {
+		t.Fatalf("alert location = %q/%q, want current guest %q/%q", alert.Instance, alert.Node, current.Instance, current.Node)
+	}
+	if alert.ResourceName != current.Name+" backup" {
+		t.Fatalf("alert resource name = %q, want %q", alert.ResourceName, current.Name+" backup")
+	}
+}
+
 // TestCheckBackupsVMIDCollisionNonMatchingNamespace verifies that when multiple guests
 // share a VMID and the PBS backup namespace matches none of them, the alert uses the
 // generic PBS key rather than falsely attributing to a specific guest.

internal/alerts/backup_snapshot.go

@@ -257,6 +257,23 @@ func filterGuestsByBackupType(guests []GuestLookup, backupType string) []GuestLo
 	return filtered
 }
 
+// preferLiveGuestCandidates prevents last-known display metadata from
+// competing with current inventory during backup attribution. Persisted
+// entries intentionally have no ResourceID; they remain useful when a backup
+// is genuinely orphaned, but a live typed guest is stronger identity evidence.
+func preferLiveGuestCandidates(guests []GuestLookup) []GuestLookup {
+	live := make([]GuestLookup, 0, len(guests))
+	for _, guest := range guests {
+		if strings.TrimSpace(guest.ResourceID) != "" {
+			live = append(live, guest)
+		}
+	}
+	if len(live) > 0 {
+		return live
+	}
+	return guests
+}
+
 func backupOrphanInventoryReady(scope *BackupInventoryScope, record backupRecord) bool {
 	if scope == nil || scope.PVEOrphanInventoryReady == nil {
 		return true
@@ -691,6 +708,8 @@ func (m *Manager) CheckBackupsWithInventory(
 			node        string
 			vmID        string
 			subjectType string
+			refInstance string
+			refNode     string
 		)
 
 		ref := rollup.SubjectRef
@@ -701,22 +720,28 @@ func (m *Manager) CheckBackupsWithInventory(
 		// Primary: subjectRef.ID is the canonical proxmox guest source ID (instance:node:vmid) when linked.
 		if ref != nil && strings.TrimSpace(ref.ID) != "" {
 			if inst, nd, vmid, ok := parseGuestID(ref.ID); ok {
-				key = BuildGuestKey(inst, nd, vmid)
-				info = guestsByKey[key]
+				refInstance = inst
+				refNode = nd
+				candidateKey := BuildGuestKey(inst, nd, vmid)
+				candidate, live := guestsByKey[candidateKey]
 				// A vm and a container can share instance:node:vmid; never
 				// attribute the backup to a guest of the wrong kind.
-				if !guestMatchesBackupType(info, subjectType) {
-					info = GuestLookup{}
+				if live && guestMatchesBackupType(candidate, subjectType) {
+					key = candidateKey
+					info = candidate
+					instance = inst
+					node = nd
 				}
-				instance = inst
-				node = nd
 				vmID = strconv.Itoa(vmid)
 			}
 		}
 
 		// Secondary: attempt to map by VMID for orphaned/ambiguous backups.
 		if key == "" && ref != nil {
-			vmidStr := strings.TrimSpace(ref.ID)
+			vmidStr := strings.TrimSpace(vmID)
+			if vmidStr == "" {
+				vmidStr = strings.TrimSpace(ref.ID)
+			}
 			if vmidStr == "" {
 				vmidStr = strings.TrimSpace(ref.Name)
 			}
@@ -725,7 +750,7 @@ func (m *Manager) CheckBackupsWithInventory(
 					vmID = vmidStr
 					// Drop same-VMID guests of the wrong kind so a vm backup is
 					// not matched to an lxc container (or vice-versa).
-					guests := filterGuestsByBackupType(guestsByVMID[vmidStr], subjectType)
+					guests := preferLiveGuestCandidates(filterGuestsByBackupType(guestsByVMID[vmidStr], subjectType))
 					if len(guests) == 1 {
 						info = guests[0]
 					} else if len(guests) > 1 && strings.TrimSpace(ref.Namespace) != "" {
@@ -767,6 +792,20 @@ func (m *Manager) CheckBackupsWithInventory(
 			}
 		}
 
+		// A PVE rollup's source ref belongs to the authoritative PVE
+		// enumeration, so retain its location for a genuine orphan after the
+		// current-inventory fallback fails. PBS historical links are not given
+		// that authority: they can outlive or predate the guest's current PVE
+		// placement and must remain unattributed when the live candidates are
+		// ambiguous.
+		if key == "" && source == "PVE" && refInstance != "" && refNode != "" && vmID != "" {
+			if vmid, err := strconv.Atoi(vmID); err == nil && vmid > 0 {
+				key = BuildGuestKey(refInstance, refNode, vmid)
+				instance = refInstance
+				node = refNode
+			}
+		}
+
 		if key == "" {
 			// Stable fallback for non-guest subjects and orphans.
 			key = strings.TrimSpace(rollup.RollupID)