From dc286a38fc9e5c0552cf96adb43c89e6ba23f8b7 Mon Sep 17 00:00:00 2001
From: Scott Seago <sseago@redhat.com>
Date: Fri, 12 Jul 2024 10:11:20 -0400
Subject: [PATCH] Reuse existing plugin manager for get/put volume info
Signed-off-by: Scott Seago <sseago@redhat.com>
---
changelogs/unreleased/8012-sseago | 1 +
pkg/backup/backup.go | 27 +++++--------------
pkg/backup/backup_test.go | 2 +-
pkg/controller/backup_controller_test.go | 1 +
pkg/controller/backup_finalizer_controller.go | 1 +
.../backup_finalizer_controller_test.go | 2 +-
6 files changed, 12 insertions(+), 22 deletions(-)
create mode 100644 changelogs/unreleased/8012-sseago
diff --git a/changelogs/unreleased/8012-sseago b/changelogs/unreleased/8012-sseago
new file mode 100644
index 000000000..6f56e867a
--- /dev/null
+++ b/changelogs/unreleased/8012-sseago
@@ -0,0 +1 @@
+Reuse existing plugin manager for get/put volume info
diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go
index 9e6da6fe7..7acae3060 100644
--- a/pkg/backup/backup.go
+++ b/pkg/backup/backup.go
@@ -95,6 +95,7 @@ type Backupper interface {
outBackupFile io.Writer,
backupItemActionResolver framework.BackupItemActionResolverV2,
asyncBIAOperations []*itemoperation.BackupOperation,
+ backupStore persistence.BackupStore,
) error
}
@@ -610,6 +611,7 @@ func (kb *kubernetesBackupper) FinalizeBackup(
outBackupFile io.Writer,
backupItemActionResolver framework.BackupItemActionResolverV2,
asyncBIAOperations []*itemoperation.BackupOperation,
+ backupStore persistence.BackupStore,
) error {
gzw := gzip.NewWriter(outBackupFile)
defer gzw.Close()
@@ -726,7 +728,7 @@ func (kb *kubernetesBackupper) FinalizeBackup(
}).Infof("Updated %d items out of an estimated total of %d (estimate will change throughout the backup finalizer)", len(backupRequest.BackedUpItems), totalItems)
}
- backupStore, volumeInfos, err := kb.getVolumeInfos(*backupRequest.Backup, log)
+ volumeInfos, err := kb.getVolumeInfos(*backupRequest.Backup, backupStore, log)
if err != nil {
log.WithError(err).Errorf("fail to get the backup VolumeInfos for backup %s", backupRequest.Name)
return err
@@ -812,30 +814,15 @@ type tarWriter interface {
func (kb *kubernetesBackupper) getVolumeInfos(
backup velerov1api.Backup,
+ backupStore persistence.BackupStore,
log logrus.FieldLogger,
-) (persistence.BackupStore, []*volume.BackupVolumeInfo, error) {
- location := &velerov1api.BackupStorageLocation{}
- if err := kb.kbClient.Get(context.Background(), kbclient.ObjectKey{
- Namespace: backup.Namespace,
- Name: backup.Spec.StorageLocation,
- }, location); err != nil {
- return nil, nil, errors.WithStack(err)
- }
-
- pluginManager := kb.pluginManager(log)
- defer pluginManager.CleanupClients()
-
- backupStore, storeErr := kb.backupStoreGetter.Get(location, pluginManager, log)
- if storeErr != nil {
- return nil, nil, storeErr
- }
-
+) ([]*volume.BackupVolumeInfo, error) {
volumeInfos, err := backupStore.GetBackupVolumeInfos(backup.Name)
if err != nil {
- return nil, nil, err
+ return nil, err
}
- return backupStore, volumeInfos, nil
+ return volumeInfos, nil
}
// updateVolumeInfos update the VolumeInfos according to the AsyncOperations
diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go
index 0c46070a9..0fa981f9a 100644
--- a/pkg/backup/backup_test.go
+++ b/pkg/backup/backup_test.go
@@ -4532,7 +4532,7 @@ func TestGetVolumeInfos(t *testing.T) {
bsl := builder.ForBackupStorageLocation("velero", "default").Result()
require.NoError(t, h.backupper.kbClient.Create(context.Background(), bsl))
- _, _, err := h.backupper.getVolumeInfos(*backup, h.log)
+ _, err := h.backupper.getVolumeInfos(*backup, backupStore, h.log)
require.NoError(t, err)
}
diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go
index 46f123f1a..bf1d83622 100644
--- a/pkg/controller/backup_controller_test.go
+++ b/pkg/controller/backup_controller_test.go
@@ -85,6 +85,7 @@ func (b *fakeBackupper) FinalizeBackup(
outBackupFile io.Writer,
backupItemActionResolver framework.BackupItemActionResolverV2,
asyncBIAOperations []*itemoperation.BackupOperation,
+ backupStore persistence.BackupStore,
) error {
args := b.Called(logger, backup, inBackupFile, outBackupFile, backupItemActionResolver, asyncBIAOperations)
return args.Error(0)
diff --git a/pkg/controller/backup_finalizer_controller.go b/pkg/controller/backup_finalizer_controller.go
index 2bf17e9bf..74386b1cd 100644
--- a/pkg/controller/backup_finalizer_controller.go
+++ b/pkg/controller/backup_finalizer_controller.go
@@ -184,6 +184,7 @@ func (r *backupFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
outBackupFile,
backupItemActionsResolver,
operations,
+ backupStore,
)
if err != nil {
log.WithError(err).Error("error finalizing Backup")
diff --git a/pkg/controller/backup_finalizer_controller_test.go b/pkg/controller/backup_finalizer_controller_test.go
index 8c1a5e361..898c5c28d 100644
--- a/pkg/controller/backup_finalizer_controller_test.go
+++ b/pkg/controller/backup_finalizer_controller_test.go
@@ -225,7 +225,7 @@ func TestBackupFinalizerReconcile(t *testing.T) {
backupStore.On("GetBackupVolumeInfos", mock.Anything).Return(nil, nil)
backupStore.On("PutBackupVolumeInfos", mock.Anything, mock.Anything).Return(nil)
pluginManager.On("GetBackupItemActionsV2").Return(nil, nil)
- backupper.On("FinalizeBackup", mock.Anything, mock.Anything, mock.Anything, mock.Anything, framework.BackupItemActionResolverV2{}, mock.Anything).Return(nil)
+ backupper.On("FinalizeBackup", mock.Anything, mock.Anything, mock.Anything, mock.Anything, framework.BackupItemActionResolverV2{}, mock.Anything, mock.Anything).Return(nil)
_, err := reconciler.Reconcile(context.TODO(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace, Name: test.backup.Name}})
gotErr := err != nil
assert.Equal(t, test.expectError, gotErr)