Issue #8772 ensure pv removed (#8777)

* ensure pv has been deleted

Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

* ensure delete pv unit test

Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

* comment, errors

Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

* updated changelog
Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

* pass value

Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>
Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

* function renamed as suggested

Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>

---------

Signed-off-by: Roger Zimmermann <roger.zimmermann@inventx.ch>
Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>

changelogs/unreleased/8777-ix-rzi

@@ -0,0 +1 @@
+ensure that PV is removed before VS is deleted

pkg/util/kube/pvc_pv.go

@@ -74,6 +74,10 @@ func DeletePVAndPVCIfAny(ctx context.Context, client corev1client.CoreV1Interfac
 	if err := EnsureDeletePVC(ctx, client, pvcName, pvcNamespace, ensureTimeout); err != nil {
 		log.Warnf("failed to delete pvc %s/%s with err %v", pvcNamespace, pvcName, err)
 	}
+
+	if err := EnsurePVDeleted(ctx, client, pvcObj.Spec.VolumeName, ensureTimeout); err != nil {
+		log.Warnf("pv %s was not removed with err %v", pvcObj.Spec.VolumeName, err)
+	}
 }
 
 // WaitPVCBound wait for binding of a PVC specified by name and returns the bound PV object
@@ -157,6 +161,37 @@ func EnsureDeletePVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface
 	return nil
 }
 
+// EnsurePVDeleted ensures a PV has been deleted. This function is supposed to be called after EnsureDeletePVC
+// If timeout is 0, it doesn't wait and return nil
+func EnsurePVDeleted(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvName string, timeout time.Duration) error {
+	if timeout == 0 {
+		return nil
+	}
+
+	err := wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) {
+		_, err := pvGetter.PersistentVolumes().Get(ctx, pvName, metav1.GetOptions{})
+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				return true, nil
+			}
+
+			return false, errors.Wrapf(err, "error to get pv %s", pvName)
+		}
+
+		return false, nil
+	})
+
+	if err != nil {
+		if errors.Is(err, context.DeadlineExceeded) {
+			return errors.Errorf("timeout to assure pv %s is deleted", pvName)
+		} else {
+			return errors.Wrapf(err, "error to ensure pv is deleted for %s", pvName)
+		}
+	}
+
+	return nil
+}
+
 // RebindPVC rebinds a PVC by modifying its VolumeName to the specific PV
 func RebindPVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface,
 	pvc *corev1api.PersistentVolumeClaim, pv string) (*corev1api.PersistentVolumeClaim, error) {

pkg/util/kube/pvc_pv_test.go

@@ -707,6 +707,94 @@ func TestEnsureDeletePVC(t *testing.T) {
 	}
 }
 
+func TestEnsureDeletePV(t *testing.T) {
+	pvObject := &corev1api.PersistentVolume{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "fake-pv",
+		},
+	}
+
+	tests := []struct {
+		name      string
+		clientObj []runtime.Object
+		pvName    string
+		reactors  []reactor
+		timeout   time.Duration
+		err       string
+	}{
+		{
+			name:   "get fail",
+			pvName: "fake-pv",
+			err:    "error to get pv fake-pv: persistentvolumes \"fake-pv\" not found",
+		},
+		{
+			name:      "0 timeout",
+			pvName:    "fake-pv",
+			clientObj: []runtime.Object{pvObject},
+			reactors: []reactor{
+				{
+					verb:     "get",
+					resource: "persistentvolumes",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, pvObject, nil
+					},
+				},
+			},
+		},
+		{
+			name:      "wait fail",
+			pvName:    "fake-pv",
+			clientObj: []runtime.Object{pvObject},
+			timeout:   time.Millisecond,
+			reactors: []reactor{
+				{
+					verb:     "get",
+					resource: "persistentvolumes",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, errors.New("fake-get-error")
+					},
+				},
+			},
+			err: "error to ensure pv is deleted for fake-pv: error to get pv fake-pv: fake-get-error",
+		},
+		{
+			name:      "wait timeout",
+			pvName:    "fake-pv",
+			clientObj: []runtime.Object{pvObject},
+			timeout:   time.Millisecond,
+			reactors: []reactor{
+				{
+					verb:     "get",
+					resource: "persistentvolumes",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, pvObject, nil
+					},
+				},
+			},
+			err: "timeout to assure pv fake-pv is deleted",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			fakeKubeClient := fake.NewSimpleClientset(test.clientObj...)
+
+			for _, reactor := range test.reactors {
+				fakeKubeClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc)
+			}
+
+			var kubeClient kubernetes.Interface = fakeKubeClient
+
+			err := EnsurePVDeleted(context.Background(), kubeClient.CoreV1(), test.pvName, test.timeout)
+			if err != nil {
+				assert.EqualError(t, err, test.err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestRebindPVC(t *testing.T) {
 	pvcObject := &corev1api.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{