diff --git a/pkg/apis/ark/v1/backup.go b/pkg/apis/ark/v1/backup.go index 1f0e19653f..a1d1c6c551 100644 --- a/pkg/apis/ark/v1/backup.go +++ b/pkg/apis/ark/v1/backup.go @@ -173,7 +173,12 @@ type BackupStatus struct { // VolumeBackups is a map of PersistentVolume names to // information about the backed-up volume in the cloud // provider API. - VolumeBackups map[string]*VolumeBackupInfo `json:"volumeBackups"` + // + // Deprecated: this field is considered read-only as of v0.10 + // and will be removed in a subsequent release. The information + // previously contained here is now stored in a file in backup + // storage. + VolumeBackups map[string]*VolumeBackupInfo `json:"volumeBackups,omitempty"` // ValidationErrors is a slice of all validation errors (if // applicable). diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index be16ce0573..60b650829a 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -398,21 +398,21 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) { }{ { name: "location name does not correspond to any existing location", - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations([]string{"random-name"}), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations("random-name"), locations: arktest.NewTestVolumeSnapshotLocation().WithName(dupLocationNames[0]).WithProviderConfig(dupLocationList), expectedErrors: "error getting volume snapshot location named random-name: volumesnapshotlocation.ark.heptio.com \"random-name\" not found", expectedSuccess: false, }, { name: "duplicate locationName per provider: should filter out dups", - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(dupLocationNames), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(dupLocationNames...), locations: arktest.NewTestVolumeSnapshotLocation().WithName(dupLocationNames[0]).WithProviderConfig(dupLocationList), expectedVolumeSnapshotLocationNames: []string{dupLocationNames[0]}, expectedSuccess: true, }, { name: "multiple location names per provider", - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(multipleLocationNames), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(multipleLocationNames...), locations: arktest.NewTestVolumeSnapshotLocation().WithName(multipleLocationNames[0]).WithProviderConfig(multipleLocationList), expectedErrors: "more than one VolumeSnapshotLocation name specified for provider aws: aws-us-east-1; unexpected name was aws-us-west-1", expectedSuccess: false, @@ -431,7 +431,7 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) { }, { name: "multiple location names for a provider, default location name for another provider", - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(dupLocationNames), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(dupLocationNames...), locations: arktest.NewTestVolumeSnapshotLocation().WithName(dupLocationNames[0]).WithProviderConfig(dupLocationList), defaultLocations: defaultLocationsFake, expectedVolumeSnapshotLocationNames: []string{dupLocationNames[0], defaultLocationsFake["fake-provider"].Name}, diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index 2abe76c9ad..59e17ebbee 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -245,25 +245,47 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e if backupStore != nil { log.Info("Removing PV snapshots") - 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 + if len(backup.Status.VolumeBackups) > 0 { + // pre-v0.10 backup + locations, err := c.snapshotLocationLister.VolumeSnapshotLocations(backup.Namespace).List(labels.Everything()) + if err != nil { + errs = append(errs, errors.Wrap(err, "error listing volume snapshot locations").Error()) + } else if len(locations) != 1 { + errs = append(errs, errors.Errorf("unable to delete pre-v0.10 volume snapshots because exactly one volume snapshot location must exist, got %d", len(locations)).Error()) + } else { + blockStore, err := blockStoreForSnapshotLocation(backup.Namespace, locations[0].Name, c.snapshotLocationLister, pluginManager) + if err != nil { + errs = append(errs, err.Error()) + } else { + for _, snapshot := range backup.Status.VolumeBackups { + if err := blockStore.DeleteSnapshot(snapshot.SnapshotID); err != nil { + errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", snapshot.SnapshotID).Error()) + } } - blockStores[snapshot.Spec.Location] = blockStore } + } + } else { + // v0.10+ backup + 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()) + if err := blockStore.DeleteSnapshot(snapshot.Status.ProviderSnapshotID); err != nil { + errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", snapshot.Status.ProviderSnapshotID).Error()) + } } } } diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 425e735b4e..072d388f89 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -324,6 +324,139 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { assert.Equal(t, expectedActions, td.client.Actions()) }) + t.Run("pre-v0.10 backup with snapshots, no errors", func(t *testing.T) { + backup := arktest.NewTestBackup().WithName("foo").Backup + backup.UID = "uid" + backup.Spec.StorageLocation = "primary" + backup.Status.VolumeBackups = map[string]*v1.VolumeBackupInfo{ + "pv-1": { + SnapshotID: "snap-1", + }, + } + + restore1 := arktest.NewTestRestore("heptio-ark", "restore-1", v1.RestorePhaseCompleted).WithBackup("foo").Restore + restore2 := arktest.NewTestRestore("heptio-ark", "restore-2", v1.RestorePhaseCompleted).WithBackup("foo").Restore + restore3 := arktest.NewTestRestore("heptio-ark", "restore-3", v1.RestorePhaseCompleted).WithBackup("some-other-backup").Restore + + td := setupBackupDeletionControllerTest(backup, restore1, restore2, restore3) + + td.sharedInformers.Ark().V1().Restores().Informer().GetStore().Add(restore1) + 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)) + + 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) + + td.client.PrependReactor("get", "backups", func(action core.Action) (bool, runtime.Object, error) { + return true, backup, nil + }) + td.blockStore.SnapshotsTaken.Insert("snap-1") + + td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) { + return true, td.req, nil + }) + + td.client.PrependReactor("patch", "backups", func(action core.Action) (bool, runtime.Object, error) { + return true, backup, nil + }) + + 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("DeleteBackup", td.req.Spec.BackupName).Return(nil) + td.backupStore.On("DeleteRestore", "restore-1").Return(nil) + td.backupStore.On("DeleteRestore", "restore-2").Return(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(`{"metadata":{"labels":{"ark.heptio.com/backup-name":"foo"}},"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(`{"metadata":{"labels":{"ark.heptio.com/backup-uid":"uid"}}}`), + ), + core.NewPatchAction( + v1.SchemeGroupVersion.WithResource("backups"), + td.req.Namespace, + td.req.Spec.BackupName, + []byte(`{"status":{"phase":"Deleting"}}`), + ), + core.NewDeleteAction( + v1.SchemeGroupVersion.WithResource("restores"), + td.req.Namespace, + "restore-1", + ), + core.NewDeleteAction( + v1.SchemeGroupVersion.WithResource("restores"), + td.req.Namespace, + "restore-2", + ), + core.NewDeleteAction( + 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":{"phase":"Processed"}}`), + ), + core.NewDeleteCollectionAction( + v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + td.req.Namespace, + pkgbackup.NewDeleteBackupRequestListOptions(td.req.Spec.BackupName, "uid"), + ), + } + + arktest.CompareActions(t, expectedActions, td.client.Actions()) + + // Make sure snapshot was deleted + assert.Equal(t, 0, td.blockStore.SnapshotsTaken.Len()) + }) + t.Run("full delete, no errors", func(t *testing.T) { backup := arktest.NewTestBackup().WithName("foo").Backup backup.UID = "uid" diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index bde359a242..6796b36f09 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -343,6 +343,26 @@ func (c *restoreController) validateAndComplete(restore *api.Restore, pluginMana return backupInfo{} } + // Ensure that we have either .status.volumeBackups (for pre-v0.10 backups) OR a + // volumesnapshots.json.gz file in obj storage (for v0.10+ backups), but not both. + // If we have .status.volumeBackups, ensure that there's only one volume snapshot + // location configured. + if info.backup.Status.VolumeBackups != nil { + snapshots, err := info.backupStore.GetBackupVolumeSnapshots(info.backup.Name) + if err != nil { + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, errors.Wrap(err, "Error checking for volumesnapshots file").Error()) + } else if len(snapshots) > 0 { + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "Backup must not have both .status.volumeBackups and a volumesnapshots.json.gz file in object storage") + } else { + locations, err := c.snapshotLocationLister.VolumeSnapshotLocations(restore.Namespace).List(labels.Everything()) + if err != nil { + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, errors.Wrap(err, "Error listing volume snapshot locations").Error()) + } else if len(locations) > 1 { + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "Cannot restore backup with .status.volumeBackups when more than one volume snapshot location exists") + } + } + } + // Fill in the ScheduleName so it's easier to consume for metrics. if restore.Spec.ScheduleName == "" { restore.Spec.ScheduleName = info.backup.GetLabels()["ark-schedule"] diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 3f307214a7..38db1e1625 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -626,6 +626,114 @@ func TestProcessRestore(t *testing.T) { } } +func TestValidateAndComplete(t *testing.T) { + tests := []struct { + name string + storageLocation *api.BackupStorageLocation + snapshotLocations []*api.VolumeSnapshotLocation + backup *api.Backup + volumeSnapshots []*volume.Snapshot + restore *api.Restore + expectedErrs []string + }{ + { + name: "backup with .status.volumeBackups and no volumesnapshots.json file does not error", + storageLocation: arktest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, + backup: arktest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, + volumeSnapshots: nil, + restore: arktest.NewDefaultTestRestore().WithBackup("backup-1").Restore, + expectedErrs: nil, + }, + { + name: "backup with no .status.volumeBackups and volumesnapshots.json file does not error", + storageLocation: arktest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, + backup: arktest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").Backup, + volumeSnapshots: []*volume.Snapshot{{}}, + restore: arktest.NewDefaultTestRestore().WithBackup("backup-1").Restore, + expectedErrs: nil, + }, + { + name: "backup with both .status.volumeBackups and volumesnapshots.json file errors", + storageLocation: arktest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, + backup: arktest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, + volumeSnapshots: []*volume.Snapshot{{}}, + restore: arktest.NewDefaultTestRestore().WithBackup("backup-1").Restore, + expectedErrs: []string{"Backup must not have both .status.volumeBackups and a volumesnapshots.json.gz file in object storage"}, + }, + { + name: "backup with .status.volumeBackups, and >1 volume snapshot locations exist, errors", + storageLocation: arktest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, + snapshotLocations: []*api.VolumeSnapshotLocation{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: api.DefaultNamespace, + Name: "vsl-1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: api.DefaultNamespace, + Name: "vsl-2", + }, + }, + }, + backup: arktest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, + restore: arktest.NewDefaultTestRestore().WithBackup("backup-1").Restore, + expectedErrs: []string{"Cannot restore backup with .status.volumeBackups when more than one volume snapshot location exists"}, + }, + { + name: "backup with .status.volumeBackups, and 1 volume snapshot location exists, does not error", + storageLocation: arktest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, + snapshotLocations: []*api.VolumeSnapshotLocation{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: api.DefaultNamespace, + Name: "vsl-1", + }, + }, + }, + backup: arktest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, + restore: arktest.NewDefaultTestRestore().WithBackup("backup-1").Restore, + expectedErrs: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ( + clientset = fake.NewSimpleClientset() + sharedInformers = informers.NewSharedInformerFactory(clientset, 0) + logger = arktest.NewLogger() + backupStore = &persistencemocks.BackupStore{} + controller = &restoreController{ + genericController: &genericController{ + logger: logger, + }, + namespace: api.DefaultNamespace, + backupLister: sharedInformers.Ark().V1().Backups().Lister(), + backupLocationLister: sharedInformers.Ark().V1().BackupStorageLocations().Lister(), + snapshotLocationLister: sharedInformers.Ark().V1().VolumeSnapshotLocations().Lister(), + newBackupStore: func(*api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) { + return backupStore, nil + }, + } + ) + + require.NoError(t, sharedInformers.Ark().V1().BackupStorageLocations().Informer().GetStore().Add(tc.storageLocation)) + require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(tc.backup)) + for _, loc := range tc.snapshotLocations { + require.NoError(t, sharedInformers.Ark().V1().VolumeSnapshotLocations().Informer().GetStore().Add(loc)) + } + backupStore.On("GetBackupVolumeSnapshots", tc.backup.Name).Return(tc.volumeSnapshots, nil) + + controller.validateAndComplete(tc.restore, nil) + + assert.Equal(t, tc.expectedErrs, tc.restore.Status.ValidationErrors) + }) + } + +} + func TestvalidateAndCompleteWhenScheduleNameSpecified(t *testing.T) { var ( client = fake.NewSimpleClientset() diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 0e39b51947..844b7faa62 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -237,8 +237,7 @@ func (kr *kubernetesRestorer) Restore( pvRestorer := &pvRestorer{ logger: log, - backupName: restore.Spec.BackupName, - backupNamespace: backup.Namespace, + backup: backup, snapshotVolumes: backup.Spec.SnapshotVolumes, restorePVs: restore.Spec.RestorePVs, volumeSnapshots: volumeSnapshots, @@ -907,8 +906,7 @@ type PVRestorer interface { type pvRestorer struct { logger logrus.FieldLogger - backupName string - backupNamespace string + backup *api.Backup snapshotVolumes *bool restorePVs *bool volumeSnapshots []*volume.Snapshot @@ -916,6 +914,66 @@ type pvRestorer struct { snapshotLocationLister listers.VolumeSnapshotLocationLister } +type snapshotInfo struct { + providerSnapshotID string + volumeType string + volumeAZ string + volumeIOPS *int64 + location *api.VolumeSnapshotLocation +} + +func getSnapshotInfo(pvName string, backup *api.Backup, volumeSnapshots []*volume.Snapshot, snapshotLocationLister listers.VolumeSnapshotLocationLister) (*snapshotInfo, error) { + // pre-v0.10 backup + if backup.Status.VolumeBackups != nil { + volumeBackup := backup.Status.VolumeBackups[pvName] + if volumeBackup == nil { + return nil, nil + } + + locations, err := snapshotLocationLister.VolumeSnapshotLocations(backup.Namespace).List(labels.Everything()) + if err != nil { + return nil, errors.WithStack(err) + } + if len(locations) != 1 { + return nil, errors.Errorf("unable to restore pre-v0.10 volume snapshot because exactly one volume snapshot location must exist, got %d", len(locations)) + } + + return &snapshotInfo{ + providerSnapshotID: volumeBackup.SnapshotID, + volumeType: volumeBackup.Type, + volumeAZ: volumeBackup.AvailabilityZone, + volumeIOPS: volumeBackup.Iops, + location: locations[0], + }, nil + } + + // v0.10+ backup + var pvSnapshot *volume.Snapshot + for _, snapshot := range volumeSnapshots { + if snapshot.Spec.PersistentVolumeName == pvName { + pvSnapshot = snapshot + break + } + } + + if pvSnapshot == nil { + return nil, nil + } + + loc, err := snapshotLocationLister.VolumeSnapshotLocations(backup.Namespace).Get(pvSnapshot.Spec.Location) + if err != nil { + return nil, errors.WithStack(err) + } + + return &snapshotInfo{ + providerSnapshotID: pvSnapshot.Status.ProviderSnapshotID, + volumeType: pvSnapshot.Spec.VolumeType, + volumeAZ: pvSnapshot.Spec.VolumeAZ, + volumeIOPS: pvSnapshot.Spec.VolumeIOPS, + location: loc, + }, nil +} + func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { pvName := obj.GetName() if pvName == "" { @@ -942,40 +1000,31 @@ func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructu log := r.logger.WithFields(logrus.Fields{"persistentVolume": pvName}) - var foundSnapshot *volume.Snapshot - for _, snapshot := range r.volumeSnapshots { - if snapshot.Spec.PersistentVolumeName == pvName { - foundSnapshot = snapshot - break - } + snapshotInfo, err := getSnapshotInfo(pvName, r.backup, r.volumeSnapshots, r.snapshotLocationLister) + if err != nil { + return nil, err } - if foundSnapshot == nil { - log.Info("skipping no snapshot found") + if snapshotInfo == nil { + log.Infof("No snapshot found for persistent volume") return obj, nil } - location, err := r.snapshotLocationLister.VolumeSnapshotLocations(r.backupNamespace).Get(foundSnapshot.Spec.Location) - if err != nil { - return nil, errors.WithStack(err) - } - - blockStore, err := r.blockStoreGetter.GetBlockStore(location.Spec.Provider) + blockStore, err := r.blockStoreGetter.GetBlockStore(snapshotInfo.location.Spec.Provider) if err != nil { return nil, errors.WithStack(err) } - if err := blockStore.Init(location.Spec.Config); err != nil { + if err := blockStore.Init(snapshotInfo.location.Spec.Config); err != nil { return nil, errors.WithStack(err) } - volumeID, err := blockStore.CreateVolumeFromSnapshot(foundSnapshot.Status.ProviderSnapshotID, foundSnapshot.Spec.VolumeType, foundSnapshot.Spec.VolumeAZ, foundSnapshot.Spec.VolumeIOPS) + volumeID, err := blockStore.CreateVolumeFromSnapshot(snapshotInfo.providerSnapshotID, snapshotInfo.volumeType, snapshotInfo.volumeAZ, snapshotInfo.volumeIOPS) if err != nil { return nil, errors.WithStack(err) } - log = log.WithFields(logrus.Fields{"snapshot": foundSnapshot.Status.ProviderSnapshotID}) - log.Info("successfully restored persistent volume from snapshot") - // used to be update1 which is then cast to Unstructured and returned + log.WithField("providerSnapshotID", snapshotInfo.providerSnapshotID).Info("successfully restored persistent volume from snapshot") + updated1, err := blockStore.SetVolumeID(obj, volumeID) if err != nil { return nil, errors.WithStack(err) diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 48069807c4..c7ff2985c0 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -23,6 +23,7 @@ import ( api "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/cloudprovider" + cloudprovidermocks "github.com/heptio/ark/pkg/cloudprovider/mocks" "github.com/heptio/ark/pkg/generated/clientset/versioned/fake" informers "github.com/heptio/ark/pkg/generated/informers/externalversions" "github.com/heptio/ark/pkg/kuberesource" @@ -635,6 +636,7 @@ func TestRestoreResourceForNamespace(t *testing.T) { volumeID: "volume-1", }, snapshotLocationLister: snapshotLocationLister, + backup: &api.Backup{}, }, } @@ -1190,31 +1192,31 @@ func TestIsCompleted(t *testing.T) { } } -func TestExecutePVAction(t *testing.T) { - iops := int64(1000) - - locationsFake := map[string]*api.VolumeSnapshotLocation{ - "default": arktest.NewTestVolumeSnapshotLocation().WithName("default-name").VolumeSnapshotLocation, - } - - var locations []string - for key := range locationsFake { - locations = append(locations, key) +func newSnapshot(pvName, location, volumeType, volumeAZ, snapshotID string, volumeIOPS int64) *volume.Snapshot { + return &volume.Snapshot{ + Spec: volume.SnapshotSpec{ + PersistentVolumeName: pvName, + Location: location, + VolumeType: volumeType, + VolumeAZ: volumeAZ, + VolumeIOPS: &volumeIOPS, + }, + Status: volume.SnapshotStatus{ + ProviderSnapshotID: snapshotID, + }, } +} +func TestExecutePVAction_NoSnapshotRestores(t *testing.T) { tests := []struct { - name string - obj *unstructured.Unstructured - restore *api.Restore - backup *arktest.TestBackup - volumeSnapshots []*volume.Snapshot - locations map[string]*api.VolumeSnapshotLocation - volumeMap map[api.VolumeBackupInfo]string - noBlockStore bool - expectedErr bool - expectedRes *unstructured.Unstructured - volumeID string - expectSetVolumeID bool + name string + obj *unstructured.Unstructured + restore *api.Restore + backup *api.Backup + volumeSnapshots []*volume.Snapshot + locations []*api.VolumeSnapshotLocation + expectedErr bool + expectedRes *unstructured.Unstructured }{ { name: "no name should error", @@ -1232,145 +1234,209 @@ func TestExecutePVAction(t *testing.T) { name: "ensure spec.claimRef, spec.storageClassName are deleted", obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "storageClassName", "someOtherField").Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(false).Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithVolumeSnapshotLocations(locations), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).Backup, expectedRes: NewTestUnstructured().WithAnnotations("a", "b").WithName("pv-1").WithSpec("someOtherField").Unstructured, }, { name: "if backup.spec.snapshotVolumes is false, ignore restore.spec.restorePVs and return early", obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "storageClassName", "someOtherField").Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithSnapshotVolumes(false), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithSnapshotVolumes(false).Backup, expectedRes: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("someOtherField").Unstructured, }, { - name: "not restoring, return early", + name: "restore.spec.restorePVs=false, return early", obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(false).Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithVolumeSnapshotLocations(locations), + backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).Backup, volumeSnapshots: []*volume.Snapshot{ - { - Spec: volume.SnapshotSpec{BackupName: "backup1", Location: "default-name", ProviderVolumeID: "volume-1", PersistentVolumeName: "pv-1", VolumeType: "gp", VolumeIOPS: &iops}, - Status: volume.SnapshotStatus{ProviderSnapshotID: "snap-1"}, - }, + newSnapshot("pv-1", "loc-1", "gp", "az-1", "snap-1", 1000), + }, + locations: []*api.VolumeSnapshotLocation{ + arktest.NewTestVolumeSnapshotLocation().WithName("loc-1").VolumeSnapshotLocation, }, - locations: locationsFake, expectedErr: false, expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, }, { - name: "restoring, return without error if there is no PV->BackupInfo map", - obj: NewTestUnstructured().WithName("pv-1").WithSpec("xyz").Unstructured, + name: "backup.status.volumeBackups non-nil and no entry for PV: return early", + obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithVolumeSnapshotLocations(locations), - locations: locationsFake, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec("xyz").Unstructured, + backup: arktest.NewTestBackup().WithName("backup-1").WithSnapshot("non-matching-pv", "snap").Backup, + expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, }, { - name: "restoring, return early if there is PV->BackupInfo map but no entry for this PV", - obj: NewTestUnstructured().WithName("pv-1").WithSpec("xyz").Unstructured, + name: "backup.status.volumeBackups has entry for PV, >1 VSLs configured: return error", + obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithVolumeSnapshotLocations(locations), - volumeSnapshots: []*volume.Snapshot{ - { - Spec: volume.SnapshotSpec{BackupName: "backup1", Location: "default-name", ProviderVolumeID: "volume-1", PersistentVolumeName: "another-pv", VolumeType: "gp", VolumeIOPS: &iops}, - Status: volume.SnapshotStatus{ProviderSnapshotID: "another-snap-1"}, - }, + backup: arktest.NewTestBackup().WithName("backup-1").WithSnapshot("pv-1", "snap").Backup, + locations: []*api.VolumeSnapshotLocation{ + arktest.NewTestVolumeSnapshotLocation().WithName("loc-1").VolumeSnapshotLocation, + arktest.NewTestVolumeSnapshotLocation().WithName("loc-2").VolumeSnapshotLocation, }, - locations: locationsFake, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec("xyz").Unstructured, + expectedErr: true, }, { - name: "volume type and IOPS are correctly passed to CreateVolume", - obj: NewTestUnstructured().WithName("pv-1").WithSpec("xyz").Unstructured, - restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithVolumeSnapshotLocations(locations), - locations: locationsFake, - volumeSnapshots: []*volume.Snapshot{ - { - Spec: volume.SnapshotSpec{BackupName: "backup1", Location: "default-name", ProviderVolumeID: "volume-1", PersistentVolumeName: "pv-1", VolumeType: "gp", VolumeIOPS: &iops}, - Status: volume.SnapshotStatus{ProviderSnapshotID: "snap-1"}, - }, + name: "volumeSnapshots is empty: return early", + obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, + restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, + backup: arktest.NewTestBackup().WithName("backup-1").Backup, + locations: []*api.VolumeSnapshotLocation{ + arktest.NewTestVolumeSnapshotLocation().WithName("loc-1").VolumeSnapshotLocation, + arktest.NewTestVolumeSnapshotLocation().WithName("loc-2").VolumeSnapshotLocation, }, - volumeMap: map[api.VolumeBackupInfo]string{{SnapshotID: "snap-1", Type: "gp", Iops: &iops}: "volume-1"}, - volumeID: "volume-1", - expectedErr: false, - expectSetVolumeID: true, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec("xyz").Unstructured, + volumeSnapshots: []*volume.Snapshot{}, + expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, }, { - name: "restoring, blockStore=nil, backup has at least 1 snapshot -> error", - obj: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, - restore: arktest.NewDefaultTestRestore().Restore, - backup: arktest.NewTestBackup().WithName("backup1").WithPhase(api.BackupPhaseInProgress).WithVolumeSnapshotLocations(locations), + name: "volumeSnapshots doesn't have a snapshot for PV: return early", + obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, + restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, + backup: arktest.NewTestBackup().WithName("backup-1").Backup, + locations: []*api.VolumeSnapshotLocation{ + arktest.NewTestVolumeSnapshotLocation().WithName("loc-1").VolumeSnapshotLocation, + arktest.NewTestVolumeSnapshotLocation().WithName("loc-2").VolumeSnapshotLocation, + }, volumeSnapshots: []*volume.Snapshot{ - { - Spec: volume.SnapshotSpec{BackupName: "backup1", Location: "default-name", ProviderVolumeID: "volume-1", PersistentVolumeName: "pv-1", VolumeType: "gp", VolumeIOPS: &iops}, - Status: volume.SnapshotStatus{ProviderSnapshotID: "snap-1"}, - }, + newSnapshot("non-matching-pv-1", "loc-1", "type-1", "az-1", "snap-1", 1), + newSnapshot("non-matching-pv-2", "loc-2", "type-2", "az-2", "snap-2", 2), }, - noBlockStore: true, - expectedErr: true, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, + expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { var ( - blockStoreGetter BlockStoreGetter - testBlockStoreGetter *fakeBlockStoreGetter - - client = fake.NewSimpleClientset() - sharedInformers = informers.NewSharedInformerFactory(client, 0) - snapshotLocationLister = sharedInformers.Ark().V1().VolumeSnapshotLocations().Lister() + client = fake.NewSimpleClientset() + snapshotLocationInformer = informers.NewSharedInformerFactory(client, 0).Ark().V1().VolumeSnapshotLocations() ) - if !test.noBlockStore { - testBlockStoreGetter = &fakeBlockStoreGetter{ - volumeMap: test.volumeMap, - volumeID: test.volumeID, - } - testBlockStoreGetter.GetBlockStore("default") - blockStoreGetter = testBlockStoreGetter - for _, location := range test.locations { - require.NoError(t, sharedInformers.Ark().V1().VolumeSnapshotLocations().Informer().GetStore().Add(location)) - } - } else { - assert.Equal(t, nil, blockStoreGetter) + r := &pvRestorer{ + logger: arktest.NewLogger(), + restorePVs: tc.restore.Spec.RestorePVs, + snapshotLocationLister: snapshotLocationInformer.Lister(), + } + if tc.backup != nil { + r.backup = tc.backup + r.snapshotVolumes = tc.backup.Spec.SnapshotVolumes } - r := &pvRestorer{ - logger: logging.DefaultLogger(logrus.DebugLevel), - backupName: "backup1", - backupNamespace: api.DefaultNamespace, - restorePVs: test.restore.Spec.RestorePVs, - snapshotLocationLister: snapshotLocationLister, - blockStoreGetter: blockStoreGetter, + for _, loc := range tc.locations { + require.NoError(t, snapshotLocationInformer.Informer().GetStore().Add(loc)) } - if test.backup != nil { - backup := test.backup.DeepCopy() - backup.Spec.VolumeSnapshotLocations = test.backup.Spec.VolumeSnapshotLocations - r.snapshotVolumes = test.backup.Spec.SnapshotVolumes - r.volumeSnapshots = test.volumeSnapshots + res, err := r.executePVAction(tc.obj) + switch tc.expectedErr { + case true: + assert.Nil(t, res) + assert.NotNil(t, err) + case false: + assert.Equal(t, tc.expectedRes, res) + assert.Nil(t, err) } + }) + } +} - res, err := r.executePVAction(test.obj) +func int64Ptr(val int) *int64 { + r := int64(val) + return &r +} + +type providerToBlockStoreMap map[string]cloudprovider.BlockStore + +func (g providerToBlockStoreMap) GetBlockStore(provider string) (cloudprovider.BlockStore, error) { + if bs, ok := g[provider]; !ok { + return nil, errors.New("block store not found for provider") + } else { + return bs, nil + } +} - if test.expectedErr { - require.Error(t, err) - return +func TestExecutePVAction_SnapshotRestores(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + restore *api.Restore + backup *api.Backup + volumeSnapshots []*volume.Snapshot + locations []*api.VolumeSnapshotLocation + expectedProvider string + expectedSnapshotID string + expectedVolumeType string + expectedVolumeAZ string + expectedVolumeIOPS *int64 + expectedSnapshot *volume.Snapshot + }{ + { + name: "pre-v0.10 backup with .status.volumeBackups with entry for PV and single VSL executes restore", + obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, + restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, + backup: arktest.NewTestBackup().WithName("backup-1"). + WithVolumeBackupInfo("pv-1", "snap-1", "type-1", "az-1", int64Ptr(1)). + WithVolumeBackupInfo("pv-2", "snap-2", "type-2", "az-2", int64Ptr(2)). + Backup, + locations: []*api.VolumeSnapshotLocation{ + arktest.NewTestVolumeSnapshotLocation().WithName("loc-1").WithProvider("provider-1").VolumeSnapshotLocation, + }, + expectedProvider: "provider-1", + expectedSnapshotID: "snap-1", + expectedVolumeType: "type-1", + expectedVolumeAZ: "az-1", + expectedVolumeIOPS: int64Ptr(1), + }, + { + name: "v0.10+ backup with a matching volume.Snapshot for PV executes restore", + obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, + restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, + backup: arktest.NewTestBackup().WithName("backup-1").Backup, + locations: []*api.VolumeSnapshotLocation{ + arktest.NewTestVolumeSnapshotLocation().WithName("loc-1").WithProvider("provider-1").VolumeSnapshotLocation, + arktest.NewTestVolumeSnapshotLocation().WithName("loc-2").WithProvider("provider-2").VolumeSnapshotLocation, + }, + volumeSnapshots: []*volume.Snapshot{ + newSnapshot("pv-1", "loc-1", "type-1", "az-1", "snap-1", 1), + newSnapshot("pv-2", "loc-2", "type-2", "az-2", "snap-2", 2), + }, + expectedProvider: "provider-1", + expectedSnapshotID: "snap-1", + expectedVolumeType: "type-1", + expectedVolumeAZ: "az-1", + expectedVolumeIOPS: int64Ptr(1), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ( + blockStore = new(cloudprovidermocks.BlockStore) + blockStoreGetter = providerToBlockStoreMap(map[string]cloudprovider.BlockStore{ + tc.expectedProvider: blockStore, + }) + locationsInformer = informers.NewSharedInformerFactory(fake.NewSimpleClientset(), 0).Ark().V1().VolumeSnapshotLocations() + ) + + for _, loc := range tc.locations { + require.NoError(t, locationsInformer.Informer().GetStore().Add(loc)) } - require.NoError(t, err) - if test.expectSetVolumeID { - assert.Equal(t, test.volumeID, testBlockStoreGetter.fakeBlockStore.VolumeIDSet) - } else { - assert.Equal(t, "", testBlockStoreGetter.fakeBlockStore.VolumeIDSet) + r := &pvRestorer{ + logger: arktest.NewLogger(), + backup: tc.backup, + volumeSnapshots: tc.volumeSnapshots, + snapshotLocationLister: locationsInformer.Lister(), + blockStoreGetter: blockStoreGetter, } - assert.Equal(t, test.expectedRes, res) + + blockStore.On("Init", mock.Anything).Return(nil) + blockStore.On("CreateVolumeFromSnapshot", tc.expectedSnapshotID, tc.expectedVolumeType, tc.expectedVolumeAZ, tc.expectedVolumeIOPS).Return("volume-1", nil) + blockStore.On("SetVolumeID", tc.obj, "volume-1").Return(tc.obj, nil) + + _, err := r.executePVAction(tc.obj) + assert.NoError(t, err) + + blockStore.AssertExpectations(t) }) } } diff --git a/pkg/util/test/test_backup.go b/pkg/util/test/test_backup.go index e3a764daf9..bc3970a313 100644 --- a/pkg/util/test/test_backup.go +++ b/pkg/util/test/test_backup.go @@ -105,6 +105,21 @@ func (b *TestBackup) WithSnapshot(pv string, snapshot string) *TestBackup { return b } +func (b *TestBackup) WithVolumeBackupInfo(pv, snapshotID, volumeType, az string, iops *int64) *TestBackup { + if b.Status.VolumeBackups == nil { + b.Status.VolumeBackups = make(map[string]*v1.VolumeBackupInfo) + } + + b.Status.VolumeBackups[pv] = &v1.VolumeBackupInfo{ + SnapshotID: snapshotID, + Type: volumeType, + AvailabilityZone: az, + Iops: iops, + } + + return b +} + func (b *TestBackup) WithSnapshotVolumes(value bool) *TestBackup { b.Spec.SnapshotVolumes = &value return b @@ -141,7 +156,7 @@ func (b *TestBackup) WithStorageLocation(location string) *TestBackup { return b } -func (b *TestBackup) WithVolumeSnapshotLocations(locations []string) *TestBackup { +func (b *TestBackup) WithVolumeSnapshotLocations(locations ...string) *TestBackup { b.Spec.VolumeSnapshotLocations = locations return b } diff --git a/pkg/util/test/test_volume_snapshot_location.go b/pkg/util/test/test_volume_snapshot_location.go index 5bfb7e496e..831b952418 100644 --- a/pkg/util/test/test_volume_snapshot_location.go +++ b/pkg/util/test/test_volume_snapshot_location.go @@ -45,6 +45,11 @@ func (location *TestVolumeSnapshotLocation) WithName(name string) *TestVolumeSna return location } +func (location *TestVolumeSnapshotLocation) WithProvider(name string) *TestVolumeSnapshotLocation { + location.Spec.Provider = name + return location +} + func (location *TestVolumeSnapshotLocation) WithProviderConfig(info []LocationInfo) []*TestVolumeSnapshotLocation { var locations []*TestVolumeSnapshotLocation