backup deletion controller: use backup location for object store

Signed-off-by: Steve Kriss <steve@heptio.com>

pkg/cmd/server/server.go

@@ -682,13 +682,13 @@ func (s *server) runControllers(config *api.Config) error {
 			s.arkClient.ArkV1(), // deleteBackupRequestClient
 			s.arkClient.ArkV1(), // backupClient
 			s.blockStore,
-			s.objectStore,
-			config.BackupStorageProvider.Bucket,
 			s.sharedInformerFactory.Ark().V1().Restores(),
 			s.arkClient.ArkV1(), // restoreClient
 			backupTracker,
 			s.resticManager,
 			s.sharedInformerFactory.Ark().V1().PodVolumeBackups(),
+			s.sharedInformerFactory.Ark().V1().BackupStorageLocations(),
+			s.pluginRegistry,
 		)
 		wg.Add(1)
 		go func() {

pkg/controller/backup_deletion_controller.go

@@ -28,6 +28,7 @@ import (
 	arkv1client "github.com/heptio/ark/pkg/generated/clientset/versioned/typed/ark/v1"
 	informers "github.com/heptio/ark/pkg/generated/informers/externalversions/ark/v1"
 	listers "github.com/heptio/ark/pkg/generated/listers/ark/v1"
+	"github.com/heptio/ark/pkg/plugin"
 	"github.com/heptio/ark/pkg/restic"
 	"github.com/heptio/ark/pkg/util/kube"
 	"github.com/pkg/errors"
@@ -50,17 +51,17 @@ type backupDeletionController struct {
 	deleteBackupRequestLister listers.DeleteBackupRequestLister
 	backupClient              arkv1client.BackupsGetter
 	blockStore                cloudprovider.BlockStore
-	objectStore               cloudprovider.ObjectStore
-	bucket                    string
 	restoreLister             listers.RestoreLister
 	restoreClient             arkv1client.RestoresGetter
 	backupTracker             BackupTracker
 	resticMgr                 restic.RepositoryManager
 	podvolumeBackupLister     listers.PodVolumeBackupLister
-
+	backupLocationLister      listers.BackupStorageLocationLister
-	deleteBackupDir    cloudprovider.DeleteBackupDirFunc
+	pluginRegistry            plugin.Registry
-	processRequestFunc func(*v1.DeleteBackupRequest) error
+	deleteBackupDir           cloudprovider.DeleteBackupDirFunc
-	clock              clock.Clock
+	processRequestFunc        func(*v1.DeleteBackupRequest) error
+	clock                     clock.Clock
+	newPluginManager          func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager
 }
 
 // NewBackupDeletionController creates a new backup deletion controller.
@@ -70,13 +71,13 @@ func NewBackupDeletionController(
 	deleteBackupRequestClient arkv1client.DeleteBackupRequestsGetter,
 	backupClient arkv1client.BackupsGetter,
 	blockStore cloudprovider.BlockStore,
-	objectStore cloudprovider.ObjectStore,
-	bucket string,
 	restoreInformer informers.RestoreInformer,
 	restoreClient arkv1client.RestoresGetter,
 	backupTracker BackupTracker,
 	resticMgr restic.RepositoryManager,
 	podvolumeBackupInformer informers.PodVolumeBackupInformer,
+	backupLocationInformer informers.BackupStorageLocationInformer,
+	pluginRegistry plugin.Registry,
 ) Interface {
 	c := &backupDeletionController{
 		genericController:         newGenericController("backup-deletion", logger),
@@ -84,16 +85,20 @@ func NewBackupDeletionController(
 		deleteBackupRequestLister: deleteBackupRequestInformer.Lister(),
 		backupClient:              backupClient,
 		blockStore:                blockStore,
-		objectStore:               objectStore,
-		bucket:                    bucket,
 		restoreLister:             restoreInformer.Lister(),
 		restoreClient:             restoreClient,
 		backupTracker:             backupTracker,
 		resticMgr:                 resticMgr,
+		podvolumeBackupLister:     podvolumeBackupInformer.Lister(),
+		backupLocationLister:      backupLocationInformer.Lister(),
+		pluginRegistry:            pluginRegistry,
 
-		podvolumeBackupLister: podvolumeBackupInformer.Lister(),
+		// use variables to refer to these functions so they can be
-		deleteBackupDir:       cloudprovider.DeleteBackupDir,
+		// replaced with fakes for testing.
-		clock:                 &clock.RealClock{},
+		deleteBackupDir:  cloudprovider.DeleteBackupDir,
+		newPluginManager: plugin.NewManager,
+
+		clock: &clock.RealClock{},
 	}
 
 	c.syncHandler = c.processQueueItem
@@ -102,6 +107,7 @@ func NewBackupDeletionController(
 		deleteBackupRequestInformer.Informer().HasSynced,
 		restoreInformer.Informer().HasSynced,
 		podvolumeBackupInformer.Informer().HasSynced,
+		backupLocationInformer.Informer().HasSynced,
 	)
 	c.processRequestFunc = c.processRequest
 
@@ -240,7 +246,6 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 
 	var errs []string
 
-	// Try to delete snapshots
 	log.Info("Removing PV snapshots")
 	for _, volumeBackup := range backup.Status.VolumeBackups {
 		log.WithField("snapshotID", volumeBackup.SnapshotID).Info("Removing snapshot associated with backup")
@@ -249,7 +254,6 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 		}
 	}
 
-	// Try to delete restic snapshots
 	log.Info("Removing restic snapshots")
 	if deleteErrs := c.deleteResticSnapshots(backup); len(deleteErrs) > 0 {
 		for _, err := range deleteErrs {
@@ -257,13 +261,11 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 		}
 	}
 
-	// Try to delete backup from backup storage
 	log.Info("Removing backup from backup storage")
-	if err := c.deleteBackupDir(log, c.objectStore, c.bucket, backup.Name); err != nil {
+	if err := c.deleteBackupFromStorage(backup, log); err != nil {
-		errs = append(errs, errors.Wrap(err, "error deleting backup from backup storage").Error())
+		errs = append(errs, err.Error())
 	}
 
-	// Try to delete restores
 	log.Info("Removing restores")
 	if restores, err := c.restoreLister.Restores(backup.Namespace).List(labels.Everything()); err != nil {
 		log.WithError(errors.WithStack(err)).Error("Error listing restore API objects")
@@ -312,6 +314,27 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 	return nil
 }
 
+func (c *backupDeletionController) deleteBackupFromStorage(backup *v1.Backup, log *logrus.Entry) error {
+	pluginManager := c.newPluginManager(log, log.Level, c.pluginRegistry)
+	defer pluginManager.CleanupClients()
+
+	backupLocation, err := c.backupLocationLister.BackupStorageLocations(backup.Namespace).Get(backup.Spec.StorageLocation)
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
+	objectStore, err := getObjectStoreForLocation(backupLocation, pluginManager)
+	if err != nil {
+		return err
+	}
+
+	if err := c.deleteBackupDir(log, objectStore, backupLocation.Spec.ObjectStorage.Bucket, backup.Name); err != nil {
+		return errors.Wrap(err, "error deleting backup from backup storage")
+	}
+
+	return nil
+}
+
 func (c *backupDeletionController) deleteExistingDeletionRequests(req *v1.DeleteBackupRequest, log logrus.FieldLogger) []error {
 	log.Info("Removing existing deletion requests for backup")
 	selector := labels.SelectorFromSet(labels.Set(map[string]string{

pkg/controller/backup_deletion_controller_test.go

@@ -26,10 +26,13 @@ import (
 	"github.com/heptio/ark/pkg/cloudprovider"
 	"github.com/heptio/ark/pkg/generated/clientset/versioned/fake"
 	informers "github.com/heptio/ark/pkg/generated/informers/externalversions"
+	"github.com/heptio/ark/pkg/plugin"
+	pluginmocks "github.com/heptio/ark/pkg/plugin/mocks"
 	arktest "github.com/heptio/ark/pkg/util/test"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -49,13 +52,13 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) {
 		client.ArkV1(), // deleteBackupRequestClient
 		client.ArkV1(), // backupClient
 		nil,            // blockStore
-		nil,            // backupService
-		"bucket",
 		sharedInformers.Ark().V1().Restores(),
 		client.ArkV1(), // restoreClient
 		NewBackupTracker(),
 		nil, // restic repository manager
 		sharedInformers.Ark().V1().PodVolumeBackups(),
+		sharedInformers.Ark().V1().BackupStorageLocations(),
+		nil, // pluginRegistry
 	).(*backupDeletionController)
 
 	// Error splitting key
@@ -109,37 +112,51 @@ type backupDeletionControllerTestData struct {
 	client          *fake.Clientset
 	sharedInformers informers.SharedInformerFactory
 	blockStore      *arktest.FakeBlockStore
+	objectStore     *arktest.ObjectStore
 	controller      *backupDeletionController
 	req             *v1.DeleteBackupRequest
 }
 
 func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletionControllerTestData {
-	client := fake.NewSimpleClientset(objects...)
+	var (
-	sharedInformers := informers.NewSharedInformerFactory(client, 0)
+		client          = fake.NewSimpleClientset(objects...)
-	blockStore := &arktest.FakeBlockStore{SnapshotsTaken: sets.NewString()}
+		sharedInformers = informers.NewSharedInformerFactory(client, 0)
-	req := pkgbackup.NewDeleteBackupRequest("foo", "uid")
+		blockStore      = &arktest.FakeBlockStore{SnapshotsTaken: sets.NewString()}
+		pluginManager   = &pluginmocks.Manager{}
+		objectStore     = &arktest.ObjectStore{}
+		req             = pkgbackup.NewDeleteBackupRequest("foo", "uid")
+	)
 
 	data := &backupDeletionControllerTestData{
 		client:          client,
 		sharedInformers: sharedInformers,
 		blockStore:      blockStore,
+		objectStore:     objectStore,
 		controller: NewBackupDeletionController(
 			arktest.NewLogger(),
 			sharedInformers.Ark().V1().DeleteBackupRequests(),
 			client.ArkV1(), // deleteBackupRequestClient
 			client.ArkV1(), // backupClient
 			blockStore,
-			nil, // objectStore
-			"bucket",
 			sharedInformers.Ark().V1().Restores(),
 			client.ArkV1(), // restoreClient
 			NewBackupTracker(),
 			nil, // restic repository manager
 			sharedInformers.Ark().V1().PodVolumeBackups(),
+			sharedInformers.Ark().V1().BackupStorageLocations(),
+			nil, // pluginRegistry
 		).(*backupDeletionController),
 
 		req: req,
 	}
+
+	data.controller.newPluginManager = func(_ logrus.FieldLogger, _ logrus.Level, _ plugin.Registry) plugin.Manager {
+		return pluginManager
+	}
+
+	pluginManager.On("GetObjectStore", "objStoreProvider").Return(objectStore, nil)
+	pluginManager.On("CleanupClients").Return(nil)
+
 	req.Namespace = "heptio-ark"
 	req.Name = "foo-abcde"
 
@@ -347,6 +364,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
 	t.Run("full delete, no errors", func(t *testing.T) {
 		backup := arktest.NewTestBackup().WithName("foo").WithSnapshot("pv-1", "snap-1").Backup
 		backup.UID = "uid"
+		backup.Spec.StorageLocation = "primary"
 
 		restore1 := arktest.NewTestRestore("heptio-ark", "restore-1", v1.RestorePhaseCompleted).WithBackup("foo").Restore
 		restore2 := arktest.NewTestRestore("heptio-ark", "restore-2", v1.RestorePhaseCompleted).WithBackup("foo").Restore
@@ -358,6 +376,24 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
 		td.sharedInformers.Ark().V1().Restores().Informer().GetStore().Add(restore2)
 		td.sharedInformers.Ark().V1().Restores().Informer().GetStore().Add(restore3)
 
+		location := &v1.BackupStorageLocation{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: backup.Namespace,
+				Name:      backup.Spec.StorageLocation,
+			},
+			Spec: v1.BackupStorageLocationSpec{
+				Provider: "objStoreProvider",
+				StorageType: v1.StorageType{
+					ObjectStorage: &v1.ObjectStorageLocation{
+						Bucket: "bucket",
+					},
+				},
+			},
+		}
+		require.NoError(t, td.sharedInformers.Ark().V1().BackupStorageLocations().Informer().GetStore().Add(location))
+
+		td.objectStore.On("Init", mock.Anything).Return(nil)
+
 		// Clear out req labels to make sure the controller adds them
 		td.req.Labels = make(map[string]string)
 
@@ -374,8 +410,9 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
 			return true, backup, nil
 		})
 
-		td.controller.deleteBackupDir = func(_ logrus.FieldLogger, _ cloudprovider.ObjectStore, bucket, backupName string) error {
+		td.controller.deleteBackupDir = func(_ logrus.FieldLogger, objectStore cloudprovider.ObjectStore, bucket, backupName string) error {
-			require.Equal(t, "bucket", bucket)
+			require.NotNil(t, objectStore)
+			require.Equal(t, location.Spec.ObjectStorage.Bucket, bucket)
 			require.Equal(t, td.req.Spec.BackupName, backupName)
 			return nil
 		}
@@ -561,13 +598,13 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) {
 				client.ArkV1(), // deleteBackupRequestClient
 				client.ArkV1(), // backupClient
 				nil,            // blockStore
-				nil,            // backupService
-				"bucket",
 				sharedInformers.Ark().V1().Restores(),
 				client.ArkV1(), // restoreClient
 				NewBackupTracker(),
 				nil,
 				sharedInformers.Ark().V1().PodVolumeBackups(),
+				sharedInformers.Ark().V1().BackupStorageLocations(),
+				nil, // pluginRegistry
 			).(*backupDeletionController)
 
 			fakeClock := &clock.FakeClock{}