fix: backup matching uses instance+VMID to prevent cross-instance collisions
Previously, SyncGuestBackupTimes matched backups to guests using only VMID. This caused newly created containers to incorrectly show old backup times from different containers on other Proxmox instances that happened to have the same VMID. Now uses composite key (instance+VMID) for PVE storage backups to ensure proper isolation. PBS backups still use VMID matching (since they aggregate from multiple sources) but only as a fallback. Fixes issue where ollama LXC showed 'last backup 3 months ago' despite being created yesterday.
This commit is contained in:
parent
26133cf0cf
commit
0b65d68682
2 changed files with 99 additions and 14 deletions
|
|
@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue