Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Retry-After in clients #26874

Merged
merged 1 commit into from
Jun 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/client/restclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ type RESTClient struct {
// serializers contain all serializers for undelying content type.
serializers Serializers

// creates BackoffManager that is passed to requests.
createBackoffMgr func() BackoffManager

// TODO extract this into a wrapper interface via the RESTClient interface in kubectl.
Throttle flowcontrol.RateLimiter

Expand Down Expand Up @@ -105,6 +108,7 @@ func NewRESTClient(baseURL *url.URL, versionedAPIPath string, config ContentConf
versionedAPIPath: versionedAPIPath,
contentConfig: config,
serializers: *serializers,
createBackoffMgr: readExpBackoffConfig,
Throttle: throttle,
Client: client,
}, nil
Expand Down Expand Up @@ -181,7 +185,7 @@ func createSerializers(config ContentConfig) (*Serializers, error) {
// list, ok := resp.(*api.PodList)
//
func (c *RESTClient) Verb(verb string) *Request {
backoff := readExpBackoffConfig()
backoff := c.createBackoffMgr()

if c.Client == nil {
return NewRequest(nil, verb, c.base, c.versionedAPIPath, c.contentConfig, c.serializers, backoff, c.Throttle)
Expand Down
26 changes: 24 additions & 2 deletions pkg/client/restclient/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,21 @@ func TestBackoffLifecycle(t *testing.T) {
}
}

type testBackoffManager struct {
sleeps []time.Duration
}

func (b *testBackoffManager) UpdateBackoff(actualUrl *url.URL, err error, responseCode int) {
}

func (b *testBackoffManager) CalculateBackoff(actualUrl *url.URL) time.Duration {
return time.Duration(0)
}

func (b *testBackoffManager) Sleep(d time.Duration) {
b.sleeps = append(b.sleeps, d)
}

func TestCheckRetryClosesBody(t *testing.T) {
count := 0
ch := make(chan struct{})
Expand All @@ -849,12 +864,16 @@ func TestCheckRetryClosesBody(t *testing.T) {
close(ch)
return
}
w.Header().Set("Retry-After", "0")
w.WriteHeader(apierrors.StatusTooManyRequests)
w.Header().Set("Retry-After", "1")
http.Error(w, "Too many requests, please try again later.", apierrors.StatusTooManyRequests)
}))
defer testServer.Close()

backoffMgr := &testBackoffManager{}
expectedSleeps := []time.Duration{0, time.Second, 0, time.Second, 0, time.Second, 0, time.Second, 0}

c := testRESTClient(t, testServer)
c.createBackoffMgr = func() BackoffManager { return backoffMgr }
_, err := c.Verb("POST").
Prefix("foo", "bar").
Suffix("baz").
Expand All @@ -868,6 +887,9 @@ func TestCheckRetryClosesBody(t *testing.T) {
if count != 5 {
t.Errorf("unexpected retries: %d", count)
}
if !reflect.DeepEqual(backoffMgr.sleeps, expectedSleeps) {
t.Errorf("unexpected sleeps, expected: %v, got: %v", expectedSleeps, backoffMgr.sleeps)
}
}

func TestCheckRetryHandles429And5xx(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/client/restclient/urlbackoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ type NoBackoff struct {
func (n *NoBackoff) UpdateBackoff(actualUrl *url.URL, err error, responseCode int) {
// do nothing.
}

func (n *NoBackoff) CalculateBackoff(actualUrl *url.URL) time.Duration {
return 0 * time.Second
}

func (n *NoBackoff) Sleep(d time.Duration) {
return
time.Sleep(d)
}

// Disable makes the backoff trivial, i.e., sets it to zero. This might be used
Expand Down