diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 0a40e7b21ba..8853be8a9f5 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -36,6 +36,8 @@ import ( var FileExtensions = []string{".json", ".yaml", ".yml"} var InputExtensions = append(FileExtensions, "stdin") +const defaultHttpGetAttempts int = 3 + // Builder provides convenience functions for taking arguments and parameters // from the command line and converting them to a list of resources to iterate // over using the Visitor interface. @@ -109,7 +111,7 @@ func (b *Builder) FilenameParam(enforceNamespace, recursive bool, paths ...strin b.errs = append(b.errs, fmt.Errorf("the URL passed to filename %q is not valid: %v", s, err)) continue } - b.URL(url) + b.URL(defaultHttpGetAttempts, url) default: b.Path(recursive, s) } @@ -123,11 +125,12 @@ func (b *Builder) FilenameParam(enforceNamespace, recursive bool, paths ...strin } // URL accepts a number of URLs directly. -func (b *Builder) URL(urls ...*url.URL) *Builder { +func (b *Builder) URL(httpAttemptCount int, urls ...*url.URL) *Builder { for _, u := range urls { b.paths = append(b.paths, &URLVisitor{ - URL: u, - StreamVisitor: NewStreamVisitor(nil, b.mapper, u.String(), b.schema), + URL: u, + StreamVisitor: NewStreamVisitor(nil, b.mapper, u.String(), b.schema), + HttpAttemptCount: httpAttemptCount, }) } return b diff --git a/pkg/kubectl/resource/visitor.go b/pkg/kubectl/resource/visitor.go index be138b210e6..288bfa85aca 100644 --- a/pkg/kubectl/resource/visitor.go +++ b/pkg/kubectl/resource/visitor.go @@ -24,6 +24,7 @@ import ( "net/url" "os" "path/filepath" + "time" "k8s.io/kubernetes/pkg/api/meta" "k8s.io/kubernetes/pkg/api/unversioned" @@ -221,22 +222,69 @@ func ValidateSchema(data []byte, schema validation.Schema) error { type URLVisitor struct { URL *url.URL *StreamVisitor + HttpAttemptCount int } func (v *URLVisitor) Visit(fn VisitorFunc) error { - res, err := http.Get(v.URL.String()) + body, err := readHttpWithRetries(httpgetImpl, time.Second, v.URL.String(), v.HttpAttemptCount) if err != nil { return err } - defer res.Body.Close() - if res.StatusCode != 200 { - return fmt.Errorf("unable to read URL %q, server reported %d %s", v.URL, res.StatusCode, res.Status) - } - - v.StreamVisitor.Reader = res.Body + defer body.Close() + v.StreamVisitor.Reader = body return v.StreamVisitor.Visit(fn) } +// readHttpWithRetries tries to http.Get the v.URL retries times before giving up. +func readHttpWithRetries(get httpget, duration time.Duration, u string, attempts int) (io.ReadCloser, error) { + var err error + var body io.ReadCloser + if attempts <= 0 { + return nil, fmt.Errorf("http attempts must be greater than 0, was %d", attempts) + } + for i := 0; i < attempts; i++ { + var statusCode int + var status string + if i > 0 { + time.Sleep(duration) + } + + // Try to get the URL + statusCode, status, body, err = get(u) + + // Retry Errors + if err != nil { + continue + } + + // Error - Set the error condition from the StatusCode + if statusCode != 200 { + err = fmt.Errorf("unable to read URL %q, server reported %d %s", u, statusCode, status) + } + + if statusCode >= 500 && statusCode < 600 { + // Retry 500's + continue + } else { + // Don't retry other StatusCodes + break + } + } + return body, err +} + +// httpget Defines function to retrieve a url and return the results. Exists for unit test stubbing. +type httpget func(url string) (int, string, io.ReadCloser, error) + +// httpgetImpl Implements a function to retrieve a url and return the results. +func httpgetImpl(url string) (int, string, io.ReadCloser, error) { + resp, err := http.Get(url) + if err != nil { + return 0, "", nil, err + } + return resp.StatusCode, resp.Status, resp.Body, nil +} + // DecoratedVisitor will invoke the decorators in order prior to invoking the visitor function // passed to Visit. An error will terminate the visit. type DecoratedVisitor struct { diff --git a/pkg/kubectl/resource/visitor_test.go b/pkg/kubectl/resource/visitor_test.go new file mode 100644 index 00000000000..e781c90c249 --- /dev/null +++ b/pkg/kubectl/resource/visitor_test.go @@ -0,0 +1,102 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resource + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestVisitorHttpGet(t *testing.T) { + // Test retries on errors + i := 0 + expectedErr := fmt.Errorf("Failed to get http") + actualBytes, actualErr := readHttpWithRetries(func(url string) (int, string, io.ReadCloser, error) { + assert.Equal(t, "hello", url) + i++ + if i > 2 { + return 0, "", nil, expectedErr + } + return 0, "", nil, fmt.Errorf("Unexpected error") + }, 0, "hello", 3) + assert.Equal(t, expectedErr, actualErr) + assert.Nil(t, actualBytes) + assert.Equal(t, 3, i) + + // Test that 500s are retried. + i = 0 + actualBytes, actualErr = readHttpWithRetries(func(url string) (int, string, io.ReadCloser, error) { + assert.Equal(t, "hello", url) + i++ + return 501, "Status", nil, nil + }, 0, "hello", 3) + assert.Error(t, actualErr) + assert.Nil(t, actualBytes) + assert.Equal(t, 3, i) + + // Test that 300s are not retried + i = 0 + actualBytes, actualErr = readHttpWithRetries(func(url string) (int, string, io.ReadCloser, error) { + assert.Equal(t, "hello", url) + i++ + return 300, "Status", nil, nil + }, 0, "hello", 3) + assert.Error(t, actualErr) + assert.Nil(t, actualBytes) + assert.Equal(t, 1, i) + + // Test attempt count is respected + i = 0 + actualBytes, actualErr = readHttpWithRetries(func(url string) (int, string, io.ReadCloser, error) { + assert.Equal(t, "hello", url) + i++ + return 501, "Status", nil, nil + }, 0, "hello", 1) + assert.Error(t, actualErr) + assert.Nil(t, actualBytes) + assert.Equal(t, 1, i) + + // Test attempts less than 1 results in an error + i = 0 + b := bytes.Buffer{} + actualBytes, actualErr = readHttpWithRetries(func(url string) (int, string, io.ReadCloser, error) { + return 200, "Status", ioutil.NopCloser(&b), nil + }, 0, "hello", 0) + assert.Error(t, actualErr) + assert.Nil(t, actualBytes) + assert.Equal(t, 0, i) + + // Test Success + i = 0 + b = bytes.Buffer{} + actualBytes, actualErr = readHttpWithRetries(func(url string) (int, string, io.ReadCloser, error) { + assert.Equal(t, "hello", url) + i++ + if i > 1 { + return 200, "Status", ioutil.NopCloser(&b), nil + } + return 501, "Status", nil, nil + }, 0, "hello", 3) + assert.Nil(t, actualErr) + assert.NotNil(t, actualBytes) + assert.Equal(t, 2, i) +}