From 5b6b1663ef3c19ff9b03058347af672da2502e12 Mon Sep 17 00:00:00 2001 From: Ivan Kalita Date: Wed, 5 Jun 2019 23:12:07 +0300 Subject: [PATCH] backend/http: implement retries for the http backend (#19702) Fixes #19619 --- backend/remote-state/http/backend.go | 30 ++++++++++++++-- backend/remote-state/http/backend_test.go | 13 +++++++ backend/remote-state/http/client.go | 5 +-- backend/remote-state/http/client_test.go | 43 ++++++++++++++++++----- command/apply_test.go | 3 ++ website/docs/backends/types/http.html.md | 5 +++ 6 files changed, 86 insertions(+), 13 deletions(-) diff --git a/backend/remote-state/http/backend.go b/backend/remote-state/http/backend.go index aaf2515fa8e3..ea5d8772b49a 100644 --- a/backend/remote-state/http/backend.go +++ b/backend/remote-state/http/backend.go @@ -6,8 +6,10 @@ import ( "fmt" "net/http" "net/url" + "time" - cleanhttp "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/go-retryablehttp" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/state" @@ -66,6 +68,24 @@ func New() backend.Backend { Default: false, Description: "Whether to skip TLS verification.", }, + "retry_max": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + Default: 2, + Description: "The number of HTTP request retries.", + }, + "retry_wait_min": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + Default: 1, + Description: "The minimum time in seconds to wait between HTTP request attempts.", + }, + "retry_wait_max": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + Default: 30, + Description: "The maximum time in seconds to wait between HTTP request attempts.", + }, }, } @@ -131,6 +151,12 @@ func (b *Backend) configure(ctx context.Context) error { } } + rClient := retryablehttp.NewClient() + rClient.HTTPClient = client + rClient.RetryMax = data.Get("retry_max").(int) + rClient.RetryWaitMin = time.Duration(data.Get("retry_wait_min").(int)) * time.Second + rClient.RetryWaitMax = time.Duration(data.Get("retry_wait_max").(int)) * time.Second + b.client = &httpClient{ URL: updateURL, UpdateMethod: updateMethod, @@ -144,7 +170,7 @@ func (b *Backend) configure(ctx context.Context) error { Password: data.Get("password").(string), // accessible only for testing use - Client: client, + Client: rClient, } return nil } diff --git a/backend/remote-state/http/backend_test.go b/backend/remote-state/http/backend_test.go index dad47c17ca6c..dc91129c7b1a 100644 --- a/backend/remote-state/http/backend_test.go +++ b/backend/remote-state/http/backend_test.go @@ -2,6 +2,7 @@ package http import ( "testing" + "time" "github.com/hashicorp/terraform/configs" "github.com/zclconf/go-cty/cty" @@ -51,6 +52,9 @@ func TestHTTPClientFactory(t *testing.T) { "unlock_method": cty.StringVal("BLOOP"), "username": cty.StringVal("user"), "password": cty.StringVal("pass"), + "retry_max": cty.StringVal("999"), + "retry_wait_min": cty.StringVal("15"), + "retry_wait_max": cty.StringVal("150"), } b = backend.TestBackendConfig(t, New(), configs.SynthBody("synth", conf)).(*Backend) @@ -74,4 +78,13 @@ func TestHTTPClientFactory(t *testing.T) { t.Fatalf("Unexpected username \"%s\" vs \"%s\" or password \"%s\" vs \"%s\"", client.Username, conf["username"], client.Password, conf["password"]) } + if client.Client.RetryMax != 999 { + t.Fatalf("Expected retry_max \"%d\", got \"%d\"", 999, client.Client.RetryMax) + } + if client.Client.RetryWaitMin != 15*time.Second { + t.Fatalf("Expected retry_wait_min \"%s\", got \"%s\"", 15*time.Second, client.Client.RetryWaitMin) + } + if client.Client.RetryWaitMax != 150*time.Second { + t.Fatalf("Expected retry_wait_max \"%s\", got \"%s\"", 150*time.Second, client.Client.RetryWaitMax) + } } diff --git a/backend/remote-state/http/client.go b/backend/remote-state/http/client.go index 62fb39fa2f23..152d266896fb 100644 --- a/backend/remote-state/http/client.go +++ b/backend/remote-state/http/client.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" + "github.com/hashicorp/go-retryablehttp" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/state/remote" ) @@ -28,7 +29,7 @@ type httpClient struct { UnlockMethod string // HTTP - Client *http.Client + Client *retryablehttp.Client Username string Password string @@ -44,7 +45,7 @@ func (c *httpClient) httpRequest(method string, url *url.URL, data *[]byte, what } // Create the request - req, err := http.NewRequest(method, url.String(), reader) + req, err := retryablehttp.NewRequest(method, url.String(), reader) if err != nil { return nil, fmt.Errorf("Failed to make %s HTTP request: %s", what, err) } diff --git a/backend/remote-state/http/client_test.go b/backend/remote-state/http/client_test.go index 2f7884a7b299..7b6f04e62342 100644 --- a/backend/remote-state/http/client_test.go +++ b/backend/remote-state/http/client_test.go @@ -10,7 +10,7 @@ import ( "reflect" "testing" - cleanhttp "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/go-retryablehttp" "github.com/hashicorp/terraform/state/remote" ) @@ -30,14 +30,14 @@ func TestHTTPClient(t *testing.T) { } // Test basic get/update - client := &httpClient{URL: url, Client: cleanhttp.DefaultClient()} + client := &httpClient{URL: url, Client: retryablehttp.NewClient()} remote.TestClient(t, client) // test just a single PUT p := &httpClient{ URL: url, UpdateMethod: "PUT", - Client: cleanhttp.DefaultClient(), + Client: retryablehttp.NewClient(), } remote.TestClient(t, p) @@ -49,7 +49,7 @@ func TestHTTPClient(t *testing.T) { LockMethod: "LOCK", UnlockURL: url, UnlockMethod: "UNLOCK", - Client: cleanhttp.DefaultClient(), + Client: retryablehttp.NewClient(), } b := &httpClient{ URL: url, @@ -58,7 +58,7 @@ func TestHTTPClient(t *testing.T) { LockMethod: "LOCK", UnlockURL: url, UnlockMethod: "UNLOCK", - Client: cleanhttp.DefaultClient(), + Client: retryablehttp.NewClient(), } remote.TestRemoteLocks(t, a, b) @@ -68,13 +68,23 @@ func TestHTTPClient(t *testing.T) { defer ts.Close() url, err = url.Parse(ts.URL) - c := &httpClient{ + client = &httpClient{ URL: url, UpdateMethod: "PUT", - Client: cleanhttp.DefaultClient(), + Client: retryablehttp.NewClient(), } - remote.TestClient(t, c) // first time through: 201 - remote.TestClient(t, c) // second time, with identical data: 204 + remote.TestClient(t, client) // first time through: 201 + remote.TestClient(t, client) // second time, with identical data: 204 + + // test a broken backend + brokenHandler := new(testBrokenHTTPHandler) + brokenHandler.handler = new(testHTTPHandler) + ts = httptest.NewServer(http.HandlerFunc(brokenHandler.Handle)) + defer ts.Close() + + url, err = url.Parse(ts.URL) + client = &httpClient{URL: url, Client: retryablehttp.NewClient()} + remote.TestClient(t, client) } func assertError(t *testing.T, err error, expected string) { @@ -149,3 +159,18 @@ func (h *testHTTPHandler) HandleWebDAV(w http.ResponseWriter, r *http.Request) { w.Write([]byte(fmt.Sprintf("Unknown method: %s", r.Method))) } } + +type testBrokenHTTPHandler struct { + lastRequestWasBroken bool + handler *testHTTPHandler +} + +func (h *testBrokenHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { + if h.lastRequestWasBroken { + h.lastRequestWasBroken = false + h.handler.Handle(w, r) + } else { + h.lastRequestWasBroken = true + w.WriteHeader(500) + } +} diff --git a/command/apply_test.go b/command/apply_test.go index c27d184d89e6..9f7a968f8195 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -677,6 +677,9 @@ func TestApply_plan_remoteState(t *testing.T) { "username": cty.NullVal(cty.String), "password": cty.NullVal(cty.String), "skip_cert_verification": cty.NullVal(cty.Bool), + "retry_max": cty.NullVal(cty.String), + "retry_wait_min": cty.NullVal(cty.String), + "retry_wait_max": cty.NullVal(cty.String), }) backendConfigRaw, err := plans.NewDynamicValue(backendConfig, backendConfig.Type()) if err != nil { diff --git a/website/docs/backends/types/http.html.md b/website/docs/backends/types/http.html.md index 19254ce6968b..ba0a723eb55e 100644 --- a/website/docs/backends/types/http.html.md +++ b/website/docs/backends/types/http.html.md @@ -60,3 +60,8 @@ The following configuration options are supported: * `password` - (Optional) The password for HTTP basic authentication * `skip_cert_verification` - (Optional) Whether to skip TLS verification. Defaults to `false`. + * `retry_max` – (Optional) The number of HTTP request retries. Defaults to `2`. + * `retry_wait_min` – (Optional) The minimum time in seconds to wait between HTTP request attempts. + Defaults to `1`. + * `retry_wait_max` – (Optional) The maximum time in seconds to wait between HTTP request attempts. + Defaults to `30`.