Skip to content

Commit 196ffaa

Browse files
authored
Merge pull request #126 from saschagrunert/retry-api
Use `go-retry` for HTTP retry
2 parents de44574 + 6676419 commit 196ffaa

File tree

5 files changed

+226
-116
lines changed

5 files changed

+226
-116
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module sigs.k8s.io/release-utils
33
go 1.23
44

55
require (
6+
github.com/avast/retry-go/v4 v4.6.0
67
github.com/blang/semver/v4 v4.0.0
78
github.com/common-nighthawk/go-figure v0.0.0-20210622060536-734e95fb86be
89
github.com/maxbrunsfeld/counterfeiter/v6 v6.11.2

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEK
22
github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=
33
github.com/Masterminds/semver/v3 v3.3.1 h1:QtNSWtVZ3nBfk8mAOu/B6v7FMJ+NHTIgUPi7rj+4nv4=
44
github.com/Masterminds/semver/v3 v3.3.1/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM=
5+
github.com/avast/retry-go/v4 v4.6.0 h1:K9xNA+KeB8HHc2aWFuLb25Offp+0iVRXEvFx8IinRJA=
6+
github.com/avast/retry-go/v4 v4.6.0/go.mod h1:gvWlPhBVsvBbLkVGDg/KwvBv0bEkCOLRRSHKIr2PyOE=
57
github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM=
68
github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ=
79
github.com/common-nighthawk/go-figure v0.0.0-20210622060536-734e95fb86be h1:J5BL2kskAlV9ckgEsNQXscjIaLiOYiZ75d4e94E6dcQ=

http/agent.go

Lines changed: 61 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ import (
2323
"io"
2424
"math"
2525
"net/http"
26+
"net/url"
2627
"sync"
2728
"time"
2829

30+
"github.com/avast/retry-go/v4"
2931
"github.com/nozzle/throttler"
3032
"github.com/sirupsen/logrus"
3133
)
@@ -59,7 +61,8 @@ type agentOptions struct {
5961
FailOnHTTPError bool // Set to true to fail on HTTP Status > 299
6062
Retries uint // Number of times to retry when errors happen
6163
Timeout time.Duration // Timeout when fetching URLs
62-
MaxWaitTime time.Duration // Max waiting time when backing off
64+
WaitTime time.Duration // Initial wait time for backing off on retry
65+
MaxWaitTime time.Duration // Max waiting time when backing off on retry
6366
PostContentType string // Content type to send when posting data
6467
MaxParallel uint // Maximum number of parallel requests when requesting groups
6568
}
@@ -76,6 +79,7 @@ var defaultAgentOptions = &agentOptions{
7679
FailOnHTTPError: true,
7780
Retries: 3,
7881
Timeout: 3 * time.Second,
82+
WaitTime: 2 * time.Second,
7983
MaxWaitTime: 60 * time.Second,
8084
PostContentType: defaultPostContentType,
8185
MaxParallel: 5,
@@ -108,6 +112,20 @@ func (a *Agent) WithRetries(retries uint) *Agent {
108112
return a
109113
}
110114

115+
// WithWaitTime sets the initial wait time for request retry.
116+
func (a *Agent) WithWaitTime(time time.Duration) *Agent {
117+
a.options.WaitTime = time
118+
119+
return a
120+
}
121+
122+
// WithMaxWaitTime sets the maximum wait time for request retry.
123+
func (a *Agent) WithMaxWaitTime(time time.Duration) *Agent {
124+
a.options.MaxWaitTime = time
125+
126+
return a
127+
}
128+
111129
// WithFailOnHTTPError determines if the agent fails on HTTP errors (HTTP status not in 200s).
112130
func (a *Agent) WithFailOnHTTPError(flag bool) *Agent {
113131
a.options.FailOnHTTPError = flag
@@ -145,28 +163,9 @@ func (a *Agent) Get(url string) (content []byte, err error) {
145163
func (a *Agent) GetRequest(url string) (response *http.Response, err error) {
146164
logrus.Debugf("Sending GET request to %s", url)
147165

148-
var try uint
149-
150-
for {
151-
response, err = a.AgentImplementation.SendGetRequest(a.Client(), url)
152-
try++
153-
154-
if err == nil || try >= a.options.Retries {
155-
return response, err
156-
}
157-
// Do exponential backoff...
158-
waitTime := math.Pow(2, float64(try))
159-
// ... but wait no more than 1 min
160-
if waitTime > 60 {
161-
waitTime = a.options.MaxWaitTime.Seconds()
162-
}
163-
164-
logrus.Errorf(
165-
"Error getting URL (will retry %d more times in %.0f secs): %s",
166-
a.options.Retries-try, waitTime, err.Error(),
167-
)
168-
time.Sleep(time.Duration(waitTime) * time.Second)
169-
}
166+
return a.retryRequest(func() (*http.Response, error) {
167+
return a.AgentImplementation.SendGetRequest(a.Client(), url)
168+
})
170169
}
171170

172171
// Post returns the body of a POST request.
@@ -184,28 +183,49 @@ func (a *Agent) Post(url string, postData []byte) (content []byte, err error) {
184183
func (a *Agent) PostRequest(url string, postData []byte) (response *http.Response, err error) {
185184
logrus.Debugf("Sending POST request to %s", url)
186185

187-
var try uint
188-
189-
for {
190-
response, err = a.AgentImplementation.SendPostRequest(a.Client(), url, postData, a.options.PostContentType)
191-
try++
186+
return a.retryRequest(func() (*http.Response, error) {
187+
return a.AgentImplementation.SendPostRequest(a.Client(), url, postData, a.options.PostContentType)
188+
})
189+
}
192190

193-
if err == nil || try >= a.options.Retries {
194-
return response, err
195-
}
196-
// Do exponential backoff...
197-
waitTime := math.Pow(2, float64(try))
198-
// ... but wait no more than 1 min
199-
if waitTime > 60 {
200-
waitTime = a.options.MaxWaitTime.Seconds()
191+
func (a *Agent) retryRequest(do func() (*http.Response, error)) (response *http.Response, err error) {
192+
err = retry.Do(func() error {
193+
//nolint:bodyclose // The API consumer should close the body
194+
response, err = do()
195+
if retryErr := shouldRetry(response, err); retryErr != nil {
196+
return retryErr
201197
}
202198

203-
logrus.Errorf(
204-
"Error getting URL (will retry %d more times in %.0f secs): %s",
205-
a.options.Retries-try, waitTime, err.Error(),
206-
)
207-
time.Sleep(time.Duration(waitTime) * time.Second)
199+
return nil
200+
},
201+
retry.Attempts(a.options.Retries),
202+
retry.Delay(a.options.WaitTime),
203+
retry.MaxDelay(a.options.MaxWaitTime),
204+
retry.DelayType(retry.BackOffDelay),
205+
retry.OnRetry(func(attempt uint, err error) {
206+
logrus.Errorf("Unable to do request (attempt %d/%d): %v", attempt+1, a.options.Retries, err)
207+
}),
208+
)
209+
210+
return response, err
211+
}
212+
213+
func shouldRetry(resp *http.Response, err error) error {
214+
urlErr := &url.Error{}
215+
if err != nil && errors.As(err, &urlErr) {
216+
return err
217+
}
218+
219+
if resp.StatusCode == http.StatusTooManyRequests {
220+
return fmt.Errorf("retry %d: %s", resp.StatusCode, resp.Status)
208221
}
222+
223+
if resp.StatusCode == 0 || (resp.StatusCode >= 500 &&
224+
resp.StatusCode != http.StatusNotImplemented) {
225+
return fmt.Errorf("retry unexpected HTTP status %d: %s", resp.StatusCode, resp.Status)
226+
}
227+
228+
return nil
209229
}
210230

211231
// Head returns the body of a HEAD request.

http/agent_test.go

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*
2+
Copyright 2025 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+
17+
package http_test
18+
19+
import (
20+
"errors"
21+
"net/http"
22+
"net/url"
23+
"testing"
24+
25+
"github.com/stretchr/testify/assert"
26+
"github.com/stretchr/testify/require"
27+
28+
rhttp "sigs.k8s.io/release-utils/http"
29+
"sigs.k8s.io/release-utils/http/httpfakes"
30+
)
31+
32+
func TestGetRequest(t *testing.T) {
33+
for _, tc := range map[string]struct {
34+
prepare func(*httpfakes.FakeAgentImplementation)
35+
assert func(*http.Response, error)
36+
}{
37+
"should succeed": {
38+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
39+
mock.SendGetRequestReturns(&http.Response{StatusCode: http.StatusOK}, nil)
40+
},
41+
assert: func(response *http.Response, err error) {
42+
require.NoError(t, err)
43+
assert.Equal(t, http.StatusOK, response.StatusCode)
44+
},
45+
},
46+
"should succeed on retry": {
47+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
48+
mock.SendGetRequestReturnsOnCall(0, &http.Response{StatusCode: http.StatusInternalServerError}, nil)
49+
mock.SendGetRequestReturnsOnCall(1, &http.Response{StatusCode: http.StatusOK}, nil)
50+
},
51+
assert: func(response *http.Response, err error) {
52+
require.NoError(t, err)
53+
assert.Equal(t, http.StatusOK, response.StatusCode)
54+
},
55+
},
56+
"should retry on internal server error": {
57+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
58+
mock.SendGetRequestReturns(&http.Response{StatusCode: http.StatusInternalServerError}, nil)
59+
},
60+
assert: func(response *http.Response, err error) {
61+
require.Error(t, err)
62+
assert.NotNil(t, response)
63+
},
64+
},
65+
"should retry on too many requests": {
66+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
67+
mock.SendGetRequestReturns(&http.Response{StatusCode: http.StatusTooManyRequests}, nil)
68+
},
69+
assert: func(response *http.Response, err error) {
70+
require.Error(t, err)
71+
assert.NotNil(t, response)
72+
},
73+
},
74+
"should retry on URL error": {
75+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
76+
mock.SendGetRequestReturns(nil, &url.Error{Err: errors.New("test")})
77+
},
78+
assert: func(response *http.Response, err error) {
79+
require.Error(t, err)
80+
require.Contains(t, err.Error(), "test")
81+
assert.Nil(t, response)
82+
},
83+
},
84+
} {
85+
agent := rhttp.NewAgent().WithWaitTime(0)
86+
mock := &httpfakes.FakeAgentImplementation{}
87+
agent.SetImplementation(mock)
88+
89+
if tc.prepare != nil {
90+
tc.prepare(mock)
91+
}
92+
93+
//nolint:bodyclose // no need to close for mocked tests
94+
tc.assert(agent.GetRequest(""))
95+
}
96+
}
97+
98+
func TestPostRequest(t *testing.T) {
99+
for _, tc := range map[string]struct {
100+
prepare func(*httpfakes.FakeAgentImplementation)
101+
assert func(*http.Response, error)
102+
}{
103+
"should succeed": {
104+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
105+
mock.SendPostRequestReturns(&http.Response{StatusCode: http.StatusOK}, nil)
106+
},
107+
assert: func(response *http.Response, err error) {
108+
require.NoError(t, err)
109+
assert.Equal(t, http.StatusOK, response.StatusCode)
110+
},
111+
},
112+
"should succeed on retry": {
113+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
114+
mock.SendPostRequestReturnsOnCall(0, &http.Response{StatusCode: http.StatusInternalServerError}, nil)
115+
mock.SendPostRequestReturnsOnCall(1, &http.Response{StatusCode: http.StatusOK}, nil)
116+
},
117+
assert: func(response *http.Response, err error) {
118+
require.NoError(t, err)
119+
assert.Equal(t, http.StatusOK, response.StatusCode)
120+
},
121+
},
122+
"should retry on internal server error": {
123+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
124+
mock.SendPostRequestReturns(&http.Response{StatusCode: http.StatusInternalServerError}, nil)
125+
},
126+
assert: func(response *http.Response, err error) {
127+
require.Error(t, err)
128+
assert.NotNil(t, response)
129+
},
130+
},
131+
"should retry on too many requests": {
132+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
133+
mock.SendPostRequestReturns(&http.Response{StatusCode: http.StatusTooManyRequests}, nil)
134+
},
135+
assert: func(response *http.Response, err error) {
136+
require.Error(t, err)
137+
assert.NotNil(t, response)
138+
},
139+
},
140+
"should retry on URL error": {
141+
prepare: func(mock *httpfakes.FakeAgentImplementation) {
142+
mock.SendPostRequestReturns(nil, &url.Error{Err: errors.New("test")})
143+
},
144+
assert: func(response *http.Response, err error) {
145+
require.Error(t, err)
146+
require.Contains(t, err.Error(), "test")
147+
assert.Nil(t, response)
148+
},
149+
},
150+
} {
151+
agent := rhttp.NewAgent().WithWaitTime(0)
152+
mock := &httpfakes.FakeAgentImplementation{}
153+
agent.SetImplementation(mock)
154+
155+
if tc.prepare != nil {
156+
tc.prepare(mock)
157+
}
158+
159+
//nolint:bodyclose // no need to close for mocked tests
160+
tc.assert(agent.PostRequest("", nil))
161+
}
162+
}

0 commit comments

Comments
 (0)