action: provide reason for failures count reset

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

Author: hiddeco

internal/action/reset.go

@@ -25,25 +25,33 @@ import (
 	intchartutil "github.com/fluxcd/helm-controller/internal/chartutil"
 )
 
-// MustResetFailures returns true if the HelmRelease's status indicates that
-// the HelmRelease failure counters must be reset. This is the case if the
-// data used to make the last (failed) attempt has changed in a way that
-// indicates that a new attempt should be made. For example, a change in
-// generation, chart version, or values.
-func MustResetFailures(obj *v2.HelmRelease, chart *chart.Metadata, values chartutil.Values) bool {
+const (
+	differentGenerationReason = "generation differs from last attempt"
+	differentRevisionReason   = "chart version differs from last attempt"
+	differentValuesReason     = "values differ from last attempt"
+)
+
+// MustResetFailures returns a reason and true if the HelmRelease's status
+// indicates that the HelmRelease failure counters must be reset.
+// This is the case if the data used to make the last (failed) attempt has
+// changed in a way that indicates that a new attempt should be made.
+// For example, a change in generation, chart version, or values.
+// If no change is detected, an empty string is returned along with false.
+func MustResetFailures(obj *v2.HelmRelease, chart *chart.Metadata, values chartutil.Values) (string, bool) {
 	switch {
 	case obj.Status.LastAttemptedGeneration != obj.Generation:
-		return true
+		return differentGenerationReason, true
 	case obj.Status.LastAttemptedRevision != chart.Version:
-		return true
+		return differentRevisionReason, true
 	case obj.Status.LastAttemptedConfigDigest != "" || obj.Status.LastAttemptedValuesChecksum != "":
 		d := obj.Status.LastAttemptedConfigDigest
 		if d == "" {
 			// TODO: remove this when the deprecated field is removed.
 			d = "sha1:" + obj.Status.LastAttemptedValuesChecksum
 		}
-		return !intchartutil.VerifyValues(digest.Digest(d), values)
-	default:
-		return false
+		if ok := intchartutil.VerifyValues(digest.Digest(d), values); !ok {
+			return differentValuesReason, true
+		}
 	}
+	return "", false
 }

internal/action/reset_test.go

@@ -19,6 +19,7 @@ package action
 import (
 	"testing"
 
+	. "github.com/onsi/gomega"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -28,11 +29,12 @@ import (
 
 func TestMustResetFailures(t *testing.T) {
 	tests := []struct {
-		name   string
-		obj    *v2.HelmRelease
-		chart  *chart.Metadata
-		values chartutil.Values
-		want   bool
+		name       string
+		obj        *v2.HelmRelease
+		chart      *chart.Metadata
+		values     chartutil.Values
+		want       bool
+		wantReason string
 	}{
 		{
 			name: "on generation change",
@@ -44,7 +46,8 @@ func TestMustResetFailures(t *testing.T) {
 					LastAttemptedGeneration: 2,
 				},
 			},
-			want: true,
+			want:       true,
+			wantReason: differentGenerationReason,
 		},
 		{
 			name: "on revision change",
@@ -60,7 +63,8 @@ func TestMustResetFailures(t *testing.T) {
 			chart: &chart.Metadata{
 				Version: "1.1.0",
 			},
-			want: true,
+			want:       true,
+			wantReason: differentRevisionReason,
 		},
 		{
 			name: "on config digest change",
@@ -80,7 +84,8 @@ func TestMustResetFailures(t *testing.T) {
 			values: chartutil.Values{
 				"foo": "bar",
 			},
-			want: true,
+			want:       true,
+			wantReason: differentValuesReason,
 		},
 		{
 			name: "on (deprecated) values checksum change",
@@ -100,7 +105,8 @@ func TestMustResetFailures(t *testing.T) {
 			values: chartutil.Values{
 				"foo": "bar",
 			},
-			want: true,
+			want:       true,
+			wantReason: differentValuesReason,
 		},
 		{
 			name: "without change no reset",
@@ -125,9 +131,11 @@ func TestMustResetFailures(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := MustResetFailures(tt.obj, tt.chart, tt.values); got != tt.want {
-				t.Errorf("MustResetFailures() = %v, want %v", got, tt.want)
-			}
+			g := NewWithT(t)
+
+			reason, got := MustResetFailures(tt.obj, tt.chart, tt.values)
+			g.Expect(got).To(Equal(tt.want))
+			g.Expect(reason).To(Equal(tt.wantReason))
 		})
 	}
 }

internal/controller/helmrelease_controller.go

@@ -321,7 +321,8 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	obj.Status.StorageNamespace = obj.GetStorageNamespace()
 
 	// Reset the failure count if the chart or values have changed.
-	if action.MustResetFailures(obj, loadedChart.Metadata, values) {
+	if reason, ok := action.MustResetFailures(obj, loadedChart.Metadata, values); ok {
+		log.V(logger.DebugLevel).Info(fmt.Sprintf("resetting failure count (%s)", reason))
 		obj.Status.ClearFailures()
 	}