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 <darkowlzz@protonmail.com>

Author: darkowlzz

internal/reconcile/atomic_release.go

@@ -315,6 +315,19 @@ 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) {
+			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 +391,22 @@ 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) {
+			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))

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))
 		})
 	}
 }