Improve retry logic (#143)

.github/workflows/e2e.yaml

@@ -56,7 +56,7 @@ jobs:
       id-token: write
     strategy:
       fail-fast: false
-      max-parallel: 5
+      max-parallel: 10
       matrix:
         test: [ "basic", "private_endpoint" ]
         tf_release: ${{ fromJSON(needs.find-tf-releases.outputs.releases) }}

go.mod

@@ -3,6 +3,7 @@ module github.com/ClickHouse/terraform-provider-clickhouse
 go 1.22.5
 
 require (
+	github.com/cenkalti/backoff/v4 v4.3.0
 	github.com/gojuno/minimock/v3 v3.3.14
 	github.com/google/go-cmp v0.6.0
 	github.com/hashicorp/terraform-plugin-docs v0.19.4

go.sum

@@ -24,6 +24,8 @@ github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwN
 github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
 github.com/bufbuild/protocompile v0.4.0 h1:LbFKd2XowZvQ/kajzguUp2DC9UEIQhIq77fZZlaQsNA=
 github.com/bufbuild/protocompile v0.4.0/go.mod h1:3v93+mbWn/v3xzN+31nwkJfrEpAUwp+BagBSZWx+TP8=
+github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
+github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/cloudflare/circl v1.3.7 h1:qlCDlTPz2n9fu58M0Nh1J/JzcFpfgkFHHX3O35r5vcU=
 github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBSc8r4zxgA=
 github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=

pkg/internal/api/client.go

@@ -8,6 +8,8 @@ import (
 	"net/http"
 	"strings"
 	"time"
+
+	"github.com/cenkalti/backoff/v4"
 )
 
 type ClientImpl struct {
@@ -98,37 +100,43 @@ func (c *ClientImpl) doRequest(req *http.Request) ([]byte, error) {
 	authHeader := fmt.Sprintf("Basic %s", base64Credentials)
 	req.Header.Set("Authorization", authHeader)
 
-	res, err := c.HttpClient.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
+	makeRequest := func(req *http.Request) func() ([]byte, error) {
+		return func() ([]byte, error) {
+			res, err := c.HttpClient.Do(req)
+			if err != nil {
+				return nil, err
+			}
+			defer res.Body.Close()
 
-	body, err := io.ReadAll(res.Body)
-	if err != nil {
-		return nil, err
-	}
+			body, err := io.ReadAll(res.Body)
+			if err != nil {
+				return nil, err
+			}
 
-	if res.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("status: %d, body: %s", res.StatusCode, body)
-	}
+			if res.StatusCode != http.StatusOK {
+				if RetriableError(res.StatusCode) {
+					return nil, fmt.Errorf("status: %d, body: %s", res.StatusCode, body)
+				} else {
+					return nil, backoff.Permanent(fmt.Errorf("status: %d, body: %s", res.StatusCode, body))
+				}
+			}
 
-	return body, err
-}
+			return body, nil
+		}
+	}
 
-func (c *ClientImpl) checkStatusCode(req *http.Request) (*int, error) {
-	credentials := fmt.Sprintf("%s:%s", c.TokenKey, c.TokenSecret)
-	base64Credentials := base64.StdEncoding.EncodeToString([]byte(credentials))
-	authHeader := fmt.Sprintf("Basic %s", base64Credentials)
-	req.Header.Set("Authorization", authHeader)
+	// Retry after 5 seconds, then double wait time until max 80 seconds are elapsed.
+	backoffSettings := backoff.NewExponentialBackOff(
+		backoff.WithInitialInterval(5*time.Second),
+		backoff.WithMaxElapsedTime(81*time.Second),
+		backoff.WithMultiplier(2),
+	)
 
-	res, err := c.HttpClient.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
+	body, err := backoff.RetryNotifyWithData[[]byte](makeRequest(req), backoffSettings, func(err error, next time.Duration) {
+		fmt.Printf("Request failed with error: %s. Retrying in %.0f seconds\n", err, next.Seconds())
+	})
 
-	return &res.StatusCode, err
+	return body, err
 }
 
 // GetService - Returns a specifc order
@@ -219,6 +227,33 @@ func (c *ClientImpl) CreateService(s Service) (*Service, string, error) {
 	return &serviceResponse.Result.Service, serviceResponse.Result.Password, nil
 }
 
+func (c *ClientImpl) WaitForServiceState(serviceId string, stateChecker func(string) bool, maxWaitSeconds int) error {
+	// Wait until service is in desired status
+	checkStatus := func() error {
+		service, err := c.GetService(serviceId)
+		if err != nil {
+			return err
+		}
+
+		if stateChecker(service.State) {
+			return nil
+		}
+
+		return fmt.Errorf("service %s is in state %s", serviceId, service.State)
+	}
+
+	if maxWaitSeconds < 5 {
+		maxWaitSeconds = 5
+	}
+
+	err := backoff.Retry(checkStatus, backoff.WithMaxRetries(backoff.NewConstantBackOff(5*time.Second), uint64(maxWaitSeconds/5)))
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (c *ClientImpl) UpdateService(serviceId string, s ServiceUpdate) (*Service, error) {
 	rb, err := json.Marshal(s)
 	if err != nil {
@@ -300,27 +335,13 @@ func (c *ClientImpl) UpdateServicePassword(serviceId string, u ServicePasswordUp
 	return &serviceResponse, nil
 }
 
-func (c *ClientImpl) GetServiceStatusCode(serviceId string) (*int, error) {
-	req, err := http.NewRequest("GET", c.getServicePath(serviceId, ""), nil)
-	if err != nil {
-		return nil, err
-	}
-
-	statusCode, err := c.checkStatusCode(req)
-	if err != nil {
-		return nil, err
-	}
-
-	return statusCode, nil
-}
-
 func (c *ClientImpl) DeleteService(serviceId string) (*Service, error) {
 	service, err := c.GetService(serviceId)
 	if err != nil {
 		return nil, err
 	}
 
-	if service.State != "stopped" && service.State != "stopping" {
+	if service.State != StatusStopped && service.State != StatusStopping {
 		rb, _ := json.Marshal(ServiceStateUpdate{
 			Command: "stop",
 		})
@@ -337,22 +358,9 @@ func (c *ClientImpl) DeleteService(serviceId string) (*Service, error) {
 		}
 	}
 
-	numErrors := 0
-	for {
-		service, err := c.GetService(serviceId)
-		if err != nil {
-			numErrors++
-			if numErrors > MaxRetry {
-				return nil, err
-			}
-			time.Sleep(5 * time.Second)
-			continue
-		}
-
-		if service.State == "stopped" {
-			break
-		}
-		time.Sleep(5 * time.Second)
+	err = c.WaitForServiceState(serviceId, func(state string) bool { return state == StatusStopped }, 300)
+	if err != nil {
+		return nil, err
 	}
 
 	req, err := http.NewRequest("DELETE", c.getServicePath(serviceId, ""), nil)
@@ -371,14 +379,23 @@ func (c *ClientImpl) DeleteService(serviceId string) (*Service, error) {
 		return nil, err
 	}
 
-	for {
-		statusCode, _ := c.GetServiceStatusCode(serviceId)
-
-		if *statusCode == 404 {
-			break
+	// Wait until service is deleted
+	checkDeleted := func() error {
+		_, err := c.GetService(serviceId)
+		if IsNotFound(err) {
+			// That is what we want
+			return nil
+		} else if err != nil {
+			return err
 		}
 
-		time.Sleep(5 * time.Second)
+		return fmt.Errorf("service %s is not deleted yet", serviceId)
+	}
+
+	// Wait for up to 5 minutes for the service to be deleted
+	err = backoff.Retry(checkDeleted, backoff.WithMaxRetries(backoff.NewConstantBackOff(5*time.Second), 60))
+	if err != nil {
+		return nil, fmt.Errorf("service %s was not deleted in the allocated time", serviceId)
 	}
 
 	return &serviceResponse.Result.Service, nil