From b68aa1534446358bf21f69c315f93f513636f096 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 18 Jun 2026 16:41:04 +0800 Subject: [PATCH 01/10] wait PV detachment before deleting PV Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 7 ++++ pkg/util/kube/pvc_pv.go | 68 +++++++++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index f0ad76123..5d0f34d99 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -452,6 +452,13 @@ 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) + if err != nil { + return errors.Wrapf(err, "error waiting for retained PV %s to detach", retained.Name) + } + + curLog.WithField("retained PV", retained.Name).Info("Retained PV is detached") + rebindPV, err = kube.RebindPV(ctx, e.kubeClient.CoreV1(), uuid.NewString(), retained, targetPVC, orgReclaim, param.TargetFSType) if err != nil { return errors.Wrapf(err, "error rebinding PV for target PVC %s", param.TargetPVCName) diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index 7dea36f08..182b18995 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -324,15 +324,10 @@ func RebindPV(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvName maps.Copy(pvLabel, pvc.Spec.Selector.MatchLabels) } - pvAnnotations := make(map[string]string) - maps.Copy(pvAnnotations, source.Annotations) - delete(pvAnnotations, KubeAnnBoundByController) - pv := &corev1api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ - Name: pvName, - Labels: pvLabel, - Annotations: pvAnnotations, + Name: pvName, + Labels: pvLabel, }, Spec: corev1api.PersistentVolumeSpec{ Capacity: source.Spec.Capacity, @@ -358,6 +353,10 @@ func RebindPV(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvName func clonePVSource(source *corev1api.PersistentVolumeSource, newFSType string) corev1api.PersistentVolumeSource { newSource := source.DeepCopy() + if newSource.CSI != nil && newSource.CSI.VolumeAttributes != nil { + delete(newSource.CSI.VolumeAttributes, "storage.kubernetes.io/csiProvisionerIdentity") + } + if newFSType != "" { if newSource.CSI != nil { newSource.CSI.FSType = newFSType @@ -684,22 +683,71 @@ func GetPVAttachedNode(ctx context.Context, pv string, storageClient storagev1.S return "", nil } -func GetPVAttachedNodes(ctx context.Context, pv string, storageClient storagev1.StorageV1Interface) ([]string, error) { +func getPVAttachment(ctx context.Context, pv string, storageClient storagev1.StorageV1Interface) ([]*storagev1api.VolumeAttachment, error) { vaList, err := storageClient.VolumeAttachments().List(ctx, metav1.ListOptions{}) if err != nil { return nil, errors.Wrapf(err, "error listing volumeattachment") } - nodes := []string{} + attachments := []*storagev1api.VolumeAttachment{} for _, va := range vaList.Items { if va.Spec.Source.PersistentVolumeName != nil && *va.Spec.Source.PersistentVolumeName == pv { - nodes = append(nodes, va.Spec.NodeName) + attachments = append(attachments, &va) } } + return attachments, nil +} + +func GetPVAttachedNodes(ctx context.Context, pv string, storageClient storagev1.StorageV1Interface) ([]string, error) { + attachments, err := getPVAttachment(ctx, pv, storageClient) + if err != nil { + return nil, errors.Wrap(err, "error listing volumeattachment") + } + + nodes := []string{} + for _, attach := range attachments { + nodes = append(nodes, attach.Spec.NodeName) + } + return nodes, nil } +func WaitVolumeDetached(ctx context.Context, storageClient storagev1.StorageV1Interface, pv string, timeout time.Duration) error { + attachments, err := getPVAttachment(ctx, pv, storageClient) + if err != nil { + return errors.Wrap(err, "error listing volumeattachment") + } + + err = wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) { + left := []*storagev1api.VolumeAttachment{} + for _, attach := range attachments { + if _, err := storageClient.VolumeAttachments().Get(ctx, attach.Name, metav1.GetOptions{}); err == nil { + left = append(left, attach) + } else if !apierrors.IsNotFound(err) { + return false, err // Return the error if it's not a NotFound error + } + } + + if len(left) == 0 { + return true, nil + } + + attachments = left + + return false, nil + }) + + if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + return errors.Errorf("timeout waiting for volume %s to be detached", pv) + } + return errors.Wrapf(err, "error waiting for volume %s to be detached", pv) + } + + return nil +} + func GetVolumeTopology(ctx context.Context, volumeClient corev1client.CoreV1Interface, storageClient storagev1.StorageV1Interface, pvName string, scName string) (*corev1api.NodeSelector, error) { if pvName == "" || scName == "" { return nil, errors.Errorf("invalid parameter, pv %s, sc %s", pvName, scName) From a8cf6646e1ab3e9f45099d0d933396b7c304da2d Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 13:16:26 +0800 Subject: [PATCH 02/10] wait PV detachment before deleting PV Signed-off-by: Lyndon-Li --- changelogs/unreleased/9932-Lyndon-Li | 1 + pkg/exposer/generic_restore_test.go | 22 +++++++ pkg/util/kube/pvc_pv_test.go | 95 +++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/9932-Lyndon-Li diff --git a/changelogs/unreleased/9932-Lyndon-Li b/changelogs/unreleased/9932-Lyndon-Li new file mode 100644 index 000000000..a09169cf2 --- /dev/null +++ b/changelogs/unreleased/9932-Lyndon-Li @@ -0,0 +1 @@ +Wait restorePV detached before binding the cloned PV to avoid confusing the CSI driver \ No newline at end of file diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index c10bec06b..75da686e6 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -443,6 +443,28 @@ 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", + targetPVCName: "fake-target-pvc", + targetNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObj, + 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 retained PV fake-restore-pv to detach: error listing volumeattachment: error listing volumeattachment: fake-list-error", + }, { name: "rebind pv fail", targetPVCName: "fake-target-pvc", diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index 0f1876ecd..9b93f2971 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -26,6 +26,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" @@ -2239,9 +2240,7 @@ func TestRebindPV(t *testing.T) { "key1": "val3", "key2": "val2", }, - Annotations: map[string]string{ - "anno1": "val1", - }, + Annotations: nil, }, Spec: corev1api.PersistentVolumeSpec{ Capacity: sourcePV.Spec.Capacity, @@ -2364,3 +2363,93 @@ func TestClonePVSource(t *testing.T) { }) } } + +func TestWaitVolumeDetached(t *testing.T) { + pvName := "test-pv" + otherPVName := "other-pv" + + volAttach1 := &storagev1api.VolumeAttachment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "va-1", + }, + Spec: storagev1api.VolumeAttachmentSpec{ + Source: storagev1api.VolumeAttachmentSource{ + PersistentVolumeName: &pvName, + }, + }, + } + + volAttach2 := &storagev1api.VolumeAttachment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "va-2", + }, + Spec: storagev1api.VolumeAttachmentSpec{ + Source: storagev1api.VolumeAttachmentSource{ + PersistentVolumeName: &otherPVName, + }, + }, + } + + tests := []struct { + name string + kubeReactors []reactor + timeout time.Duration + expectedErr string + storageObjs []runtime.Object + }{ + { + name: "no volume attachments", + timeout: time.Second, + }, + { + name: "volume attachments exist and deleted", + timeout: time.Second, + storageObjs: []runtime.Object{volAttach1, volAttach2}, + kubeReactors: []reactor{ + { + verb: "get", + resource: "volumeattachments", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, apierrors.NewNotFound(schema.GroupResource{Group: "storage.k8s.io", Resource: "volumeattachments"}, "va-1") + }, + }, + }, + }, + { + name: "volume attachments exist and not deleted", + timeout: 2 * time.Millisecond, + storageObjs: []runtime.Object{volAttach1, volAttach2}, + expectedErr: "timeout waiting for volume test-pv to be detached", + }, + { + name: "get returns error", + timeout: time.Second, + storageObjs: []runtime.Object{volAttach1, volAttach2}, + kubeReactors: []reactor{ + { + verb: "get", + resource: "volumeattachments", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-error") + }, + }, + }, + expectedErr: "error waiting for volume test-pv to be detached: fake-error", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(test.storageObjs...) + for _, reactor := range test.kubeReactors { + fakeKubeClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc) + } + err := WaitVolumeDetached(t.Context(), fakeKubeClient.StorageV1(), pvName, test.timeout) + if test.expectedErr != "" { + assert.EqualError(t, err, test.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} From 77520d05220cad797cc432cb4b887d30f497282a Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 16:26:12 +0800 Subject: [PATCH 03/10] recall the way to rebind volume with restorePV Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 89 +++++++++++- pkg/exposer/generic_restore_test.go | 210 +++++++++++++++++++++++++--- pkg/util/kube/pvc_pv.go | 16 +++ pkg/util/kube/utils_test.go | 92 ++++++++++++ 4 files changed, 386 insertions(+), 21 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 5d0f34d99..8097a46e3 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") @@ -494,6 +504,83 @@ 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.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..a95c1e51e 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, @@ -466,12 +483,12 @@ 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", }, { - 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", }, { - 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) + }) + } +} From 20a0def15d7c5bc0d335641a0c8d1e0ba1a109ee Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 17:23:24 +0800 Subject: [PATCH 04/10] add wait restorePV detach to same mode route Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 8097a46e3..ab5efe2b4 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -542,6 +542,13 @@ func (e *genericRestoreExposer) rebindVolumeSameMode(ctx context.Context, ownerO 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) From eb0aa625cebefe61c1cd9c04938e86c40d328fca Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 17:24:53 +0800 Subject: [PATCH 05/10] use restorePV to cover retained and non-retained case Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 18 +++++++++++------- pkg/exposer/generic_restore_test.go | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index ab5efe2b4..f79b0b629 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -450,6 +450,10 @@ func (e *genericRestoreExposer) rebindVolumeChangeMode(ctx context.Context, owne } }() + 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) @@ -462,26 +466,26 @@ func (e *genericRestoreExposer) rebindVolumeChangeMode(ctx context.Context, owne 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 diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index a95c1e51e..b5b7530d6 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -480,7 +480,7 @@ 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: "[change mode] rebind pv fail", @@ -528,7 +528,7 @@ func TestRebindVolume(t *testing.T) { }, }, }, - 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: "[change mode] rebind target pvc fail", From 04ef90dfb7be6c700fb37538dacb3cca3a1e2879 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 17:35:44 +0800 Subject: [PATCH 06/10] recall the way to rebind volume with restorePV Signed-off-by: Lyndon-Li --- changelogs/unreleased/9933-Lyndon-Li | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/9933-Lyndon-Li 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 From 9a615430ed3e7ecf4a66e0f8b3d9f31f61fbc0da Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 16:26:12 +0800 Subject: [PATCH 07/10] recall the way to rebind volume with restorePV Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 89 +++++++++++- pkg/exposer/generic_restore_test.go | 210 +++++++++++++++++++++++++--- pkg/util/kube/pvc_pv.go | 16 +++ pkg/util/kube/utils_test.go | 92 ++++++++++++ 4 files changed, 386 insertions(+), 21 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 5d0f34d99..8097a46e3 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") @@ -494,6 +504,83 @@ 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.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..a95c1e51e 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, @@ -466,12 +483,12 @@ 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", }, { - 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", }, { - 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) + }) + } +} From 0f9874bf062b5930bca1536f6547668a779ede4a Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 17:23:24 +0800 Subject: [PATCH 08/10] add wait restorePV detach to same mode route Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 8097a46e3..ab5efe2b4 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -542,6 +542,13 @@ func (e *genericRestoreExposer) rebindVolumeSameMode(ctx context.Context, ownerO 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) From 8c3a638d884ec2730960e2c4377a35373bd78bb1 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 17:24:53 +0800 Subject: [PATCH 09/10] use restorePV to cover retained and non-retained case Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 18 +++++++++++------- pkg/exposer/generic_restore_test.go | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index ab5efe2b4..f79b0b629 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -450,6 +450,10 @@ func (e *genericRestoreExposer) rebindVolumeChangeMode(ctx context.Context, owne } }() + 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) @@ -462,26 +466,26 @@ func (e *genericRestoreExposer) rebindVolumeChangeMode(ctx context.Context, owne 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 diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index a95c1e51e..b5b7530d6 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -480,7 +480,7 @@ 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: "[change mode] rebind pv fail", @@ -528,7 +528,7 @@ func TestRebindVolume(t *testing.T) { }, }, }, - 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: "[change mode] rebind target pvc fail", From 6af6e4e85f2a6d41edc83f77e17327ff4e337894 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Mon, 22 Jun 2026 17:35:44 +0800 Subject: [PATCH 10/10] recall the way to rebind volume with restorePV Signed-off-by: Lyndon-Li --- changelogs/unreleased/9933-Lyndon-Li | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/9933-Lyndon-Li 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