diff --git a/changelogs/unreleased/9933-Lyndon-Li b/changelogs/unreleased/9933-Lyndon-Li new file mode 100644 index 000000000..1d8582fe9 --- /dev/null +++ b/changelogs/unreleased/9933-Lyndon-Li @@ -0,0 +1 @@ +Recall the old rebind volume way for the case that volumeMode is not changed; and use the new way for volumeMode changed case \ No newline at end of file diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 5d0f34d99..f79b0b629 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -396,7 +396,6 @@ func (e *genericRestoreExposer) CleanUp(ctx context.Context, ownerObject corev1a } func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject corev1api.ObjectReference, param GenericRestoreRebindVolumeParam) error { - restorePodName := ownerObject.Name restorePVCName := ownerObject.Name curLog := e.log.WithFields(logrus.Fields{ @@ -415,6 +414,17 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co return errors.Wrapf(err, "error to get PV from restore PVC %s", restorePVCName) } + if kube.GetVolumeModeByPVC(targetPVC) != kube.GetVolumeModeByPV(restorePV) { + return e.rebindVolumeChangeMode(ctx, ownerObject, param, targetPVC, restorePV, curLog) + } else { + return e.rebindVolumeSameMode(ctx, ownerObject, param, targetPVC, restorePV, curLog) + } +} + +func (e *genericRestoreExposer) rebindVolumeChangeMode(ctx context.Context, ownerObject corev1api.ObjectReference, param GenericRestoreRebindVolumeParam, targetPVC *corev1api.PersistentVolumeClaim, restorePV *corev1api.PersistentVolume, curLog logrus.FieldLogger) error { + restorePodName := ownerObject.Name + restorePVCName := ownerObject.Name + orgReclaim := restorePV.Spec.PersistentVolumeReclaimPolicy curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is retrieved") @@ -440,6 +450,10 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co } }() + if retained != nil { + restorePV = retained + } + err = kube.EnsureDeletePod(ctx, e.kubeClient.CoreV1(), restorePodName, ownerObject.Namespace, param.OperationTimeout) if err != nil { return errors.Wrapf(err, "error to delete restore pod %s", restorePodName) @@ -452,26 +466,26 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co curLog.WithField("restore PVC", restorePVCName).Info("Restore PVC is deleted") - err = kube.WaitVolumeDetached(ctx, e.kubeClient.StorageV1(), retained.Name, param.OperationTimeout) + err = kube.WaitVolumeDetached(ctx, e.kubeClient.StorageV1(), restorePV.Name, param.OperationTimeout) if err != nil { - return errors.Wrapf(err, "error waiting for retained PV %s to detach", retained.Name) + return errors.Wrapf(err, "error waiting for restore PV %s to detach", restorePV.Name) } - curLog.WithField("retained PV", retained.Name).Info("Retained PV is detached") + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is detached") - rebindPV, err = kube.RebindPV(ctx, e.kubeClient.CoreV1(), uuid.NewString(), retained, targetPVC, orgReclaim, param.TargetFSType) + rebindPV, err = kube.RebindPV(ctx, e.kubeClient.CoreV1(), uuid.NewString(), restorePV, targetPVC, orgReclaim, param.TargetFSType) if err != nil { return errors.Wrapf(err, "error rebinding PV for target PVC %s", param.TargetPVCName) } curLog.WithField("rebind PV", rebindPV.Name).Info("Rebind PV is created") - err = kube.EnsureDeletePV(ctx, e.kubeClient.CoreV1(), retained.Name, param.OperationTimeout) + err = kube.EnsureDeletePV(ctx, e.kubeClient.CoreV1(), restorePV.Name, param.OperationTimeout) if err != nil { - return errors.Wrapf(err, "error deleting PV %s", retained.Name) + return errors.Wrapf(err, "error deleting restore PV %s", restorePV.Name) } - curLog.WithField("retained PV", retained.Name).Info("Retained PV is deleted") + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is deleted") retained = nil @@ -494,6 +508,90 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co return nil } +func (e *genericRestoreExposer) rebindVolumeSameMode(ctx context.Context, ownerObject corev1api.ObjectReference, param GenericRestoreRebindVolumeParam, targetPVC *corev1api.PersistentVolumeClaim, restorePV *corev1api.PersistentVolume, curLog logrus.FieldLogger) error { + restorePodName := ownerObject.Name + restorePVCName := ownerObject.Name + + orgReclaim := restorePV.Spec.PersistentVolumeReclaimPolicy + + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is retrieved") + + retained, err := kube.SetPVReclaimPolicy(ctx, e.kubeClient.CoreV1(), restorePV, corev1api.PersistentVolumeReclaimRetain) + if err != nil { + return errors.Wrapf(err, "error to retain PV %s", restorePV.Name) + } + + curLog.WithField("restore PV", restorePV.Name).WithField("retained", (retained != nil)).Info("Restore PV is retained") + + defer func() { + if retained != nil { + curLog.WithField("retained PV", retained.Name).Info("Deleting retained PV on error") + kube.DeletePVIfAny(ctx, e.kubeClient.CoreV1(), retained.Name, curLog) + } + }() + + if retained != nil { + restorePV = retained + } + + err = kube.EnsureDeletePod(ctx, e.kubeClient.CoreV1(), restorePodName, ownerObject.Namespace, param.OperationTimeout) + if err != nil { + return errors.Wrapf(err, "error to delete restore pod %s", restorePodName) + } + + err = kube.EnsureDeletePVC(ctx, e.kubeClient.CoreV1(), restorePVCName, ownerObject.Namespace, param.OperationTimeout) + if err != nil { + return errors.Wrapf(err, "error to delete restore PVC %s", restorePVCName) + } + + curLog.WithField("restore PVC", restorePVCName).Info("Restore PVC is deleted") + + err = kube.WaitVolumeDetached(ctx, e.kubeClient.StorageV1(), restorePV.Name, param.OperationTimeout) + if err != nil { + return errors.Wrapf(err, "error waiting for restore PV %s to detach", restorePV.Name) + } + + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is detached") + + _, err = kube.RebindPVC(ctx, e.kubeClient.CoreV1(), targetPVC, restorePV.Name) + if err != nil { + return errors.Wrapf(err, "error to rebind target PVC %s/%s to %s", targetPVC.Namespace, targetPVC.Name, restorePV.Name) + } + + curLog.WithField("tartet PVC", fmt.Sprintf("%s/%s", targetPVC.Namespace, targetPVC.Name)).WithField("restore PV", restorePV.Name).Info("Target PVC is rebound to restore PV") + + var matchLabel map[string]string + if targetPVC.Spec.Selector != nil { + matchLabel = targetPVC.Spec.Selector.MatchLabels + } + + restorePVName := restorePV.Name + restorePV, err = kube.ResetPVBinding(ctx, e.kubeClient.CoreV1(), restorePV, matchLabel, targetPVC) + if err != nil { + return errors.Wrapf(err, "error to reset binding info for restore PV %s", restorePVName) + } + + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is rebound") + + restorePV, err = kube.WaitPVBound(ctx, e.kubeClient.CoreV1(), restorePV.Name, targetPVC.Name, targetPVC.Namespace, param.OperationTimeout) + if err != nil { + return errors.Wrapf(err, "error to wait restore PV bound, restore PV %s", restorePVName) + } + + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV is ready") + + retained = nil + + _, err = kube.SetPVReclaimPolicy(ctx, e.kubeClient.CoreV1(), restorePV, orgReclaim) + if err != nil { + curLog.WithField("restore PV", restorePV.Name).WithError(err).Warn("Restore PV's reclaim policy is not restored") + } else { + curLog.WithField("restore PV", restorePV.Name).Info("Restore PV's reclaim policy is restored") + } + + return nil +} + func (e *genericRestoreExposer) createRestorePod( ctx context.Context, ownerObject corev1api.ObjectReference, diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index 75da686e6..b5b7530d6 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -319,11 +319,27 @@ func TestRebindVolume(t *testing.T) { }, } - targetPVCObj := &corev1api.PersistentVolumeClaim{ + modeFilesystem := corev1api.PersistentVolumeFilesystem + modeBlock := corev1api.PersistentVolumeBlock + + targetPVCObjChangeMode := &corev1api.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Namespace: "fake-ns", Name: "fake-target-pvc", }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: &modeBlock, + }, + } + + targetPVCObjSameMode := &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-target-pvc", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: &modeFilesystem, + }, } restorePVCObj := &corev1api.PersistentVolumeClaim{ @@ -342,6 +358,7 @@ func TestRebindVolume(t *testing.T) { }, Spec: corev1api.PersistentVolumeSpec{ PersistentVolumeReclaimPolicy: corev1api.PersistentVolumeReclaimDelete, + VolumeMode: &modeFilesystem, }, } @@ -374,17 +391,17 @@ func TestRebindVolume(t *testing.T) { targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjSameMode, }, err: "error to get PV from restore PVC fake-restore: error to wait for rediness of PVC: error to get pvc velero/fake-restore: persistentvolumeclaims \"fake-restore\" not found", }, { - name: "retain target pv fail", + name: "[change mode] retain target pv fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, }, @@ -400,12 +417,12 @@ func TestRebindVolume(t *testing.T) { err: "error to retain PV fake-restore-pv: error patching PV: fake-patch-error", }, { - name: "delete restore pod fail", + name: "[change mode] delete restore pod fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, @@ -422,12 +439,12 @@ func TestRebindVolume(t *testing.T) { err: "error to delete restore pod fake-restore: error to delete pod fake-restore: fake-delete-error", }, { - name: "delete restore pvc fail", + name: "[change mode] delete restore pvc fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, @@ -444,12 +461,12 @@ func TestRebindVolume(t *testing.T) { err: "error to delete restore PVC fake-restore: error to delete pvc fake-restore: fake-delete-error", }, { - name: "wait volume detached fail", + name: "[change mode] wait volume detached fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, @@ -463,15 +480,15 @@ func TestRebindVolume(t *testing.T) { }, }, }, - err: "error waiting for retained PV fake-restore-pv to detach: error listing volumeattachment: error listing volumeattachment: fake-list-error", + err: "error waiting for restore PV fake-restore-pv to detach: error listing volumeattachment: error listing volumeattachment: fake-list-error", }, { - name: "rebind pv fail", + name: "[change mode] rebind pv fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, @@ -488,12 +505,12 @@ func TestRebindVolume(t *testing.T) { err: "error rebinding PV for target PVC fake-target-pvc: fake-create-error", }, { - name: "delete retained pv fail", + name: "[change mode] delete retained pv fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, @@ -503,19 +520,23 @@ func TestRebindVolume(t *testing.T) { verb: "delete", resource: "persistentvolumes", reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, errors.New("fake-delete-error") + // we want it to fail on the PV deletion but not the pod/pvc deletions + if action.(clientTesting.DeleteAction).GetName() == "fake-restore-pv" { + return true, nil, errors.New("fake-delete-error") + } + return false, nil, nil }, }, }, - err: "error deleting PV fake-restore-pv: error to delete pv fake-restore-pv: fake-delete-error", + err: "error deleting restore PV fake-restore-pv: error to delete pv fake-restore-pv: fake-delete-error", }, { - name: "rebind target pvc fail", + name: "[change mode] rebind target pvc fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, @@ -532,18 +553,168 @@ func TestRebindVolume(t *testing.T) { err: "error to rebind target PVC fake-ns/fake-target-pvc to", }, { - name: "wait rebind PV ready fail", + name: "[change mode] wait rebind PV ready fail", targetPVCName: "fake-target-pvc", targetNamespace: "fake-ns", ownerRestore: restore, kubeClientObj: []runtime.Object{ - targetPVCObj, + targetPVCObjChangeMode, restorePVCObj, restorePVObj, restorePod, }, err: "error to wait rebind PV ready, rebind PV", }, + { + name: "[same mode] retain target pv fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + }, + kubeReactors: []reactor{ + { + verb: "patch", + resource: "persistentvolumes", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-patch-error") + }, + }, + }, + err: "error to retain PV fake-restore-pv: error patching PV: fake-patch-error", + }, + { + name: "[same mode] delete restore pod fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + restorePod, + }, + kubeReactors: []reactor{ + { + verb: "delete", + resource: "pods", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-delete-error") + }, + }, + }, + err: "error to delete restore pod fake-restore: error to delete pod fake-restore: fake-delete-error", + }, + { + name: "[same mode] delete restore pvc fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + restorePod, + }, + kubeReactors: []reactor{ + { + verb: "delete", + resource: "persistentvolumeclaims", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-delete-error") + }, + }, + }, + err: "error to delete restore PVC fake-restore: error to delete pvc fake-restore: fake-delete-error", + }, + { + name: "[same mode] wait volume detached fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + restorePod, + }, + kubeReactors: []reactor{ + { + verb: "list", + resource: "volumeattachments", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-list-error") + }, + }, + }, + err: "error waiting for restore PV fake-restore-pv to detach: error listing volumeattachment: error listing volumeattachment: fake-list-error", + }, + { + name: "[same mode] rebind target pvc fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + restorePod, + }, + kubeReactors: []reactor{ + { + verb: "patch", + resource: "persistentvolumeclaims", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-patch-error") + }, + }, + }, + err: "error to rebind target PVC fake-ns/fake-target-pvc to fake-restore-pv: error patching PVC: fake-patch-error", + }, + { + name: "[same mode] reset pv binding fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + restorePod, + }, + kubeReactors: []reactor{ + { + verb: "patch", + resource: "persistentvolumes", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + // we need it to succeed on set reclaim policy, but fail on reset binding + patchAction := action.(clientTesting.PatchAction) + patchString := string(patchAction.GetPatch()) + if patchString != `{"spec":{"persistentVolumeReclaimPolicy":"Retain"}}` { + return true, nil, errors.New("fake-patch-error-reset") + } + return false, nil, nil + }, + }, + }, + err: "error to reset binding info for restore PV fake-restore-pv: error patching PV: fake-patch-error-reset", + }, + { + name: "[same mode] wait restore PV bound fail", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjSameMode, + restorePVCObj, + restorePVObj, + restorePod, + }, + err: "error to wait restore PV bound, restore PV fake-restore-pv: error to wait for bound of PV: context deadline exceeded", + }, } for _, test := range tests { @@ -583,7 +754,6 @@ func TestRebindVolume(t *testing.T) { }) } } - func TestRestorePeekExpose(t *testing.T) { restore := &velerov1.Restore{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index 182b18995..7db9df3e4 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -773,3 +773,19 @@ func GetVolumeTopology(ctx context.Context, volumeClient corev1client.CoreV1Inte return pv.Spec.NodeAffinity.Required, nil } + +func GetVolumeModeByPVC(pvc *corev1api.PersistentVolumeClaim) corev1api.PersistentVolumeMode { + if pvc.Spec.VolumeMode != nil { + return *pvc.Spec.VolumeMode + } + + return corev1api.PersistentVolumeFilesystem +} + +func GetVolumeModeByPV(pv *corev1api.PersistentVolume) corev1api.PersistentVolumeMode { + if pv.Spec.VolumeMode != nil { + return *pv.Spec.VolumeMode + } + + return corev1api.PersistentVolumeFilesystem +} diff --git a/pkg/util/kube/utils_test.go b/pkg/util/kube/utils_test.go index df23903a0..23db12a41 100644 --- a/pkg/util/kube/utils_test.go +++ b/pkg/util/kube/utils_test.go @@ -730,3 +730,95 @@ func TestVerifyJsonConfigs(t *testing.T) { }) } } + +func TestGetVolumeModeByPVC(t *testing.T) { + modeFilesystem := corev1api.PersistentVolumeFilesystem + modeBlock := corev1api.PersistentVolumeBlock + + tests := []struct { + name string + pvc *corev1api.PersistentVolumeClaim + expected corev1api.PersistentVolumeMode + }{ + { + name: "nil VolumeMode returns Filesystem", + pvc: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: nil, + }, + }, + expected: corev1api.PersistentVolumeFilesystem, + }, + { + name: "Filesystem VolumeMode returns Filesystem", + pvc: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: &modeFilesystem, + }, + }, + expected: corev1api.PersistentVolumeFilesystem, + }, + { + name: "Block VolumeMode returns Block", + pvc: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: &modeBlock, + }, + }, + expected: corev1api.PersistentVolumeBlock, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := GetVolumeModeByPVC(test.pvc) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestGetVolumeModeByPV(t *testing.T) { + modeFilesystem := corev1api.PersistentVolumeFilesystem + modeBlock := corev1api.PersistentVolumeBlock + + tests := []struct { + name string + pv *corev1api.PersistentVolume + expected corev1api.PersistentVolumeMode + }{ + { + name: "nil VolumeMode returns Filesystem", + pv: &corev1api.PersistentVolume{ + Spec: corev1api.PersistentVolumeSpec{ + VolumeMode: nil, + }, + }, + expected: corev1api.PersistentVolumeFilesystem, + }, + { + name: "Filesystem VolumeMode returns Filesystem", + pv: &corev1api.PersistentVolume{ + Spec: corev1api.PersistentVolumeSpec{ + VolumeMode: &modeFilesystem, + }, + }, + expected: corev1api.PersistentVolumeFilesystem, + }, + { + name: "Block VolumeMode returns Block", + pv: &corev1api.PersistentVolume{ + Spec: corev1api.PersistentVolumeSpec{ + VolumeMode: &modeBlock, + }, + }, + expected: corev1api.PersistentVolumeBlock, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := GetVolumeModeByPV(test.pv) + assert.Equal(t, test.expected, actual) + }) + } +}