Merge pull request #6041 from sseago/async-backup-deletion

Fixed backup deletion bug related to async operations

changelogs/unreleased/6041-sseago

@@ -0,0 +1 @@
+Fixed backup deletion bug related to async operations

pkg/cmd/server/server.go

@@ -803,6 +803,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			clock.RealClock{},
 			backupper,
 			newPluginManager,
+			backupTracker,
 			backupStoreGetter,
 			s.logger,
 			s.metrics,

pkg/controller/backup_controller.go

@@ -267,7 +267,12 @@ func (b *backupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	}
 
 	b.backupTracker.Add(request.Namespace, request.Name)
-	defer b.backupTracker.Delete(request.Namespace, request.Name)
+	defer func() {
+		switch request.Status.Phase {
+		case velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed, velerov1api.BackupPhaseFailed, velerov1api.BackupPhaseFailedValidation:
+			b.backupTracker.Delete(request.Namespace, request.Name)
+		}
+	}()
 
 	log.Debug("Running backup")
 

pkg/controller/backup_finalizer_controller.go

@@ -44,6 +44,7 @@ type backupFinalizerReconciler struct {
 	clock             clocks.WithTickerAndDelayedExecution
 	backupper         pkgbackup.Backupper
 	newPluginManager  func(logrus.FieldLogger) clientmgmt.Manager
+	backupTracker     BackupTracker
 	metrics           *metrics.ServerMetrics
 	backupStoreGetter persistence.ObjectBackupStoreGetter
 	log               logrus.FieldLogger
@@ -55,6 +56,7 @@ func NewBackupFinalizerReconciler(
 	clock clocks.WithTickerAndDelayedExecution,
 	backupper pkgbackup.Backupper,
 	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
+	backupTracker BackupTracker,
 	backupStoreGetter persistence.ObjectBackupStoreGetter,
 	log logrus.FieldLogger,
 	metrics *metrics.ServerMetrics,
@@ -64,6 +66,7 @@ func NewBackupFinalizerReconciler(
 		clock:             clock,
 		backupper:         backupper,
 		newPluginManager:  newPluginManager,
+		backupTracker:     backupTracker,
 		backupStoreGetter: backupStoreGetter,
 		log:               log,
 		metrics:           metrics,
@@ -102,6 +105,10 @@ func (r *backupFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 
 	original := backup.DeepCopy()
 	defer func() {
+		switch backup.Status.Phase {
+		case velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed, velerov1api.BackupPhaseFailed, velerov1api.BackupPhaseFailedValidation:
+			r.backupTracker.Delete(backup.Namespace, backup.Name)
+		}
 		// Always attempt to Patch the backup object and status after each reconciliation.
 		if err := r.client.Patch(ctx, backup, kbclient.MergeFrom(original)); err != nil {
 			log.WithError(err).Error("Error updating backup")

pkg/controller/backup_finalizer_controller_test.go

@@ -52,6 +52,7 @@ func mockBackupFinalizerReconciler(fakeClient kbclient.Client, fakeClock *testcl
 		fakeClock,
 		backupper,
 		func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+		NewBackupTracker(),
 		NewFakeSingleObjectBackupStoreGetter(backupStore),
 		logrus.StandardLogger(),
 		metrics.NewServerMetrics(),