Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions internal/kubernetes/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ func WaitForDelete(c *RetryClient, objs []runtime.Object) error {
actual := &unstructured.Unstructured{}
actual.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind())
err = c.Get(ctx, ObjectKey(obj), actual)
if err == nil || !errors.IsNotFound(err) {
return false, err
// Retry on transient API errors (return nil to keep polling) rather
// than aborting the entire wait, which would surface as a misleading
// "timed out" error well before the real deadline.
if !errors.IsNotFound(err) {
return false, nil
}
}

Expand All @@ -46,13 +49,7 @@ func WaitForSA(config *rest.Config, name, namespace string) error {
Name: name,
}
return wait.PollUntilContextTimeout(context.TODO(), 500*time.Millisecond, 60*time.Second, true, func(ctx context.Context) (done bool, err error) {
err = c.Get(ctx, key, obj)
if errors.IsNotFound(err) {
return false, nil
}
if err != nil {
return false, err
}
return true, nil
// Retry on all errors (not-found, transient) rather than aborting the wait.
return c.Get(ctx, key, obj) == nil, nil
})
}
82 changes: 82 additions & 0 deletions internal/kubernetes/wait_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package kubernetes

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
)

func testObj() runtime.Object {
obj := &unstructured.Unstructured{}
obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"})
obj.SetName("cm")
obj.SetNamespace("default")
return obj
}

func TestWaitForDelete_AlreadyGone(t *testing.T) {
cl := fake.NewClientBuilder().Build()
rc := &RetryClient{Client: cl}

err := WaitForDelete(rc, []runtime.Object{testObj()})
require.NoError(t, err)
}

func TestWaitForDelete_TransientErrorThenGone(t *testing.T) {
callCount := 0
cl := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
Get: func(context.Context, client.WithWatch, client.ObjectKey, client.Object, ...client.GetOption) error {
callCount++
if callCount <= 3 {
return fmt.Errorf("transient API error")
}
return k8serrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "cm")
},
}).Build()
rc := &RetryClient{Client: cl}

err := WaitForDelete(rc, []runtime.Object{testObj()})
require.NoError(t, err)
assert.Greater(t, callCount, 3)
}

func TestWaitForDelete_StillExistsThenGone(t *testing.T) {
callCount := 0
cl := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
Get: func(context.Context, client.WithWatch, client.ObjectKey, client.Object, ...client.GetOption) error {
callCount++
if callCount <= 2 {
return nil
}
return k8serrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "cm")
},
}).Build()
rc := &RetryClient{Client: cl}

err := WaitForDelete(rc, []runtime.Object{testObj()})
require.NoError(t, err)
assert.Greater(t, callCount, 2)
}

func TestWaitForDelete_PersistentErrorTimesOut(t *testing.T) {
cl := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
Get: func(context.Context, client.WithWatch, client.ObjectKey, client.Object, ...client.GetOption) error {
return fmt.Errorf("persistent API error")
},
}).Build()
rc := &RetryClient{Client: cl}

err := WaitForDelete(rc, []runtime.Object{testObj()})
assert.Error(t, err)
assert.ErrorIs(t, err, context.DeadlineExceeded)
}
2 changes: 1 addition & 1 deletion internal/step/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (s *Step) DeleteExisting(namespace string) error {
}
if !k8serrors.IsNotFound(err) {
lastCheckMsg = fmt.Sprintf("checking existence of %v %s failed: %v", obj.GetObjectKind().GroupVersionKind(), obj.GetName(), err)
return false, err
return false, nil
}
}

Expand Down
3 changes: 2 additions & 1 deletion internal/testcase/case.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ func (c *Case) deleteNamespace(cl clientWithKubeConfig) error {
return true, nil
}
if err != nil {
return false, fmt.Errorf("failed to check deletion of namespace %q: %w", c.ns.name, err)
cl.Logf("failed to check deletion of namespace %q (will retry): %v", c.ns.name, err)
return false, nil
}
return false, nil
})
Expand Down
Loading