diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go index 893dc317b..1c1e80058 100644 --- a/internal/reconcile/atomic_release.go +++ b/internal/reconcile/atomic_release.go @@ -33,6 +33,7 @@ import ( "github.com/fluxcd/pkg/runtime/logger" "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/ssa/jsondiff" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" @@ -315,6 +316,18 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state return NewUpgrade(r.configFactory, r.eventRecorder), nil } + // Since the release is in-sync, remove any remediated condition if + // present and replace it with upgrade succeeded condition. + // This can happen when the current release, which is the result of a + // rollback remediation, matches the new desired configuration due to + // having the same chart version and values. As a result, we are already + // in-sync without performing a release action. + if conditions.IsTrue(req.Object, v2.RemediatedCondition) { + cur := req.Object.Status.History.Latest() + msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + replaceCondition(req.Object, v2.RemediatedCondition, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg, metav1.ConditionTrue) + } + return nil, nil case ReleaseStatusLocked: log.Info(msgWithReason("release locked", state.Reason)) @@ -378,6 +391,21 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state return nil, nil case ReleaseStatusUntested: log.Info(msgWithReason("release has not been tested", state.Reason)) + + // Since an untested release indicates that the release is already + // in-sync, remove any remediated condition if present and replace it + // with upgrade succeeded condition. + // This can happen when an untested current release, which is the result + // of a rollback remediation, matches the new desired configuration due + // to having the same chart version and values, and has test enabled. As + // a result, we are already in-sync without performing a release action, + // the existing release needs to undergo testing. + if conditions.IsTrue(req.Object, v2.RemediatedCondition) { + cur := req.Object.Status.History.Latest() + msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + replaceCondition(req.Object, v2.RemediatedCondition, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg, metav1.ConditionTrue) + } + return NewTest(r.configFactory, r.eventRecorder), nil case ReleaseStatusFailed: log.Info(msgWithReason("release is in a failed state", state.Reason)) @@ -491,3 +519,24 @@ func timeoutForAction(action ActionReconciler, obj *v2.HelmRelease) time.Duratio return obj.GetTimeout().Duration } } + +// replaceCondition replaces existing target condition with replacement +// condition, if present, for the given values, retaining the +// LastTransitionTime. +func replaceCondition(obj *v2.HelmRelease, target string, replacement string, reason string, msg string, status metav1.ConditionStatus) { + c := conditions.Get(obj, target) + if c != nil { + // Remove any existing replacement condition to retain the + // LastTransitionTime set here. If the state of the new condition + // changes an existing condition, the LastTransitionTime is updated to + // the current time. + // Refer https://github.com/fluxcd/pkg/blob/runtime/v0.43.0/runtime/conditions/setter.go#L54-L55. + conditions.Delete(obj, replacement) + c.Status = status + c.Type = replacement + c.Reason = reason + c.Message = msg + conditions.Set(obj, c) + conditions.Delete(obj, target) + } +} diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go index 4e7fba159..0de1bdabb 100644 --- a/internal/reconcile/atomic_release_test.go +++ b/internal/reconcile/atomic_release_test.go @@ -1015,15 +1015,16 @@ func TestAtomicRelease_Reconcile_Scenarios(t *testing.T) { func TestAtomicRelease_actionForState(t *testing.T) { tests := []struct { - name string - releases []*helmrelease.Release - annotations map[string]string - spec func(spec *v2.HelmReleaseSpec) - status func(releases []*helmrelease.Release) v2.HelmReleaseStatus - state ReleaseState - want ActionReconciler - wantEvent *corev1.Event - wantErr error + name string + releases []*helmrelease.Release + annotations map[string]string + spec func(spec *v2.HelmReleaseSpec) + status func(releases []*helmrelease.Release) v2.HelmReleaseStatus + state ReleaseState + want ActionReconciler + wantEvent *corev1.Event + wantErr error + assertConditions []metav1.Condition }{ { name: "in-sync release does not trigger any action", @@ -1053,6 +1054,25 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, want: &Upgrade{}, }, + { + name: "in-sync release with stale remediated condition", + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + {Version: 1}, + }, + Conditions: []metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, "upgrade failed"), + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "rolled back"), + }, + } + }, + state: ReleaseState{Status: ReleaseStatusInSync}, + want: nil, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "upgrade succeeded"), + }, + }, { name: "locked release triggers unlock action", state: ReleaseState{Status: ReleaseStatusLocked}, @@ -1245,6 +1265,25 @@ func TestAtomicRelease_actionForState(t *testing.T) { state: ReleaseState{Status: ReleaseStatusUntested}, want: &Test{}, }, + { + name: "untested release with stale remediated condition", + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + {Version: 1}, + }, + Conditions: []metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, "upgrade failed"), + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "rolled back"), + }, + } + }, + state: ReleaseState{Status: ReleaseStatusUntested}, + want: &Test{}, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "upgrade succeeded"), + }, + }, { name: "failed release without active remediation triggers upgrade", state: ReleaseState{Status: ReleaseStatusFailed}, @@ -1513,6 +1552,122 @@ func TestAtomicRelease_actionForState(t *testing.T) { } else { g.Expect(recorder.GetEvents()).To(BeEmpty()) } + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions)) + }) + } +} + +func Test_replaceCondition(t *testing.T) { + g := NewWithT(t) + timestamp, err := time.Parse(time.UnixDate, "Wed Feb 25 11:06:39 GMT 2015") + g.Expect(err).ToNot(HaveOccurred()) + + tests := []struct { + name string + conditions []metav1.Condition + target string + replacement string + wantConditions []metav1.Condition + }{ + { + name: "both conditions exist", + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UpgradeFailedReason, + Message: "upgrade failed", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "rollback", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + }, + target: v2.RemediatedCondition, + replacement: v2.ReleasedCondition, + wantConditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "foo", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + }, + }, + { + name: "no existing replacement condition", + conditions: []metav1.Condition{ + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "rollback", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + }, + target: v2.RemediatedCondition, + replacement: v2.ReleasedCondition, + wantConditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "foo", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + }, + }, + { + name: "no existing target condition", + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UpgradeFailedReason, + Message: "upgrade failed", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + }, + target: v2.RemediatedCondition, + replacement: v2.ReleasedCondition, + wantConditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UpgradeFailedReason, + Message: "upgrade failed", + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(timestamp), + }, + }, + }, + { + name: "no existing target and replacement conditions", + target: v2.RemediatedCondition, + replacement: v2.ReleasedCondition, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{} + obj.Generation = 1 + obj.Status.Conditions = tt.conditions + replaceCondition(obj, tt.target, tt.replacement, v2.UpgradeSucceededReason, "foo", metav1.ConditionTrue) + g.Expect(obj.Status.Conditions).To(Equal(tt.wantConditions)) }) } }