From 4ed03f23c2630d230b68dcbc6ee1bbf525f7fda4 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 1 Jan 2026 23:22:38 +0000 Subject: [PATCH] fix: use Instance field for backup/snapshot state sync instead of ID prefix This resolves issues where snapshots/backups persist after deletion if the Instance field didn't match the ID prefix (due to case changes, name changes, etc). Now consistent with how VMs, Containers, Storage, etc. are filtered. Also adds Instance field to BackupTask model for completeness. Addresses #1009 (refs #991) --- internal/models/models.go | 10 ++++------ internal/models/state_host_test.go | 24 ++++++++++++------------ internal/monitoring/monitor.go | 1 + 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/internal/models/models.go b/internal/models/models.go index 3932dd68a..2102e2c7c 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -1117,6 +1117,7 @@ type PVEBackups struct { type BackupTask struct { ID string `json:"id"` Node string `json:"node"` + Instance string `json:"instance"` // Unique instance identifier Type string `json:"type"` VMID int `json:"vmid"` Status string `json:"status"` @@ -2364,8 +2365,7 @@ func (s *State) UpdateBackupTasksForInstance(instanceName string, tasks []Backup // Create a map of existing tasks, excluding those from this instance taskMap := make(map[string]BackupTask) for _, task := range s.PVEBackups.BackupTasks { - // Check if task ID contains the instance name - if !strings.HasPrefix(task.ID, instanceName+"-") { + if task.Instance != instanceName { taskMap[task.ID] = task } } @@ -2423,8 +2423,7 @@ func (s *State) UpdateStorageBackupsForInstance(instanceName string, backups []S // Create a map of existing backups, excluding those from this instance backupMap := make(map[string]StorageBackup) for _, backup := range s.PVEBackups.StorageBackups { - // Check if backup ID contains the instance name - if !strings.HasPrefix(backup.ID, instanceName+"-") { + if backup.Instance != instanceName { backupMap[backup.ID] = backup } } @@ -2507,8 +2506,7 @@ func (s *State) UpdateGuestSnapshotsForInstance(instanceName string, snapshots [ // Create a map of existing snapshots, excluding those from this instance snapshotMap := make(map[string]GuestSnapshot) for _, snapshot := range s.PVEBackups.GuestSnapshots { - // Check if snapshot ID contains the instance name - if !strings.HasPrefix(snapshot.ID, instanceName+"-") { + if snapshot.Instance != instanceName { snapshotMap[snapshot.ID] = snapshot } } diff --git a/internal/models/state_host_test.go b/internal/models/state_host_test.go index 736acbcc9..264c028b8 100644 --- a/internal/models/state_host_test.go +++ b/internal/models/state_host_test.go @@ -613,8 +613,8 @@ func TestUpdateBackupTasksForInstance(t *testing.T) { // Add tasks from first instance tasks1 := []BackupTask{ - {ID: "pve-1-task-1", StartTime: now}, - {ID: "pve-1-task-2", StartTime: now.Add(-time.Hour)}, + {ID: "pve-1-task-1", Instance: "pve-1", StartTime: now}, + {ID: "pve-1-task-2", Instance: "pve-1", StartTime: now.Add(-time.Hour)}, } state.UpdateBackupTasksForInstance("pve-1", tasks1) @@ -630,7 +630,7 @@ func TestUpdateBackupTasksForInstance(t *testing.T) { // Add tasks from second instance tasks2 := []BackupTask{ - {ID: "pve-2-task-1", StartTime: now.Add(-30 * time.Minute)}, + {ID: "pve-2-task-1", Instance: "pve-2", StartTime: now.Add(-30 * time.Minute)}, } state.UpdateBackupTasksForInstance("pve-2", tasks2) @@ -641,7 +641,7 @@ func TestUpdateBackupTasksForInstance(t *testing.T) { // Update first instance (should replace its tasks) tasks1Updated := []BackupTask{ - {ID: "pve-1-task-3", StartTime: now.Add(time.Hour)}, + {ID: "pve-1-task-3", Instance: "pve-1", StartTime: now.Add(time.Hour)}, } state.UpdateBackupTasksForInstance("pve-1", tasks1Updated) @@ -696,8 +696,8 @@ func TestUpdateGuestSnapshotsForInstance(t *testing.T) { // Add snapshots from first instance snapshots1 := []GuestSnapshot{ - {ID: "pve-1-snap-1", VMID: 100, Name: "snapshot1", Time: now}, - {ID: "pve-1-snap-2", VMID: 100, Name: "snapshot2", Time: now.Add(-time.Hour)}, + {ID: "pve-1-snap-1", Instance: "pve-1", VMID: 100, Name: "snapshot1", Time: now}, + {ID: "pve-1-snap-2", Instance: "pve-1", VMID: 100, Name: "snapshot2", Time: now.Add(-time.Hour)}, } state.UpdateGuestSnapshotsForInstance("pve-1", snapshots1) @@ -708,7 +708,7 @@ func TestUpdateGuestSnapshotsForInstance(t *testing.T) { // Add snapshots from second instance snapshots2 := []GuestSnapshot{ - {ID: "pve-2-snap-1", VMID: 200, Name: "snapshot1", Time: now.Add(-30 * time.Minute)}, + {ID: "pve-2-snap-1", Instance: "pve-2", VMID: 200, Name: "snapshot1", Time: now.Add(-30 * time.Minute)}, } state.UpdateGuestSnapshotsForInstance("pve-2", snapshots2) @@ -719,7 +719,7 @@ func TestUpdateGuestSnapshotsForInstance(t *testing.T) { // Update first instance (should replace its snapshots) snapshots1Updated := []GuestSnapshot{ - {ID: "pve-1-snap-3", VMID: 100, Name: "new-snapshot", Time: now.Add(time.Hour)}, + {ID: "pve-1-snap-3", Instance: "pve-1", VMID: 100, Name: "new-snapshot", Time: now.Add(time.Hour)}, } state.UpdateGuestSnapshotsForInstance("pve-1", snapshots1Updated) @@ -878,8 +878,8 @@ func TestUpdateStorageBackupsForInstance(t *testing.T) { // Add backups from first instance backups1 := []StorageBackup{ - {ID: "pve-1-backup-1", VMID: 100, Time: now, Node: "node1"}, - {ID: "pve-1-backup-2", VMID: 100, Time: now.Add(-time.Hour), Node: "node1"}, + {ID: "pve-1-backup-1", Instance: "pve-1", VMID: 100, Time: now, Node: "node1"}, + {ID: "pve-1-backup-2", Instance: "pve-1", VMID: 100, Time: now.Add(-time.Hour), Node: "node1"}, } state.UpdateStorageBackupsForInstance("pve-1", backups1) @@ -895,7 +895,7 @@ func TestUpdateStorageBackupsForInstance(t *testing.T) { // Add backups from second instance backups2 := []StorageBackup{ - {ID: "pve-2-backup-1", VMID: 200, Time: now.Add(-30 * time.Minute), Node: "node2"}, + {ID: "pve-2-backup-1", Instance: "pve-2", VMID: 200, Time: now.Add(-30 * time.Minute), Node: "node2"}, } state.UpdateStorageBackupsForInstance("pve-2", backups2) @@ -906,7 +906,7 @@ func TestUpdateStorageBackupsForInstance(t *testing.T) { // Update first instance (should replace its backups) backups1Updated := []StorageBackup{ - {ID: "pve-1-backup-3", VMID: 100, Time: now.Add(time.Hour), Node: "node1"}, + {ID: "pve-1-backup-3", Instance: "pve-1", VMID: 100, Time: now.Add(time.Hour), Node: "node1"}, } state.UpdateStorageBackupsForInstance("pve-1", backups1Updated) diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 970a2298f..bdc90bab1 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -6990,6 +6990,7 @@ func (m *Monitor) pollBackupTasks(ctx context.Context, instanceName string, clie backupTask := models.BackupTask{ ID: taskID, Node: task.Node, + Instance: instanceName, Type: task.Type, VMID: vmid, Status: task.Status,