Merge pull request #6492 from Lyndon-Li/data-mover-restore-abort-for-…

…existing-pvc Data mover restore abort for existing PVC
vmware-tanzu · Jul 14, 2023 · ddc50af · ddc50af
2 parents dfd7970 + eebb879
commit ddc50af
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -741,22 +741,25 @@ 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{
 						Name: "fake-pv",
 					},
 					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)
+		})
+	}
+}