From 0b8d725f5a04178caf09cd802305c4b8370db65e Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy <126021+ash2k@users.noreply.github.com> Date: Tue, 11 Jan 2022 06:40:26 +1100 Subject: [PATCH] Close HTTP response body on failed GET attempts (#105591) * Close response body when it's not needed * Code cleanups --- .../cli-runtime/pkg/resource/builder.go | 5 ++-- .../cli-runtime/pkg/resource/visitor.go | 30 +++++++++++-------- .../cli-runtime/pkg/resource/visitor_test.go | 8 ++--- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go index f206f3b3ef3..d9faf9b4cc6 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go @@ -1155,10 +1155,9 @@ func (b *Builder) Do() *Result { helpers = append(helpers, RetrieveLazy) } if b.continueOnError { - r.visitor = NewDecoratedVisitor(ContinueOnErrorVisitor{r.visitor}, helpers...) - } else { - r.visitor = NewDecoratedVisitor(r.visitor, helpers...) + r.visitor = ContinueOnErrorVisitor{Visitor: r.visitor} } + r.visitor = NewDecoratedVisitor(r.visitor, helpers...) return r } diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go index 41962cb5e09..48ee0235810 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go @@ -204,9 +204,9 @@ type EagerVisitorList []Visitor // Visit implements Visitor, and gathers errors that occur during processing until // all sub visitors have been visited. func (l EagerVisitorList) Visit(fn VisitorFunc) error { - errs := []error(nil) + var errs []error for i := range l { - if err := l[i].Visit(func(info *Info, err error) error { + err := l[i].Visit(func(info *Info, err error) error { if err != nil { errs = append(errs, err) return nil @@ -215,7 +215,8 @@ func (l EagerVisitorList) Visit(fn VisitorFunc) error { errs = append(errs, err) } return nil - }); err != nil { + }) + if err != nil { errs = append(errs, err) } } @@ -253,13 +254,15 @@ func (v *URLVisitor) Visit(fn VisitorFunc) error { // 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 + var ( + statusCode int + status string + body io.ReadCloser + ) if i > 0 { time.Sleep(duration) } @@ -272,10 +275,12 @@ func readHttpWithRetries(get httpget, duration time.Duration, u string, attempts continue } - // Error - Set the error condition from the StatusCode - if statusCode != http.StatusOK { - err = fmt.Errorf("unable to read URL %q, server reported %s, status code=%d", u, status, statusCode) + if statusCode == http.StatusOK { + return body, nil } + body.Close() + // Error - Set the error condition from the StatusCode + err = fmt.Errorf("unable to read URL %q, server reported %s, status code=%d", u, status, statusCode) if statusCode >= 500 && statusCode < 600 { // Retry 500's @@ -285,7 +290,7 @@ func readHttpWithRetries(get httpget, duration time.Duration, u string, attempts break } } - return body, err + return nil, err } // httpget Defines function to retrieve a url and return the results. Exists for unit test stubbing. @@ -346,7 +351,7 @@ type ContinueOnErrorVisitor struct { // returned by the visitor directly may still result in some items // not being visited. func (v ContinueOnErrorVisitor) Visit(fn VisitorFunc) error { - errs := []error{} + var errs []error err := v.Visitor.Visit(func(info *Info, err error) error { if err != nil { errs = append(errs, err) @@ -420,7 +425,7 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error { if info.Mapping != nil && !info.Mapping.GroupVersionKind.Empty() { preferredGVKs = append(preferredGVKs, info.Mapping.GroupVersionKind) } - errs := []error{} + var errs []error for i := range items { item, err := v.mapper.infoForObject(items[i], v.typer, preferredGVKs) if err != nil { @@ -439,7 +444,6 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error { } } return utilerrors.NewAggregate(errs) - }) } diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go index 1ecb5a61e30..dabb45c043a 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go @@ -75,7 +75,7 @@ func TestVisitorHttpGet(t *testing.T) { httpRetries: func(url string) (int, string, io.ReadCloser, error) { assert.Equal(t, "hello", url) i++ - return 501, "Status", nil, nil + return 501, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil }, args: httpArgs{ duration: 0, @@ -89,7 +89,7 @@ func TestVisitorHttpGet(t *testing.T) { httpRetries: func(url string) (int, string, io.ReadCloser, error) { assert.Equal(t, "hello", url) i++ - return 300, "Status", nil, nil + return 300, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil }, args: httpArgs{ @@ -104,7 +104,7 @@ func TestVisitorHttpGet(t *testing.T) { httpRetries: func(url string) (int, string, io.ReadCloser, error) { assert.Equal(t, "hello", url) i++ - return 501, "Status", nil, nil + return 501, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil }, args: httpArgs{ @@ -135,7 +135,7 @@ func TestVisitorHttpGet(t *testing.T) { if i > 1 { return 200, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil } - return 501, "Status", nil, nil + return 501, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil }, args: httpArgs{