From 528713bcc2e2be5bb592e4a855cccf719260df8d Mon Sep 17 00:00:00 2001
From: Wojciech Tyczynski <wojtekt@google.com>
Date: Mon, 6 Jun 2016 09:48:44 +0200
Subject: [PATCH] Fix Retry-After in clients

---
 pkg/client/restclient/client.go       |  6 +++++-
 pkg/client/restclient/request_test.go | 26 ++++++++++++++++++++++++--
 pkg/client/restclient/urlbackoff.go   |  4 +++-
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/pkg/client/restclient/client.go b/pkg/client/restclient/client.go
index bf813d052a0..230edd45c74 100644
--- a/pkg/client/restclient/client.go
+++ b/pkg/client/restclient/client.go
@@ -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
 
@@ -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
@@ -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)
diff --git a/pkg/client/restclient/request_test.go b/pkg/client/restclient/request_test.go
index e770a1ca083..d51b43501f2 100644
--- a/pkg/client/restclient/request_test.go
+++ b/pkg/client/restclient/request_test.go
@@ -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{})
@@ -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").
@@ -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) {
diff --git a/pkg/client/restclient/urlbackoff.go b/pkg/client/restclient/urlbackoff.go
index df453e65fdd..6c672f08af6 100644
--- a/pkg/client/restclient/urlbackoff.go
+++ b/pkg/client/restclient/urlbackoff.go
@@ -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