add replace condition helper

replaceCondition() replaces target condition with a replacement
condition, retaining the transition time. This helps ensure that
the last transition time of releases don't change when a release is
marked from remediated to released.

Signed-off-by: Sunny <darkowlzz@protonmail.com>

Author: darkowlzz

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"
@@ -322,10 +323,9 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state
 		// 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)
+			replaceCondition(req.Object, v2.RemediatedCondition, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg, metav1.ConditionTrue)
 		}
 
 		return nil, nil
@@ -401,10 +401,9 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state
 		// 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)
+			replaceCondition(req.Object, v2.RemediatedCondition, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg, metav1.ConditionTrue)
 		}
 
 		return NewTest(r.configFactory, r.eventRecorder), nil
@@ -520,3 +519,19 @@ 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 {
+		conditions.Delete(obj, replacement)
+		c.Status = status
+		c.Type = replacement
+		c.Reason = reason
+		c.Message = msg
+		conditions.Set(obj, c)
+		conditions.Delete(obj, target)
+	}
+}

internal/reconcile/atomic_release_test.go

@@ -1557,3 +1557,117 @@ func TestAtomicRelease_actionForState(t *testing.T) {
 		})
 	}
 }
+
+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))
+		})
+	}
+}