diff --git a/internal/models/models.go b/internal/models/models.go index 7fae159..0a0eeca 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -1386,48 +1386,76 @@ func (s *State) UpdateContainers(containers []Container) { s.LastUpdate = time.Now() } +// backupKey creates a composite key for backup matching using instance and VMID. +// This ensures backups are correctly matched to guests even when VMIDs are reused across instances. +func backupKey(instance string, vmid int) string { + return instance + "-" + strconv.Itoa(vmid) +} + // SyncGuestBackupTimes updates LastBackup on VMs and Containers from storage backups and PBS backups. // Call this after updating storage backups or PBS backups to ensure guest backup indicators are accurate. +// Matching is done by instance+VMID to prevent cross-instance VMID collisions. func (s *State) SyncGuestBackupTimes() { s.mu.Lock() defer s.mu.Unlock() - // Build a map of VMID -> latest backup time from all backup sources - latestBackup := make(map[int]time.Time) + // Build a map of instance+VMID -> latest backup time from all backup sources + // Using composite key prevents cross-instance VMID collision issues + latestBackup := make(map[string]time.Time) // Process PVE storage backups for _, backup := range s.PVEBackups.StorageBackups { - if backup.VMID <= 0 { + if backup.VMID <= 0 || backup.Instance == "" { continue } - if existing, ok := latestBackup[backup.VMID]; !ok || backup.Time.After(existing) { - latestBackup[backup.VMID] = backup.Time + key := backupKey(backup.Instance, backup.VMID) + if existing, ok := latestBackup[key]; !ok || backup.Time.After(existing) { + latestBackup[key] = backup.Time } } // Process PBS backups (VMID is string, BackupTime is the timestamp) + // Note: PBS backups use Instance field to identify the PBS server, but backups + // from different PVE instances can be stored in the same PBS. + // For PBS, we need to match against all guests since we can't reliably determine + // which PVE instance the backup belongs to. + pbsLatestByVMID := make(map[int]time.Time) for _, backup := range s.PBSBackups { vmid, err := strconv.Atoi(backup.VMID) if err != nil || vmid <= 0 { continue } - if existing, ok := latestBackup[vmid]; !ok || backup.BackupTime.After(existing) { - latestBackup[vmid] = backup.BackupTime + if existing, ok := pbsLatestByVMID[vmid]; !ok || backup.BackupTime.After(existing) { + pbsLatestByVMID[vmid] = backup.BackupTime } } - // Update VMs + // Update VMs - prefer instance-specific PVE backup, fall back to PBS by VMID for i := range s.VMs { - if backupTime, ok := latestBackup[s.VMs[i].VMID]; ok { + key := backupKey(s.VMs[i].Instance, s.VMs[i].VMID) + if backupTime, ok := latestBackup[key]; ok { s.VMs[i].LastBackup = backupTime } + // Check if PBS has a more recent backup + if pbsTime, ok := pbsLatestByVMID[s.VMs[i].VMID]; ok { + if s.VMs[i].LastBackup.IsZero() || pbsTime.After(s.VMs[i].LastBackup) { + s.VMs[i].LastBackup = pbsTime + } + } } - // Update Containers + // Update Containers - prefer instance-specific PVE backup, fall back to PBS by VMID for i := range s.Containers { - if backupTime, ok := latestBackup[s.Containers[i].VMID]; ok { + key := backupKey(s.Containers[i].Instance, s.Containers[i].VMID) + if backupTime, ok := latestBackup[key]; ok { s.Containers[i].LastBackup = backupTime } + // Check if PBS has a more recent backup + if pbsTime, ok := pbsLatestByVMID[s.Containers[i].VMID]; ok { + if s.Containers[i].LastBackup.IsZero() || pbsTime.After(s.Containers[i].LastBackup) { + s.Containers[i].LastBackup = pbsTime + } + } } } diff --git a/internal/models/state_host_test.go b/internal/models/state_host_test.go index 0a7c99a..736acbc 100644 --- a/internal/models/state_host_test.go +++ b/internal/models/state_host_test.go @@ -745,11 +745,11 @@ func TestSyncGuestBackupTimes(t *testing.T) { {VMID: 200, Name: "ct-200", Instance: "pve-1"}, }) - // Add storage backups for VM 100 + // Add storage backups for VM 100 (must include Instance for proper matching) state.mu.Lock() state.PVEBackups.StorageBackups = []StorageBackup{ - {ID: "pve-1-backup-1", VMID: 100, Time: oldBackup}, - {ID: "pve-1-backup-2", VMID: 100, Time: newBackup}, // newer + {ID: "pve-1-backup-1", VMID: 100, Instance: "pve-1", Time: oldBackup}, + {ID: "pve-1-backup-2", VMID: 100, Instance: "pve-1", Time: newBackup}, // newer } state.mu.Unlock() @@ -809,6 +809,63 @@ func TestSyncGuestBackupTimes(t *testing.T) { } } +// TestSyncGuestBackupTimesCrossInstance verifies that backup matching uses instance+VMID. +// This prevents a newly created container on one instance from incorrectly showing +// backup time from a different container with the same VMID on another instance. +func TestSyncGuestBackupTimesCrossInstance(t *testing.T) { + state := NewState() + + now := time.Now() + threeMonthsAgo := now.Add(-90 * 24 * time.Hour) + + // Set up containers with the SAME VMID on DIFFERENT instances + // This simulates: pve-1 has VMID 100 with old backup, pve-2 has newly created VMID 100 + state.UpdateContainers([]Container{ + {VMID: 100, Name: "old-container", Instance: "pve-1"}, + {VMID: 100, Name: "new-container", Instance: "pve-2"}, // newly created, no backup + }) + + // Add a 3-month-old backup for pve-1's container (VMID 100) + state.mu.Lock() + state.PVEBackups.StorageBackups = []StorageBackup{ + {ID: "pve-1-backup-old", VMID: 100, Instance: "pve-1", Time: threeMonthsAgo}, + } + state.mu.Unlock() + + // Sync backup times + state.SyncGuestBackupTimes() + + snapshot := state.GetSnapshot() + + // Find both containers + var oldContainer, newContainer *Container + for i := range snapshot.Containers { + if snapshot.Containers[i].Instance == "pve-1" && snapshot.Containers[i].VMID == 100 { + oldContainer = &snapshot.Containers[i] + } + if snapshot.Containers[i].Instance == "pve-2" && snapshot.Containers[i].VMID == 100 { + newContainer = &snapshot.Containers[i] + } + } + + if oldContainer == nil { + t.Fatal("pve-1 container not found") + } + if newContainer == nil { + t.Fatal("pve-2 container not found") + } + + // The old container on pve-1 SHOULD have the backup time + if !oldContainer.LastBackup.Equal(threeMonthsAgo) { + t.Errorf("pve-1 container LastBackup = %v, expected %v", oldContainer.LastBackup, threeMonthsAgo) + } + + // The NEW container on pve-2 should NOT have any backup time (it's a different container!) + if !newContainer.LastBackup.IsZero() { + t.Errorf("pve-2 container (newly created) should have no backup, got %v", newContainer.LastBackup) + } +} + func TestUpdateStorageBackupsForInstance(t *testing.T) { state := NewState()