diff --git a/google/common_operation.go b/google/common_operation.go index 3160b180..e930ca21 100644 --- a/google/common_operation.go +++ b/google/common_operation.go @@ -22,7 +22,9 @@ type Waiter interface { SetOp(interface{}) error // QueryOp sends a request to the server to get the current status of the - // operation. + // operation. It's expected that QueryOp will return exactly one of an + // operation or an error as non-nil, and that requests will be retried by + // specific implementations of the method. QueryOp() (interface{}, error) // OpName is the name of the operation and is used to log its status. @@ -90,35 +92,22 @@ func OperationDone(w Waiter) bool { func CommonRefreshFunc(w Waiter) resource.StateRefreshFunc { return func() (interface{}, string, error) { - // First, read the operation from the server. op, err := w.QueryOp() - - // If we got a non-retryable error, return it. if err != nil { - if !isRetryableError(err) { - return nil, "", fmt.Errorf("Not retriable error: %s", err) - } - - log.Printf("[DEBUG] Saw error polling for op, but dismissed as retriable: %s", err) - if op == nil { - return nil, "", fmt.Errorf("Cannot continue, Operation is nil. %s", err) - } + // Importantly, this error is in the GET to the operation, and isn't an error + // with the resource CRUD request itself. + return nil, "", fmt.Errorf("error while retrieving operation: %s", err) } - log.Printf("[DEBUG] working with op %#v", op) - - // Try to set the operation (so we can check it's Error/State), - // and fail if we can't. if err = w.SetOp(op); err != nil { - return nil, "", fmt.Errorf("Cannot continue %s", err) + return nil, "", fmt.Errorf("Cannot continue, unable to use operation: %s", err) } - // Fail if the operation object contains an error. if err = w.Error(); err != nil { return nil, "", err } - log.Printf("[DEBUG] Got %v while polling for operation %s's status", w.State(), w.OpName()) + log.Printf("[DEBUG] Got %v while polling for operation %s's status", w.State(), w.OpName()) return op, w.State(), nil } } diff --git a/google/sqladmin_operation.go b/google/sqladmin_operation.go index 1e9990ef..2bb204fe 100644 --- a/google/sqladmin_operation.go +++ b/google/sqladmin_operation.go @@ -61,7 +61,18 @@ func (w *SqlAdminOperationWaiter) QueryOp() (interface{}, error) { return nil, fmt.Errorf("Cannot query operation, service is nil.") } - return w.Service.Operations.Get(w.Project, w.Op.Name).Do() + var op interface{} + var err error + err = retryTimeDuration( + func() error { + op, err = w.Service.Operations.Get(w.Project, w.Op.Name).Do() + return err + }, + + DefaultRequestTimeout, + ) + + return op, err } func (w *SqlAdminOperationWaiter) OpName() string { diff --git a/google/transport.go b/google/transport.go index 7b788c57..5cdd5973 100644 --- a/google/transport.go +++ b/google/transport.go @@ -14,6 +14,8 @@ import ( "google.golang.org/api/googleapi" ) +var DefaultRequestTimeout = 5 * time.Minute + func isEmptyValue(v reflect.Value) bool { switch v.Kind() { case reflect.Array, reflect.Map, reflect.Slice, reflect.String: @@ -33,7 +35,7 @@ func isEmptyValue(v reflect.Value) bool { } func sendRequest(config *Config, method, rawurl string, body map[string]interface{}) (map[string]interface{}, error) { - return sendRequestWithTimeout(config, method, rawurl, body, 5*time.Minute) + return sendRequestWithTimeout(config, method, rawurl, body, DefaultRequestTimeout) } func sendRequestWithTimeout(config *Config, method, rawurl string, body map[string]interface{}, timeout time.Duration) (map[string]interface{}, error) { diff --git a/google/utils.go b/google/utils.go index 49cb6caf..4731da7f 100644 --- a/google/utils.go +++ b/google/utils.go @@ -351,11 +351,13 @@ func retryTimeDuration(retryFunc func() error, duration time.Duration) error { func isRetryableError(err error) bool { if gerr, ok := err.(*googleapi.Error); ok && (gerr.Code == 429 || gerr.Code == 500 || gerr.Code == 502 || gerr.Code == 503) { + log.Printf("[DEBUG] Dismissed an error as retryable based on error code: %s", err) return true } // These operations are always hitting googleapis.com - they should rarely // time out, and if they do, that timeout is retryable. if urlerr, ok := err.(*url.Error); ok && urlerr.Timeout() { + log.Printf("[DEBUG] Dismissed an error as retryable based on googleapis.com target: %s", err) return true } if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 409 && strings.Contains(gerr.Body, "operationInProgress") { @@ -363,6 +365,7 @@ func isRetryableError(err error) bool { // The only way right now to determine it is a SQL 409 due to concurrent calls is to // look at the contents of the error message. // See https://github.com/terraform-providers/terraform-provider-google/issues/3279 + log.Printf("[DEBUG] Dismissed an error as retryable based on error code 409 and error reason 'operationInProgress': %s", err) return true }