From 770a251ed5710762c3074b3179efa8aa1af9c83c Mon Sep 17 00:00:00 2001 From: Abhayraj Jaiswal Date: Mon, 7 Sep 2026 08:07:24 +0000 Subject: [PATCH] fix(restore): propagate context and standardize errors in CSI PVC restore action Signed-off-by: Abhayraj Jaiswal --- pkg/restore/actions/csi/pvc_action.go | 18 +++++++++--------- pkg/restore/actions/csi/pvc_action_test.go | 21 ++++++++++++++++++++- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/pkg/restore/actions/csi/pvc_action.go b/pkg/restore/actions/csi/pvc_action.go index f71d0965a..363507709 100644 --- a/pkg/restore/actions/csi/pvc_action.go +++ b/pkg/restore/actions/csi/pvc_action.go @@ -228,7 +228,7 @@ func (p *pvcRestoreItemAction) executeWithDataMove(logger *logrus.Entry, input * var dataUploadResult *velerov2alpha1.DataUploadResult dataUploadResult, err = getDataUploadResult(ctx, input.Restore, pvc, p.crClient) if err != nil { - return nil, errors.Wrapf(err, "fail get DataUploadResult for restore: %s", input.Restore.Name) + return nil, errors.Wrapf(err, "failed to get DataUploadResult for restore: %s", input.Restore.Name) } var volumeSnapshot *snapshotv1api.VolumeSnapshot @@ -279,10 +279,10 @@ func (p *pvcRestoreItemAction) executeWithDataMove(logger *logrus.Entry, input * var dataDownload *velerov2alpha1.DataDownload dataDownload, err = restoreFromDataUploadResult( - context.Background(), dataUploadResult, input.Restore, backup, pvc, existingPV, newNamespace, + ctx, dataUploadResult, input.Restore, backup, pvc, existingPV, newNamespace, operationID, string(restoreType), volumeSnapshot, p.crClient) if err != nil { - logger.Errorf("Fail to restore from DataUploadResult: %s", err.Error()) + logger.Errorf("Failed to restore from DataUploadResult: %s", err.Error()) return nil, errors.WithStack(err) } logger.Infof("DataDownload %s/%s is created successfully.", @@ -339,7 +339,7 @@ func (p *pvcRestoreItemAction) Progress( p.crClient, ) if err != nil { - logger.Errorf("fail to get DataDownload: %s", err.Error()) + logger.Errorf("Failed to get DataDownload: %s", err.Error()) return progress, err } if dataDownload.Status.Phase == velerov2alpha1.DataDownloadPhaseNew || @@ -392,13 +392,13 @@ func (p *pvcRestoreItemAction) Cancel( p.crClient, ) if err != nil { - logger.Errorf("fail to get DataDownload: %s", err.Error()) + logger.Errorf("Failed to get DataDownload: %s", err.Error()) return err } err = cancelDataDownload(context.Background(), p.crClient, dataDownload) if err != nil { - logger.Errorf("fail to cancel DataDownload %s: %s", dataDownload.Name, err.Error()) + logger.Errorf("Failed to cancel DataDownload %s: %s", dataDownload.Name, err.Error()) } return err } @@ -601,7 +601,7 @@ func restoreFromDataUploadResult( ) err := crClient.Create(ctx, dataDownload) if err != nil { - return nil, errors.Wrapf(err, "fail to create DataDownload") + return nil, errors.Wrapf(err, "failed to create DataDownload") } return dataDownload, nil @@ -654,8 +654,8 @@ func (p *pvcRestoreItemAction) deleteExistingPVC(ctx context.Context, logger *lo var err error logger.Info("ExistingVolumeDataPolicy is in-place restore. Deleting the existing PVC but keep the PV...") pv := &corev1api.PersistentVolume{} - if err = p.crClient.Get(context.Background(), crclient.ObjectKey{Name: existingPVC.Spec.VolumeName}, pv); err != nil { - return nil, errors.Errorf("Fail to get PV %s: %s", existingPVC.Spec.VolumeName, err.Error()) + if err = p.crClient.Get(ctx, crclient.ObjectKey{Name: existingPVC.Spec.VolumeName}, pv); err != nil { + return nil, errors.Wrapf(err, "failed to get PV %s", existingPVC.Spec.VolumeName) } // set reclaim policy to retain diff --git a/pkg/restore/actions/csi/pvc_action_test.go b/pkg/restore/actions/csi/pvc_action_test.go index b8e4912f6..acdfa5938 100644 --- a/pkg/restore/actions/csi/pvc_action_test.go +++ b/pkg/restore/actions/csi/pvc_action_test.go @@ -450,7 +450,7 @@ func TestExecute(t *testing.T) { restore: builder.ForRestore("velero", "testRestore").Backup("testBackup").Result(), pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName", velerov1api.VolumeSnapshotRestoreSize, "10Gi", velerov1api.DataUploadNameAnnotation, "velero/")).Result(), expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").Result(), - expectedErr: "fail get DataUploadResult for restore: testRestore: no DataUpload result cm found with labels velero.io/pvc-namespace-name=velero.testPVC,velero.io/restore-uid=,velero.io/resource-usage=DataUpload", + expectedErr: "failed to get DataUploadResult for restore: testRestore: no DataUpload result cm found with labels velero.io/pvc-namespace-name=velero.testPVC,velero.io/restore-uid=,velero.io/resource-usage=DataUpload", }, { name: "Restore from DataUploadResult", @@ -884,3 +884,22 @@ func TestNewPvcRestoreItemAction(t *testing.T) { _, err1 := plugin1(logger) require.NoError(t, err1) } + +func TestDeleteExistingPVCFailure(t *testing.T) { + pvcRIA := pvcRestoreItemAction{ + log: logrus.New(), + crClient: velerotest.NewFakeControllerRuntimeClient(t), + kubeClient: fake.NewSimpleClientset(), + } + existingPVC := builder.ForPersistentVolumeClaim("ns-1", "pvc-1"). + VolumeName("non-existent-pv"). + Phase(corev1api.ClaimBound).Result() + targetPVC := builder.ForPersistentVolumeClaim("ns-1", "pvc-1").Result() + + returnedPV, err := pvcRIA.deleteExistingPVC( + t.Context(), logrus.New().WithField("test", "fail-to-get-pv"), + targetPVC, existingPVC, time.Minute) + require.Error(t, err) + assert.Nil(t, returnedPV) + assert.Contains(t, err.Error(), "failed to get PV non-existent-pv") +}