From 441a32a8616f46ae35da2fb869f742faa19d7093 Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Mon, 14 Aug 2023 13:37:06 -0400 Subject: [PATCH 1/2] Deal with PartiallyFailed orphaned backups as well as Completed ones Fixes https://github.com/vmware-tanzu/velero/issues/6648 Signed-off-by: Scott Seago --- changelogs/unreleased/6649-sseago | 1 + pkg/controller/backup_sync_controller.go | 2 +- pkg/controller/backup_sync_controller_test.go | 25 +++++++++++++------ 3 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 changelogs/unreleased/6649-sseago diff --git a/changelogs/unreleased/6649-sseago b/changelogs/unreleased/6649-sseago new file mode 100644 index 000000000..49f43bb7b --- /dev/null +++ b/changelogs/unreleased/6649-sseago @@ -0,0 +1 @@ +Deal with PartiallyFailed orphaned backups as well as Completed ones diff --git a/pkg/controller/backup_sync_controller.go b/pkg/controller/backup_sync_controller.go index b3d996d2f..2fb61d603 100644 --- a/pkg/controller/backup_sync_controller.go +++ b/pkg/controller/backup_sync_controller.go @@ -331,7 +331,7 @@ func (b *backupSyncReconciler) deleteOrphanedBackups(ctx context.Context, locati for i, backup := range backupList.Items { log = log.WithField("backup", backup.Name) - if backup.Status.Phase != velerov1api.BackupPhaseCompleted || backupStoreBackups.Has(backup.Name) { + if !(backup.Status.Phase == velerov1api.BackupPhaseCompleted || backup.Status.Phase == velerov1api.BackupPhasePartiallyFailed) || backupStoreBackups.Has(backup.Name) { continue } diff --git a/pkg/controller/backup_sync_controller_test.go b/pkg/controller/backup_sync_controller_test.go index bc362edb4..86ca3227b 100644 --- a/pkg/controller/backup_sync_controller_test.go +++ b/pkg/controller/backup_sync_controller_test.go @@ -557,7 +557,7 @@ var _ = Describe("Backup Sync Reconciler", func() { k8sBackups: []*velerov1api.Backup{ baseBuilder("backupA").Phase(velerov1api.BackupPhaseCompleted).Result(), baseBuilder("backupB").Phase(velerov1api.BackupPhaseCompleted).Result(), - baseBuilder("backupC").Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backupC").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), }, expectedDeletes: sets.NewString("backupA", "backupB", "backupC"), }, @@ -568,9 +568,10 @@ var _ = Describe("Backup Sync Reconciler", func() { k8sBackups: []*velerov1api.Backup{ baseBuilder("backup-1").Phase(velerov1api.BackupPhaseCompleted).Result(), baseBuilder("backup-2").Phase(velerov1api.BackupPhaseCompleted).Result(), - baseBuilder("backup-C").Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backup-B").Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backup-C").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), }, - expectedDeletes: sets.NewString("backup-C"), + expectedDeletes: sets.NewString("backup-B", "backup-C"), }, { name: "all overlapping backups", @@ -579,7 +580,7 @@ var _ = Describe("Backup Sync Reconciler", func() { k8sBackups: []*velerov1api.Backup{ baseBuilder("backup-1").Phase(velerov1api.BackupPhaseCompleted).Result(), baseBuilder("backup-2").Phase(velerov1api.BackupPhaseCompleted).Result(), - baseBuilder("backup-3").Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backup-3").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), }, expectedDeletes: sets.NewString(), }, @@ -589,13 +590,14 @@ var _ = Describe("Backup Sync Reconciler", func() { cloudBackups: sets.NewString("backup-1", "backup-2", "backup-3"), k8sBackups: []*velerov1api.Backup{ baseBuilder("backupA").Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backupB").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), baseBuilder("Deleting").Phase(velerov1api.BackupPhaseDeleting).Result(), baseBuilder("Failed").Phase(velerov1api.BackupPhaseFailed).Result(), baseBuilder("FailedValidation").Phase(velerov1api.BackupPhaseFailedValidation).Result(), baseBuilder("InProgress").Phase(velerov1api.BackupPhaseInProgress).Result(), baseBuilder("New").Phase(velerov1api.BackupPhaseNew).Result(), }, - expectedDeletes: sets.NewString("backupA"), + expectedDeletes: sets.NewString("backupA", "backupB"), }, { name: "all overlapping backups and all backups that are not complete", @@ -616,12 +618,13 @@ var _ = Describe("Backup Sync Reconciler", func() { baseBuilder("backup-1").Phase(velerov1api.BackupPhaseCompleted).Result(), baseBuilder("backup-2").Phase(velerov1api.BackupPhaseCompleted).Result(), baseBuilder("backup-C").Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backup-D").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), baseBuilder("backup-4").ObjectMeta(builder.WithLabels(velerov1api.StorageLocationLabel, "alternate")).Phase(velerov1api.BackupPhaseCompleted).Result(), baseBuilder("backup-5").ObjectMeta(builder.WithLabels(velerov1api.StorageLocationLabel, "alternate")).Phase(velerov1api.BackupPhaseCompleted).Result(), - baseBuilder("backup-6").ObjectMeta(builder.WithLabels(velerov1api.StorageLocationLabel, "alternate")).Phase(velerov1api.BackupPhaseCompleted).Result(), + baseBuilder("backup-6").ObjectMeta(builder.WithLabels(velerov1api.StorageLocationLabel, "alternate")).Phase(velerov1api.BackupPhasePartiallyFailed).Result(), }, - expectedDeletes: sets.NewString("backup-C"), + expectedDeletes: sets.NewString("backup-C", "backup-D"), }, { name: "some overlapping backups", @@ -646,8 +649,14 @@ var _ = Describe("Backup Sync Reconciler", func() { ). Phase(velerov1api.BackupPhaseCompleted). Result(), + builder.ForBackup("ns-1", "backup-D"). + ObjectMeta( + builder.WithLabels(velerov1api.StorageLocationLabel, "the-really-long-location-name-that-is-much-more-than-63-c69e779"), + ). + Phase(velerov1api.BackupPhasePartiallyFailed). + Result(), }, - expectedDeletes: sets.NewString("backup-C"), + expectedDeletes: sets.NewString("backup-C", "backup-D"), useLongBSLName: true, }, } From 8d0a8bac3415fe9df247ca4eb4bbb34963bf43f5 Mon Sep 17 00:00:00 2001 From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> Date: Tue, 22 Aug 2023 10:37:59 +0800 Subject: [PATCH 2/2] Update changelogs/unreleased/6649-sseago Co-authored-by: Tiger Kaovilai Signed-off-by: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> --- changelogs/unreleased/6649-sseago | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/6649-sseago b/changelogs/unreleased/6649-sseago index 49f43bb7b..8c3b20751 100644 --- a/changelogs/unreleased/6649-sseago +++ b/changelogs/unreleased/6649-sseago @@ -1 +1 @@ -Deal with PartiallyFailed orphaned backups as well as Completed ones +Delete PartiallyFailed orphaned backups as well as Completed ones