From 38a52980cc0f25b5cba9058a40a51043d76dc481 Mon Sep 17 00:00:00 2001 From: Roger Zimmermann Date: Thu, 13 Mar 2025 15:39:25 +0100 Subject: [PATCH] Issue #8772 ensure pv removed (#8777) * ensure pv has been deleted Signed-off-by: Roger Zimmermann * ensure delete pv unit test Signed-off-by: Roger Zimmermann * comment, errors Signed-off-by: Roger Zimmermann * updated changelog Signed-off-by: Roger Zimmermann Signed-off-by: Roger Zimmermann * pass value Co-authored-by: Tiger Kaovilai Signed-off-by: Roger Zimmermann * function renamed as suggested Signed-off-by: Roger Zimmermann --------- Signed-off-by: Roger Zimmermann Co-authored-by: Tiger Kaovilai --- changelogs/unreleased/8777-ix-rzi | 1 + pkg/util/kube/pvc_pv.go | 35 ++++++++++++ pkg/util/kube/pvc_pv_test.go | 88 +++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 changelogs/unreleased/8777-ix-rzi diff --git a/changelogs/unreleased/8777-ix-rzi b/changelogs/unreleased/8777-ix-rzi new file mode 100644 index 000000000..397d4aa36 --- /dev/null +++ b/changelogs/unreleased/8777-ix-rzi @@ -0,0 +1 @@ +ensure that PV is removed before VS is deleted diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index 49eb91dbe..3b4ddc275 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/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) { diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index 06e8058f2..5f0abcf85 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/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{