From 9fa8dc62966e61d1079e2fc5be100f76391c097f Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Fri, 4 Aug 2023 12:49:28 -0400 Subject: [PATCH] adding block mode to uploader/provider interfaces Signed-off-by: Shawn Hurley --- pkg/datapath/file_system.go | 6 +++++- pkg/datapath/types.go | 1 + pkg/uploader/kopia/snapshot.go | 7 ++++++- pkg/uploader/kopia/snapshot_test.go | 15 +++++++++++++-- pkg/uploader/provider/kopia.go | 8 +++++++- pkg/uploader/provider/kopia_test.go | 22 +++++++++++++++++----- pkg/uploader/provider/mocks/Provider.go | 4 ++-- pkg/uploader/provider/provider.go | 1 + pkg/uploader/provider/restic.go | 5 +++++ pkg/uploader/provider/restic_test.go | 16 ++++++++++++++-- pkg/uploader/types.go | 9 +++++++++ 11 files changed, 80 insertions(+), 14 deletions(-) diff --git a/pkg/datapath/file_system.go b/pkg/datapath/file_system.go index 4e4b16aee0..195c5afc20 100644 --- a/pkg/datapath/file_system.go +++ b/pkg/datapath/file_system.go @@ -133,9 +133,13 @@ func (fs *fileSystemBR) StartBackup(source AccessPoint, realSource string, paren if !fs.initialized { return errors.New("file system data path is not initialized") } + volMode := uploader.PersistentVolumeFilesystem + if source.ByBlock != "" { + volMode = uploader.PersistentVolumeBlock + } go func() { - snapshotID, emptySnapshot, err := fs.uploaderProv.RunBackup(fs.ctx, source.ByPath, realSource, tags, forceFull, parentSnapshot, fs) + snapshotID, emptySnapshot, err := fs.uploaderProv.RunBackup(fs.ctx, source.ByPath, realSource, tags, forceFull, parentSnapshot, volMode, fs) if err == provider.ErrorCanceled { fs.callbacks.OnCancelled(context.Background(), fs.namespace, fs.jobName) diff --git a/pkg/datapath/types.go b/pkg/datapath/types.go index 4f85fceecb..d64badfc69 100644 --- a/pkg/datapath/types.go +++ b/pkg/datapath/types.go @@ -53,6 +53,7 @@ type Callbacks struct { // AccessPoint represents an access point that has been exposed to a data path instance type AccessPoint struct { ByPath string + ByBlock string } // AsyncBR is the interface for asynchronous data path methods diff --git a/pkg/uploader/kopia/snapshot.go b/pkg/uploader/kopia/snapshot.go index 85000ad00b..32ec54ccd0 100644 --- a/pkg/uploader/kopia/snapshot.go +++ b/pkg/uploader/kopia/snapshot.go @@ -88,10 +88,15 @@ func setupDefaultPolicy() *policy.Tree { // Backup backup specific sourcePath and update progress func Backup(ctx context.Context, fsUploader SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, - forceFull bool, parentSnapshot string, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { + forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { if fsUploader == nil { return nil, false, errors.New("get empty kopia uploader") } + + if volMode == uploader.PersistentVolumeBlock { + return nil, false, errors.New("unable to handle block storage") + } + dir, err := filepath.Abs(sourcePath) if err != nil { return nil, false, errors.Wrapf(err, "Invalid source path '%s'", sourcePath) diff --git a/pkg/uploader/kopia/snapshot_test.go b/pkg/uploader/kopia/snapshot_test.go index 0d352ab6d3..2093b55d57 100644 --- a/pkg/uploader/kopia/snapshot_test.go +++ b/pkg/uploader/kopia/snapshot_test.go @@ -564,6 +564,7 @@ func TestBackup(t *testing.T) { isSnapshotSourceError bool expectedError error expectedEmpty bool + volMode uploader.PersistentVolumeMode } manifest := &snapshot.Manifest{ ID: "test", @@ -590,10 +591,20 @@ func TestBackup(t *testing.T) { tags: nil, expectedError: errors.New("Unable to read dir"), }, + { + name: "Unable to handle block mode", + sourcePath: "/", + tags: nil, + volMode: uploader.PersistentVolumeBlock, + expectedError: errors.New("unable to handle block storage"), + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.volMode == "" { + tc.volMode = uploader.PersistentVolumeFilesystem + } s := injectSnapshotFuncs() args := []mockArgs{ {methodName: "LoadSnapshot", returns: []interface{}{manifest, nil}}, @@ -616,9 +627,9 @@ func TestBackup(t *testing.T) { var snapshotInfo *uploader.SnapshotInfo var err error if tc.isEmptyUploader { - snapshotInfo, isSnapshotEmpty, err = Backup(context.Background(), nil, s.repoWriterMock, tc.sourcePath, "", tc.forceFull, tc.parentSnapshot, tc.tags, &logrus.Logger{}) + snapshotInfo, isSnapshotEmpty, err = Backup(context.Background(), nil, s.repoWriterMock, tc.sourcePath, "", tc.forceFull, tc.parentSnapshot, tc.volMode, tc.tags, &logrus.Logger{}) } else { - snapshotInfo, isSnapshotEmpty, err = Backup(context.Background(), s.uploderMock, s.repoWriterMock, tc.sourcePath, "", tc.forceFull, tc.parentSnapshot, tc.tags, &logrus.Logger{}) + snapshotInfo, isSnapshotEmpty, err = Backup(context.Background(), s.uploderMock, s.repoWriterMock, tc.sourcePath, "", tc.forceFull, tc.parentSnapshot, tc.volMode, tc.tags, &logrus.Logger{}) } // Check if the returned error matches the expected error if tc.expectedError != nil { diff --git a/pkg/uploader/provider/kopia.go b/pkg/uploader/provider/kopia.go index 0a241c4738..49568f5687 100644 --- a/pkg/uploader/provider/kopia.go +++ b/pkg/uploader/provider/kopia.go @@ -118,6 +118,7 @@ func (kp *kopiaProvider) RunBackup( tags map[string]string, forceFull bool, parentSnapshot string, + volMode uploader.PersistentVolumeMode, updater uploader.ProgressUpdater) (string, bool, error) { if updater == nil { return "", false, errors.New("Need to initial backup progress updater first") @@ -127,6 +128,11 @@ func (kp *kopiaProvider) RunBackup( return "", false, errors.New("path is empty") } + // For now, error on block mode + if volMode == uploader.PersistentVolumeBlock { + return "", false, errors.New("unable to currently support block mode") + } + log := kp.log.WithFields(logrus.Fields{ "path": path, "realSource": realSource, @@ -153,7 +159,7 @@ func (kp *kopiaProvider) RunBackup( tags[uploader.SnapshotRequesterTag] = kp.requestorType tags[uploader.SnapshotUploaderTag] = uploader.KopiaType - snapshotInfo, isSnapshotEmpty, err := BackupFunc(ctx, kpUploader, repoWriter, path, realSource, forceFull, parentSnapshot, tags, log) + snapshotInfo, isSnapshotEmpty, err := BackupFunc(ctx, kpUploader, repoWriter, path, realSource, forceFull, parentSnapshot, volMode, tags, log) if err != nil { if kpUploader.IsCanceled() { log.Error("Kopia backup is canceled") diff --git a/pkg/uploader/provider/kopia_test.go b/pkg/uploader/provider/kopia_test.go index a1105cbe8e..b0d7e66397 100644 --- a/pkg/uploader/provider/kopia_test.go +++ b/pkg/uploader/provider/kopia_test.go @@ -68,35 +68,47 @@ func TestRunBackup(t *testing.T) { testCases := []struct { name string - hookBackupFunc func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) + hookBackupFunc func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) + volMode uploader.PersistentVolumeMode notError bool }{ { name: "success to backup", - hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { + hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { return &uploader.SnapshotInfo{}, false, nil }, notError: true, }, { name: "get error to backup", - hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { + hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { return &uploader.SnapshotInfo{}, false, errors.New("failed to backup") }, notError: false, }, { name: "got empty snapshot", - hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { + hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { return nil, true, errors.New("snapshot is empty") }, notError: false, }, + { + name: "error on vol mode", + hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { + return nil, true, nil + }, + volMode: uploader.PersistentVolumeBlock, + notError: false, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.volMode == "" { + tc.volMode = uploader.PersistentVolumeFilesystem + } BackupFunc = tc.hookBackupFunc - _, _, err := kp.RunBackup(context.Background(), "var", "", nil, false, "", &updater) + _, _, err := kp.RunBackup(context.Background(), "var", "", nil, false, "", tc.volMode, &updater) if tc.notError { assert.NoError(t, err) } else { diff --git a/pkg/uploader/provider/mocks/Provider.go b/pkg/uploader/provider/mocks/Provider.go index 54a461ea14..87f691e9f5 100644 --- a/pkg/uploader/provider/mocks/Provider.go +++ b/pkg/uploader/provider/mocks/Provider.go @@ -30,8 +30,8 @@ func (_m *Provider) Close(ctx context.Context) error { } // RunBackup provides a mock function with given fields: ctx, path, realSource, tags, forceFull, parentSnapshot, updater -func (_m *Provider) RunBackup(ctx context.Context, path string, realSource string, tags map[string]string, forceFull bool, parentSnapshot string, updater uploader.ProgressUpdater) (string, bool, error) { - ret := _m.Called(ctx, path, realSource, tags, forceFull, parentSnapshot, updater) +func (_m *Provider) RunBackup(ctx context.Context, path string, realSource string, tags map[string]string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, updater uploader.ProgressUpdater) (string, bool, error) { + ret := _m.Called(ctx, path, realSource, tags, forceFull, parentSnapshot, volMode, updater) var r0 string var r1 bool diff --git a/pkg/uploader/provider/provider.go b/pkg/uploader/provider/provider.go index e3373936d5..5f91f63b58 100644 --- a/pkg/uploader/provider/provider.go +++ b/pkg/uploader/provider/provider.go @@ -48,6 +48,7 @@ type Provider interface { tags map[string]string, forceFull bool, parentSnapshot string, + volMode uploader.PersistentVolumeMode, updater uploader.ProgressUpdater) (string, bool, error) // RunRestore which will do restore for one specific volume with given snapshot id and return error // updater is used for updating backup progress which implement by third-party diff --git a/pkg/uploader/provider/restic.go b/pkg/uploader/provider/restic.go index 22e0f5b40a..ac130fa421 100644 --- a/pkg/uploader/provider/restic.go +++ b/pkg/uploader/provider/restic.go @@ -121,6 +121,7 @@ func (rp *resticProvider) RunBackup( tags map[string]string, forceFull bool, parentSnapshot string, + volMode uploader.PersistentVolumeMode, updater uploader.ProgressUpdater) (string, bool, error) { if updater == nil { return "", false, errors.New("Need to initial backup progress updater first") @@ -134,6 +135,10 @@ func (rp *resticProvider) RunBackup( return "", false, errors.New("real source is not empty, this is not supported by restic uploader") } + if volMode == uploader.PersistentVolumeBlock { + return "", false, errors.New("unable to currently support block mode") + } + log := rp.log.WithFields(logrus.Fields{ "path": path, "parentSnapshot": parentSnapshot, diff --git a/pkg/uploader/provider/restic_test.go b/pkg/uploader/provider/restic_test.go index eaf0273ff5..0619e90d7f 100644 --- a/pkg/uploader/provider/restic_test.go +++ b/pkg/uploader/provider/restic_test.go @@ -45,6 +45,7 @@ func TestResticRunBackup(t *testing.T) { nilUpdater bool parentSnapshot string rp *resticProvider + volMode uploader.PersistentVolumeMode hookBackupFunc func(string, string, string, map[string]string) *restic.Command hookResticBackupFunc func(*restic.Command, logrus.FieldLogger, uploader.ProgressUpdater) (string, string, error) hookResticGetSnapshotFunc func(string, string, map[string]string) *restic.Command @@ -117,6 +118,14 @@ func TestResticRunBackup(t *testing.T) { return strings.Contains(err.Error(), "failed to get snapshot id") }, }, + { + name: "failed to use block mode", + rp: &resticProvider{log: logrus.New(), extraFlags: []string{"testFlags"}}, + volMode: uploader.PersistentVolumeBlock, + errorHandleFunc: func(err error) bool { + return strings.Contains(err.Error(), "unable to currently support block mode") + }, + }, } for _, tc := range testCases { @@ -135,11 +144,14 @@ func TestResticRunBackup(t *testing.T) { if tc.hookResticGetSnapshotIDFunc != nil { resticGetSnapshotIDFunc = tc.hookResticGetSnapshotIDFunc } + if tc.volMode == "" { + tc.volMode = uploader.PersistentVolumeFilesystem + } if !tc.nilUpdater { updater := FakeBackupProgressUpdater{PodVolumeBackup: &velerov1api.PodVolumeBackup{}, Log: tc.rp.log, Ctx: context.Background(), Cli: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()} - _, _, err = tc.rp.RunBackup(context.Background(), "var", "", map[string]string{}, false, parentSnapshot, &updater) + _, _, err = tc.rp.RunBackup(context.Background(), "var", "", map[string]string{}, false, parentSnapshot, tc.volMode, &updater) } else { - _, _, err = tc.rp.RunBackup(context.Background(), "var", "", map[string]string{}, false, parentSnapshot, nil) + _, _, err = tc.rp.RunBackup(context.Background(), "var", "", map[string]string{}, false, parentSnapshot, tc.volMode, nil) } tc.rp.log.Infof("test name %v error %v", tc.name, err) diff --git a/pkg/uploader/types.go b/pkg/uploader/types.go index 8df77fb3d5..02106e266e 100644 --- a/pkg/uploader/types.go +++ b/pkg/uploader/types.go @@ -28,6 +28,15 @@ const ( SnapshotUploaderTag = "snapshot-uploader" ) +type PersistentVolumeMode string + +const ( + // PersistentVolumeBlock means the volume will not be formatted with a filesystem and will remain a raw block device. + PersistentVolumeBlock PersistentVolumeMode = "Block" + // PersistentVolumeFilesystem means the volume will be or is formatted with a filesystem. + PersistentVolumeFilesystem PersistentVolumeMode = "Filesystem" +) + // ValidateUploaderType validates if the input param is a valid uploader type. // It will return an error if it's invalid. func ValidateUploaderType(t string) error {