Skip to content

Commit 424e392

Browse files
fix: set Progressing condition to False on successful reconciliation
The Progressing condition on ClusterExtension resources was always set to status: True with reason: Succeeded after a successful reconciliation. This is semantically incorrect — Progressing=True means "still working," which contradicts the Succeeded reason and the "Desired state reached" message. This fix changes the default Progressing status to ConditionFalse (done progressing) and only sets ConditionTrue when there is an active non-terminal error causing retries. Terminal errors correctly remain ConditionFalse with reason Blocked. Before: Installed=True Progressing=True reason=Succeeded "Desired state reached" After: Installed=True Progressing=False reason=Succeeded "Desired state reached" Signed-off-by: Venkatesh Durgam <vdurgam@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent be16a0a commit 424e392

3 files changed

Lines changed: 9 additions & 8 deletions

File tree

internal/operator-controller/controllers/clusterextension_controller_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,7 +1352,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
13521352
t.Log("By checking the expected progressing conditions")
13531353
progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing)
13541354
require.NotNil(t, progressingCond)
1355-
require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
1355+
require.Equal(t, metav1.ConditionFalse, progressingCond.Status)
13561356
require.Equal(t, ocv1.ReasonSucceeded, progressingCond.Reason)
13571357

13581358
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
@@ -2674,7 +2674,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) {
26742674

26752675
progCond := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
26762676
require.NotNil(t, progCond)
2677-
require.Equal(t, metav1.ConditionTrue, progCond.Status)
2677+
require.Equal(t, metav1.ConditionFalse, progCond.Status)
26782678
require.Equal(t, ocv1.ReasonSucceeded, progCond.Reason)
26792679

26802680
// Verify all conditions are present and valid after first reconcile
@@ -2695,7 +2695,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) {
26952695
// Progressing should be Succeeded (apply completed successfully)
26962696
progCond = apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
26972697
require.NotNil(t, progCond)
2698-
require.Equal(t, metav1.ConditionTrue, progCond.Status)
2698+
require.Equal(t, metav1.ConditionFalse, progCond.Status)
26992699
require.Equal(t, ocv1.ReasonSucceeded, progCond.Reason)
27002700

27012701
// Installed should be True (maintaining current version)
@@ -2828,7 +2828,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) {
28282828

28292829
progCond := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
28302830
require.NotNil(t, progCond)
2831-
require.Equal(t, metav1.ConditionTrue, progCond.Status)
2831+
require.Equal(t, metav1.ConditionFalse, progCond.Status)
28322832
require.Equal(t, ocv1.ReasonSucceeded, progCond.Reason)
28332833

28342834
// Note: When falling back without catalog access initially, deprecation conditions
@@ -2851,7 +2851,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) {
28512851

28522852
progCond = apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
28532853
require.NotNil(t, progCond)
2854-
require.Equal(t, metav1.ConditionTrue, progCond.Status)
2854+
require.Equal(t, metav1.ConditionFalse, progCond.Status)
28552855
require.Equal(t, ocv1.ReasonSucceeded, progCond.Reason)
28562856

28572857
// Verify all conditions remain valid after upgrade

internal/operator-controller/controllers/common_controller.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,13 +148,14 @@ func setInstallStatus(ext *ocv1.ClusterExtension, installStatus *ocv1.ClusterExt
148148
func setStatusProgressing(ext *ocv1.ClusterExtension, err error) {
149149
progressingCond := metav1.Condition{
150150
Type: ocv1.TypeProgressing,
151-
Status: metav1.ConditionTrue,
151+
Status: metav1.ConditionFalse,
152152
Reason: ocv1.ReasonSucceeded,
153153
Message: "Desired state reached",
154154
ObservedGeneration: ext.GetGeneration(),
155155
}
156156

157157
if err != nil {
158+
progressingCond.Status = metav1.ConditionTrue
158159
progressingCond.Reason = ocv1.ReasonRetrying
159160
// Unwrap TerminalError to avoid "terminal error:" prefix in message
160161
progressingCond.Message = errorutil.SanitizeNetworkError(errorutil.UnwrapTerminal(err))

internal/operator-controller/controllers/common_controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ func TestSetStatusProgressing(t *testing.T) {
2626
expected metav1.Condition
2727
}{
2828
{
29-
name: "non-nil ClusterExtension, nil error, Progressing condition has status True with reason Success",
29+
name: "non-nil ClusterExtension, nil error, Progressing condition has status False with reason Success",
3030
err: nil,
3131
clusterExtension: &ocv1.ClusterExtension{},
3232
expected: metav1.Condition{
3333
Type: ocv1.TypeProgressing,
34-
Status: metav1.ConditionTrue,
34+
Status: metav1.ConditionFalse,
3535
Reason: ocv1.ReasonSucceeded,
3636
Message: "Desired state reached",
3737
},

0 commit comments

Comments
 (0)