update backup deletion controller for snapshot locations
Signed-off-by: Steve Kriss <steve@heptio.com>pull/948/head
parent
38c72b8cc2
commit
4a03370f1d
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
@ -70,13 +70,13 @@ 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{
|
||||
|
@ -84,13 +84,13 @@ func NewBackupDeletionController(
|
|||
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.
|
||||
|
@ -107,6 +107,7 @@ func NewBackupDeletionController(
|
|||
restoreInformer.Informer().HasSynced,
|
||||
podvolumeBackupInformer.Informer().HasSynced,
|
||||
backupLocationInformer.Informer().HasSynced,
|
||||
snapshotLocationInformer.Informer().HasSynced,
|
||||
)
|
||||
c.processRequestFunc = c.processRequest
|
||||
|
||||
|
@ -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
|
||||
|
@ -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())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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())
|
||||
}
|
||||
|
@ -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 {
|
||||
|
|
|
@ -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) {
|
||||
|
@ -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)
|
||||
|
||||
|
@ -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),
|
||||
|
||||
|
@ -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"
|
||||
|
||||
|
@ -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)
|
||||
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
||||
|
|
Loading…
Reference in New Issue