diff --git a/changelogs/unreleased/10357-samay43 b/changelogs/unreleased/10357-samay43 new file mode 100644 index 000000000..1c25341db --- /dev/null +++ b/changelogs/unreleased/10357-samay43 @@ -0,0 +1 @@ +translate parent snapshot "auto" to an empty parent snapshot in both data mover micro services diff --git a/pkg/datamover/backup_micro_service.go b/pkg/datamover/backup_micro_service.go index 39a6b3eb0..5ac1ce6ee 100644 --- a/pkg/datamover/backup_micro_service.go +++ b/pkg/datamover/backup_micro_service.go @@ -204,12 +204,17 @@ func (r *BackupMicroService) RunCancelableDataPath(ctx context.Context) (string, velerov1api.AsyncOperationIDLabel: du.Labels[velerov1api.AsyncOperationIDLabel], } - // Modify the ParentSnapshot to "" and ForceFull to true when ParentSnapshot is "none". + // "none" requests a full backup. "auto" requests that the data mover finds the most + // recent backup of the same volume as parent, which is what an empty ParentSnapshot + // already does, so both map to "". parentSnapshot := du.Spec.ParentSnapshot forceFull := false - if du.Spec.ParentSnapshot == veleroshared.ParentSnapshotNone { + switch du.Spec.ParentSnapshot { + case veleroshared.ParentSnapshotNone: parentSnapshot = "" forceFull = true + case veleroshared.ParentSnapshotAuto: + parentSnapshot = "" } if err := dp.StartBackup(r.sourceTargetPath, du.Spec.DataMoverConfig, &datapath.BackupStartParam{ diff --git a/pkg/datamover/backup_micro_service_test.go b/pkg/datamover/backup_micro_service_test.go index 48db8351e..c9accdd77 100644 --- a/pkg/datamover/backup_micro_service_test.go +++ b/pkg/datamover/backup_micro_service_test.go @@ -32,6 +32,7 @@ import ( kbclient "sigs.k8s.io/controller-runtime/pkg/client" clientFake "sigs.k8s.io/controller-runtime/pkg/client/fake" + veleroshared "github.com/vmware-tanzu/velero/pkg/apis/velero/shared" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov2alpha1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1" "github.com/vmware-tanzu/velero/pkg/builder" @@ -445,3 +446,89 @@ func TestRunCancelableDataPath(t *testing.T) { cancel() } + +func TestRunCancelableDataPathParentSnapshot(t *testing.T) { + dataUploadName := "fake-data-upload" + + tests := []struct { + name string + parentSnapshot string + expectedParentSnapshot string + expectedForceFull bool + }{ + { + name: "empty lets the data mover search for a parent", + parentSnapshot: "", + expectedParentSnapshot: "", + expectedForceFull: false, + }, + { + name: "auto lets the data mover search for a parent", + parentSnapshot: veleroshared.ParentSnapshotAuto, + expectedParentSnapshot: "", + expectedForceFull: false, + }, + { + name: "none forces a full backup", + parentSnapshot: veleroshared.ParentSnapshotNone, + expectedParentSnapshot: "", + expectedForceFull: true, + }, + { + name: "a specific snapshot ID is passed through unchanged", + parentSnapshot: "fake-parent-snapshot-id", + expectedParentSnapshot: "fake-parent-snapshot-id", + expectedForceFull: false, + }, + } + + scheme := runtime.NewScheme() + velerov2alpha1api.AddToScheme(scheme) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + duInProgress := builder.ForDataUpload(velerov1api.DefaultNamespace, dataUploadName). + Phase(velerov2alpha1api.DataUploadPhaseInProgress). + CSISnapshot(&velerov2alpha1api.CSISnapshotSpec{VolumeSnapshot: "fake-snapshot"}). + Result() + duInProgress.Spec.ParentSnapshot = test.parentSnapshot + + fakeClient := clientFake.NewClientBuilder().WithScheme(scheme). + WithRuntimeObjects(duInProgress).Build() + + bs := &BackupMicroService{ + namespace: velerov1api.DefaultNamespace, + dataUploadName: dataUploadName, + ctx: t.Context(), + client: fakeClient, + dataPathMgr: datapath.NewManager(1), + eventRecorder: &backupMsTestHelper{}, + resultSignal: make(chan dataPathResult), + logger: velerotest.NewLogger(), + } + + var startParam *datapath.BackupStartParam + datapath.VGDPCreator = func(string, string, kbclient.Client, string, datapath.Callbacks, logrus.FieldLogger) datapath.AsyncBR { + fsBR := datapathmockes.NewAsyncBR(t) + fsBR.On("Init", mock.Anything, mock.Anything).Return(nil) + fsBR.On("StartBackup", mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + startParam = args.Get(2).(*datapath.BackupStartParam) + }).Return(nil) + return fsBR + } + + go func() { + time.Sleep(time.Millisecond * 500) + bs.resultSignal <- dataPathResult{result: "fake-succeed-result"} + }() + + _, err := bs.RunCancelableDataPath(t.Context()) + require.NoError(t, err) + + require.NotNil(t, startParam) + assert.Equal(t, test.expectedParentSnapshot, startParam.ParentSnapshot) + assert.Equal(t, test.expectedForceFull, startParam.ForceFull) + }) + } +} diff --git a/pkg/podvolume/backup_micro_service.go b/pkg/podvolume/backup_micro_service.go index d9e24ada8..1f71ba0b2 100644 --- a/pkg/podvolume/backup_micro_service.go +++ b/pkg/podvolume/backup_micro_service.go @@ -193,12 +193,17 @@ func (r *BackupMicroService) RunCancelableDataPath(ctx context.Context) (string, tags := map[string]string{} - // Modify the ParentSnapshot to "" and ForceFull to true when ParentSnapshot is "none". + // "none" requests a full backup. "auto" requests that the data mover finds the most + // recent backup of the same volume as parent, which is what an empty ParentSnapshot + // already does, so both map to "". parentSnapshot := pvb.Spec.ParentSnapshot forceFull := false - if pvb.Spec.ParentSnapshot == veleroshared.ParentSnapshotNone { + switch pvb.Spec.ParentSnapshot { + case veleroshared.ParentSnapshotNone: parentSnapshot = "" forceFull = true + case veleroshared.ParentSnapshotAuto: + parentSnapshot = "" } if err := fsBackup.StartBackup(r.sourceTargetPath, pvb.Spec.UploaderSettings, &datapath.BackupStartParam{ diff --git a/pkg/podvolume/backup_micro_service_test.go b/pkg/podvolume/backup_micro_service_test.go index eac17e4de..2de4705af 100644 --- a/pkg/podvolume/backup_micro_service_test.go +++ b/pkg/podvolume/backup_micro_service_test.go @@ -34,6 +34,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/datapath" "github.com/vmware-tanzu/velero/pkg/uploader" + veleroshared "github.com/vmware-tanzu/velero/pkg/apis/velero/shared" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" clientFake "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -446,3 +447,88 @@ func TestRunCancelableDataPath(t *testing.T) { cancel() } + +func TestRunCancelableDataPathParentSnapshot(t *testing.T) { + pvbName := "fake-pvb" + + tests := []struct { + name string + parentSnapshot string + expectedParentSnapshot string + expectedForceFull bool + }{ + { + name: "empty lets the data mover search for a parent", + parentSnapshot: "", + expectedParentSnapshot: "", + expectedForceFull: false, + }, + { + name: "auto lets the data mover search for a parent", + parentSnapshot: veleroshared.ParentSnapshotAuto, + expectedParentSnapshot: "", + expectedForceFull: false, + }, + { + name: "none forces a full backup", + parentSnapshot: veleroshared.ParentSnapshotNone, + expectedParentSnapshot: "", + expectedForceFull: true, + }, + { + name: "a specific snapshot ID is passed through unchanged", + parentSnapshot: "fake-parent-snapshot-id", + expectedParentSnapshot: "fake-parent-snapshot-id", + expectedForceFull: false, + }, + } + + scheme := runtime.NewScheme() + velerov1api.AddToScheme(scheme) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pvbInProgress := builder.ForPodVolumeBackup(velerov1api.DefaultNamespace, pvbName). + Phase(velerov1api.PodVolumeBackupPhaseInProgress). + Result() + pvbInProgress.Spec.ParentSnapshot = test.parentSnapshot + + fakeClient := clientFake.NewClientBuilder().WithScheme(scheme). + WithRuntimeObjects(pvbInProgress).Build() + + bs := &BackupMicroService{ + namespace: velerov1api.DefaultNamespace, + pvbName: pvbName, + ctx: t.Context(), + client: fakeClient, + dataPathMgr: datapath.NewManager(1), + eventRecorder: &backupMsTestHelper{}, + resultSignal: make(chan dataPathResult), + logger: velerotest.NewLogger(), + } + + var startParam *datapath.BackupStartParam + datapath.VGDPCreator = func(string, string, kbclient.Client, string, datapath.Callbacks, logrus.FieldLogger) datapath.AsyncBR { + fsBR := datapathmockes.NewAsyncBR(t) + fsBR.On("Init", mock.Anything, mock.Anything).Return(nil) + fsBR.On("StartBackup", mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + startParam = args.Get(2).(*datapath.BackupStartParam) + }).Return(nil) + return fsBR + } + + go func() { + time.Sleep(time.Millisecond * 500) + bs.resultSignal <- dataPathResult{result: "fake-succeed-result"} + }() + + _, err := bs.RunCancelableDataPath(t.Context()) + require.NoError(t, err) + + require.NotNil(t, startParam) + assert.Equal(t, test.expectedParentSnapshot, startParam.ParentSnapshot) + assert.Equal(t, test.expectedForceFull, startParam.ForceFull) + }) + } +}