Skip to content

Commit d76d23e

Browse files
Initial retry transport (#3196) (#1856)
* retry transport * lint * remove import, comments * Rename copyRequest, change comments, remove redundant line * refactor test * clarifications, defers * lint/fmt Signed-off-by: Modular Magician <[email protected]>
1 parent 1844a94 commit d76d23e

File tree

5 files changed

+491
-21
lines changed

5 files changed

+491
-21
lines changed

.changelog/3196.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:enhancement
2+
provider: Added provider-wide request retries for common temporary GCP error codes and network errors
3+
```

google-beta/config.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ import (
88
"regexp"
99
"time"
1010

11+
"github.com/hashicorp/go-cleanhttp"
1112
"github.com/hashicorp/terraform-plugin-sdk/helper/logging"
1213
"github.com/hashicorp/terraform-plugin-sdk/helper/pathorcontents"
1314
"github.com/hashicorp/terraform-plugin-sdk/httpclient"
1415
"github.com/terraform-providers/terraform-provider-google-beta/version"
16+
"google.golang.org/api/option"
1517

1618
"golang.org/x/oauth2"
1719
googleoauth "golang.org/x/oauth2/google"
@@ -40,7 +42,6 @@ import (
4042
"google.golang.org/api/iam/v1"
4143
iamcredentials "google.golang.org/api/iamcredentials/v1"
4244
cloudlogging "google.golang.org/api/logging/v2"
43-
"google.golang.org/api/option"
4445
"google.golang.org/api/pubsub/v1"
4546
runtimeconfig "google.golang.org/api/runtimeconfig/v1beta1"
4647
"google.golang.org/api/servicemanagement/v1"
@@ -284,8 +285,23 @@ func (c *Config) LoadAndValidate(ctx context.Context) error {
284285
}
285286
c.tokenSource = tokenSource
286287

287-
client := oauth2.NewClient(context.Background(), tokenSource)
288-
client.Transport = logging.NewTransport("Google", client.Transport)
288+
cleanCtx := context.WithValue(ctx, oauth2.HTTPClient, cleanhttp.DefaultClient())
289+
290+
// 1. OAUTH2 TRANSPORT/CLIENT - sets up proper auth headers
291+
client := oauth2.NewClient(cleanCtx, tokenSource)
292+
293+
// 2. Logging Transport - ensure we log HTTP requests to GCP APIs.
294+
loggingTransport := logging.NewTransport("Google", client.Transport)
295+
296+
// 3. Retry Transport - retries common temporary errors
297+
// Keep order for wrapping logging so we log each retried request as well.
298+
// This value should be used if needed to create shallow copies with additional retry predicates.
299+
// See ClientWithAdditionalRetries
300+
retryTransport := NewTransportWithDefaultRetries(loggingTransport)
301+
302+
// Set final transport value.
303+
client.Transport = retryTransport
304+
289305
// This timeout is a timeout per HTTP request, not per logical operation.
290306
client.Timeout = c.synchronousTimeout()
291307

@@ -394,7 +410,8 @@ func (c *Config) LoadAndValidate(ctx context.Context) error {
394410

395411
pubsubClientBasePath := removeBasePathVersion(c.PubsubBasePath)
396412
log.Printf("[INFO] Instantiating Google Pubsub client for path %s", pubsubClientBasePath)
397-
c.clientPubsub, err = pubsub.NewService(ctx, option.WithHTTPClient(client))
413+
wrappedPubsubClient := ClientWithAdditionalRetries(client, retryTransport, pubsubTopicProjectNotReady)
414+
c.clientPubsub, err = pubsub.NewService(ctx, option.WithHTTPClient(wrappedPubsubClient))
398415
if err != nil {
399416
return err
400417
}
@@ -493,7 +510,8 @@ func (c *Config) LoadAndValidate(ctx context.Context) error {
493510

494511
bigQueryClientBasePath := c.BigQueryBasePath
495512
log.Printf("[INFO] Instantiating Google Cloud BigQuery client for path %s", bigQueryClientBasePath)
496-
c.clientBigQuery, err = bigquery.NewService(ctx, option.WithHTTPClient(client))
513+
wrappedBigQueryClient := ClientWithAdditionalRetries(client, retryTransport, iamMemberMissing)
514+
c.clientBigQuery, err = bigquery.NewService(ctx, option.WithHTTPClient(wrappedBigQueryClient))
497515
if err != nil {
498516
return err
499517
}

google-beta/error_retry_predicates.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"net/url"
99
"strings"
1010

11-
"golang.org/x/oauth2"
1211
"google.golang.org/api/googleapi"
1312
)
1413

@@ -72,21 +71,7 @@ const connectionResetByPeerErr = ": connection reset by peer"
7271

7372
func isConnectionResetNetworkError(err error) (bool, string) {
7473
if strings.HasSuffix(err.Error(), connectionResetByPeerErr) {
75-
//TODO(emilymye, TPG#3957): Remove these debug logs
76-
log.Printf("[DEBUG] Found connection reset by peer error of type %T", err)
77-
switch err.(type) {
78-
case *url.Error:
79-
case *net.OpError:
80-
log.Printf("[DEBUG] Connection reset error returned from net/url")
81-
case *googleapi.Error:
82-
log.Printf("[DEBUG] Connection reset error wrapped by googleapi.Error")
83-
case *oauth2.RetrieveError:
84-
log.Printf("[DEBUG] Connection reset error wrapped by oauth2")
85-
default:
86-
log.Printf("[DEBUG] Connection reset error wrapped by %T", err)
87-
}
88-
89-
return true, fmt.Sprintf("reset connection")
74+
return true, fmt.Sprintf("reset connection error: %v", err)
9075
}
9176
return false, ""
9277
}

google-beta/retry_transport.go

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
// A http.RoundTripper that retries common errors, with convenience constructors.
2+
//
3+
// NOTE: This meant for TEMPORARY, TRANSIENT ERRORS.
4+
// Do not use for waiting on operations or polling of resource state,
5+
// especially if the expected state (operation done, resource ready, etc)
6+
// takes longer to reach than the default client Timeout.
7+
// In those cases, retryTimeDuration(...)/resource.Retry with appropriate timeout
8+
// and error predicates/handling should be used as a wrapper around the request
9+
// instead.
10+
//
11+
// Example Usage:
12+
// For handwritten/Go clients, the retry transport should be provided via
13+
// the main client or a shallow copy of the HTTP resources, depending on the
14+
// API-specific retry predicates.
15+
// Example Usage in Terraform Config:
16+
// client := oauth2.NewClient(ctx, tokenSource)
17+
// // Create with default retry predicates
18+
// client.Transport := NewTransportWithDefaultRetries(client.Transport, defaultTimeout)
19+
//
20+
// // If API uses just default retry predicates:
21+
// c.clientCompute, err = compute.NewService(ctx, option.WithHTTPClient(client))
22+
// ...
23+
// // If API needs custom additional retry predicates:
24+
// sqlAdminHttpClient := ClientWithAdditionalRetries(client, retryTransport,
25+
// isTemporarySqlError1,
26+
// isTemporarySqlError2)
27+
// c.clientSqlAdmin, err = compute.NewService(ctx, option.WithHTTPClient(sqlAdminHttpClient))
28+
// ...
29+
30+
package google
31+
32+
import (
33+
"bytes"
34+
"context"
35+
"errors"
36+
"fmt"
37+
"github.com/hashicorp/errwrap"
38+
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
39+
"google.golang.org/api/googleapi"
40+
"io/ioutil"
41+
"log"
42+
"net/http"
43+
"net/http/httputil"
44+
"time"
45+
)
46+
47+
const defaultRetryTransportTimeoutSec = 30
48+
49+
// NewTransportWithDefaultRetries constructs a default retryTransport that will retry common temporary errors
50+
func NewTransportWithDefaultRetries(t http.RoundTripper) *retryTransport {
51+
return &retryTransport{
52+
retryPredicates: defaultErrorRetryPredicates,
53+
internal: t,
54+
}
55+
}
56+
57+
// Helper method to create a shallow copy of an HTTP client with a shallow-copied retryTransport
58+
// s.t. the base HTTP transport is the same (i.e. client connection pools are shared, retryPredicates are different)
59+
func ClientWithAdditionalRetries(baseClient *http.Client, baseRetryTransport *retryTransport, predicates ...RetryErrorPredicateFunc) *http.Client {
60+
copied := *baseClient
61+
if baseRetryTransport == nil {
62+
baseRetryTransport = NewTransportWithDefaultRetries(baseClient.Transport)
63+
}
64+
copied.Transport = baseRetryTransport.WithAddedPredicates(predicates...)
65+
return &copied
66+
}
67+
68+
// Returns a shallow copy of the retry transport with additional retry
69+
// predicates but same wrapped http.RoundTripper
70+
func (t *retryTransport) WithAddedPredicates(predicates ...RetryErrorPredicateFunc) *retryTransport {
71+
copyT := *t
72+
copyT.retryPredicates = append(t.retryPredicates, predicates...)
73+
return &copyT
74+
}
75+
76+
type retryTransport struct {
77+
retryPredicates []RetryErrorPredicateFunc
78+
internal http.RoundTripper
79+
}
80+
81+
// RoundTrip implements the RoundTripper interface method.
82+
// It retries the given HTTP request based on the retry predicates
83+
// registered under the retryTransport.
84+
func (t *retryTransport) RoundTrip(req *http.Request) (resp *http.Response, respErr error) {
85+
// Set timeout to default value.
86+
ctx := req.Context()
87+
var ccancel context.CancelFunc
88+
if _, ok := ctx.Deadline(); !ok {
89+
ctx, ccancel = context.WithTimeout(ctx, defaultRetryTransportTimeoutSec*time.Second)
90+
defer func() {
91+
if ctx.Err() == nil {
92+
// Cleanup child context created for retry loop if ctx not done.
93+
ccancel()
94+
}
95+
}()
96+
}
97+
98+
attempts := 0
99+
backoff := time.Millisecond * 500
100+
nextBackoff := time.Millisecond * 500
101+
102+
log.Printf("[DEBUG] Retry Transport: starting RoundTrip retry loop")
103+
Retry:
104+
for {
105+
log.Printf("[DEBUG] Retry Transport: request attempt %d", attempts)
106+
107+
// Copy the request - we dont want to use the original request as
108+
// RoundTrip contract says request body can/will be consumed
109+
newRequest, copyErr := copyHttpRequest(req)
110+
if copyErr != nil {
111+
respErr = errwrap.Wrapf("unable to copy invalid http.Request for retry: {{err}}", copyErr)
112+
break Retry
113+
}
114+
115+
// Do the wrapped Roundtrip. This is one request in the retry loop.
116+
resp, respErr = t.internal.RoundTrip(newRequest)
117+
attempts++
118+
119+
retryErr := t.checkForRetryableError(resp, respErr)
120+
if retryErr == nil {
121+
log.Printf("[DEBUG] Retry Transport: Stopping retries, last request was successful")
122+
break Retry
123+
}
124+
if !retryErr.Retryable {
125+
log.Printf("[DEBUG] Retry Transport: Stopping retries, last request failed with non-retryable error: %s", retryErr.Err)
126+
break Retry
127+
}
128+
129+
log.Printf("[DEBUG] Retry Transport: Waiting %s before trying request again", backoff)
130+
select {
131+
case <-ctx.Done():
132+
log.Printf("[DEBUG] Retry Transport: Stopping retries, context done: %v", ctx.Err())
133+
break Retry
134+
case <-time.After(backoff):
135+
log.Printf("[DEBUG] Retry Transport: Finished waiting %s before next retry", backoff)
136+
137+
// Fibonnaci backoff - 0.5, 1, 1.5, 2.5, 4, 6.5, 10.5, ...
138+
lastBackoff := backoff
139+
backoff = backoff + nextBackoff
140+
nextBackoff = lastBackoff
141+
continue
142+
}
143+
}
144+
145+
log.Printf("[DEBUG] Retry Transport: Returning after %d attempts", attempts)
146+
return resp, respErr
147+
}
148+
149+
// copyHttpRequest provides an copy of the given HTTP request for one RoundTrip.
150+
// If the request has a non-empty body (io.ReadCloser), the body is deep copied
151+
// so it can be consumed.
152+
func copyHttpRequest(req *http.Request) (*http.Request, error) {
153+
newRequest := *req
154+
155+
if req.Body == nil || req.Body == http.NoBody {
156+
return &newRequest, nil
157+
}
158+
159+
// Helpers like http.NewRequest add a GetBody for copying.
160+
// If not given, we should reject the request.
161+
if req.GetBody == nil {
162+
return nil, errors.New("invalid HTTP request for transport, expected request.GetBody for non-empty Body")
163+
}
164+
165+
bd, err := req.GetBody()
166+
if err != nil {
167+
return nil, err
168+
}
169+
170+
newRequest.Body = bd
171+
return &newRequest, nil
172+
}
173+
174+
// checkForRetryableError uses the googleapi.CheckResponse util to check for
175+
// errors in the response, and determines whether there is a retryable error.
176+
// in response/response error.
177+
func (t *retryTransport) checkForRetryableError(resp *http.Response, respErr error) *resource.RetryError {
178+
var errToCheck error
179+
180+
if respErr != nil {
181+
errToCheck = respErr
182+
} else {
183+
respToCheck := *resp
184+
// The RoundTrip contract states that the HTTP response/response error
185+
// returned cannot be edited. We need to consume the Body to check for
186+
// errors, so we need to create a copy if the Response has a body.
187+
if resp.Body != nil && resp.Body != http.NoBody {
188+
// Use httputil.DumpResponse since the only important info is
189+
// error code and messages in the response body.
190+
dumpBytes, err := httputil.DumpResponse(resp, true)
191+
if err != nil {
192+
return resource.NonRetryableError(fmt.Errorf("unable to check response for error: %v", err))
193+
}
194+
respToCheck.Body = ioutil.NopCloser(bytes.NewReader(dumpBytes))
195+
}
196+
errToCheck = googleapi.CheckResponse(&respToCheck)
197+
}
198+
199+
if isRetryableError(errToCheck, t.retryPredicates...) {
200+
return resource.RetryableError(errToCheck)
201+
}
202+
return resource.NonRetryableError(errToCheck)
203+
}

0 commit comments

Comments
 (0)