From d87018a32641592dd8f014ea01ef649ce6814c1d Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 8 Dec 2023 10:45:12 +0000 Subject: [PATCH] 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 --- internal/reconcile/atomic_release.go | 23 ++++- internal/reconcile/atomic_release_test.go | 114 ++++++++++++++++++++++ 2 files changed, 133 insertions(+), 4 deletions(-) diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go index 30e26a2cb..27a4f190c 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" @@ -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) + } +} diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go index fab5df98d..0de1bdabb 100644 --- a/internal/reconcile/atomic_release_test.go +++ b/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)) + }) + } +}