Skip to content

Commit

Permalink
action: provide reason for failures count reset
Browse files Browse the repository at this point in the history
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
  • Loading branch information
hiddeco committed Nov 22, 2023
1 parent 3ce6e8d commit 4a8d2ff
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 24 deletions.
30 changes: 19 additions & 11 deletions internal/action/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
32 changes: 20 additions & 12 deletions internal/action/reset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand All @@ -44,7 +46,8 @@ func TestMustResetFailures(t *testing.T) {
LastAttemptedGeneration: 2,
},
},
want: true,
want: true,
wantReason: differentGenerationReason,
},
{
name: "on revision change",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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))
})
}
}
3 changes: 2 additions & 1 deletion internal/controller/helmrelease_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down

0 comments on commit 4a8d2ff

Please sign in to comment.