Skip to content

Commit 77795d6

Browse files
committed
feat: retry IBM Cloud Global Catalog calls on HTTP 429
Add a shared DoWithRetry helper to handle IBM Cloud API rate limiting (HTTP 429) using exponential backoff and honoring the Retry-After header when present. Apply the retry wrapper to GlobalCatalog client call sites: - GetCatalogEntryWithContext - ListCatalogEntriesWithContext - GetPricing Add unit tests covering: - successful call without retry - non-429 passthrough - retry then success - retry exhaustion - context cancellation - Retry-After handling without real sleep - Retry InvalidRetryAfterFallsBack Signed-off-by: Anand Nekkunti <anand.nekkunti@ibm.com>
1 parent 18e7a4b commit 77795d6

File tree

3 files changed

+251
-1
lines changed

3 files changed

+251
-1
lines changed

pkg/cloudprovider/ibm/catalog.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ func (c *GlobalCatalogClient) GetPricing(ctx context.Context, catalogEntryID str
147147
ID: &catalogEntryID,
148148
}
149149

150-
pricingData, _, err := sdkClient.GetPricing(pricingOptions)
150+
pricingData, err := DoWithRateLimitRetry(ctx, func() (*globalcatalogv1.PricingGet, *core.DetailedResponse, error) {
151+
return sdkClient.GetPricing(pricingOptions)
152+
})
151153
if err != nil {
152154
return nil, fmt.Errorf("calling GetPricing API: %w", err)
153155
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
Copyright The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package ibm
17+
18+
import (
19+
"context"
20+
"fmt"
21+
"net/http"
22+
"strconv"
23+
"time"
24+
25+
"github.com/IBM/go-sdk-core/v5/core"
26+
"sigs.k8s.io/controller-runtime/pkg/log"
27+
)
28+
29+
const (
30+
initialBackoff = 100 * time.Millisecond
31+
maxBackoff = 30 * time.Second
32+
maxAttempts = 5
33+
retryAfterKey = "Retry-After"
34+
)
35+
36+
// DoWithRateLimitRetry handles HTTP 429 rate limiting with exponential backoff.
37+
// It performs up to maxAttempts (1 initial + 4 retries), respecting the
38+
// Retry-After header when present (seconds or HTTP-date per RFC 7231).
39+
func DoWithRateLimitRetry[T any](ctx context.Context, fn func() (T, *core.DetailedResponse, error)) (T, error) {
40+
var zero T
41+
backoff := initialBackoff
42+
43+
for attempt := 0; attempt < maxAttempts; attempt++ {
44+
result, response, err := fn()
45+
46+
// Success or non-rate-limit error
47+
if response == nil || response.StatusCode != http.StatusTooManyRequests {
48+
return result, err
49+
}
50+
51+
// Use Retry-After for this sleep only; keep exponential backoff progression separate.
52+
// Retry-After can be delay-seconds (integer) or HTTP-date
53+
delay := backoff
54+
if response.Headers != nil {
55+
if ra := response.Headers.Get(retryAfterKey); ra != "" {
56+
if secs, parseErr := strconv.Atoi(ra); parseErr == nil && secs > 0 {
57+
delay = time.Duration(secs) * time.Second
58+
} else if t, parseErr := http.ParseTime(ra); parseErr == nil {
59+
if d := time.Until(t); d > 0 {
60+
delay = d
61+
}
62+
}
63+
}
64+
}
65+
66+
log.FromContext(ctx).V(1).Info("rate limited (429), sleeping before retry", "delay", delay, "attempt", attempt+1, "maxAttempts", maxAttempts)
67+
68+
timer := time.NewTimer(delay)
69+
select {
70+
case <-ctx.Done():
71+
if !timer.Stop() {
72+
<-timer.C
73+
}
74+
return zero, ctx.Err()
75+
case <-timer.C:
76+
backoff = min(backoff*2, maxBackoff)
77+
}
78+
}
79+
80+
return zero, fmt.Errorf("rate limited after %d attempts", maxAttempts)
81+
}
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
/*
2+
Copyright The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package ibm
17+
18+
import (
19+
"context"
20+
"net/http"
21+
"testing"
22+
"time"
23+
24+
"github.com/IBM/go-sdk-core/v5/core"
25+
"github.com/stretchr/testify/assert"
26+
"github.com/stretchr/testify/require"
27+
)
28+
29+
func TestDoWithRateLimitRetry_SuccessFirstTry(t *testing.T) {
30+
ctx := context.Background()
31+
calls := 0
32+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
33+
calls++
34+
return "ok", &core.DetailedResponse{StatusCode: 200}, nil
35+
})
36+
require.NoError(t, err)
37+
assert.Equal(t, "ok", result)
38+
assert.Equal(t, 1, calls)
39+
}
40+
41+
func TestDoWithRateLimitRetry_Non429ErrorReturnsImmediately(t *testing.T) {
42+
ctx := context.Background()
43+
calls := 0
44+
apiErr := assert.AnError
45+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
46+
calls++
47+
return "", &core.DetailedResponse{StatusCode: 500}, apiErr
48+
})
49+
require.Error(t, err)
50+
assert.ErrorIs(t, err, apiErr)
51+
assert.Equal(t, "", result)
52+
assert.Equal(t, 1, calls)
53+
}
54+
55+
func TestDoWithRateLimitRetry_NilResponseReturnsImmediately(t *testing.T) {
56+
ctx := context.Background()
57+
calls := 0
58+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
59+
calls++
60+
return "ok", nil, nil
61+
})
62+
require.NoError(t, err)
63+
assert.Equal(t, "ok", result)
64+
assert.Equal(t, 1, calls)
65+
}
66+
67+
func TestDoWithRateLimitRetry_429ThenSuccess(t *testing.T) {
68+
ctx := context.Background()
69+
calls := 0
70+
result, err := DoWithRateLimitRetry(ctx, func() (int, *core.DetailedResponse, error) {
71+
calls++
72+
if calls == 1 {
73+
return 0, &core.DetailedResponse{StatusCode: 429}, nil
74+
}
75+
return 42, &core.DetailedResponse{StatusCode: 200}, nil
76+
})
77+
require.NoError(t, err)
78+
assert.Equal(t, 42, result)
79+
assert.Equal(t, 2, calls)
80+
}
81+
82+
func TestDoWithRateLimitRetry_ExhaustedRetries(t *testing.T) {
83+
ctx := context.Background()
84+
calls := 0
85+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
86+
calls++
87+
return "", &core.DetailedResponse{StatusCode: 429}, nil
88+
})
89+
require.Error(t, err)
90+
assert.Contains(t, err.Error(), "rate limited after 5 attempts")
91+
assert.Equal(t, "", result)
92+
assert.Equal(t, 5, calls)
93+
}
94+
95+
func TestDoWithRateLimitRetry_ContextCanceledDuringBackoff(t *testing.T) {
96+
ctx, cancel := context.WithCancel(context.Background())
97+
calls := 0
98+
done := make(chan struct{})
99+
var result string
100+
var err error
101+
go func() {
102+
result, err = DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
103+
calls++
104+
return "", &core.DetailedResponse{StatusCode: 429}, nil
105+
})
106+
close(done)
107+
}()
108+
cancel()
109+
<-done
110+
require.Error(t, err)
111+
assert.ErrorIs(t, err, context.Canceled)
112+
assert.Equal(t, "", result)
113+
assert.Equal(t, 1, calls)
114+
}
115+
116+
func TestDoWithRateLimitRetry_RespectsRetryAfterHeaderSeconds(t *testing.T) {
117+
ctx := context.Background()
118+
calls := 0
119+
headers := http.Header{}
120+
headers.Set("Retry-After", "1")
121+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
122+
calls++
123+
if calls == 1 {
124+
return "", &core.DetailedResponse{StatusCode: 429, Headers: headers}, nil
125+
}
126+
return "ok", &core.DetailedResponse{StatusCode: 200}, nil
127+
})
128+
require.NoError(t, err)
129+
assert.Equal(t, "ok", result)
130+
assert.Equal(t, 2, calls)
131+
}
132+
133+
func TestDoWithRateLimitRetry_RespectsRetryAfterHeaderHTTPDate(t *testing.T) {
134+
ctx := context.Background()
135+
calls := 0
136+
headers := http.Header{}
137+
// HTTP-date format (RFC 1123): 1 second from now so delay is short
138+
headers.Set("Retry-After", time.Now().Add(1*time.Second).UTC().Format(time.RFC1123))
139+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
140+
calls++
141+
if calls == 1 {
142+
return "", &core.DetailedResponse{StatusCode: 429, Headers: headers}, nil
143+
}
144+
return "ok", &core.DetailedResponse{StatusCode: 200}, nil
145+
})
146+
require.NoError(t, err)
147+
assert.Equal(t, "ok", result)
148+
assert.Equal(t, 2, calls)
149+
}
150+
151+
func TestDoWithRateLimitRetry_InvalidRetryAfterFallsBackToBackoff(t *testing.T) {
152+
ctx := context.Background()
153+
calls := 0
154+
headers := http.Header{}
155+
// Invalid Retry-After value (not a number, not a valid HTTP-date) -> should use exponential backoff
156+
headers.Set("Retry-After", "not-a-number-or-date")
157+
result, err := DoWithRateLimitRetry(ctx, func() (string, *core.DetailedResponse, error) {
158+
calls++
159+
if calls == 1 {
160+
return "", &core.DetailedResponse{StatusCode: 429, Headers: headers}, nil
161+
}
162+
return "ok", &core.DetailedResponse{StatusCode: 200}, nil
163+
})
164+
require.NoError(t, err)
165+
assert.Equal(t, "ok", result)
166+
assert.Equal(t, 2, calls)
167+
}

0 commit comments

Comments
 (0)