From 05c4e35ae70eade9b910d14640e2e52ae9b74ca7 Mon Sep 17 00:00:00 2001
From: Scott Seago <sseago@redhat.com>
Date: Mon, 27 Mar 2023 09:47:23 -0400
Subject: [PATCH] Fixed backup deletion bug related to async operations

Signed-off-by: Scott Seago <sseago@redhat.com>
---
 changelogs/unreleased/6041-sseago                  | 1 +
 pkg/cmd/server/server.go                           | 1 +
 pkg/controller/backup_controller.go                | 7 ++++++-
 pkg/controller/backup_finalizer_controller.go      | 7 +++++++
 pkg/controller/backup_finalizer_controller_test.go | 1 +
 5 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/6041-sseago

diff --git a/changelogs/unreleased/6041-sseago b/changelogs/unreleased/6041-sseago
new file mode 100644
index 000000000..1a9a66a6c
--- /dev/null
+++ b/changelogs/unreleased/6041-sseago
@@ -0,0 +1 @@
+Fixed backup deletion bug related to async operations
diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go
index b8901e1c3..a2a0516f2 100644
--- a/pkg/cmd/server/server.go
+++ b/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,
diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go
index 1d808b914..d5c6eea38 100644
--- a/pkg/controller/backup_controller.go
+++ b/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")
 
diff --git a/pkg/controller/backup_finalizer_controller.go b/pkg/controller/backup_finalizer_controller.go
index 76df47d37..fd0f1ac3c 100644
--- a/pkg/controller/backup_finalizer_controller.go
+++ b/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")
diff --git a/pkg/controller/backup_finalizer_controller_test.go b/pkg/controller/backup_finalizer_controller_test.go
index e062e8a72..1e0f88511 100644
--- a/pkg/controller/backup_finalizer_controller_test.go
+++ b/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(),