diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 15105be47..ca1655fae 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -855,9 +855,6 @@ func (e *genericRestoreExposer) createRestorePVC(ctx context.Context, ownerObjec Resources: targetPVC.Spec.Resources, }, } - if targetPV != nil { - pvcObj.Spec.VolumeName = targetPV.Name - } if selectedNode != "" { if pvcObj.Annotations == nil { @@ -874,12 +871,56 @@ func (e *genericRestoreExposer) createRestorePVC(ctx context.Context, ownerObjec *pvcObj.Spec.VolumeMode = corev1api.PersistentVolumeBlock } + volumeName := "" + sameVolumeMode := true + if targetPV != nil { + volumeName = targetPV.Name + sameVolumeMode = kube.GetVolumeModeByPVC(pvcObj) == kube.GetVolumeModeByPV(targetPV) + if !sameVolumeMode { + volumeName = ownerObject.Name + } + pvcObj.Spec.VolumeName = volumeName + } + restorePVC, err := e.kubeClient.CoreV1().PersistentVolumeClaims(pvcObj.Namespace).Create(ctx, pvcObj, metav1.CreateOptions{}) if err != nil { return nil, errors.Wrapf(err, "fail to create the restore PVC %s in namespace %s", pvcObj.Name, pvcObj.Namespace) } + defer func() { + if err != nil { + kube.DeletePVCIfAny(ctx, e.kubeClient.CoreV1(), pvcObj.Name, pvcObj.Namespace, 0, e.log) + } + }() + if targetPV != nil { + if !sameVolumeMode { + tmpPV := &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + }, + Spec: *targetPV.Spec.DeepCopy(), + } + tmpPV.Spec.VolumeMode = restorePVC.Spec.VolumeMode + e.log.Infof("the volume mode is different, creating temporary PV %s with volume mode %s", tmpPV.Name, tmpPV.Spec.VolumeMode) + tmpPV, err = e.kubeClient.CoreV1().PersistentVolumes().Create(ctx, tmpPV, metav1.CreateOptions{}) + if err != nil { + return nil, errors.Wrapf(err, "fail to create the temporary PV %s", volumeName) + } + + defer func() { + if err != nil { + kube.DeletePVIfAny(ctx, e.kubeClient.CoreV1(), tmpPV.Name, e.log) + } + }() + + e.log.Infof("deleting the target PV %s", targetPV.Name) + if err = e.kubeClient.CoreV1().PersistentVolumes().Delete(ctx, targetPV.Name, metav1.DeleteOptions{}); err != nil { + return nil, errors.Wrapf(err, "fail to delete the target PV %s", targetPV.Name) + } + targetPV = tmpPV + } + if _, err = kube.ResetPVBinding(ctx, e.kubeClient.CoreV1(), targetPV, nil, restorePVC); err != nil { return nil, errors.Wrapf(err, "fail to reset PV %s binding to restore PVC %s/%s", targetPV.Name, restorePVC.Namespace, restorePVC.Name) } diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index 3a9e4bd68..7703e710f 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -67,6 +67,16 @@ func TestRestoreExpose(t *testing.T) { }, } + modeBlock := corev1api.PersistentVolumeBlock + targetPVObjWithDifferentVolumeMode := &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-target-pv", + }, + Spec: corev1api.PersistentVolumeSpec{ + VolumeMode: &modeBlock, + }, + } + modeFilesystem := corev1api.PersistentVolumeFilesystem targetPVCObjWithVolumeMode := &corev1api.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -130,6 +140,7 @@ func TestRestoreExpose(t *testing.T) { expectBackupPod bool expectBackupPVC bool expectCachePVC bool + expectBackupPV bool err string }{ { @@ -244,6 +255,96 @@ func TestRestoreExpose(t *testing.T) { expectBackupPod: true, expectBackupPVC: true, }, + { + name: "create temporary PV fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + targetPVName: "fake-target-pv", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObj, + targetPVObjWithDifferentVolumeMode, + daemonSet, + storageClass, + }, + kubeReactors: []reactor{ + { + verb: "create", + resource: "persistentvolumes", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-create-pv-error") + }, + }, + }, + err: "error to create restore pvc: fail to create the temporary PV fake-restore: fake-create-pv-error", + }, + { + name: "delete original PV fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + targetPVName: "fake-target-pv", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObj, + targetPVObjWithDifferentVolumeMode, + daemonSet, + storageClass, + }, + kubeReactors: []reactor{ + { + verb: "delete", + resource: "persistentvolumes", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + deleteAction := action.(clientTesting.DeleteAction) + if deleteAction.GetName() == "fake-target-pv" { + return true, nil, errors.New("fake-delete-pv-error") + } + return false, nil, nil + }, + }, + }, + err: "error to create restore pvc: fail to delete the target PV fake-target-pv: fake-delete-pv-error", + }, + { + name: "succeed with target PV set and different volume mode", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + targetPVName: "fake-target-pv", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObj, + targetPVObjWithDifferentVolumeMode, + daemonSet, + storageClass, + }, + kubeReactors: []reactor{ + { + verb: "get", + resource: "persistentvolumeclaims", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + getAction := action.(clientTesting.GetAction) + if getAction.GetName() == "fake-restore" { + return true, &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-restore", + Namespace: velerov1.DefaultNamespace, + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "fake-restore", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, nil + } + return false, nil, nil + }, + }, + }, + expectBackupPod: true, + expectBackupPVC: true, + expectBackupPV: true, + }, { name: "succeed, cache config, no cache volume", targetPVCName: "fake-target-pvc", @@ -375,7 +476,7 @@ func TestRestoreExpose(t *testing.T) { if test.expectBackupPod { require.NoError(t, err) } else { - require.True(t, apierrors.IsNotFound(err)) + require.True(t, apierrors.IsNotFound(err), "expected IsNotFound, got %v", err) } pvc, err := exposer.kubeClient.CoreV1().PersistentVolumeClaims(ownerObject.Namespace).Get(t.Context(), ownerObject.Name, metav1.GetOptions{}) @@ -386,14 +487,31 @@ func TestRestoreExpose(t *testing.T) { require.Equal(t, corev1api.PersistentVolumeBlock, *pvc.Spec.VolumeMode) } } else { - require.True(t, apierrors.IsNotFound(err)) + require.True(t, apierrors.IsNotFound(err), "expected IsNotFound, got %v", err) } _, err = exposer.kubeClient.CoreV1().PersistentVolumeClaims(ownerObject.Namespace).Get(t.Context(), getCachePVCName(ownerObject), metav1.GetOptions{}) if test.expectCachePVC { require.NoError(t, err) } else { - require.True(t, apierrors.IsNotFound(err)) + require.True(t, apierrors.IsNotFound(err), "expected IsNotFound, got %v", err) + } + + _, err = exposer.kubeClient.CoreV1().PersistentVolumes().Get(t.Context(), ownerObject.Name, metav1.GetOptions{}) + if test.expectBackupPV { + require.NoError(t, err) + } else { + require.True(t, apierrors.IsNotFound(err), "expected IsNotFound, got %v", err) + } + + if test.targetPVName != "" && !test.expectBackupPV && test.err == "" { + // if targetPVName was provided, and sameVolumeMode was true, the original PV should still exist + _, err = exposer.kubeClient.CoreV1().PersistentVolumes().Get(t.Context(), test.targetPVName, metav1.GetOptions{}) + require.NoError(t, err) + } else if test.targetPVName != "" && test.expectBackupPV { + // if targetPVName was provided, and sameVolumeMode was false (expectBackupPV is true), the original PV should be deleted + _, err = exposer.kubeClient.CoreV1().PersistentVolumes().Get(t.Context(), test.targetPVName, metav1.GetOptions{}) + require.True(t, apierrors.IsNotFound(err), "expected original PV %s to be deleted, but it still exists", test.targetPVName) } }) }