diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 6a0d7da4..2ddf28a1 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -344,7 +344,7 @@ }, { "ImportPath": "k8s.io/api", - "Rev": "b5bd82427fa8" + "Rev": "464a0b549922" }, { "ImportPath": "k8s.io/apimachinery", diff --git a/go.mod b/go.mod index 26b47ac6..0dd0818f 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 golang.org/x/time v0.0.0-20191024005414-555d28b269f0 google.golang.org/appengine v1.5.0 // indirect - k8s.io/api v0.0.0-20200326015715-b5bd82427fa8 + k8s.io/api v0.0.0-20200402140318-464a0b549922 k8s.io/apimachinery v0.0.0-20200331220056-7e441e0f246a k8s.io/klog v1.0.0 k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89 @@ -38,6 +38,6 @@ require ( replace ( golang.org/x/sys => golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a // pinned to release-branch.go1.13 golang.org/x/tools => golang.org/x/tools v0.0.0-20190821162956-65e3620a7ae7 // pinned to release-branch.go1.13 - k8s.io/api => k8s.io/api v0.0.0-20200326015715-b5bd82427fa8 + k8s.io/api => k8s.io/api v0.0.0-20200402140318-464a0b549922 k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20200331220056-7e441e0f246a ) diff --git a/go.sum b/go.sum index 1489c9ed..e388f7cc 100644 --- a/go.sum +++ b/go.sum @@ -190,7 +190,7 @@ gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -k8s.io/api v0.0.0-20200326015715-b5bd82427fa8/go.mod h1:ccyYpAL1p/8sTW0H16JKHFFZ+99445VJK+SEiiyYEcg= +k8s.io/api v0.0.0-20200402140318-464a0b549922/go.mod h1:wh8DIsZ4VHj7B2M8dhAqWn8pUsGy1dNzmTOP4lIDMbI= k8s.io/apimachinery v0.0.0-20200331220056-7e441e0f246a/go.mod h1:MwmRUlFgPZZjQ9mmX205Ve0gth+HzXB7tiAFmJilVME= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= diff --git a/rest/request.go b/rest/request.go index 1acd189e..fe048ec4 100644 --- a/rest/request.go +++ b/rest/request.go @@ -91,6 +91,7 @@ type Request struct { rateLimiter flowcontrol.RateLimiter backoff BackoffManager timeout time.Duration + maxRetries int // generic components accessible via method setters verb string @@ -139,6 +140,7 @@ func NewRequest(c *RESTClient) *Request { backoff: backoff, timeout: timeout, pathPrefix: pathPrefix, + maxRetries: 10, } switch { @@ -391,6 +393,18 @@ func (r *Request) Timeout(d time.Duration) *Request { return r } +// MaxRetries makes the request use the given integer as a ceiling of retrying upon receiving +// "Retry-After" headers and 429 status-code in the response. The default is 10 unless this +// function is specifically called with a different value. +// A zero maxRetries prevent it from doing retires and return an error immediately. +func (r *Request) MaxRetries(maxRetries int) *Request { + if maxRetries < 0 { + maxRetries = 0 + } + r.maxRetries = maxRetries + return r +} + // Body makes the request use obj as the body. Optional. // If obj is a string, try to read a file of that name. // If obj is a []byte, send it directly. @@ -831,7 +845,6 @@ func (r *Request) request(ctx context.Context, fn func(*http.Request, *http.Resp } // Right now we make about ten retry attempts if we get a Retry-After response. - maxRetries := 10 retries := 0 for { @@ -894,7 +907,7 @@ func (r *Request) request(ctx context.Context, fn func(*http.Request, *http.Resp }() retries++ - if seconds, wait := checkWait(resp); wait && retries < maxRetries { + if seconds, wait := checkWait(resp); wait && retries <= r.maxRetries { if seeker, ok := r.body.(io.Seeker); ok && r.body != nil { _, err := seeker.Seek(0, 0) if err != nil { diff --git a/rest/request_test.go b/rest/request_test.go index 2a183ba4..1c199160 100644 --- a/rest/request_test.go +++ b/rest/request_test.go @@ -1403,7 +1403,8 @@ func TestConnectionResetByPeerIsRetried(t *testing.T) { return nil, &net.OpError{Err: syscall.ECONNRESET} }), }, - backoff: backoff, + backoff: backoff, + maxRetries: 10, } // We expect two retries of "connection reset by peer" and the success. _, err := req.Do(context.Background()).Raw() @@ -2218,3 +2219,62 @@ func TestThrottledLogger(t *testing.T) { t.Fatalf("expected %v log messages, but got %v", e, a) } } + +func TestRequestMaxRetries(t *testing.T) { + successAtNthCalls := 1 + actualCalls := 0 + retryOneTimeHandler := func(w http.ResponseWriter, req *http.Request) { + defer func() { actualCalls++ }() + if actualCalls >= successAtNthCalls { + w.WriteHeader(http.StatusOK) + return + } + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + actualCalls++ + } + testServer := httptest.NewServer(http.HandlerFunc(retryOneTimeHandler)) + defer testServer.Close() + + u, err := url.Parse(testServer.URL) + if err != nil { + t.Error(err) + } + + testCases := []struct { + name string + maxRetries int + expectError bool + }{ + { + name: "no retrying should fail", + maxRetries: 0, + expectError: true, + }, + { + name: "1 max-retry should exactly work", + maxRetries: 1, + expectError: false, + }, + { + name: "5 max-retry should work", + maxRetries: 5, + expectError: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + defer func() { actualCalls = 0 }() + _, err := NewRequestWithClient(u, "", defaultContentConfig(), testServer.Client()). + Verb("get"). + MaxRetries(testCase.maxRetries). + AbsPath("/foo"). + DoRaw(context.TODO()) + hasError := err != nil + if testCase.expectError != hasError { + t.Error(" failed checking error") + } + }) + } +}