From eebb879278d4a7217928188ebc9eb8a1c1760385 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 13 Jul 2023 18:29:43 +0800 Subject: [PATCH] data mover restore abort for existing pvc Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 4 ++ pkg/exposer/generic_restore_test.go | 20 ++++++++++ pkg/util/kube/pvc_pv.go | 9 ++++- pkg/util/kube/pvc_pv_test.go | 60 +++++++++++++++++++++++++---- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 3617c53a8..f11ac27c8 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -78,6 +78,10 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1.O curLog.WithField("target PVC", targetPVCName).WithField("selected node", selectedNode).Info("Target PVC is consumed") + if kube.IsPVCBound(targetPVC) { + return errors.Errorf("Target PVC %s/%s has already been bound, abort", sourceNamespace, targetPVCName) + } + restorePod, err := e.createRestorePod(ctx, ownerObject, hostingPodLabels, selectedNode) if err != nil { return errors.Wrapf(err, "error to create restore pod") diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index 2b7f4f0a6..23dd3f480 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -54,6 +54,16 @@ func TestRestoreExpose(t *testing.T) { }, } + targetPVCObjBound := &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-target-pvc", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "fake-pv", + }, + } + tests := []struct { name string kubeClientObj []runtime.Object @@ -70,6 +80,16 @@ func TestRestoreExpose(t *testing.T) { ownerRestore: restore, err: "error to wait target PVC consumed, fake-ns/fake-target-pvc: error to wait for PVC: error to get pvc fake-ns/fake-target-pvc: persistentvolumeclaims \"fake-target-pvc\" not found", }, + { + name: "target pvc is already bound", + targetPVCName: "fake-target-pvc", + sourceNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjBound, + }, + err: "Target PVC fake-ns/fake-target-pvc has already been bound, abort", + }, { name: "create restore pod fail", targetPVCName: "fake-target-pvc", diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index a24525894..8e13b6b1c 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -284,11 +284,11 @@ func WaitPVBound(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvN } if tmpPV.Spec.ClaimRef.Name != pvcName { - return false, nil + return false, errors.Errorf("pv has been bound by unexpected pvc %s/%s", tmpPV.Spec.ClaimRef.Namespace, tmpPV.Spec.ClaimRef.Name) } if tmpPV.Spec.ClaimRef.Namespace != pvcNamespace { - return false, nil + return false, errors.Errorf("pv has been bound by unexpected pvc %s/%s", tmpPV.Spec.ClaimRef.Namespace, tmpPV.Spec.ClaimRef.Name) } updated = tmpPV @@ -302,3 +302,8 @@ func WaitPVBound(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvN return updated, nil } } + +// IsPVCBound returns true if the specified PVC has been bound +func IsPVCBound(pvc *corev1api.PersistentVolumeClaim) bool { + return pvc.Spec.VolumeName != "" +} diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index d31c5d57d..b883a3ea5 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -741,9 +741,10 @@ func TestWaitPVBound(t *testing.T) { err: "error to wait for bound of PV: timed out waiting for the condition", }, { - name: "pvc claimRef pvc name mismatch", - pvName: "fake-pv", - pvcName: "fake-pvc", + name: "pvc claimRef pvc name mismatch", + pvName: "fake-pv", + pvcName: "fake-pvc", + pvcNamespace: "fake-ns", kubeClientObj: []runtime.Object{ &corev1api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -751,12 +752,14 @@ func TestWaitPVBound(t *testing.T) { }, Spec: corev1api.PersistentVolumeSpec{ ClaimRef: &corev1api.ObjectReference{ - Kind: "fake-kind", + Kind: "fake-kind", + Namespace: "fake-ns", + Name: "fake-pvc-1", }, }, }, }, - err: "error to wait for bound of PV: timed out waiting for the condition", + err: "error to wait for bound of PV: pv has been bound by unexpected pvc fake-ns/fake-pvc-1", }, { name: "pvc claimRef pvc namespace mismatch", @@ -770,13 +773,14 @@ func TestWaitPVBound(t *testing.T) { }, Spec: corev1api.PersistentVolumeSpec{ ClaimRef: &corev1api.ObjectReference{ - Kind: "fake-kind", - Name: "fake-pvc", + Kind: "fake-kind", + Namespace: "fake-ns-1", + Name: "fake-pvc", }, }, }, }, - err: "error to wait for bound of PV: timed out waiting for the condition", + err: "error to wait for bound of PV: pv has been bound by unexpected pvc fake-ns-1/fake-pvc", }, { name: "success", @@ -834,3 +838,43 @@ func TestWaitPVBound(t *testing.T) { }) } } + +func TestIsPVCBound(t *testing.T) { + tests := []struct { + name string + pvc *corev1api.PersistentVolumeClaim + expect bool + }{ + { + name: "expect bound", + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-pvc", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "fake-volume", + }, + }, + expect: true, + }, + { + name: "expect not bound", + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-pvc", + }, + }, + expect: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := IsPVCBound(test.pvc) + + assert.Equal(t, test.expect, result) + }) + } +}