diff --git a/pkg/builder/pod_volume_restore_builder.go b/pkg/builder/pod_volume_restore_builder.go index 3d4da94d6..47acd3cd0 100644 --- a/pkg/builder/pod_volume_restore_builder.go +++ b/pkg/builder/pod_volume_restore_builder.go @@ -97,3 +97,9 @@ func (b *PodVolumeRestoreBuilder) UploaderType(uploaderType string) *PodVolumeRe b.object.Spec.UploaderType = uploaderType return b } + +// OwnerReference sets the OwnerReference for this PodVolumeRestore. +func (b *PodVolumeRestoreBuilder) OwnerReference(ownerRef []metav1.OwnerReference) *PodVolumeRestoreBuilder { + b.object.OwnerReferences = ownerRef + return b +} diff --git a/pkg/cmd/cli/podvolume/restore.go b/pkg/cmd/cli/podvolume/restore.go index ba7a838d7..8d5924532 100644 --- a/pkg/cmd/cli/podvolume/restore.go +++ b/pkg/cmd/cli/podvolume/restore.go @@ -92,6 +92,19 @@ func NewRestoreCommand(f client.Factory) *cobra.Command { _ = command.MarkFlagRequired("pod-volume-restore") _ = command.MarkFlagRequired("resource-timeout") + command.PreRunE = func(cmd *cobra.Command, args []string) error { + if config.resourceTimeout <= 0 { + return errors.New("resource-timeout must be greater than 0") + } + if config.volumePath == "" { + return errors.New("volume-path cannot be empty") + } + if config.pvrName == "" { + return errors.New("pod-volume-restore name cannot be empty") + } + return nil + } + return command } diff --git a/pkg/cmd/cli/podvolume/restore_test.go b/pkg/cmd/cli/podvolume/restore_test.go new file mode 100644 index 000000000..74fbb8090 --- /dev/null +++ b/pkg/cmd/cli/podvolume/restore_test.go @@ -0,0 +1,166 @@ +/* +Copyright The Velero Contributors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package podvolume + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + cacheMock "github.com/vmware-tanzu/velero/pkg/cmd/cli/datamover/mocks" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func fakeCreateRestoreDataPathServiceWithErr(_ *podVolumeRestore) (dataPathService, error) { + return nil, errors.New("fake-create-data-path-error") +} + +func fakeCreateRestoreDataPathService(_ *podVolumeRestore) (dataPathService, error) { + return frHelper, nil +} + +func TestRunRestoreDataPath(t *testing.T) { + tests := []struct { + name string + pvrName string + createDataPathFail bool + initDataPathErr error + runCancelableDataPathErr error + runCancelableDataPathResult string + expectedMessage string + expectedSucceed bool + }{ + { + name: "create data path failed", + pvrName: "fake-name", + createDataPathFail: true, + expectedMessage: "Failed to create data path service for PVR fake-name: fake-create-data-path-error", + }, + { + name: "init data path failed", + pvrName: "fake-name", + initDataPathErr: errors.New("fake-init-data-path-error"), + expectedMessage: "Failed to init data path service for PVR fake-name: fake-init-data-path-error", + }, + { + name: "run data path failed", + pvrName: "fake-name", + runCancelableDataPathErr: errors.New("fake-run-data-path-error"), + expectedMessage: "Failed to run data path service for PVR fake-name: fake-run-data-path-error", + }, + { + name: "succeed", + pvrName: "fake-name", + runCancelableDataPathResult: "fake-run-data-path-result", + expectedMessage: "fake-run-data-path-result", + expectedSucceed: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + frHelper = &fakeRunHelper{ + initErr: test.initDataPathErr, + runCancelableDataPathErr: test.runCancelableDataPathErr, + runCancelableDataPathResult: test.runCancelableDataPathResult, + } + + if test.createDataPathFail { + funcCreateDataPathRestore = fakeCreateRestoreDataPathServiceWithErr + } else { + funcCreateDataPathRestore = fakeCreateRestoreDataPathService + } + + funcExitWithMessage = frHelper.ExitWithMessage + + s := &podVolumeRestore{ + logger: velerotest.NewLogger(), + cancelFunc: func() {}, + config: podVolumeRestoreConfig{ + pvrName: test.pvrName, + }, + } + + s.runDataPath() + + assert.Equal(t, test.expectedMessage, frHelper.exitMessage) + assert.Equal(t, test.expectedSucceed, frHelper.succeed) + }) + } +} + +func TestCreateRestoreDataPathService(t *testing.T) { + tests := []struct { + name string + fileStoreErr error + secretStoreErr error + mockGetInformer bool + getInformerErr error + expectedError string + }{ + { + name: "create credential file store error", + fileStoreErr: errors.New("fake-file-store-error"), + expectedError: "error to create credential file store: fake-file-store-error", + }, + { + name: "create credential secret store", + secretStoreErr: errors.New("fake-secret-store-error"), + expectedError: "error to create credential secret store: fake-secret-store-error", + }, + { + name: "get informer error", + mockGetInformer: true, + getInformerErr: errors.New("fake-get-informer-error"), + expectedError: "error to get controller-runtime informer from manager: fake-get-informer-error", + }, + { + name: "succeed", + mockGetInformer: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fcHelper := &fakeCreateDataPathServiceHelper{ + fileStoreErr: test.fileStoreErr, + secretStoreErr: test.secretStoreErr, + } + + funcNewCredentialFileStore = fcHelper.NewNamespacedFileStore + funcNewCredentialSecretStore = fcHelper.NewNamespacedSecretStore + + cache := cacheMock.NewCache(t) + if test.mockGetInformer { + cache.On("GetInformer", mock.Anything, mock.Anything).Return(nil, test.getInformerErr) + } + + funcExitWithMessage = frHelper.ExitWithMessage + + s := &podVolumeRestore{ + cache: cache, + } + + _, err := s.createDataPathService() + + if test.expectedError != "" { + assert.EqualError(t, err, test.expectedError) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/podvolume/restore_micro_service.go b/pkg/podvolume/restore_micro_service.go index f7614bf7c..01f33a68a 100644 --- a/pkg/podvolume/restore_micro_service.go +++ b/pkg/podvolume/restore_micro_service.go @@ -304,6 +304,10 @@ func (r *RestoreMicroService) cancelPodVolumeRestore(pvr *velerov1api.PodVolumeR } } +var funcRemoveAll = os.RemoveAll +var funcMkdirAll = os.MkdirAll +var funcWriteFile = os.WriteFile + func writeCompletionMark(pvr *velerov1api.PodVolumeRestore, result datapath.RestoreResult, log logrus.FieldLogger) error { volumePath := result.Target.ByPath if volumePath == "" { @@ -314,7 +318,7 @@ func writeCompletionMark(pvr *velerov1api.PodVolumeRestore, result datapath.Rest // of this volume, which we don't want to carry over). If this fails for any reason, log and continue, since // this is non-essential cleanup (the done files are named based on restore UID and the init container looks // for the one specific to the restore being executed). - if err := os.RemoveAll(filepath.Join(volumePath, ".velero")); err != nil { + if err := funcRemoveAll(filepath.Join(volumePath, ".velero")); err != nil { log.WithError(err).Warnf("Failed to remove .velero directory from directory %s", volumePath) } @@ -326,14 +330,14 @@ func writeCompletionMark(pvr *velerov1api.PodVolumeRestore, result datapath.Rest // Create the .velero directory within the volume dir so we can write a done file // for this restore. - if err := os.MkdirAll(filepath.Join(volumePath, ".velero"), 0755); err != nil { + if err := funcMkdirAll(filepath.Join(volumePath, ".velero"), 0755); err != nil { return errors.Wrapf(err, "error creating .velero directory for done file") } // Write a done file with name= into the just-created .velero dir // within the volume. The velero init container on the pod is waiting // for this file to exist in each restored volume before completing. - if err := os.WriteFile(filepath.Join(volumePath, ".velero", string(restoreUID)), nil, 0644); err != nil { //nolint:gosec // Internal usage. No need to check. + if err := funcWriteFile(filepath.Join(volumePath, ".velero", string(restoreUID)), nil, 0644); err != nil { return errors.Wrapf(err, "error writing done file") } diff --git a/pkg/podvolume/restore_micro_service_test.go b/pkg/podvolume/restore_micro_service_test.go index 0968bfeec..dabbfb83f 100644 --- a/pkg/podvolume/restore_micro_service_test.go +++ b/pkg/podvolume/restore_micro_service_test.go @@ -19,6 +19,7 @@ package podvolume import ( "context" "fmt" + "os" "sync" "testing" "time" @@ -42,6 +43,8 @@ import ( kbclient "sigs.k8s.io/controller-runtime/pkg/client" datapathmockes "github.com/vmware-tanzu/velero/pkg/datapath/mocks" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) type restoreMsTestHelper struct { @@ -468,3 +471,152 @@ func TestRunCancelableDataPathRestore(t *testing.T) { cancel() } + +func TestWriteCompletionMark(t *testing.T) { + tests := []struct { + name string + pvr *velerov1api.PodVolumeRestore + result datapath.RestoreResult + funcRemoveAll func(string) error + funcMkdirAll func(string, os.FileMode) error + funcWriteFile func(string, []byte, os.FileMode) error + expectedErr string + expectedLog string + }{ + { + name: "no volume path", + result: datapath.RestoreResult{}, + expectedErr: "target volume is empty in restore result", + }, + { + name: "no owner reference", + result: datapath.RestoreResult{ + Target: datapath.AccessPoint{ + ByPath: "fake-volume-path", + }, + }, + pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, "fake-pvr").Result(), + funcRemoveAll: func(string) error { + return nil + }, + expectedErr: "error finding restore UID", + }, + { + name: "mkdir fail", + result: datapath.RestoreResult{ + Target: datapath.AccessPoint{ + ByPath: "fake-volume-path", + }, + }, + pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, "fake-pvr").OwnerReference([]metav1.OwnerReference{ + { + UID: "fake-uid", + }, + }).Result(), + funcRemoveAll: func(string) error { + return nil + }, + funcMkdirAll: func(string, os.FileMode) error { + return errors.New("fake-mk-dir-error") + }, + expectedErr: "error creating .velero directory for done file: fake-mk-dir-error", + }, + { + name: "write file fail", + result: datapath.RestoreResult{ + Target: datapath.AccessPoint{ + ByPath: "fake-volume-path", + }, + }, + pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, "fake-pvr").OwnerReference([]metav1.OwnerReference{ + { + UID: "fake-uid", + }, + }).Result(), + funcRemoveAll: func(string) error { + return nil + }, + funcMkdirAll: func(string, os.FileMode) error { + return nil + }, + funcWriteFile: func(string, []byte, os.FileMode) error { + return errors.New("fake-write-file-error") + }, + expectedErr: "error writing done file: fake-write-file-error", + }, + { + name: "succeed", + result: datapath.RestoreResult{ + Target: datapath.AccessPoint{ + ByPath: "fake-volume-path", + }, + }, + pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, "fake-pvr").OwnerReference([]metav1.OwnerReference{ + { + UID: "fake-uid", + }, + }).Result(), + funcRemoveAll: func(string) error { + return nil + }, + funcMkdirAll: func(string, os.FileMode) error { + return nil + }, + funcWriteFile: func(string, []byte, os.FileMode) error { + return nil + }, + }, + { + name: "succeed but previous dir is not removed", + result: datapath.RestoreResult{ + Target: datapath.AccessPoint{ + ByPath: "fake-volume-path", + }, + }, + pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, "fake-pvr").OwnerReference([]metav1.OwnerReference{ + { + UID: "fake-uid", + }, + }).Result(), + funcRemoveAll: func(string) error { + return errors.New("fake-remove-dir-error") + }, + funcMkdirAll: func(string, os.FileMode) error { + return nil + }, + funcWriteFile: func(string, []byte, os.FileMode) error { + return nil + }, + expectedLog: "Failed to remove .velero directory from directory fake-volume-path", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.funcRemoveAll != nil { + funcRemoveAll = test.funcRemoveAll + } + + if test.funcMkdirAll != nil { + funcMkdirAll = test.funcMkdirAll + } + + if test.funcWriteFile != nil { + funcWriteFile = test.funcWriteFile + } + + logBuffer := "" + err := writeCompletionMark(test.pvr, test.result, velerotest.NewSingleLogger(&logBuffer)) + + if test.expectedErr == "" { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, test.expectedErr) + } + + if test.expectedLog != "" { + assert.Contains(t, logBuffer, test.expectedLog) + } + }) + } +}