From 7baedcb6e73716e09c4ecdbe9ea2afce26909870 Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 7 Dec 2023 19:49:32 +0000 Subject: [PATCH] Remove stale remediated condition when in-sync Remediation can roll back to a version that matches with the next good config. In such situation, release will be in-sync and no action will be performed. The status conditions will continue to show Remediated=True and Released=False. Check and remove stale Remediated condition and add a Released=True condition with message constructed from the latest release. Signed-off-by: Sunny --- internal/reconcile/atomic_release.go | 20 ++++++++ internal/reconcile/atomic_release_test.go | 59 +++++++++++++++++++---- 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go index 893dc317b..4c94d7cd9 100644 --- a/internal/reconcile/atomic_release.go +++ b/internal/reconcile/atomic_release.go @@ -315,6 +315,15 @@ 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. + if conditions.IsTrue(req.Object, v2.RemediatedCondition) { + conditions.Delete(req.Object, v2.RemediatedCondition) + cur := req.Object.Status.History.Latest() + msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg) + } + return nil, nil case ReleaseStatusLocked: log.Info(msgWithReason("release locked", state.Reason)) @@ -378,6 +387,17 @@ 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. + if conditions.IsTrue(req.Object, v2.RemediatedCondition) { + conditions.Delete(req.Object, v2.RemediatedCondition) + cur := req.Object.Status.History.Latest() + msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg) + } + return NewTest(r.configFactory, r.eventRecorder), nil case ReleaseStatusFailed: log.Info(msgWithReason("release is in a failed state", state.Reason)) diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go index 4e7fba159..fab5df98d 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,8 @@ func TestAtomicRelease_actionForState(t *testing.T) { } else { g.Expect(recorder.GetEvents()).To(BeEmpty()) } + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions)) }) } }