Merge pull request #8016 from sseago/plugin-leak-1.14

[release-1.14] Reuse existing plugin manager for get/put volume info

changelogs/unreleased/8016-sseago

@@ -0,0 +1 @@
+Reuse existing plugin manager for get/put volume info

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 := backupStore.GetBackupVolumeInfos(backupRequest.Backup.Name)
 	if err != nil {
 		log.WithError(err).Errorf("fail to get the backup VolumeInfos for backup %s", backupRequest.Name)
 		return err
@@ -810,34 +812,6 @@ type tarWriter interface {
 	WriteHeader(*tar.Header) error
 }
 
-func (kb *kubernetesBackupper) getVolumeInfos(
-	backup velerov1api.Backup,
-	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
-	}
-
-	volumeInfos, err := backupStore.GetBackupVolumeInfos(backup.Name)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return backupStore, volumeInfos, nil
-}
-
 // updateVolumeInfos update the VolumeInfos according to the AsyncOperations
 func updateVolumeInfos(
 	volumeInfos []*volume.BackupVolumeInfo,

pkg/backup/backup_test.go

@@ -54,8 +54,6 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
 	"github.com/vmware-tanzu/velero/pkg/persistence"
 	persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
-	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
-	pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	biav2 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v2"
 	vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
@@ -4519,23 +4517,6 @@ func TestBackupNamespaces(t *testing.T) {
 	}
 }
 
-func TestGetVolumeInfos(t *testing.T) {
-	h := newHarness(t)
-	pluginManager := new(pluginmocks.Manager)
-	backupStore := new(persistencemocks.BackupStore)
-	h.backupper.pluginManager = func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager }
-	h.backupper.backupStoreGetter = NewFakeSingleObjectBackupStoreGetter(backupStore)
-	backupStore.On("GetBackupVolumeInfos", "backup-01").Return([]*volume.BackupVolumeInfo{}, nil)
-	pluginManager.On("CleanupClients").Return()
-
-	backup := builder.ForBackup("velero", "backup-01").StorageLocation("default").Result()
-	bsl := builder.ForBackupStorageLocation("velero", "default").Result()
-	require.NoError(t, h.backupper.kbClient.Create(context.Background(), bsl))
-
-	_, _, err := h.backupper.getVolumeInfos(*backup, h.log)
-	require.NoError(t, err)
-}
-
 func TestUpdateVolumeInfos(t *testing.T) {
 	timeExample := time.Date(2014, 6, 5, 11, 56, 45, 0, time.Local)
 	now := metav1.NewTime(timeExample)

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)

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")

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)