diff --git a/compute/metadata/metadata.go b/compute/metadata/metadata.go index 646950811c2..471adfdb1de 100644 --- a/compute/metadata/metadata.go +++ b/compute/metadata/metadata.go @@ -308,20 +308,24 @@ func (c *Client) getETag(suffix string) (value, etag string, err error) { req.Header.Set("Metadata-Flavor", "Google") req.Header.Set("User-Agent", userAgent) var res *http.Response + var reqErr error retryer := newRetryer() for { - var err error - res, err = c.hc.Do(req) - if err == nil { - break + res, reqErr = c.hc.Do(req) + var code int + if res != nil { + code = res.StatusCode } - if delay, shouldRetry := retryer.Retry(res.StatusCode, err); shouldRetry { + if delay, shouldRetry := retryer.Retry(code, reqErr); shouldRetry { if err := gax.Sleep(ctx, delay); err != nil { return "", "", err } continue } - return "", "", err + break + } + if reqErr != nil { + return "", "", nil } defer res.Body.Close() if res.StatusCode == http.StatusNotFound { diff --git a/compute/metadata/metadata_go113_test.go b/compute/metadata/metadata_go113_test.go new file mode 100644 index 00000000000..94e2594c1c9 --- /dev/null +++ b/compute/metadata/metadata_go113_test.go @@ -0,0 +1,96 @@ +// Copyright 2016 Google LLC +// +// 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. + +//go:build go1.13 +// +build go1.13 + +package metadata + +import ( + "io" + "io/ioutil" + "net/http" + "strings" + "testing" +) + +func TestRetry(t *testing.T) { + tests := []struct { + name string + timesToFail int + failCode int + failErr error + response string + }{ + { + name: "no retries", + response: "test", + }, + { + name: "retry 500 once", + response: "test", + failCode: 500, + timesToFail: 1, + }, + { + name: "retry io.ErrUnexpectedEOF once", + response: "test", + failErr: io.ErrUnexpectedEOF, + timesToFail: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ft := &failingTransport{ + timesToFail: tt.timesToFail, + failCode: tt.failCode, + failErr: tt.failErr, + response: tt.response, + } + c := NewClient(&http.Client{Transport: ft}) + s, err := c.Get("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ft.called != ft.failedAttempts+1 { + t.Fatalf("failed %d times, want %d", ft.called, ft.failedAttempts+1) + } + if s != tt.response { + t.Fatalf("c.Get() = %q, want %q", s, tt.response) + } + }) + } +} + +type failingTransport struct { + timesToFail int + failCode int + failErr error + response string + + failedAttempts int + called int +} + +func (r *failingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + r.called++ + if r.failedAttempts < r.timesToFail { + r.failedAttempts++ + if r.failErr != nil { + return nil, r.failErr + } + return &http.Response{StatusCode: r.failCode}, nil + } + return &http.Response{StatusCode: http.StatusOK, Body: ioutil.NopCloser(strings.NewReader(r.response))}, nil +} diff --git a/compute/metadata/retry.go b/compute/metadata/retry.go index e290452234e..b1d1e7981a1 100644 --- a/compute/metadata/retry.go +++ b/compute/metadata/retry.go @@ -16,6 +16,7 @@ package metadata import ( "io" + "net/http" "time" "github.com/googleapis/gax-go/v2" @@ -43,6 +44,9 @@ type metadataRetryer struct { } func (r *metadataRetryer) Retry(status int, err error) (time.Duration, bool) { + if status == http.StatusOK { + return 0, false + } retryOk := shouldRetry(status, err) if !retryOk { return 0, false diff --git a/compute/metadata/retry_test.go b/compute/metadata/retry_test.go index 5908ea07933..c96ef054ae9 100644 --- a/compute/metadata/retry_test.go +++ b/compute/metadata/retry_test.go @@ -87,6 +87,13 @@ func TestMetadataRetryer(t *testing.T) { wantDelay: 0, wantShouldRetry: false, }, + { + name: "don't retry 200", + code: 200, + err: nil, + wantDelay: 0, + wantShouldRetry: false, + }, } for _, tc := range tests {