Skip to content

Commit 2e8c1d2

Browse files
camilamacedo86camilamacedo86
committed
Use Installing/Upgrading reasons for active operations
Show Installing when first install is rolling out and Upgrading when an existing bundle is being updated. This gives users clearer status about what's happening instead of the vague Absent reason. Assisted-by: Cursor
1 parent 2bdb702 commit 2e8c1d2

4 files changed

Lines changed: 200 additions & 18 deletions

File tree

‎api/v1/common_types.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ const (
2121
TypeProgressing = "Progressing"
2222

2323
// Installed reasons
24-
ReasonAbsent = "Absent"
24+
ReasonAbsent = "Absent"
25+
ReasonInstalling = "Installing"
26+
ReasonUpgrading = "Upgrading"
2527

2628
// Progressing reasons
2729
ReasonRollingOut = "RollingOut"

‎internal/operator-controller/conditionsets/conditionsets.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ var ConditionReasons = []string{
4343
ocv1.ReasonInvalidConfiguration,
4444
ocv1.ReasonRetrying,
4545
ocv1.ReasonAbsent,
46+
ocv1.ReasonInstalling,
47+
ocv1.ReasonUpgrading,
4648
ocv1.ReasonRollingOut,
4749
ocv1.ReasonProgressDeadlineExceeded,
4850
}

‎internal/operator-controller/controllers/common_controller.go‎

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,38 +57,56 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
5757
// Nothing is installed
5858
if revisionStates.Installed == nil {
5959
setInstallStatus(ext, nil)
60-
reason := determineFailureReason(revisionStates.RollingOut)
60+
reason := determineInstalledReason(revisionStates.RollingOut)
6161
setInstalledStatusConditionFalse(ext, reason, "No bundle installed")
6262
return
6363
}
64-
// Something is installed
64+
65+
// Something is installed - check if upgrade is in progress
6566
installStatus := &ocv1.ClusterExtensionInstallStatus{
6667
Bundle: revisionStates.Installed.BundleMetadata,
6768
}
6869
setInstallStatus(ext, installStatus)
70+
71+
if len(revisionStates.RollingOut) > 0 {
72+
latestRevision := revisionStates.RollingOut[len(revisionStates.RollingOut)-1]
73+
progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
74+
75+
if progressingCond != nil && progressingCond.Reason == string(ocv1.ReasonRollingOut) {
76+
setInstalledStatusConditionUpgrading(ext, fmt.Sprintf("Upgrading from %s", revisionStates.Installed.Image))
77+
return
78+
}
79+
}
80+
6981
setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image))
7082
}
7183

72-
// determineFailureReason determines the appropriate reason for the Installed condition
84+
// determineInstalledReason determines the appropriate reason for the Installed condition
7385
// when no bundle is installed (Installed: False).
7486
//
7587
// Returns Failed when:
7688
// - No rolling revisions exist (nothing to install)
7789
// - The latest rolling revision has Reason: Retrying (indicates an error occurred)
7890
//
91+
// Returns Installing when:
92+
// - The latest rolling revision explicitly has Reason: RollingOut (healthy installation in progress)
93+
//
7994
// Returns Absent when:
80-
// - Rolling revisions exist with the latest having Reason: RollingOut (healthy phased rollout in progress)
95+
// - Rolling revisions exist but have no conditions set (rollout just started)
8196
//
8297
// Rationale:
8398
// - Failed: Semantically indicates an error prevented installation
84-
// - Absent: Semantically indicates "not there yet" (neutral state, e.g., during healthy rollout)
99+
// - Installing: Semantically indicates a first-time installation is actively in progress
100+
// - Absent: Neutral state when rollout exists but hasn't progressed enough to determine health
85101
// - Retrying reason indicates an error (config validation, apply failure, etc.)
86-
// - RollingOut reason indicates healthy progress (not an error)
102+
// - RollingOut reason indicates confirmed healthy progress
87103
// - Only the LATEST revision matters - old errors superseded by newer healthy revisions should not cause Failed
88104
//
105+
// Note: This function is only called when Installed == nil (first-time installation scenario).
89106
// Note: rollingRevisions are sorted in ascending order by Spec.Revision (oldest to newest),
90-
// so the latest revision is the LAST element in the array.
91-
func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
107+
//
108+
// so the latest revision is the LAST element in the array.
109+
func determineInstalledReason(rollingRevisions []*RevisionMetadata) string {
92110
if len(rollingRevisions) == 0 {
93111
return ocv1.ReasonFailed
94112
}
@@ -97,14 +115,21 @@ func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
97115
// Latest revision is the last element in the array (sorted ascending by Spec.Revision)
98116
latestRevision := rollingRevisions[len(rollingRevisions)-1]
99117
progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
100-
if progressingCond != nil && progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) {
101-
// Retrying indicates an error occurred (config, apply, validation, etc.)
102-
// Use Failed for semantic correctness: installation failed due to error
103-
return ocv1.ReasonFailed
118+
if progressingCond != nil {
119+
if progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) {
120+
// Retrying indicates an error occurred (config, apply, validation, etc.)
121+
// Use Failed for semantic correctness: installation failed due to error
122+
return ocv1.ReasonFailed
123+
}
124+
if progressingCond.Reason == string(ocv1.ReasonRollingOut) {
125+
// RollingOut indicates healthy progress is confirmed
126+
// Use Installing to communicate that a first-time installation is actively in progress
127+
return ocv1.ReasonInstalling
128+
}
104129
}
105130

106-
// No error detected in latest revision - it's progressing healthily (RollingOut) or no conditions set
107-
// Use Absent for neutral "not installed yet" state
131+
// No progressing condition or unknown reason - rollout just started or hasn't progressed
132+
// Use Absent as neutral state
108133
return ocv1.ReasonAbsent
109134
}
110135

@@ -119,6 +144,17 @@ func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message stri
119144
})
120145
}
121146

147+
// setInstalledStatusConditionUpgrading sets the installed status condition to upgrading.
148+
func setInstalledStatusConditionUpgrading(ext *ocv1.ClusterExtension, message string) {
149+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
150+
Type: ocv1.TypeInstalled,
151+
Status: metav1.ConditionTrue,
152+
Reason: ocv1.ReasonUpgrading,
153+
Message: message,
154+
ObservedGeneration: ext.GetGeneration(),
155+
})
156+
}
157+
122158
// setInstalledStatusConditionFailed sets the installed status condition to failed.
123159
func setInstalledStatusConditionFalse(ext *ocv1.ClusterExtension, reason string, message string) {
124160
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{

‎internal/operator-controller/controllers/common_controller_test.go‎

Lines changed: 145 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,107 @@ func TestSetStatusConditionWrapper(t *testing.T) {
253253
}
254254
}
255255

256+
func TestSetInstalledStatusFromRevisionStates_Upgrade(t *testing.T) {
257+
tests := []struct {
258+
name string
259+
revisionStates *RevisionStates
260+
expectedInstalledCond metav1.Condition
261+
}{
262+
{
263+
name: "installed bundle with healthy upgrade in progress - uses Upgrading",
264+
revisionStates: &RevisionStates{
265+
Installed: &RevisionMetadata{
266+
RevisionName: "rev-1",
267+
Image: "quay.io/example/bundle:v1.0.0",
268+
BundleMetadata: ocv1.BundleMetadata{
269+
Name: "example.v1.0.0",
270+
Version: "1.0.0",
271+
},
272+
},
273+
RollingOut: []*RevisionMetadata{
274+
{
275+
RevisionName: "rev-2",
276+
Conditions: []metav1.Condition{
277+
{
278+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
279+
Status: metav1.ConditionTrue,
280+
Reason: ocv1.ReasonRollingOut,
281+
Message: "Upgrading to v2.0.0",
282+
},
283+
},
284+
},
285+
},
286+
},
287+
expectedInstalledCond: metav1.Condition{
288+
Type: ocv1.TypeInstalled,
289+
Status: metav1.ConditionTrue,
290+
Reason: ocv1.ReasonUpgrading,
291+
},
292+
},
293+
{
294+
name: "installed bundle with no rolling revisions - uses Succeeded",
295+
revisionStates: &RevisionStates{
296+
Installed: &RevisionMetadata{
297+
RevisionName: "rev-1",
298+
Image: "quay.io/example/bundle:v1.0.0",
299+
BundleMetadata: ocv1.BundleMetadata{
300+
Name: "example.v1.0.0",
301+
Version: "1.0.0",
302+
},
303+
},
304+
RollingOut: nil,
305+
},
306+
expectedInstalledCond: metav1.Condition{
307+
Type: ocv1.TypeInstalled,
308+
Status: metav1.ConditionTrue,
309+
Reason: ocv1.ReasonSucceeded,
310+
},
311+
},
312+
{
313+
name: "installed bundle with rolling revision not yet healthy - uses Succeeded",
314+
revisionStates: &RevisionStates{
315+
Installed: &RevisionMetadata{
316+
RevisionName: "rev-1",
317+
Image: "quay.io/example/bundle:v1.0.0",
318+
BundleMetadata: ocv1.BundleMetadata{
319+
Name: "example.v1.0.0",
320+
Version: "1.0.0",
321+
},
322+
},
323+
RollingOut: []*RevisionMetadata{
324+
{
325+
RevisionName: "rev-2",
326+
Conditions: []metav1.Condition{},
327+
},
328+
},
329+
},
330+
expectedInstalledCond: metav1.Condition{
331+
Type: ocv1.TypeInstalled,
332+
Status: metav1.ConditionTrue,
333+
Reason: ocv1.ReasonSucceeded,
334+
},
335+
},
336+
}
337+
338+
for _, tt := range tests {
339+
t.Run(tt.name, func(t *testing.T) {
340+
ext := &ocv1.ClusterExtension{
341+
ObjectMeta: metav1.ObjectMeta{
342+
Name: "test-ext",
343+
Generation: 1,
344+
},
345+
}
346+
347+
setInstalledStatusFromRevisionStates(ext, tt.revisionStates)
348+
349+
cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled)
350+
require.NotNil(t, cond)
351+
require.Equal(t, tt.expectedInstalledCond.Status, cond.Status)
352+
require.Equal(t, tt.expectedInstalledCond.Reason, cond.Reason)
353+
})
354+
}
355+
}
356+
256357
func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T) {
257358
tests := []struct {
258359
name string
@@ -331,7 +432,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T
331432
},
332433
},
333434
{
334-
name: "rolling revision with RollingOut reason - uses Absent",
435+
name: "rolling revision with RollingOut reason - uses Installing",
335436
revisionStates: &RevisionStates{
336437
Installed: nil,
337438
RollingOut: []*RevisionMetadata{
@@ -351,11 +452,11 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T
351452
expectedInstalledCond: metav1.Condition{
352453
Type: ocv1.TypeInstalled,
353454
Status: metav1.ConditionFalse,
354-
Reason: ocv1.ReasonAbsent,
455+
Reason: ocv1.ReasonInstalling,
355456
},
356457
},
357458
{
358-
name: "old revision with Retrying superseded by latest healthy - uses Absent",
459+
name: "old revision with Retrying superseded by latest healthy - uses Installing",
359460
revisionStates: &RevisionStates{
360461
Installed: nil,
361462
RollingOut: []*RevisionMetadata{
@@ -383,6 +484,47 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T
383484
},
384485
},
385486
},
487+
expectedInstalledCond: metav1.Condition{
488+
Type: ocv1.TypeInstalled,
489+
Status: metav1.ConditionFalse,
490+
Reason: ocv1.ReasonInstalling,
491+
},
492+
},
493+
{
494+
name: "rolling revision with no conditions set - uses Absent",
495+
revisionStates: &RevisionStates{
496+
Installed: nil,
497+
RollingOut: []*RevisionMetadata{
498+
{
499+
RevisionName: "rev-1",
500+
Conditions: []metav1.Condition{},
501+
},
502+
},
503+
},
504+
expectedInstalledCond: metav1.Condition{
505+
Type: ocv1.TypeInstalled,
506+
Status: metav1.ConditionFalse,
507+
Reason: ocv1.ReasonAbsent,
508+
},
509+
},
510+
{
511+
name: "rolling revision with unknown reason - uses Absent",
512+
revisionStates: &RevisionStates{
513+
Installed: nil,
514+
RollingOut: []*RevisionMetadata{
515+
{
516+
RevisionName: "rev-1",
517+
Conditions: []metav1.Condition{
518+
{
519+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
520+
Status: metav1.ConditionTrue,
521+
Reason: "SomeOtherReason",
522+
Message: "Revision is in an unknown state",
523+
},
524+
},
525+
},
526+
},
527+
},
386528
expectedInstalledCond: metav1.Condition{
387529
Type: ocv1.TypeInstalled,
388530
Status: metav1.ConditionFalse,

0 commit comments

Comments
 (0)