From 1e919d484d5b97eafb7fe22d5f639d0a9213a365 Mon Sep 17 00:00:00 2001 From: Sunny Date: Tue, 14 Mar 2023 14:12:08 +0000 Subject: [PATCH] runtime/reconcile: Fix Ready check in Finalizer Fix the Ready condition check in the ResultFinalizer. `IsReady()` takes into account the Reconciling and Stalling conditons too. Use `IsTrue()` to precisely check if Ready=True. This ensure that Ready=True is toggled when there's a reconciliation error in presence or other negative conditions. Signed-off-by: Sunny --- runtime/reconcile/result.go | 2 +- runtime/reconcile/result_test.go | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/runtime/reconcile/result.go b/runtime/reconcile/result.go index 82fc3518..49ff5d74 100644 --- a/runtime/reconcile/result.go +++ b/runtime/reconcile/result.go @@ -134,7 +134,7 @@ func (rs ResultFinalizer) Finalize(obj conditions.Setter, res ctrl.Result, recEr // Ready=False with the reconcile error. If Ready is already False with a // reason, preserve the value. if recErr != nil { - if conditions.IsUnknown(obj, meta.ReadyCondition) || conditions.IsReady(obj) { + if conditions.IsUnknown(obj, meta.ReadyCondition) || conditions.IsTrue(obj, meta.ReadyCondition) { conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, recErr.Error()) } } diff --git a/runtime/reconcile/result_test.go b/runtime/reconcile/result_test.go index b3697b05..66d9d9a4 100644 --- a/runtime/reconcile/result_test.go +++ b/runtime/reconcile/result_test.go @@ -170,6 +170,20 @@ func TestResultFinalizer(t *testing.T) { *conditions.TrueCondition(meta.ReconcilingCondition, "SomeReasonX", "some msg X"), }, }, + { + name: "result with error, ready and reconciling, change to not ready", + beforeFunc: func(obj conditions.Setter) { + conditions.MarkReconciling(obj, "SomeReasonX", "some msg X") + conditions.MarkTrue(obj, meta.ReadyCondition, "SomeReasonY", "some msg Y") + }, + result: resultFailed, + recErr: errors.New("foo failed"), + wantErr: true, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, meta.FailedReason, "foo failed"), + *conditions.TrueCondition(meta.ReconcilingCondition, "SomeReasonX", "some msg X"), + }, + }, { name: "stalled and reconciling, Ready=False, remove reconciling, retain ready", beforeFunc: func(obj conditions.Setter) {