From f6d19d80081645d6621b4c01a7ab3fc8b3217cd9 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Wed, 13 Dec 2023 10:34:37 +0100
Subject: [PATCH] loader: log HTTP errors to provide faster feedback

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 internal/controller/helmrelease_controller.go | 17 ++-----
 internal/loader/client.go                     | 44 +++++++++++++++++++
 2 files changed, 48 insertions(+), 13 deletions(-)
 create mode 100644 internal/loader/client.go

diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go
index 82bebd6a1..653295783 100644
--- a/internal/controller/helmrelease_controller.go
+++ b/internal/controller/helmrelease_controller.go
@@ -23,7 +23,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hashicorp/go-retryablehttp"
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -90,8 +89,8 @@ type HelmReleaseReconciler struct {
 	FieldManager          string
 	DefaultServiceAccount string
 
-	httpClient        *retryablehttp.Client
-	requeueDependency time.Duration
+	requeueDependency    time.Duration
+	artifactFetchRetries int
 }
 
 type HelmReleaseReconcilerOptions struct {
@@ -122,15 +121,7 @@ func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
 	}
 
 	r.requeueDependency = opts.DependencyRequeueInterval
-
-	// Configure the retryable http client used for fetching artifacts.
-	// By default, it retries 10 times within a 3.5 minutes window.
-	httpClient := retryablehttp.NewClient()
-	httpClient.RetryWaitMin = 5 * time.Second
-	httpClient.RetryWaitMax = 30 * time.Second
-	httpClient.RetryMax = opts.HTTPRetry
-	httpClient.Logger = nil
-	r.httpClient = httpClient
+	r.artifactFetchRetries = opts.HTTPRetry
 
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&v2.HelmRelease{}, builder.WithPredicates(
@@ -294,7 +285,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	}
 
 	// Load chart from artifact.
-	loadedChart, err := loader.SecureLoadChartFromURL(r.httpClient, hc.GetArtifact().URL, hc.GetArtifact().Digest)
+	loadedChart, err := loader.SecureLoadChartFromURL(loader.NewRetryableHTTPClient(ctx, r.artifactFetchRetries), hc.GetArtifact().URL, hc.GetArtifact().Digest)
 	if err != nil {
 		if errors.Is(err, loader.ErrFileNotFound) {
 			msg := fmt.Sprintf("Chart not ready: artifact not found. Retrying in %s", r.requeueDependency.String())
diff --git a/internal/loader/client.go b/internal/loader/client.go
new file mode 100644
index 000000000..a6efaf0d8
--- /dev/null
+++ b/internal/loader/client.go
@@ -0,0 +1,44 @@
+package loader
+
+import (
+	"context"
+	"time"
+
+	"github.com/go-logr/logr"
+	"github.com/hashicorp/go-retryablehttp"
+	ctrl "sigs.k8s.io/controller-runtime"
+)
+
+// NewRetryableHTTPClient returns a new retrying HTTP client for loading
+// artifacts. The client will retry up to the given number of times before
+// giving up. The context is used to log errors.
+func NewRetryableHTTPClient(ctx context.Context, retries int) *retryablehttp.Client {
+	httpClient := retryablehttp.NewClient()
+	httpClient.RetryWaitMin = 5 * time.Second
+	httpClient.RetryWaitMax = 30 * time.Second
+	httpClient.RetryMax = retries
+	httpClient.Logger = errorLogger{logger: ctrl.LoggerFrom(ctx)}
+	return httpClient
+}
+
+// errorLogger is a wrapper around logr.Logger that implements the
+// retryablehttp.LeveledLogger interface while only logging errors.
+type errorLogger struct {
+	logger logr.Logger
+}
+
+func (l *errorLogger) Error(msg string, keysAndValues ...interface{}) {
+	l.logger.Info(msg, keysAndValues...)
+}
+
+func (l *errorLogger) Info(msg string, keysAndValues ...interface{}) {
+	// Do nothing.
+}
+
+func (l *errorLogger) Debug(msg string, keysAndValues ...interface{}) {
+	// Do nothing.
+}
+
+func (l *errorLogger) Warn(msg string, keysAndValues ...interface{}) {
+	// Do nothing.
+}