diff --git a/pkg/apis/velero/v1/backup.go b/pkg/apis/velero/v1/backup.go index 27a78f38b..aec79aec4 100644 --- a/pkg/apis/velero/v1/backup.go +++ b/pkg/apis/velero/v1/backup.go @@ -170,16 +170,6 @@ type BackupStatus struct { // Phase is the current state of the Backup. Phase BackupPhase `json:"phase"` - // VolumeBackups is a map of PersistentVolume names to - // information about the backed-up volume in the cloud - // provider API. - // - // 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). ValidationErrors []string `json:"validationErrors"` diff --git a/pkg/apis/velero/v1/zz_generated.deepcopy.go b/pkg/apis/velero/v1/zz_generated.deepcopy.go index 136bfc48e..6e61e6fe2 100644 --- a/pkg/apis/velero/v1/zz_generated.deepcopy.go +++ b/pkg/apis/velero/v1/zz_generated.deepcopy.go @@ -254,21 +254,6 @@ func (in *BackupSpec) DeepCopy() *BackupSpec { func (in *BackupStatus) DeepCopyInto(out *BackupStatus) { *out = *in in.Expiration.DeepCopyInto(&out.Expiration) - if in.VolumeBackups != nil { - in, out := &in.VolumeBackups, &out.VolumeBackups - *out = make(map[string]*VolumeBackupInfo, len(*in)) - for key, val := range *in { - var outVal *VolumeBackupInfo - if val == nil { - (*out)[key] = nil - } else { - in, out := &val, &outVal - *out = new(VolumeBackupInfo) - (*in).DeepCopyInto(*out) - } - (*out)[key] = outVal - } - } if in.ValidationErrors != nil { in, out := &in.ValidationErrors, &out.ValidationErrors *out = make([]string, len(*in)) diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index c268cf5e2..7e31dd5e7 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -210,17 +210,7 @@ func DescribeBackupStatus(d *Describer, backup *velerov1api.Backup, details bool d.Printf("Expiration:\t%s\n", status.Expiration.Time) d.Println() - if len(status.VolumeBackups) > 0 { - // pre-v0.10 backup - d.Printf("Persistent Volumes:\n") - for pvName, info := range status.VolumeBackups { - printSnapshot(d, pvName, info.SnapshotID, info.Type, info.AvailabilityZone, info.Iops) - } - return - } - if status.VolumeSnapshotsAttempted > 0 { - // v0.10+ backup if !details { d.Printf("Persistent Volumes:\t%d of %d snapshots completed successfully (specify --details for more information)\n", status.VolumeSnapshotsCompleted, status.VolumeSnapshotsAttempted) return diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index 329c19eff..edd8d1b00 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -252,47 +252,26 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e if backupStore != nil { log.Info("Removing PV snapshots") - 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 { - volumeSnapshotter, err := volumeSnapshotterForSnapshotLocation(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 := volumeSnapshotter.DeleteSnapshot(snapshot.SnapshotID); err != nil { - errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", snapshot.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 { - // 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 { - volumeSnapshotters := make(map[string]velero.VolumeSnapshotter) + volumeSnapshotters := make(map[string]velero.VolumeSnapshotter) - for _, snapshot := range snapshots { - log.WithField("providerSnapshotID", snapshot.Status.ProviderSnapshotID).Info("Removing snapshot associated with backup") + for _, snapshot := range snapshots { + log.WithField("providerSnapshotID", snapshot.Status.ProviderSnapshotID).Info("Removing snapshot associated with backup") - volumeSnapshotter, ok := volumeSnapshotters[snapshot.Spec.Location] - if !ok { - if volumeSnapshotter, err = volumeSnapshotterForSnapshotLocation(backup.Namespace, snapshot.Spec.Location, c.snapshotLocationLister, pluginManager); err != nil { - errs = append(errs, err.Error()) - continue - } - volumeSnapshotters[snapshot.Spec.Location] = volumeSnapshotter + volumeSnapshotter, ok := volumeSnapshotters[snapshot.Spec.Location] + if !ok { + if volumeSnapshotter, err = volumeSnapshotterForSnapshotLocation(backup.Namespace, snapshot.Spec.Location, c.snapshotLocationLister, pluginManager); err != nil { + errs = append(errs, err.Error()) + continue } + volumeSnapshotters[snapshot.Spec.Location] = volumeSnapshotter + } - if err := volumeSnapshotter.DeleteSnapshot(snapshot.Status.ProviderSnapshotID); err != nil { - errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", snapshot.Status.ProviderSnapshotID).Error()) - } + if err := volumeSnapshotter.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 1b77b44bd..093d7e3e1 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -275,7 +275,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("patching backup to Deleting fails", func(t *testing.T) { - backup := velerotest.NewTestBackup().WithName("foo").WithSnapshot("pv-1", "snap-1").Backup + backup := velerotest.NewTestBackup().WithName("foo").Backup td := setupBackupDeletionControllerTest(backup) td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) { @@ -326,139 +326,6 @@ 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 := velerotest.NewTestBackup().WithName("foo").Backup - backup.UID = "uid" - backup.Spec.StorageLocation = "primary" - backup.Status.VolumeBackups = map[string]*v1.VolumeBackupInfo{ - "pv-1": { - SnapshotID: "snap-1", - }, - } - - restore1 := velerotest.NewTestRestore("velero", "restore-1", v1.RestorePhaseCompleted).WithBackup("foo").Restore - restore2 := velerotest.NewTestRestore("velero", "restore-2", v1.RestorePhaseCompleted).WithBackup("foo").Restore - restore3 := velerotest.NewTestRestore("velero", "restore-3", v1.RestorePhaseCompleted).WithBackup("some-other-backup").Restore - - td := setupBackupDeletionControllerTest(backup, restore1, restore2, restore3) - - td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore1) - td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore2) - td.sharedInformers.Velero().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.Velero().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.Velero().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.volumeSnapshotter.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("GetVolumeSnapshotter", "provider-1").Return(td.volumeSnapshotter, nil) - pluginManager.On("CleanupClients") - td.controller.newPluginManager = func(logrus.FieldLogger) clientmgmt.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":{"velero.io/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":{"velero.io/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"), - ), - } - - velerotest.CompareActions(t, expectedActions, td.client.Actions()) - - // Make sure snapshot was deleted - assert.Equal(t, 0, td.volumeSnapshotter.SnapshotsTaken.Len()) - }) - t.Run("full delete, no errors", func(t *testing.T) { backup := velerotest.NewTestBackup().WithName("foo").Backup backup.UID = "uid" diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 833a2c16e..1af0b0834 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -329,26 +329,6 @@ 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()[velerov1api.ScheduleNameLabel] diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 82a33f0ef..63d00f367 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -611,113 +611,6 @@ func TestProcessQueueItem(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: velerotest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, - backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, - volumeSnapshots: nil, - restore: velerotest.NewDefaultTestRestore().WithBackup("backup-1").Restore, - expectedErrs: nil, - }, - { - name: "backup with no .status.volumeBackups and volumesnapshots.json file does not error", - storageLocation: velerotest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, - backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").Backup, - volumeSnapshots: []*volume.Snapshot{{}}, - restore: velerotest.NewDefaultTestRestore().WithBackup("backup-1").Restore, - expectedErrs: nil, - }, - { - name: "backup with both .status.volumeBackups and volumesnapshots.json file errors", - storageLocation: velerotest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, - backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, - volumeSnapshots: []*volume.Snapshot{{}}, - restore: velerotest.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: velerotest.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: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, - restore: velerotest.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: velerotest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation, - snapshotLocations: []*api.VolumeSnapshotLocation{ - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: api.DefaultNamespace, - Name: "vsl-1", - }, - }, - }, - backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("loc-1").WithSnapshot("pv-1", "snap-1").Backup, - restore: velerotest.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 = velerotest.NewLogger() - backupStore = &persistencemocks.BackupStore{} - controller = &restoreController{ - genericController: &genericController{ - logger: logger, - }, - namespace: api.DefaultNamespace, - backupLister: sharedInformers.Velero().V1().Backups().Lister(), - backupLocationLister: sharedInformers.Velero().V1().BackupStorageLocations().Lister(), - snapshotLocationLister: sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(), - newBackupStore: func(*api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) { - return backupStore, nil - }, - } - ) - - require.NoError(t, sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(tc.storageLocation)) - require.NoError(t, sharedInformers.Velero().V1().Backups().Informer().GetStore().Add(tc.backup)) - for _, loc := range tc.snapshotLocations { - require.NoError(t, sharedInformers.Velero().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/pv_restorer.go b/pkg/restore/pv_restorer.go index b5157cf89..c18259528 100644 --- a/pkg/restore/pv_restorer.go +++ b/pkg/restore/pv_restorer.go @@ -20,7 +20,6 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/labels" api "github.com/heptio/velero/pkg/apis/velero/v1" listers "github.com/heptio/velero/pkg/generated/listers/velero/v1" @@ -119,31 +118,6 @@ type snapshotInfo struct { } 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 { diff --git a/pkg/restore/pv_restorer_test.go b/pkg/restore/pv_restorer_test.go index 22be8b865..014fa2334 100644 --- a/pkg/restore/pv_restorer_test.go +++ b/pkg/restore/pv_restorer_test.go @@ -92,24 +92,6 @@ func TestExecutePVAction_NoSnapshotRestores(t *testing.T) { expectedErr: false, expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, }, - { - name: "backup.status.volumeBackups non-nil and no entry for PV: return early", - obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, - restore: velerotest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: velerotest.NewTestBackup().WithName("backup-1").WithSnapshot("non-matching-pv", "snap").Backup, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, - }, - { - name: "backup.status.volumeBackups has entry for PV, >1 VSLs configured: return error", - obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, - restore: velerotest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: velerotest.NewTestBackup().WithName("backup-1").WithSnapshot("pv-1", "snap").Backup, - locations: []*api.VolumeSnapshotLocation{ - velerotest.NewTestVolumeSnapshotLocation().WithName("loc-1").VolumeSnapshotLocation, - velerotest.NewTestVolumeSnapshotLocation().WithName("loc-2").VolumeSnapshotLocation, - }, - expectedErr: true, - }, { name: "volumeSnapshots is empty: return early", obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, @@ -189,24 +171,7 @@ func TestExecutePVAction_SnapshotRestores(t *testing.T) { 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: velerotest.NewDefaultTestRestore().WithRestorePVs(true).Restore, - backup: velerotest.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{ - velerotest.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", + name: "backup with a matching volume.Snapshot for PV executes restore", obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, restore: velerotest.NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: velerotest.NewTestBackup().WithName("backup-1").Backup, diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 0e381594d..36411ea4a 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -831,16 +831,10 @@ func (ctx *context) restoreItem(obj *unstructured.Unstructured, groupResource sc if groupResource == kuberesource.PersistentVolumes { var hasSnapshot bool - if len(ctx.backup.Status.VolumeBackups) > 0 { - // pre-v0.10 backup - _, hasSnapshot = ctx.backup.Status.VolumeBackups[name] - } else { - // v0.10+ backup - for _, snapshot := range ctx.volumeSnapshots { - if snapshot.Spec.PersistentVolumeName == name { - hasSnapshot = true - break - } + for _, snapshot := range ctx.volumeSnapshots { + if snapshot.Spec.PersistentVolumeName == name { + hasSnapshot = true + break } } diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index d82a66915..9b55645e6 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -840,7 +840,6 @@ status: tests := []struct { name string haveSnapshot bool - legacyBackup bool reclaimPolicy string expectPVCVolumeName bool expectedPVCAnnotationsMissing sets.String @@ -848,68 +847,38 @@ status: expectPVFound bool }{ { - name: "legacy backup, have snapshot, reclaim policy delete, no existing PV found", + name: "backup has snapshot, reclaim policy delete, no existing PV found", haveSnapshot: true, - legacyBackup: true, reclaimPolicy: "Delete", expectPVCVolumeName: true, expectPVCreation: true, }, { - name: "non-legacy backup, have snapshot, reclaim policy delete, no existing PV found", + name: "backup has snapshot, reclaim policy delete, existing PV found", haveSnapshot: true, - legacyBackup: false, - reclaimPolicy: "Delete", - expectPVCVolumeName: true, - expectPVCreation: true, - }, - { - name: "non-legacy backup, have snapshot, reclaim policy delete, existing PV found", - haveSnapshot: true, - legacyBackup: false, reclaimPolicy: "Delete", expectPVCVolumeName: true, expectPVCreation: false, expectPVFound: true, }, { - name: "legacy backup, have snapshot, reclaim policy retain, no existing PV found", + name: "backup has snapshot, reclaim policy retain, no existing PV found", haveSnapshot: true, - legacyBackup: true, reclaimPolicy: "Retain", expectPVCVolumeName: true, expectPVCreation: true, }, { - name: "legacy backup, have snapshot, reclaim policy retain, existing PV found", + name: "backup has snapshot, reclaim policy retain, existing PV found", haveSnapshot: true, - legacyBackup: true, reclaimPolicy: "Retain", expectPVCVolumeName: true, expectPVCreation: false, expectPVFound: true, }, { - name: "non-legacy backup, have snapshot, reclaim policy retain, no existing PV found", + name: "backup has snapshot, reclaim policy retain, existing PV found", haveSnapshot: true, - legacyBackup: false, - reclaimPolicy: "Retain", - expectPVCVolumeName: true, - expectPVCreation: true, - }, - { - name: "non-legacy backup, have snapshot, reclaim policy retain, existing PV found", - haveSnapshot: true, - legacyBackup: false, - reclaimPolicy: "Retain", - expectPVCVolumeName: true, - expectPVCreation: false, - expectPVFound: true, - }, - { - name: "non-legacy backup, have snapshot, reclaim policy retain, existing PV found", - haveSnapshot: true, - legacyBackup: false, reclaimPolicy: "Retain", expectPVCVolumeName: true, expectPVCreation: false, @@ -979,13 +948,6 @@ status: nsClient.On("Get", pvcObj.Namespace, mock.Anything).Return(ns, nil) backup := &api.Backup{} - if test.haveSnapshot && test.legacyBackup { - backup.Status.VolumeBackups = map[string]*api.VolumeBackupInfo{ - "pvc-6a74b5af-78a5-11e8-a0d8-e2ad1e9734ce": { - SnapshotID: "snap", - }, - } - } pvRestorer := new(mockPVRestorer) defer pvRestorer.AssertExpectations(t) @@ -1017,7 +979,7 @@ status: restoredItems: make(map[velero.ResourceIdentifier]struct{}), } - if test.haveSnapshot && !test.legacyBackup { + if test.haveSnapshot { ctx.volumeSnapshots = append(ctx.volumeSnapshots, &volume.Snapshot{ Spec: volume.SnapshotSpec{ PersistentVolumeName: "pvc-6a74b5af-78a5-11e8-a0d8-e2ad1e9734ce", diff --git a/pkg/util/test/test_backup.go b/pkg/util/test/test_backup.go index 2257714cb..be371c959 100644 --- a/pkg/util/test/test_backup.go +++ b/pkg/util/test/test_backup.go @@ -97,29 +97,6 @@ func (b *TestBackup) WithVersion(version int) *TestBackup { return b } -func (b *TestBackup) WithSnapshot(pv string, snapshot string) *TestBackup { - if b.Status.VolumeBackups == nil { - b.Status.VolumeBackups = make(map[string]*v1.VolumeBackupInfo) - } - b.Status.VolumeBackups[pv] = &v1.VolumeBackupInfo{SnapshotID: snapshot} - 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