Merge pull request #6738 from Lyndon-Li/issue-fix-6733

Fix issue 6733
vmware-tanzu · Sep 1, 2023 · b9b2c88 · b9b2c88
2 parents 1615cfd + c4443d5
commit b9b2c88
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 4 deletions.
diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go
@@ -221,7 +221,7 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co
 	}
 
 	restorePVName := restorePV.Name
-	restorePV, err = kube.ResetPVBinding(ctx, e.kubeClient.CoreV1(), restorePV, matchLabel)
+	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)
 	}

diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go
@@ -155,14 +155,19 @@ func RebindPVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface,
 }
 
 // ResetPVBinding resets the binding info of a PV and adds the required labels so as to make it ready for binding
-func ResetPVBinding(ctx context.Context, pvGetter corev1client.CoreV1Interface, pv *corev1api.PersistentVolume, labels map[string]string) (*corev1api.PersistentVolume, error) {
+func ResetPVBinding(ctx context.Context, pvGetter corev1client.CoreV1Interface, pv *corev1api.PersistentVolume,
+	labels map[string]string, pvc *corev1api.PersistentVolumeClaim) (*corev1api.PersistentVolume, error) {
 	origBytes, err := json.Marshal(pv)
 	if err != nil {
 		return nil, errors.Wrap(err, "error marshaling original PV")
 	}
 
 	updated := pv.DeepCopy()
-	updated.Spec.ClaimRef = nil
+	updated.Spec.ClaimRef = &corev1api.ObjectReference{
+		Kind:      pvc.Kind,
+		Namespace: pvc.Namespace,
+		Name:      pvc.Name,
+	}
 	delete(updated.Annotations, KubeAnnBoundByController)
 
 	if labels != nil {
@@ -283,6 +288,10 @@ func WaitPVBound(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvN
 			return false, nil
 		}
 
+		if tmpPV.Status.Phase != corev1api.VolumeBound {
+			return false, nil
+		}
+
 		if tmpPV.Spec.ClaimRef.Name != pvcName {
 			return false, errors.Errorf("pv has been bound by unexpected pvc %s/%s", tmpPV.Spec.ClaimRef.Namespace, tmpPV.Spec.ClaimRef.Name)
 		}

diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go
@@ -568,10 +568,25 @@ func TestResetPVBinding(t *testing.T) {
 		},
 	}
 
+	pvcObject := &corev1api.PersistentVolumeClaim{
+		TypeMeta: metav1.TypeMeta{
+			Kind: "fake-kind-1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "fake-ns-1",
+			Name:      "fake-pvc-1",
+			Annotations: map[string]string{
+				KubeAnnBindCompleted:     "true",
+				KubeAnnBoundByController: "true",
+			},
+		},
+	}
+
 	tests := []struct {
 		name      string
 		clientObj []runtime.Object
 		pv        *corev1api.PersistentVolume
+		pvc       *corev1api.PersistentVolumeClaim
 		labels    map[string]string
 		reactors  []reactor
 		result    *corev1api.PersistentVolume
@@ -580,6 +595,7 @@ func TestResetPVBinding(t *testing.T) {
 		{
 			name:      "path fail",
 			pv:        pvObject,
+			pvc:       pvcObject,
 			clientObj: []runtime.Object{pvObject},
 			reactors: []reactor{
 				{
@@ -595,6 +611,7 @@ func TestResetPVBinding(t *testing.T) {
 		{
 			name: "succeed",
 			pv:   pvObject,
+			pvc:  pvcObject,
 			labels: map[string]string{
 				"fake-label-1": "fake-value-1",
 				"fake-label-2": "fake-value-2",
@@ -608,6 +625,13 @@ func TestResetPVBinding(t *testing.T) {
 						"fake-label-2": "fake-value-2",
 					},
 				},
+				Spec: corev1api.PersistentVolumeSpec{
+					ClaimRef: &corev1api.ObjectReference{
+						Kind:      "fake-kind-1",
+						Namespace: "fake-ns-1",
+						Name:      "fake-pvc-1",
+					},
+				},
 			},
 		},
 	}
@@ -622,7 +646,7 @@ func TestResetPVBinding(t *testing.T) {
 
 			var kubeClient kubernetes.Interface = fakeKubeClient
 
-			result, err := ResetPVBinding(context.Background(), kubeClient.CoreV1(), test.pv, test.labels)
+			result, err := ResetPVBinding(context.Background(), kubeClient.CoreV1(), test.pv, test.labels, test.pvc)
 			if err != nil {
 				assert.EqualError(t, err, test.err)
 			} else {
@@ -740,6 +764,18 @@ func TestWaitPVBound(t *testing.T) {
 			},
 			err: "error to wait for bound of PV: timed out waiting for the condition",
 		},
+		{
+			name:   "pvc status not bound",
+			pvName: "fake-pv",
+			kubeClientObj: []runtime.Object{
+				&corev1api.PersistentVolume{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "fake-pv",
+					},
+				},
+			},
+			err: "error to wait for bound of PV: timed out waiting for the condition",
+		},
 		{
 			name:         "pvc claimRef pvc name mismatch",
 			pvName:       "fake-pv",
@@ -757,6 +793,9 @@ func TestWaitPVBound(t *testing.T) {
 							Name:      "fake-pvc-1",
 						},
 					},
+					Status: corev1api.PersistentVolumeStatus{
+						Phase: "Bound",
+					},
 				},
 			},
 			err: "error to wait for bound of PV: pv has been bound by unexpected pvc fake-ns/fake-pvc-1",
@@ -778,6 +817,9 @@ func TestWaitPVBound(t *testing.T) {
 							Name:      "fake-pvc",
 						},
 					},
+					Status: corev1api.PersistentVolumeStatus{
+						Phase: "Bound",
+					},
 				},
 			},
 			err: "error to wait for bound of PV: pv has been bound by unexpected pvc fake-ns-1/fake-pvc",
@@ -799,6 +841,9 @@ func TestWaitPVBound(t *testing.T) {
 							Namespace: "fake-ns",
 						},
 					},
+					Status: corev1api.PersistentVolumeStatus{
+						Phase: "Bound",
+					},
 				},
 			},
 			expectedPV: &corev1api.PersistentVolume{
@@ -812,6 +857,9 @@ func TestWaitPVBound(t *testing.T) {
 						Namespace: "fake-ns",
 					},
 				},
+				Status: corev1api.PersistentVolumeStatus{
+					Phase: "Bound",
+				},
 			},
 		},
 	}