diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index 9aabead7d..c8c8d778c 100644 --- a/internal/alerts/alerts.go +++ b/internal/alerts/alerts.go @@ -5157,9 +5157,28 @@ func (m *Manager) CheckBackups( var node string if exists && len(guests) > 0 { - // If we have exactly one match, use it - // If we have multiple matches, use the first one (we can't disambiguate without PVE origin metadata) - info = guests[0] + // If we have exactly one match, use it directly + // If we have multiple matches, try to disambiguate using the PBS namespace + if len(guests) == 1 { + info = guests[0] + } else if backup.Namespace != "" { + // Try to match namespace to instance name + matched := false + for _, g := range guests { + if namespaceMatchesInstance(backup.Namespace, g.Instance) { + info = g + matched = true + break + } + } + if !matched { + // No namespace match found, fall back to first guest + info = guests[0] + } + } else { + // No namespace available, fall back to first guest + info = guests[0] + } if info.Instance != "" && info.Node != "" { key = BuildGuestKey(info.Instance, info.Node, info.VMID) displayName = info.Name @@ -6179,6 +6198,46 @@ func abs(x float64) float64 { return x } +// namespaceMatchesInstance checks if a PBS namespace likely corresponds to a PVE instance. +// This helps disambiguate backups when multiple PVE instances have VMs with the same VMID. +// Examples: namespace "pve1" matches instance "pve1", namespace "nat" matches instance "pve-nat" +func namespaceMatchesInstance(namespace, instance string) bool { + if namespace == "" || instance == "" { + return false + } + + // Normalize both strings: lowercase and keep only alphanumeric + normalize := func(s string) string { + var b strings.Builder + for _, r := range strings.ToLower(s) { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + b.WriteRune(r) + } + } + return b.String() + } + + ns := normalize(namespace) + inst := normalize(instance) + + if ns == "" || inst == "" { + return false + } + + // Exact match after normalization + if ns == inst { + return true + } + + // Check if namespace is contained in instance or vice versa + // e.g., namespace "nat" matches instance "pvenat" (normalized from "pve-nat") + if strings.Contains(inst, ns) || strings.Contains(ns, inst) { + return true + } + + return false +} + // AcknowledgeAlert acknowledges an alert func (m *Manager) AcknowledgeAlert(alertID, user string) error { m.mu.Lock() diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index ca262867a..1765c8261 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -1093,6 +1093,91 @@ func TestCheckBackupsHandlesPbsOnlyGuests(t *testing.T) { } } +func TestCheckBackupsDisambiguatesWithNamespace(t *testing.T) { + // Test that when multiple guests have the same VMID from different instances, + // the namespace is used to match the backup to the correct guest. + // This addresses issue #1095 where users have multiple PVE instances with + // overlapping VMIDs and separate PBS instances backing them up. + 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() + + // Two guests with the same VMID (100) but on different instances + guestsByKey := map[string]GuestLookup{ + "pve-node1-100": { + ResourceID: "qemu/100", + Name: "webserver-pve", + Instance: "pve", + Node: "node1", + Type: "qemu", + VMID: 100, + }, + "pve-nat-node2-100": { + ResourceID: "qemu/100", + Name: "webserver-nat", + Instance: "pve-nat", + Node: "node2", + Type: "qemu", + VMID: 100, + }, + } + + // Both guests have VMID "100" + guestsByVMID := map[string][]GuestLookup{ + "100": { + guestsByKey["pve-node1-100"], + guestsByKey["pve-nat-node2-100"], + }, + } + + // PBS backup with namespace "nat" should match the "pve-nat" instance + pbsBackups := []models.PBSBackup{ + { + ID: "pbs-backup-100-nat", + Instance: "pbs-main", + Datastore: "backup-store", + Namespace: "nat", // This namespace should match "pve-nat" + BackupType: "qemu", + VMID: "100", + BackupTime: now.Add(-6 * 24 * time.Hour), // Critical + }, + } + + m.CheckBackups(nil, pbsBackups, nil, guestsByKey, guestsByVMID) + + m.mu.RLock() + defer m.mu.RUnlock() + + // Should find an alert keyed to the pve-nat instance (node2), not pve (node1) + expectedKey := "backup-age-pve-nat-node2-100" + alert, exists := m.activeAlerts[expectedKey] + if !exists { + // List what keys we do have for debugging + var keys []string + for k := range m.activeAlerts { + keys = append(keys, k) + } + t.Fatalf("expected alert with key %q not found; found keys: %v", expectedKey, keys) + } + + if alert.ResourceName != "webserver-nat backup" { + t.Errorf("expected ResourceName 'webserver-nat backup', got %q", alert.ResourceName) + } + if alert.Instance != "pve-nat" { + t.Errorf("expected Instance 'pve-nat', got %q", alert.Instance) + } +} + func TestCheckBackupsHandlesPmgBackups(t *testing.T) { m := newTestManager(t) m.ClearActiveAlerts() @@ -15482,3 +15567,58 @@ func TestLoadActiveAlerts(t *testing.T) { } }) } + +func TestNamespaceMatchesInstance(t *testing.T) { + tests := []struct { + name string + namespace string + instance string + expected bool + }{ + // Exact matches + {"exact match", "pve", "pve", true}, + {"exact match with numbers", "pve1", "pve1", true}, + + // Partial matches (namespace in instance) + {"namespace contained in instance", "nat", "pve-nat", true}, + {"namespace contained in instance no dash", "nat", "pvenat", true}, + {"longer namespace in instance", "production", "my-production-server", true}, + + // Partial matches (instance in namespace) + {"instance contained in namespace", "pve-backups", "pve", true}, + + // Case insensitive + {"case insensitive exact", "PVE", "pve", true}, + {"case insensitive partial", "NAT", "pve-nat", true}, + + // Special characters ignored + {"special chars in namespace", "pve_nat", "pvenat", true}, + {"special chars in instance", "pvenat", "pve-nat", true}, + {"both have special chars", "pve-1", "pve_1", true}, + + // No matches + {"no match", "production", "staging", false}, + {"no match different names", "pve1", "pve2", false}, + {"no match partial mismatch", "abc", "xyz", false}, + + // Empty values + {"empty namespace", "", "pve", false}, + {"empty instance", "pve", "", false}, + {"both empty", "", "", false}, + + // Real-world scenarios from issue #1095 + {"pve namespace with pve instance", "pve", "pve", true}, + {"nat namespace with pve-nat instance", "nat", "pve-nat", true}, + {"pve1 namespace with pve1 instance", "pve1", "pve1", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := namespaceMatchesInstance(tt.namespace, tt.instance) + if result != tt.expected { + t.Errorf("namespaceMatchesInstance(%q, %q) = %v, want %v", + tt.namespace, tt.instance, result, tt.expected) + } + }) + } +}