Skip to content

Commit

Permalink
Merge pull request #8508 from Lyndon-Li/issue-fix-8267-info-when-expo…
Browse files Browse the repository at this point in the history
…se-error

Issue 8267: enhance the error message when expose fails
  • Loading branch information
Lyndon-Li authored Dec 12, 2024
2 parents cb7758f + 8b54553 commit cd01222
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 14 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8508-Lyndon-Li
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #8267, enhance the error message when expose fails
21 changes: 17 additions & 4 deletions pkg/util/csi/volume_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
77 changes: 77 additions & 0 deletions pkg/util/csi/volume_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
10 changes: 8 additions & 2 deletions pkg/util/kube/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
40 changes: 40 additions & 0 deletions pkg/util/kube/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand Down
18 changes: 12 additions & 6 deletions pkg/util/kube/pvc_pv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 27 additions & 2 deletions pkg/util/kube/pvc_pv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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 []",
},
}

Expand Down

0 comments on commit cd01222

Please sign in to comment.