fix(compute/metadata): fix retry logic to not panic on error

compute/metadata/metadata.go

@@ -312,16 +312,17 @@ func (c *Client) getETag(suffix string) (value, etag string, err error) {
 	for {
 		var err error
 		res, err = c.hc.Do(req)
-		if err == nil {
-			break
+		var code int
+		if res != nil {
+			code = res.StatusCode
 		}
-		if delay, shouldRetry := retryer.Retry(res.StatusCode, err); shouldRetry {
+		if delay, shouldRetry := retryer.Retry(code, err); shouldRetry {
 			if err := gax.Sleep(ctx, delay); err != nil {
 				return "", "", err
 			}
 			continue
 		}
-		return "", "", err
+		break
 	}
 	defer res.Body.Close()
 	if res.StatusCode == http.StatusNotFound {

compute/metadata/metadata_test.go

@@ -16,10 +16,12 @@ package metadata
 
 import (
 	"bytes"
+	"io"
 	"io/ioutil"
 	"log"
 	"net/http"
 	"os"
+	"strings"
 	"sync"
 	"testing"
 )
@@ -100,6 +102,54 @@ func TestGet_LeadingSlash(t *testing.T) {
 	}
 }
 
+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.failedAttempts, tt.timesToFail)
+			}
+			if s != tt.response {
+				t.Fatalf("c.Get() = %q, want %q", s, tt.response)
+			}
+		})
+	}
+}
+
 type captureTransport struct {
 	url string
 }
@@ -127,3 +177,22 @@ func (r *rrt) RoundTrip(req *http.Request) (*http.Response, error) {
 	r.gotUserAgent = req.Header.Get("User-Agent")
 	return &http.Response{Body: ioutil.NopCloser(bytes.NewReader(nil))}, nil
 }
+
+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++
+		return &http.Response{StatusCode: r.failCode}, r.failErr
+	}
+	return &http.Response{StatusCode: http.StatusOK, Body: ioutil.NopCloser(strings.NewReader(r.response))}, nil
+}

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

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 {