Skip to content

Commit

Permalink
update backup deletion controller for snapshot locations
Browse files Browse the repository at this point in the history 
Signed-off-by: Steve Kriss <[email protected]>
  • Loading branch information
@skriss
skriss committed Oct 17, 2018
1 parent 38c72b8 commit 4a03370
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 70 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,13 +697,13 @@ func (s *server) runControllers(config *api.Config, defaultVolumeSnapshotLocatio
s.sharedInformerFactory.Ark().V1().DeleteBackupRequests(),
s.arkClient.ArkV1(), // deleteBackupRequestClient
s.arkClient.ArkV1(), // backupClient
nil,
s.sharedInformerFactory.Ark().V1().Restores(),
s.arkClient.ArkV1(), // restoreClient
backupTracker,
s.resticManager,
s.sharedInformerFactory.Ark().V1().PodVolumeBackups(),
s.sharedInformerFactory.Ark().V1().BackupStorageLocations(),
s.sharedInformerFactory.Ark().V1().VolumeSnapshotLocations(),
newPluginManager,
)
wg.Add(1)
Expand Down
81 changes: 55 additions & 26 deletions pkg/controller/backup_deletion_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ type backupDeletionController struct {
deleteBackupRequestClient arkv1client.DeleteBackupRequestsGetter
deleteBackupRequestLister listers.DeleteBackupRequestLister
backupClient arkv1client.BackupsGetter
blockStore cloudprovider.BlockStore
restoreLister listers.RestoreLister
restoreClient arkv1client.RestoresGetter
backupTracker BackupTracker
resticMgr restic.RepositoryManager
podvolumeBackupLister listers.PodVolumeBackupLister
backupLocationLister listers.BackupStorageLocationLister
snapshotLocationLister listers.VolumeSnapshotLocationLister
processRequestFunc func(*v1.DeleteBackupRequest) error
clock clock.Clock
newPluginManager func(logrus.FieldLogger) plugin.Manager
Expand All @@ -70,27 +70,27 @@ func NewBackupDeletionController(
deleteBackupRequestInformer informers.DeleteBackupRequestInformer,
deleteBackupRequestClient arkv1client.DeleteBackupRequestsGetter,
backupClient arkv1client.BackupsGetter,
blockStore cloudprovider.BlockStore,
restoreInformer informers.RestoreInformer,
restoreClient arkv1client.RestoresGetter,
backupTracker BackupTracker,
resticMgr restic.RepositoryManager,
podvolumeBackupInformer informers.PodVolumeBackupInformer,
backupLocationInformer informers.BackupStorageLocationInformer,
snapshotLocationInformer informers.VolumeSnapshotLocationInformer,
newPluginManager func(logrus.FieldLogger) plugin.Manager,
) Interface {
c := &backupDeletionController{
genericController: newGenericController("backup-deletion", logger),
deleteBackupRequestClient: deleteBackupRequestClient,
deleteBackupRequestLister: deleteBackupRequestInformer.Lister(),
backupClient: backupClient,
blockStore: blockStore,
restoreLister: restoreInformer.Lister(),
restoreClient: restoreClient,
backupTracker: backupTracker,
resticMgr: resticMgr,
podvolumeBackupLister: podvolumeBackupInformer.Lister(),
backupLocationLister: backupLocationInformer.Lister(),
snapshotLocationLister: snapshotLocationInformer.Lister(),

// use variables to refer to these functions so they can be
// replaced with fakes for testing.
Expand All @@ -107,6 +107,7 @@ func NewBackupDeletionController(
restoreInformer.Informer().HasSynced,
podvolumeBackupInformer.Informer().HasSynced,
backupLocationInformer.Informer().HasSynced,
snapshotLocationInformer.Informer().HasSynced,
)
c.processRequestFunc = c.processRequest

Expand Down Expand Up @@ -223,17 +224,6 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
}
}

// If the backup includes snapshots but we don't currently have a PVProvider, we don't
// want to orphan the snapshots so skip deletion.
if c.blockStore == nil && len(backup.Status.VolumeBackups) > 0 {
req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) {
r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed
r.Status.Errors = []string{"unable to delete backup because it includes PV snapshots and Ark is not configured with a PersistentVolumeProvider"}
})

return err
}

// Set backup status to Deleting
backup, err = c.patchBackup(backup, func(b *v1.Backup) {
b.Status.Phase = v1.BackupPhaseDeleting
Expand All @@ -245,11 +235,36 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e

var errs []string

pluginManager := c.newPluginManager(log)
defer pluginManager.CleanupClients()

backupStore, backupStoreErr := c.backupStoreForBackup(backup, pluginManager, log)
if backupStoreErr != nil {
errs = append(errs, backupStoreErr.Error())
// TODO need to not proceed since backupStore will be nil
}

log.Info("Removing PV snapshots")
for _, volumeBackup := range backup.Status.VolumeBackups {
log.WithField("snapshotID", volumeBackup.SnapshotID).Info("Removing snapshot associated with backup")
if err := c.blockStore.DeleteSnapshot(volumeBackup.SnapshotID); err != nil {
errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", volumeBackup.SnapshotID).Error())
if snapshots, err := backupStore.GetBackupVolumeSnapshots(backup.Name); err != nil {
errs = append(errs, errors.Wrap(err, "error getting backup's volume snapshots").Error())
} else {
blockStores := make(map[string]cloudprovider.BlockStore)

for _, snapshot := range snapshots {
log.WithField("providerSnapshotID", snapshot.Status.ProviderSnapshotID).Info("Removing snapshot associated with backup")

blockStore, ok := blockStores[snapshot.Spec.Location]
if !ok {
if blockStore, err = blockStoreForSnapshotLocation(backup.Namespace, snapshot.Spec.Location, c.snapshotLocationLister, pluginManager); err != nil {
errs = append(errs, err.Error())
continue
}
blockStores[snapshot.Spec.Location] = blockStore
}

if err := blockStore.DeleteSnapshot(snapshot.Status.ProviderSnapshotID); err != nil {
errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", snapshot.Status.ProviderSnapshotID).Error())
}
}
}

Expand All @@ -261,14 +276,6 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
}

log.Info("Removing backup from backup storage")
pluginManager := c.newPluginManager(log)
defer pluginManager.CleanupClients()

backupStore, backupStoreErr := c.backupStoreForBackup(backup, pluginManager, log)
if backupStoreErr != nil {
errs = append(errs, backupStoreErr.Error())
}

if err := backupStore.DeleteBackup(backup.Name); err != nil {
errs = append(errs, err.Error())
}
Expand Down Expand Up @@ -328,6 +335,28 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
return nil
}

func blockStoreForSnapshotLocation(
namespace, snapshotLocationName string,
snapshotLocationLister listers.VolumeSnapshotLocationLister,
pluginManager plugin.Manager,
) (cloudprovider.BlockStore, error) {
snapshotLocation, err := snapshotLocationLister.VolumeSnapshotLocations(namespace).Get(snapshotLocationName)
if err != nil {
return nil, errors.Wrapf(err, "error getting volume snapshot location %s", snapshotLocationName)
}

blockStore, err := pluginManager.GetBlockStore(snapshotLocation.Spec.Provider)
if err != nil {
return nil, errors.Wrapf(err, "error getting block store for provider %s", snapshotLocation.Spec.Provider)
}

if err = blockStore.Init(snapshotLocation.Spec.Config); err != nil {
return nil, errors.Wrapf(err, "error initializing block store for volume snapshot location %s", snapshotLocationName)
}

return blockStore, nil
}

func (c *backupDeletionController) backupStoreForBackup(backup *v1.Backup, pluginManager plugin.Manager, log logrus.FieldLogger) (persistence.BackupStore, error) {
backupLocation, err := c.backupLocationLister.BackupStorageLocations(backup.Namespace).Get(backup.Spec.StorageLocation)
if err != nil {
Expand Down
76 changes: 33 additions & 43 deletions pkg/controller/backup_deletion_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/heptio/ark/pkg/plugin"
pluginmocks "github.com/heptio/ark/pkg/plugin/mocks"
arktest "github.com/heptio/ark/pkg/util/test"
"github.com/heptio/ark/pkg/volume"
)

func TestBackupDeletionControllerProcessQueueItem(t *testing.T) {
Expand All @@ -53,13 +54,13 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) {
sharedInformers.Ark().V1().DeleteBackupRequests(),
client.ArkV1(), // deleteBackupRequestClient
client.ArkV1(), // backupClient
nil, // blockStore
sharedInformers.Ark().V1().Restores(),
client.ArkV1(), // restoreClient
NewBackupTracker(),
nil, // restic repository manager
sharedInformers.Ark().V1().PodVolumeBackups(),
sharedInformers.Ark().V1().BackupStorageLocations(),
sharedInformers.Ark().V1().VolumeSnapshotLocations(),
nil, // new plugin manager func
).(*backupDeletionController)

Expand Down Expand Up @@ -139,13 +140,13 @@ func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletio
sharedInformers.Ark().V1().DeleteBackupRequests(),
client.ArkV1(), // deleteBackupRequestClient
client.ArkV1(), // backupClient
blockStore,
sharedInformers.Ark().V1().Restores(),
client.ArkV1(), // restoreClient
NewBackupTracker(),
nil, // restic repository manager
sharedInformers.Ark().V1().PodVolumeBackups(),
sharedInformers.Ark().V1().BackupStorageLocations(),
sharedInformers.Ark().V1().VolumeSnapshotLocations(),
func(logrus.FieldLogger) plugin.Manager { return pluginManager },
).(*backupDeletionController),

Expand Down Expand Up @@ -323,47 +324,8 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
assert.Equal(t, expectedActions, td.client.Actions())
})

t.Run("no block store, backup has snapshots", func(t *testing.T) {
td := setupBackupDeletionControllerTest()
td.controller.blockStore = nil

td.client.PrependReactor("get", "backups", func(action core.Action) (bool, runtime.Object, error) {
backup := arktest.NewTestBackup().WithName("backup-1").WithSnapshot("pv-1", "snap-1").Backup
return true, backup, nil
})

td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) {
return true, td.req, nil
})

err := td.controller.processRequest(td.req)
require.NoError(t, err)

expectedActions := []core.Action{
core.NewPatchAction(
v1.SchemeGroupVersion.WithResource("deletebackuprequests"),
td.req.Namespace,
td.req.Name,
[]byte(`{"status":{"phase":"InProgress"}}`),
),
core.NewGetAction(
v1.SchemeGroupVersion.WithResource("backups"),
td.req.Namespace,
td.req.Spec.BackupName,
),
core.NewPatchAction(
v1.SchemeGroupVersion.WithResource("deletebackuprequests"),
td.req.Namespace,
td.req.Name,
[]byte(`{"status":{"errors":["unable to delete backup because it includes PV snapshots and Ark is not configured with a PersistentVolumeProvider"],"phase":"Processed"}}`),
),
}

assert.Equal(t, expectedActions, td.client.Actions())
})

t.Run("full delete, no errors", func(t *testing.T) {
backup := arktest.NewTestBackup().WithName("foo").WithSnapshot("pv-1", "snap-1").Backup
backup := arktest.NewTestBackup().WithName("foo").Backup
backup.UID = "uid"
backup.Spec.StorageLocation = "primary"

Expand Down Expand Up @@ -393,6 +355,17 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
}
require.NoError(t, td.sharedInformers.Ark().V1().BackupStorageLocations().Informer().GetStore().Add(location))

snapshotLocation := &v1.VolumeSnapshotLocation{
ObjectMeta: metav1.ObjectMeta{
Namespace: backup.Namespace,
Name: "vsl-1",
},
Spec: v1.VolumeSnapshotLocationSpec{
Provider: "provider-1",
},
}
require.NoError(t, td.sharedInformers.Ark().V1().VolumeSnapshotLocations().Informer().GetStore().Add(snapshotLocation))

// Clear out req labels to make sure the controller adds them
td.req.Labels = make(map[string]string)

Expand All @@ -409,6 +382,23 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
return true, backup, nil
})

snapshots := []*volume.Snapshot{
{
Spec: volume.SnapshotSpec{
Location: "vsl-1",
},
Status: volume.SnapshotStatus{
ProviderSnapshotID: "snap-1",
},
},
}

pluginManager := &pluginmocks.Manager{}
pluginManager.On("GetBlockStore", "provider-1").Return(td.blockStore, nil)
pluginManager.On("CleanupClients")
td.controller.newPluginManager = func(logrus.FieldLogger) plugin.Manager { return pluginManager }

td.backupStore.On("GetBackupVolumeSnapshots", td.req.Spec.BackupName).Return(snapshots, nil)
td.backupStore.On("DeleteBackup", td.req.Spec.BackupName).Return(nil)
td.backupStore.On("DeleteRestore", "restore-1").Return(nil)
td.backupStore.On("DeleteRestore", "restore-2").Return(nil)
Expand Down Expand Up @@ -593,13 +583,13 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) {
sharedInformers.Ark().V1().DeleteBackupRequests(),
client.ArkV1(), // deleteBackupRequestClient
client.ArkV1(), // backupClient
nil, // blockStore
sharedInformers.Ark().V1().Restores(),
client.ArkV1(), // restoreClient
NewBackupTracker(),
nil,
sharedInformers.Ark().V1().PodVolumeBackups(),
sharedInformers.Ark().V1().BackupStorageLocations(),
sharedInformers.Ark().V1().VolumeSnapshotLocations(),
nil, // new plugin manager func
).(*backupDeletionController)

Expand Down

0 comments on commit 4a03370

Please sign in to comment.