mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-11 14:00:29 +00:00
fix(alerts): disambiguate PBS backups using namespace for multi-PVE setups (#1095)
When multiple PVE instances have VMs with overlapping VMIDs, PBS backups were being matched to the wrong VM because the code would just use the first matching guest. Now when a PBS backup has a namespace, it attempts to match that namespace to the PVE instance name to find the correct VM. This helps users who have separate PBS instances backing up different PVE clusters with namespaces like "pve1", "nat", etc.
This commit is contained in:
rcourtman
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user