Skip to content

Commit 62d025b

Browse files
porridgeclaude
andcommitted
fix: retry transient API errors in wait/poll loops instead of aborting
All four wait.Poll* call sites treated any non-NotFound error from Get as a fatal poll error, causing PollUntilContext* to abort immediately rather than retrying. This meant transient Kubernetes API errors (e.g. Internal, ServerTimeout, network blips) would terminate the poll loop well before the configured timeout was reached. Change all four sites to treat non-NotFound errors as retryable by returning (false, nil) instead of (false, err). If the error persists, the poll will eventually time out as intended. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 59cb892 commit 62d025b

4 files changed

Lines changed: 92 additions & 5 deletions

File tree

internal/kubernetes/wait.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@ func WaitForDelete(c *RetryClient, objs []runtime.Object) error {
2121
actual := &unstructured.Unstructured{}
2222
actual.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind())
2323
err = c.Get(ctx, ObjectKey(obj), actual)
24-
if err == nil || !errors.IsNotFound(err) {
25-
return false, err
24+
// Retry on transient API errors (return nil to keep polling) rather
25+
// than aborting the entire wait, which would surface as a misleading
26+
// "timed out" error well before the real deadline.
27+
if !errors.IsNotFound(err) {
28+
return false, nil
2629
}
2730
}
2831

@@ -50,8 +53,9 @@ func WaitForSA(config *rest.Config, name, namespace string) error {
5053
if errors.IsNotFound(err) {
5154
return false, nil
5255
}
56+
// Retry on transient API errors rather than aborting the wait.
5357
if err != nil {
54-
return false, err
58+
return false, nil
5559
}
5660
return true, nil
5761
})

internal/kubernetes/wait_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package kubernetes
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
k8serrors "k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/apimachinery/pkg/runtime/schema"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
15+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
16+
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
17+
)
18+
19+
func testObj() runtime.Object {
20+
obj := &unstructured.Unstructured{}
21+
obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"})
22+
obj.SetName("cm")
23+
obj.SetNamespace("default")
24+
return obj
25+
}
26+
27+
func TestWaitForDelete_AlreadyGone(t *testing.T) {
28+
cl := fake.NewClientBuilder().Build()
29+
rc := &RetryClient{Client: cl}
30+
31+
err := WaitForDelete(rc, []runtime.Object{testObj()})
32+
require.NoError(t, err)
33+
}
34+
35+
func TestWaitForDelete_TransientErrorThenGone(t *testing.T) {
36+
callCount := 0
37+
cl := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
38+
Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
39+
callCount++
40+
if callCount <= 3 {
41+
return fmt.Errorf("transient API error")
42+
}
43+
return k8serrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "cm")
44+
},
45+
}).Build()
46+
rc := &RetryClient{Client: cl}
47+
48+
err := WaitForDelete(rc, []runtime.Object{testObj()})
49+
require.NoError(t, err)
50+
assert.Greater(t, callCount, 3)
51+
}
52+
53+
func TestWaitForDelete_StillExistsThenGone(t *testing.T) {
54+
callCount := 0
55+
cl := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
56+
Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
57+
callCount++
58+
if callCount <= 2 {
59+
return nil
60+
}
61+
return k8serrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "cm")
62+
},
63+
}).Build()
64+
rc := &RetryClient{Client: cl}
65+
66+
err := WaitForDelete(rc, []runtime.Object{testObj()})
67+
require.NoError(t, err)
68+
assert.Greater(t, callCount, 2)
69+
}
70+
71+
func TestWaitForDelete_PersistentErrorTimesOut(t *testing.T) {
72+
cl := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{
73+
Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
74+
return fmt.Errorf("persistent API error")
75+
},
76+
}).Build()
77+
rc := &RetryClient{Client: cl}
78+
79+
err := WaitForDelete(rc, []runtime.Object{testObj()})
80+
assert.Error(t, err)
81+
assert.ErrorIs(t, err, context.DeadlineExceeded)
82+
}

internal/step/step.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func (s *Step) DeleteExisting(namespace string) error {
179179
}
180180
if !k8serrors.IsNotFound(err) {
181181
lastCheckMsg = fmt.Sprintf("checking existence of %v %s failed: %v", obj.GetObjectKind().GroupVersionKind(), obj.GetName(), err)
182-
return false, err
182+
return false, nil
183183
}
184184
}
185185

internal/testcase/case.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,8 @@ func (c *Case) deleteNamespace(cl clientWithKubeConfig) error {
197197
return true, nil
198198
}
199199
if err != nil {
200-
return false, fmt.Errorf("failed to check deletion of namespace %q: %w", c.ns.name, err)
200+
cl.Logf("failed to check deletion of namespace %q (will retry): %v", c.ns.name, err)
201+
return false, nil
201202
}
202203
return false, nil
203204
})

0 commit comments

Comments
 (0)