From 5bde12939e469de10801a55060f13b37045a984d Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Mon, 30 Mar 2020 14:26:23 -0400 Subject: [PATCH] Address review feedback on object store Signed-off-by: Nolan Brubaker --- pkg/persistence/object_store.go | 36 ++++++++++++++------------ pkg/persistence/object_store_layout.go | 5 ++-- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pkg/persistence/object_store.go b/pkg/persistence/object_store.go index 97c341e10..2b4169553 100644 --- a/pkg/persistence/object_store.go +++ b/pkg/persistence/object_store.go @@ -58,7 +58,7 @@ type BackupStore interface { GetBackupVolumeSnapshots(name string) ([]*volume.Snapshot, error) GetPodVolumeBackups(name string) ([]*velerov1api.PodVolumeBackup, error) GetBackupContents(name string) (io.ReadCloser, error) - // TODO(nrb-csi): Do we need Get methods for the CSI types? + // TODO(nrb-csi): Any Get methods relevant to the CSI VolumeSnapshots should be added with the client-side PRs. // BackupExists checks if the backup metadata file exists in object storage. BackupExists(bucket, backupName string) (bool, error) @@ -218,6 +218,7 @@ func (s *objectBackupStore) PutBackup(info BackupInfo) error { return kerrors.NewAggregate([]error{err, deleteErr}) } + // TODO: How do we reduce duplication from here to the end of the function? if err := seekAndPutObject(s.objectStore, s.bucket, s.layout.getPodVolumeBackupsKey(info.Name), info.PodVolumeBackups); err != nil { errs := []error{err} @@ -254,29 +255,32 @@ func (s *objectBackupStore) PutBackup(info BackupInfo) error { return kerrors.NewAggregate(errs) } - // TODO(nrb-csi): only upload these if they're non-nil and/or if EnableCSI is on // TODO(nrb-csi): Add tests? - if err := seekAndPutObject(s.objectStore, s.bucket, s.layout.getCSIVolumeSnapshotContentsKey(info.Name), info.CSIVolumeSnapshots); err != nil { - errs := []error{err} + if info.CSIVolumeSnapshots != nil { + if err := seekAndPutObject(s.objectStore, s.bucket, s.layout.getCSIVolumeSnapshotKey(info.Name), info.CSIVolumeSnapshots); err != nil { + errs := []error{err} - deleteErr := s.objectStore.DeleteObject(s.bucket, s.layout.getCSIVolumeSnapshotContentsKey(info.Name)) - errs = append(errs, deleteErr) + deleteErr := s.objectStore.DeleteObject(s.bucket, s.layout.getCSIVolumeSnapshotKey(info.Name)) + errs = append(errs, deleteErr) - deleteErr = s.objectStore.DeleteObject(s.bucket, s.layout.getBackupMetadataKey(info.Name)) - errs = append(errs, deleteErr) + deleteErr = s.objectStore.DeleteObject(s.bucket, s.layout.getBackupMetadataKey(info.Name)) + errs = append(errs, deleteErr) - return kerrors.NewAggregate(errs) + return kerrors.NewAggregate(errs) + } } - if err := seekAndPutObject(s.objectStore, s.bucket, s.layout.getVolumeSnapshotContentsKey(info.Name), info.VolumeSnapshotContents); err != nil { - errs := []error{err} + if info.VolumeSnapshotContents != nil { + if err := seekAndPutObject(s.objectStore, s.bucket, s.layout.getVolumeSnapshotContentsKey(info.Name), info.VolumeSnapshotContents); err != nil { + errs := []error{err} - deleteErr := s.objectStore.DeleteObject(s.bucket, s.layout.getVolumeSnapshotContentsKey(info.Name)) - errs = append(errs, deleteErr) + deleteErr := s.objectStore.DeleteObject(s.bucket, s.layout.getVolumeSnapshotContentsKey(info.Name)) + errs = append(errs, deleteErr) - deleteErr = s.objectStore.DeleteObject(s.bucket, s.layout.getBackupMetadataKey(info.Name)) - errs = append(errs, deleteErr) - return kerrors.NewAggregate(errs) + deleteErr = s.objectStore.DeleteObject(s.bucket, s.layout.getBackupMetadataKey(info.Name)) + errs = append(errs, deleteErr) + return kerrors.NewAggregate(errs) + } } return nil diff --git a/pkg/persistence/object_store_layout.go b/pkg/persistence/object_store_layout.go index baf2fa196..70e955968 100644 --- a/pkg/persistence/object_store_layout.go +++ b/pkg/persistence/object_store_layout.go @@ -100,9 +100,8 @@ func (l *ObjectStoreLayout) getRestoreResultsKey(restore string) string { return path.Join(l.subdirs["restores"], restore, fmt.Sprintf("restore-%s-results.gz", restore)) } -// TODO: Add keys for VS & VSContents json.gz files -func (l *ObjectStoreLayout) getCSIVolumeSnapshotContentsKey(backup string) string { - return path.Join(l.subdirs["backups"], backup, fmt.Sprintf("%s-csivolumesnapshotcontents.json.gz", backup)) +func (l *ObjectStoreLayout) getCSIVolumeSnapshotKey(backup string) string { + return path.Join(l.subdirs["backups"], backup, fmt.Sprintf("%s-csivolumesnapshot.json.gz", backup)) } func (l *ObjectStoreLayout) getVolumeSnapshotContentsKey(backup string) string {