Improve backup-age alerts to show VM/CT names in multi-cluster setups (related to #668)
This change fixes backup-age alert notifications to display VM/CT names instead of just "VMID XXX" in multi-cluster environments where backups are stored on PBS. Changes: - Store all guests per VMID (not just first match) to handle VMID collisions across clusters - Persist last-known guest names/types in metadata store for deleted VMs - Enrich backup correlation with persisted metadata when live inventory is empty - Update CheckBackups to handle multiple VMID matches intelligently The fix addresses two scenarios: 1. Multiple PVE clusters with same VMID backing up to one PBS 2. VMs deleted from Proxmox but backups still exist on PBS Backup-age alerts will now show proper VM/CT names when: - A unique guest exists with that VMID (live or persisted) - Multiple guests share a VMID (uses first match, consistent with current behavior) When truly ambiguous (multiple live VMs, same VMID, no way to determine origin), the alert gracefully falls back to showing "VMID XXX".
This commit is contained in:
rcourtman
parent
082c6c2201
commit
8f05fc0a57
4 changed files with 109 additions and 20 deletions
|
|
@ -4285,7 +4285,7 @@ func (m *Manager) CheckBackups(
|
|||
pbsBackups []models.PBSBackup,
|
||||
pmgBackups []models.PMGBackup,
|
||||
guestsByKey map[string]GuestLookup,
|
||||
guestsByVMID map[string]GuestLookup,
|
||||
guestsByVMID map[string][]GuestLookup,
|
||||
) {
|
||||
m.mu.RLock()
|
||||
enabled := m.config.Enabled
|
||||
|
|
@ -4368,17 +4368,28 @@ func (m *Manager) CheckBackups(
|
|||
continue
|
||||
}
|
||||
|
||||
info, exists := guestsByVMID[backup.VMID]
|
||||
guests, exists := guestsByVMID[backup.VMID]
|
||||
var info GuestLookup
|
||||
var key string
|
||||
var displayName string
|
||||
var instance string
|
||||
var node string
|
||||
|
||||
if exists && info.Instance != "" && info.Node != "" {
|
||||
key = BuildGuestKey(info.Instance, info.Node, info.VMID)
|
||||
displayName = info.Name
|
||||
instance = info.Instance
|
||||
node = info.Node
|
||||
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 info.Instance != "" && info.Node != "" {
|
||||
key = BuildGuestKey(info.Instance, info.Node, info.VMID)
|
||||
displayName = info.Name
|
||||
instance = info.Instance
|
||||
node = info.Node
|
||||
} else {
|
||||
key = fmt.Sprintf("pbs:%s:%s:%s", backup.Instance, backup.BackupType, backup.VMID)
|
||||
displayName = fmt.Sprintf("VMID %s", backup.VMID)
|
||||
instance = fmt.Sprintf("PBS:%s", backup.Instance)
|
||||
node = backup.Datastore
|
||||
}
|
||||
} else {
|
||||
key = fmt.Sprintf("pbs:%s:%s:%s", backup.Instance, backup.BackupType, backup.VMID)
|
||||
displayName = fmt.Sprintf("VMID %s", backup.VMID)
|
||||
|
|
|
|||
|
|
@ -790,8 +790,8 @@ func TestCheckBackupsCreatesAndClearsAlerts(t *testing.T) {
|
|||
VMID: 100,
|
||||
},
|
||||
}
|
||||
guestsByVMID := map[string]GuestLookup{
|
||||
"100": guestsByKey[key],
|
||||
guestsByVMID := map[string][]GuestLookup{
|
||||
"100": {guestsByKey[key]},
|
||||
}
|
||||
|
||||
m.CheckBackups(storageBackups, nil, nil, guestsByKey, guestsByVMID)
|
||||
|
|
@ -843,7 +843,7 @@ func TestCheckBackupsHandlesPbsOnlyGuests(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
m.CheckBackups(nil, pbsBackups, nil, map[string]GuestLookup{}, map[string]GuestLookup{})
|
||||
m.CheckBackups(nil, pbsBackups, nil, map[string]GuestLookup{}, map[string][]GuestLookup{})
|
||||
|
||||
m.mu.RLock()
|
||||
found := false
|
||||
|
|
@ -887,7 +887,7 @@ func TestCheckBackupsHandlesPmgBackups(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
m.CheckBackups(nil, nil, pmgBackups, map[string]GuestLookup{}, map[string]GuestLookup{})
|
||||
m.CheckBackups(nil, nil, pmgBackups, map[string]GuestLookup{}, map[string][]GuestLookup{})
|
||||
|
||||
m.mu.RLock()
|
||||
found := false
|
||||
|
|
|
|||
|
|
@ -16,6 +16,9 @@ type GuestMetadata struct {
|
|||
CustomURL string `json:"customUrl"` // Custom URL for the guest
|
||||
Description string `json:"description"` // Optional description
|
||||
Tags []string `json:"tags"` // Optional tags for categorization
|
||||
// Last-known identity (persisted even after guest deletion)
|
||||
LastKnownName string `json:"lastKnownName,omitempty"` // Last known guest name
|
||||
LastKnownType string `json:"lastKnownType,omitempty"` // Last known guest type (qemu, lxc)
|
||||
}
|
||||
|
||||
// GuestMetadataStore manages guest metadata
|
||||
|
|
|
|||
|
|
@ -8330,7 +8330,7 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName
|
|||
|
||||
if m.alertManager != nil {
|
||||
snapshot := m.state.GetSnapshot()
|
||||
guestsByKey, guestsByVMID := buildGuestLookups(snapshot)
|
||||
guestsByKey, guestsByVMID := buildGuestLookups(snapshot, m.guestMetadataStore)
|
||||
pveStorage := snapshot.Backups.PVE.StorageBackups
|
||||
if len(pveStorage) == 0 && len(snapshot.PVEBackups.StorageBackups) > 0 {
|
||||
pveStorage = snapshot.PVEBackups.StorageBackups
|
||||
|
|
@ -8370,9 +8370,9 @@ func shouldPreservePBSBackups(datastoreCount, datastoreFetches int) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
func buildGuestLookups(snapshot models.StateSnapshot) (map[string]alerts.GuestLookup, map[string]alerts.GuestLookup) {
|
||||
func buildGuestLookups(snapshot models.StateSnapshot, metadataStore *config.GuestMetadataStore) (map[string]alerts.GuestLookup, map[string][]alerts.GuestLookup) {
|
||||
byKey := make(map[string]alerts.GuestLookup)
|
||||
byVMID := make(map[string]alerts.GuestLookup)
|
||||
byVMID := make(map[string][]alerts.GuestLookup)
|
||||
|
||||
for _, vm := range snapshot.VMs {
|
||||
info := alerts.GuestLookup{
|
||||
|
|
@ -8386,8 +8386,11 @@ func buildGuestLookups(snapshot models.StateSnapshot) (map[string]alerts.GuestLo
|
|||
byKey[key] = info
|
||||
|
||||
vmidKey := fmt.Sprintf("%d", vm.VMID)
|
||||
if _, exists := byVMID[vmidKey]; !exists {
|
||||
byVMID[vmidKey] = info
|
||||
byVMID[vmidKey] = append(byVMID[vmidKey], info)
|
||||
|
||||
// Persist last-known name and type for this guest
|
||||
if metadataStore != nil && vm.Name != "" {
|
||||
persistGuestIdentity(metadataStore, key, vm.Name, vm.Type)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -8405,14 +8408,86 @@ func buildGuestLookups(snapshot models.StateSnapshot) (map[string]alerts.GuestLo
|
|||
}
|
||||
|
||||
vmidKey := fmt.Sprintf("%d", ct.VMID)
|
||||
if _, exists := byVMID[vmidKey]; !exists {
|
||||
byVMID[vmidKey] = info
|
||||
byVMID[vmidKey] = append(byVMID[vmidKey], info)
|
||||
|
||||
// Persist last-known name and type for this guest
|
||||
if metadataStore != nil && ct.Name != "" {
|
||||
persistGuestIdentity(metadataStore, key, ct.Name, ct.Type)
|
||||
}
|
||||
}
|
||||
|
||||
// Augment byVMID with persisted metadata for deleted guests
|
||||
if metadataStore != nil {
|
||||
enrichWithPersistedMetadata(metadataStore, byVMID)
|
||||
}
|
||||
|
||||
return byKey, byVMID
|
||||
}
|
||||
|
||||
// enrichWithPersistedMetadata adds entries from the metadata store for guests
|
||||
// that no longer exist in the live inventory but have persisted identity data
|
||||
func enrichWithPersistedMetadata(metadataStore *config.GuestMetadataStore, byVMID map[string][]alerts.GuestLookup) {
|
||||
allMetadata := metadataStore.GetAll()
|
||||
for guestKey, meta := range allMetadata {
|
||||
if meta.LastKnownName == "" {
|
||||
continue // No name persisted, skip
|
||||
}
|
||||
|
||||
// Parse the guest key (format: instance:node:vmid)
|
||||
// We need to extract instance, node, and vmid
|
||||
var instance, node string
|
||||
var vmid int
|
||||
if _, err := fmt.Sscanf(guestKey, "%[^:]:%[^:]:%d", &instance, &node, &vmid); err != nil {
|
||||
continue // Invalid key format
|
||||
}
|
||||
|
||||
vmidKey := fmt.Sprintf("%d", vmid)
|
||||
|
||||
// Check if we already have a live entry for this exact guest
|
||||
hasLiveEntry := false
|
||||
for _, existing := range byVMID[vmidKey] {
|
||||
if existing.Instance == instance && existing.Node == node && existing.VMID == vmid {
|
||||
hasLiveEntry = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
// Only add persisted metadata if no live entry exists
|
||||
if !hasLiveEntry {
|
||||
byVMID[vmidKey] = append(byVMID[vmidKey], alerts.GuestLookup{
|
||||
Name: meta.LastKnownName,
|
||||
Instance: instance,
|
||||
Node: node,
|
||||
Type: meta.LastKnownType,
|
||||
VMID: vmid,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// persistGuestIdentity updates the metadata store with the last-known name and type for a guest
|
||||
func persistGuestIdentity(metadataStore *config.GuestMetadataStore, guestKey, name, guestType string) {
|
||||
existing := metadataStore.Get(guestKey)
|
||||
if existing == nil {
|
||||
existing = &config.GuestMetadata{
|
||||
ID: guestKey,
|
||||
Tags: []string{},
|
||||
}
|
||||
}
|
||||
|
||||
// Only update if the name or type has changed
|
||||
if existing.LastKnownName != name || existing.LastKnownType != guestType {
|
||||
existing.LastKnownName = name
|
||||
existing.LastKnownType = guestType
|
||||
// Save asynchronously to avoid blocking the monitor
|
||||
go func() {
|
||||
if err := metadataStore.Set(guestKey, existing); err != nil {
|
||||
log.Error().Err(err).Str("guestKey", guestKey).Msg("Failed to persist guest identity")
|
||||
}
|
||||
}()
|
||||
}
|
||||
}
|
||||
|
||||
func (m *Monitor) calculateBackupOperationTimeout(instanceName string) time.Duration {
|
||||
const (
|
||||
minTimeout = 2 * time.Minute
|
||||
|
|
@ -9067,7 +9142,7 @@ func (m *Monitor) pollPBSBackups(ctx context.Context, instanceName string, clien
|
|||
|
||||
if m.alertManager != nil {
|
||||
snapshot := m.state.GetSnapshot()
|
||||
guestsByKey, guestsByVMID := buildGuestLookups(snapshot)
|
||||
guestsByKey, guestsByVMID := buildGuestLookups(snapshot, m.guestMetadataStore)
|
||||
pveStorage := snapshot.Backups.PVE.StorageBackups
|
||||
if len(pveStorage) == 0 && len(snapshot.PVEBackups.StorageBackups) > 0 {
|
||||
pveStorage = snapshot.PVEBackups.StorageBackups
|
||||
|
|
@ -9123,7 +9198,7 @@ func (m *Monitor) checkMockAlerts() {
|
|||
Msg("Collecting resources for alert cleanup in mock mode")
|
||||
m.alertManager.CleanupAlertsForNodes(existingNodes)
|
||||
|
||||
guestsByKey, guestsByVMID := buildGuestLookups(state)
|
||||
guestsByKey, guestsByVMID := buildGuestLookups(state, m.guestMetadataStore)
|
||||
pveStorage := state.Backups.PVE.StorageBackups
|
||||
if len(pveStorage) == 0 && len(state.PVEBackups.StorageBackups) > 0 {
|
||||
pveStorage = state.PVEBackups.StorageBackups
|
||||
|
|
|
|||
Loading…
Reference in a new issue