diff --git a/internal/action/reset.go b/internal/action/reset.go index 0c46d9f1e..da0caf69f 100644 --- a/internal/action/reset.go +++ b/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 } diff --git a/internal/action/reset_test.go b/internal/action/reset_test.go index 2c2ecd694..465df0e81 100644 --- a/internal/action/reset_test.go +++ b/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)) }) } } diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index ad9445459..7380913c7 100644 --- a/internal/controller/helmrelease_controller.go +++ b/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() }