fix: use hashicorp/go-retryablehttp for retries

yannh · Jun 30, 2024 · 2ecdfe9 · 2ecdfe9
1 parent 7b9163b
commit 2ecdfe9
Show file tree

Hide file tree

Showing 21 changed files with 2,026 additions and 23 deletions.
diff --git a/go.mod b/go.mod
@@ -6,3 +6,8 @@ require (
 	github.com/santhosh-tekuri/jsonschema/v5 v5.3.1
 	sigs.k8s.io/yaml v1.4.0
 )
+
+require (
+	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
+	github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
+)
diff --git a/go.sum b/go.sum
@@ -1,5 +1,9 @@
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
+github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
+github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISHxT2Q8+VepXU=
+github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk=
 github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 h1:lZUw3E0/J3roVtGQ+SCrUrg3ON6NgVqpn3+iol9aGu4=
 github.com/santhosh-tekuri/jsonschema/v5 v5.3.1/go.mod h1:uToXkOrWAZ6/Oc07xWQrPOhJotwFIyu2bBVN41fcDUY=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=

diff --git a/pkg/registry/http.go b/pkg/registry/http.go
@@ -6,11 +6,11 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"net"
 	"net/http"
 	"os"
 	"time"
 
+	retryablehttp "github.com/hashicorp/go-retryablehttp"
 	"github.com/yannh/kubeconform/pkg/cache"
 )
 
@@ -52,8 +52,13 @@ func newHTTPRegistry(schemaPathTemplate string, cacheFolder string, strict bool,
 		filecache = cache.NewOnDiskCache(cacheFolder)
 	}
 
+	// retriable http client
+	retryClient := retryablehttp.NewClient()
+	retryClient.RetryMax = 4
+	retryClient.HTTPClient = &http.Client{Transport: reghttp}
+
 	return &SchemaRegistry{
-		c:                  &http.Client{Transport: reghttp},
+		c:                  retryClient.StandardClient(),
 		schemaPathTemplate: schemaPathTemplate,
 		cache:              filecache,
 		strict:             strict,
@@ -75,24 +80,6 @@ func (r SchemaRegistry) DownloadSchema(resourceKind, resourceAPIVersion, k8sVers
 	}
 
 	resp, err := r.c.Get(url)
-	// retry on transient errors, ie. connection reset by peer
-	if err != nil {
-		if opErr, ok := err.(*net.OpError); ok {
-			if r.debug {
-				log.Printf("failed downloading schema at %s due to network error, retrying: %s", url, opErr)
-			}
-			time.Sleep(1 * time.Second)
-			resp, err = r.c.Get(url)
-		}
-	}
-	// retry on server errors
-	if resp != nil && resp.StatusCode >= 500 {
-		if r.debug {
-			log.Printf("failed downloading schema at %s due to server error, retrying: %d", url, resp.StatusCode)
-		}
-		time.Sleep(1 * time.Second)
-		resp, err = r.c.Get(url)
-	}
 	if err != nil {
 		msg := fmt.Sprintf("failed downloading schema at %s: %s", url, err)
 		if r.debug {

diff --git a/pkg/registry/http_test.go b/pkg/registry/http_test.go
@@ -137,11 +137,14 @@ func TestDownloadSchema(t *testing.T) {
 
 		_, res, err := reg.DownloadSchema(testCase.resourceKind, testCase.resourceAPIVersion, testCase.k8sversion)
 		if err == nil || testCase.expectErr == nil {
-			if err != testCase.expectErr {
-				t.Errorf("during test '%s': expected error, got:\n%s\n%s\n", testCase.name, testCase.expectErr, err)
+			if err == nil && testCase.expectErr != nil {
+				t.Errorf("during test '%s': expected error\n%s, got nil", testCase.name, testCase.expectErr)
+			}
+			if err != nil && testCase.expectErr == nil {
+				t.Errorf("during test '%s': expected no error, got\n%s\n", testCase.name, err)
 			}
 		} else if err.Error() != testCase.expectErr.Error() {
-			t.Errorf("during test '%s': expected error, got:\n%s\n%s\n", testCase.name, testCase.expectErr, err)
+			t.Errorf("during test '%s': expected error\n%s, got:\n%s\n", testCase.name, testCase.expectErr, err)
 		}
 
 		if !bytes.Equal(res, testCase.expect) {