From 8b545532e2fdeb4604472c0cee470f0184bc23ef Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 12 Dec 2024 11:01:04 +0800 Subject: [PATCH] issue 8267: add informative logs when expose error Signed-off-by: Lyndon-Li --- changelogs/unreleased/8508-Lyndon-Li | 1 + pkg/util/csi/volume_snapshot.go | 21 ++++++-- pkg/util/csi/volume_snapshot_test.go | 77 ++++++++++++++++++++++++++++ pkg/util/kube/pod.go | 10 +++- pkg/util/kube/pod_test.go | 40 +++++++++++++++ pkg/util/kube/pvc_pv.go | 18 ++++--- pkg/util/kube/pvc_pv_test.go | 29 ++++++++++- 7 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 changelogs/unreleased/8508-Lyndon-Li diff --git a/changelogs/unreleased/8508-Lyndon-Li b/changelogs/unreleased/8508-Lyndon-Li new file mode 100644 index 0000000000..7033701379 --- /dev/null +++ b/changelogs/unreleased/8508-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #8267, enhance the error message when expose fails \ No newline at end of file diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go index 76a4d59fa5..0b12caa76f 100644 --- a/pkg/util/csi/volume_snapshot.go +++ b/pkg/util/csi/volume_snapshot.go @@ -167,8 +167,9 @@ func EnsureDeleteVS(ctx context.Context, snapshotClient snapshotter.SnapshotV1In return errors.Wrap(err, "error to delete volume snapshot") } + var updated *snapshotv1api.VolumeSnapshot err = wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) { - _, err := snapshotClient.VolumeSnapshots(vsNamespace).Get(ctx, vsName, metav1.GetOptions{}) + vs, err := snapshotClient.VolumeSnapshots(vsNamespace).Get(ctx, vsName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { return true, nil @@ -177,11 +178,16 @@ func EnsureDeleteVS(ctx context.Context, snapshotClient snapshotter.SnapshotV1In return false, errors.Wrapf(err, fmt.Sprintf("error to get VolumeSnapshot %s", vsName)) } + updated = vs return false, nil }) if err != nil { - return errors.Wrapf(err, "error to assure VolumeSnapshot is deleted, %s", vsName) + if errors.Is(err, context.DeadlineExceeded) { + return errors.Errorf("timeout to assure VolumeSnapshot %s is deleted, finalizers in VS %v", vsName, updated.Finalizers) + } else { + return errors.Wrapf(err, "error to assure VolumeSnapshot is deleted, %s", vsName) + } } return nil @@ -219,8 +225,10 @@ func EnsureDeleteVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1I if err != nil && !apierrors.IsNotFound(err) { return errors.Wrap(err, "error to delete volume snapshot content") } + + var updated *snapshotv1api.VolumeSnapshotContent err = wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) { - _, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vscName, metav1.GetOptions{}) + vsc, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vscName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { return true, nil @@ -229,11 +237,16 @@ func EnsureDeleteVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1I return false, errors.Wrapf(err, fmt.Sprintf("error to get VolumeSnapshotContent %s", vscName)) } + updated = vsc return false, nil }) if err != nil { - return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vscName) + if errors.Is(err, context.DeadlineExceeded) { + return errors.Errorf("timeout to assure VolumeSnapshotContent %s is deleted, finalizers in VSC %v", vscName, updated.Finalizers) + } else { + return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vscName) + } } return nil diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index 3876d96edb..ad0c184d0c 100644 --- a/pkg/util/csi/volume_snapshot_test.go +++ b/pkg/util/csi/volume_snapshot_test.go @@ -304,6 +304,14 @@ func TestEnsureDeleteVS(t *testing.T) { }, } + vsObjWithFinalizer := &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vs", + Namespace: "fake-ns", + Finalizers: []string{"fake-finalizer-1", "fake-finalizer-2"}, + }, + } + tests := []struct { name string clientObj []runtime.Object @@ -334,6 +342,38 @@ func TestEnsureDeleteVS(t *testing.T) { }, err: "error to assure VolumeSnapshot is deleted, fake-vs: error to get VolumeSnapshot fake-vs: fake-get-error", }, + { + name: "wait timeout", + vsName: "fake-vs", + namespace: "fake-ns", + clientObj: []runtime.Object{vsObjWithFinalizer}, + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshots", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + }, + err: "timeout to assure VolumeSnapshot fake-vs is deleted, finalizers in VS [fake-finalizer-1 fake-finalizer-2]", + }, + { + name: "wait timeout, no finalizer", + vsName: "fake-vs", + namespace: "fake-ns", + clientObj: []runtime.Object{vsObj}, + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshots", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + }, + err: "timeout to assure VolumeSnapshot fake-vs is deleted, finalizers in VS []", + }, { name: "success", vsName: "fake-vs", @@ -367,6 +407,13 @@ func TestEnsureDeleteVSC(t *testing.T) { }, } + vscObjWithFinalizer := &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vsc", + Finalizers: []string{"fake-finalizer-1", "fake-finalizer-2"}, + }, + } + tests := []struct { name string clientObj []runtime.Object @@ -408,6 +455,36 @@ func TestEnsureDeleteVSC(t *testing.T) { }, err: "error to assure VolumeSnapshotContent is deleted, fake-vsc: error to get VolumeSnapshotContent fake-vsc: fake-get-error", }, + { + name: "wait timeout", + vscName: "fake-vsc", + clientObj: []runtime.Object{vscObjWithFinalizer}, + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshotcontents", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + }, + err: "timeout to assure VolumeSnapshotContent fake-vsc is deleted, finalizers in VSC [fake-finalizer-1 fake-finalizer-2]", + }, + { + name: "wait timeout, no finalizer", + vscName: "fake-vsc", + clientObj: []runtime.Object{vscObj}, + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshotcontents", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + }, + err: "timeout to assure VolumeSnapshotContent fake-vsc is deleted, finalizers in VSC []", + }, { name: "success", vscName: "fake-vsc", diff --git a/pkg/util/kube/pod.go b/pkg/util/kube/pod.go index 593d1541f2..ddcd43b83b 100644 --- a/pkg/util/kube/pod.go +++ b/pkg/util/kube/pod.go @@ -105,8 +105,9 @@ func EnsureDeletePod(ctx context.Context, podGetter corev1client.CoreV1Interface return errors.Wrapf(err, "error to delete pod %s", pod) } + var updated *corev1api.Pod err = wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) { - _, err := podGetter.Pods(namespace).Get(ctx, pod, metav1.GetOptions{}) + po, err := podGetter.Pods(namespace).Get(ctx, pod, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { return true, nil @@ -115,11 +116,16 @@ func EnsureDeletePod(ctx context.Context, podGetter corev1client.CoreV1Interface return false, errors.Wrapf(err, "error to get pod %s", pod) } + updated = po return false, nil }) if err != nil { - return errors.Wrapf(err, "error to assure pod is deleted, %s", pod) + if errors.Is(err, context.DeadlineExceeded) { + return errors.Errorf("timeout to assure pod %s is deleted, finalizers in pod %v", pod, updated.Finalizers) + } else { + return errors.Wrapf(err, "error to assure pod is deleted, %s", pod) + } } return nil diff --git a/pkg/util/kube/pod_test.go b/pkg/util/kube/pod_test.go index 0e76899a5e..88002e8bbb 100644 --- a/pkg/util/kube/pod_test.go +++ b/pkg/util/kube/pod_test.go @@ -47,6 +47,14 @@ func TestEnsureDeletePod(t *testing.T) { }, } + podObjectWithFinalizer := &corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-pod", + Finalizers: []string{"fake-finalizer-1", "fake-finalizer-2"}, + }, + } + tests := []struct { name string clientObj []runtime.Object @@ -61,6 +69,38 @@ func TestEnsureDeletePod(t *testing.T) { namespace: "fake-ns", err: "error to delete pod fake-pod: pods \"fake-pod\" not found", }, + { + name: "wait timeout", + podName: "fake-pod", + namespace: "fake-ns", + clientObj: []runtime.Object{podObjectWithFinalizer}, + reactors: []reactor{ + { + verb: "delete", + resource: "pods", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + }, + err: "timeout to assure pod fake-pod is deleted, finalizers in pod [fake-finalizer-1 fake-finalizer-2]", + }, + { + name: "wait timeout, no finalizer", + podName: "fake-pod", + namespace: "fake-ns", + clientObj: []runtime.Object{podObject}, + reactors: []reactor{ + { + verb: "delete", + resource: "pods", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + }, + err: "timeout to assure pod fake-pod is deleted, finalizers in pod []", + }, { name: "wait fail", podName: "fake-pod", diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index 1811a2c1df..d21e68d882 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -120,31 +120,37 @@ func DeletePVIfAny(ctx context.Context, pvGetter corev1client.CoreV1Interface, p // EnsureDeletePVC asserts the existence of a PVC by name, deletes it and waits for its disappearance and returns errors on any failure // If timeout is 0, it doesn't wait and return nil -func EnsureDeletePVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface, pvc string, namespace string, timeout time.Duration) error { - err := pvcGetter.PersistentVolumeClaims(namespace).Delete(ctx, pvc, metav1.DeleteOptions{}) +func EnsureDeletePVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface, pvcName string, namespace string, timeout time.Duration) error { + err := pvcGetter.PersistentVolumeClaims(namespace).Delete(ctx, pvcName, metav1.DeleteOptions{}) if err != nil { - return errors.Wrapf(err, "error to delete pvc %s", pvc) + return errors.Wrapf(err, "error to delete pvc %s", pvcName) } if timeout == 0 { return nil } + var updated *corev1api.PersistentVolumeClaim err = wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) { - _, err := pvcGetter.PersistentVolumeClaims(namespace).Get(ctx, pvc, metav1.GetOptions{}) + pvc, err := pvcGetter.PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { return true, nil } - return false, errors.Wrapf(err, "error to get pvc %s", pvc) + return false, errors.Wrapf(err, "error to get pvc %s", pvcName) } + updated = pvc return false, nil }) if err != nil { - return errors.Wrapf(err, "error to ensure pvc deleted for %s", pvc) + if errors.Is(err, context.DeadlineExceeded) { + return errors.Errorf("timeout to assure pvc %s is deleted, finalizers in pvc %v", pvcName, updated.Finalizers) + } else { + return errors.Wrapf(err, "error to ensure pvc deleted for %s", pvcName) + } } return nil diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index 5cbe02dc06..ab14cffc4a 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -462,7 +462,7 @@ func TestDeletePVCIfAny(t *testing.T) { }, }, ensureTimeout: time.Second, - logMessage: "failed to delete pvc fake-namespace/fake-pvc with err error to ensure pvc deleted for fake-pvc: context deadline exceeded", + logMessage: "failed to delete pvc fake-namespace/fake-pvc with err timeout to assure pvc fake-pvc is deleted, finalizers in pvc []", logLevel: "level=warning", }, { @@ -584,6 +584,14 @@ func TestEnsureDeletePVC(t *testing.T) { }, } + pvcObjectWithFinalizer := &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-pvc", + Finalizers: []string{"fake-finalizer-1", "fake-finalizer-2"}, + }, + } + tests := []struct { name string clientObj []runtime.Object @@ -635,6 +643,23 @@ func TestEnsureDeletePVC(t *testing.T) { name: "wait timeout", pvcName: "fake-pvc", namespace: "fake-ns", + clientObj: []runtime.Object{pvcObjectWithFinalizer}, + timeout: time.Millisecond, + reactors: []reactor{ + { + verb: "delete", + resource: "persistentvolumeclaims", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, pvcObject, nil + }, + }, + }, + err: "timeout to assure pvc fake-pvc is deleted, finalizers in pvc [fake-finalizer-1 fake-finalizer-2]", + }, + { + name: "wait timeout, no finalizer", + pvcName: "fake-pvc", + namespace: "fake-ns", clientObj: []runtime.Object{pvcObject}, timeout: time.Millisecond, reactors: []reactor{ @@ -646,7 +671,7 @@ func TestEnsureDeletePVC(t *testing.T) { }, }, }, - err: "error to ensure pvc deleted for fake-pvc: context deadline exceeded", + err: "timeout to assure pvc fake-pvc is deleted, finalizers in pvc []", }, }