From 2e8c1d2732a7d3b0c95c8f2d234db5f24ef6471f Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Mon, 2 Feb 2026 10:50:07 +0000 Subject: [PATCH 1/2] 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 --- api/v1/common_types.go | 4 +- .../conditionsets/conditionsets.go | 2 + .../controllers/common_controller.go | 64 ++++++-- .../controllers/common_controller_test.go | 148 +++++++++++++++++- 4 files changed, 200 insertions(+), 18 deletions(-) diff --git a/api/v1/common_types.go b/api/v1/common_types.go index 53215dbde..bd29b271f 100644 --- a/api/v1/common_types.go +++ b/api/v1/common_types.go @@ -21,7 +21,9 @@ const ( TypeProgressing = "Progressing" // Installed reasons - ReasonAbsent = "Absent" + ReasonAbsent = "Absent" + ReasonInstalling = "Installing" + ReasonUpgrading = "Upgrading" // Progressing reasons ReasonRollingOut = "RollingOut" diff --git a/internal/operator-controller/conditionsets/conditionsets.go b/internal/operator-controller/conditionsets/conditionsets.go index 97073a02d..812de85e5 100644 --- a/internal/operator-controller/conditionsets/conditionsets.go +++ b/internal/operator-controller/conditionsets/conditionsets.go @@ -43,6 +43,8 @@ var ConditionReasons = []string{ ocv1.ReasonInvalidConfiguration, ocv1.ReasonRetrying, ocv1.ReasonAbsent, + ocv1.ReasonInstalling, + ocv1.ReasonUpgrading, ocv1.ReasonRollingOut, ocv1.ReasonProgressDeadlineExceeded, } diff --git a/internal/operator-controller/controllers/common_controller.go b/internal/operator-controller/controllers/common_controller.go index cb6834c6b..1067a7728 100644 --- a/internal/operator-controller/controllers/common_controller.go +++ b/internal/operator-controller/controllers/common_controller.go @@ -57,38 +57,56 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt // Nothing is installed if revisionStates.Installed == nil { setInstallStatus(ext, nil) - reason := determineFailureReason(revisionStates.RollingOut) + reason := determineInstalledReason(revisionStates.RollingOut) setInstalledStatusConditionFalse(ext, reason, "No bundle installed") return } - // Something is installed + + // Something is installed - check if upgrade is in progress installStatus := &ocv1.ClusterExtensionInstallStatus{ Bundle: revisionStates.Installed.BundleMetadata, } setInstallStatus(ext, installStatus) + + if len(revisionStates.RollingOut) > 0 { + latestRevision := revisionStates.RollingOut[len(revisionStates.RollingOut)-1] + progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + + if progressingCond != nil && progressingCond.Reason == string(ocv1.ReasonRollingOut) { + setInstalledStatusConditionUpgrading(ext, fmt.Sprintf("Upgrading from %s", revisionStates.Installed.Image)) + return + } + } + setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image)) } -// determineFailureReason determines the appropriate reason for the Installed condition +// determineInstalledReason determines the appropriate reason for the Installed condition // when no bundle is installed (Installed: False). // // Returns Failed when: // - No rolling revisions exist (nothing to install) // - The latest rolling revision has Reason: Retrying (indicates an error occurred) // +// Returns Installing when: +// - The latest rolling revision explicitly has Reason: RollingOut (healthy installation in progress) +// // Returns Absent when: -// - Rolling revisions exist with the latest having Reason: RollingOut (healthy phased rollout in progress) +// - Rolling revisions exist but have no conditions set (rollout just started) // // Rationale: // - Failed: Semantically indicates an error prevented installation -// - Absent: Semantically indicates "not there yet" (neutral state, e.g., during healthy rollout) +// - Installing: Semantically indicates a first-time installation is actively in progress +// - Absent: Neutral state when rollout exists but hasn't progressed enough to determine health // - Retrying reason indicates an error (config validation, apply failure, etc.) -// - RollingOut reason indicates healthy progress (not an error) +// - RollingOut reason indicates confirmed healthy progress // - Only the LATEST revision matters - old errors superseded by newer healthy revisions should not cause Failed // +// Note: This function is only called when Installed == nil (first-time installation scenario). // Note: rollingRevisions are sorted in ascending order by Spec.Revision (oldest to newest), -// so the latest revision is the LAST element in the array. -func determineFailureReason(rollingRevisions []*RevisionMetadata) string { +// +// so the latest revision is the LAST element in the array. +func determineInstalledReason(rollingRevisions []*RevisionMetadata) string { if len(rollingRevisions) == 0 { return ocv1.ReasonFailed } @@ -97,14 +115,21 @@ func determineFailureReason(rollingRevisions []*RevisionMetadata) string { // Latest revision is the last element in the array (sorted ascending by Spec.Revision) latestRevision := rollingRevisions[len(rollingRevisions)-1] progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) - if progressingCond != nil && progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) { - // Retrying indicates an error occurred (config, apply, validation, etc.) - // Use Failed for semantic correctness: installation failed due to error - return ocv1.ReasonFailed + if progressingCond != nil { + if progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) { + // Retrying indicates an error occurred (config, apply, validation, etc.) + // Use Failed for semantic correctness: installation failed due to error + return ocv1.ReasonFailed + } + if progressingCond.Reason == string(ocv1.ReasonRollingOut) { + // RollingOut indicates healthy progress is confirmed + // Use Installing to communicate that a first-time installation is actively in progress + return ocv1.ReasonInstalling + } } - // No error detected in latest revision - it's progressing healthily (RollingOut) or no conditions set - // Use Absent for neutral "not installed yet" state + // No progressing condition or unknown reason - rollout just started or hasn't progressed + // Use Absent as neutral state return ocv1.ReasonAbsent } @@ -119,6 +144,17 @@ func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message stri }) } +// setInstalledStatusConditionUpgrading sets the installed status condition to upgrading. +func setInstalledStatusConditionUpgrading(ext *ocv1.ClusterExtension, message string) { + SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonUpgrading, + Message: message, + ObservedGeneration: ext.GetGeneration(), + }) +} + // setInstalledStatusConditionFailed sets the installed status condition to failed. func setInstalledStatusConditionFalse(ext *ocv1.ClusterExtension, reason string, message string) { SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ diff --git a/internal/operator-controller/controllers/common_controller_test.go b/internal/operator-controller/controllers/common_controller_test.go index 3cc757543..c2740a33b 100644 --- a/internal/operator-controller/controllers/common_controller_test.go +++ b/internal/operator-controller/controllers/common_controller_test.go @@ -253,6 +253,107 @@ func TestSetStatusConditionWrapper(t *testing.T) { } } +func TestSetInstalledStatusFromRevisionStates_Upgrade(t *testing.T) { + tests := []struct { + name string + revisionStates *RevisionStates + expectedInstalledCond metav1.Condition + }{ + { + name: "installed bundle with healthy upgrade in progress - uses Upgrading", + revisionStates: &RevisionStates{ + Installed: &RevisionMetadata{ + RevisionName: "rev-1", + Image: "quay.io/example/bundle:v1.0.0", + BundleMetadata: ocv1.BundleMetadata{ + Name: "example.v1.0.0", + Version: "1.0.0", + }, + }, + RollingOut: []*RevisionMetadata{ + { + RevisionName: "rev-2", + Conditions: []metav1.Condition{ + { + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonRollingOut, + Message: "Upgrading to v2.0.0", + }, + }, + }, + }, + }, + expectedInstalledCond: metav1.Condition{ + Type: ocv1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonUpgrading, + }, + }, + { + name: "installed bundle with no rolling revisions - uses Succeeded", + revisionStates: &RevisionStates{ + Installed: &RevisionMetadata{ + RevisionName: "rev-1", + Image: "quay.io/example/bundle:v1.0.0", + BundleMetadata: ocv1.BundleMetadata{ + Name: "example.v1.0.0", + Version: "1.0.0", + }, + }, + RollingOut: nil, + }, + expectedInstalledCond: metav1.Condition{ + Type: ocv1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonSucceeded, + }, + }, + { + name: "installed bundle with rolling revision not yet healthy - uses Succeeded", + revisionStates: &RevisionStates{ + Installed: &RevisionMetadata{ + RevisionName: "rev-1", + Image: "quay.io/example/bundle:v1.0.0", + BundleMetadata: ocv1.BundleMetadata{ + Name: "example.v1.0.0", + Version: "1.0.0", + }, + }, + RollingOut: []*RevisionMetadata{ + { + RevisionName: "rev-2", + Conditions: []metav1.Condition{}, + }, + }, + }, + expectedInstalledCond: metav1.Condition{ + Type: ocv1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonSucceeded, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext", + Generation: 1, + }, + } + + setInstalledStatusFromRevisionStates(ext, tt.revisionStates) + + cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(t, cond) + require.Equal(t, tt.expectedInstalledCond.Status, cond.Status) + require.Equal(t, tt.expectedInstalledCond.Reason, cond.Reason) + }) + } +} + func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T) { tests := []struct { name string @@ -331,7 +432,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, { - name: "rolling revision with RollingOut reason - uses Absent", + name: "rolling revision with RollingOut reason - uses Installing", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -351,11 +452,11 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T expectedInstalledCond: metav1.Condition{ Type: ocv1.TypeInstalled, Status: metav1.ConditionFalse, - Reason: ocv1.ReasonAbsent, + Reason: ocv1.ReasonInstalling, }, }, { - name: "old revision with Retrying superseded by latest healthy - uses Absent", + name: "old revision with Retrying superseded by latest healthy - uses Installing", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -383,6 +484,47 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, }, + expectedInstalledCond: metav1.Condition{ + Type: ocv1.TypeInstalled, + Status: metav1.ConditionFalse, + Reason: ocv1.ReasonInstalling, + }, + }, + { + name: "rolling revision with no conditions set - uses Absent", + revisionStates: &RevisionStates{ + Installed: nil, + RollingOut: []*RevisionMetadata{ + { + RevisionName: "rev-1", + Conditions: []metav1.Condition{}, + }, + }, + }, + expectedInstalledCond: metav1.Condition{ + Type: ocv1.TypeInstalled, + Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + }, + }, + { + name: "rolling revision with unknown reason - uses Absent", + revisionStates: &RevisionStates{ + Installed: nil, + RollingOut: []*RevisionMetadata{ + { + RevisionName: "rev-1", + Conditions: []metav1.Condition{ + { + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionTrue, + Reason: "SomeOtherReason", + Message: "Revision is in an unknown state", + }, + }, + }, + }, + }, expectedInstalledCond: metav1.Condition{ Type: ocv1.TypeInstalled, Status: metav1.ConditionFalse, From 245d64f656bb730a971addb13e2b94123ee5769b Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Mon, 2 Feb 2026 14:40:54 +0000 Subject: [PATCH 2/2] Test --- test/e2e/features/install.feature | 24 ++++++++++++++ test/e2e/features/update.feature | 3 +- test/e2e/steps/steps.go | 52 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index 1848bc2f0..47aa6e7d6 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -359,6 +359,30 @@ Feature: Install ClusterExtension invalid ClusterExtension configuration: invalid configuration: unknown field "watchNamespace" """ + Scenario: Report Installing status during initial installation + When ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + namespace: ${TEST_NAMESPACE} + serviceAccount: + name: olm-sa + source: + sourceType: Catalog + catalog: + packageName: test + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": test-catalog + """ + Then ClusterExtension reports Installed as False with Reason Installing or has progressed past it + And ClusterExtension is rolled out + And ClusterExtension is available + And bundle "test-operator.1.2.0" is installed in version "1.2.0" + @BoxcutterRuntime @ProgressDeadline Scenario: Report ClusterExtension as not progressing if the rollout does not complete within given timeout diff --git a/test/e2e/features/update.feature b/test/e2e/features/update.feature index dee45e32a..ecab4b607 100644 --- a/test/e2e/features/update.feature +++ b/test/e2e/features/update.feature @@ -31,7 +31,8 @@ Feature: Update ClusterExtension And ClusterExtension is rolled out And ClusterExtension is available When ClusterExtension is updated to version "1.0.1" - Then ClusterExtension is rolled out + Then ClusterExtension reports Installed as True with Reason Upgrading or has progressed past it + And ClusterExtension is rolled out And ClusterExtension is available And bundle "test-operator.1.0.1" is installed in version "1.0.1" diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 9bcffa159..d0ca57291 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -69,6 +69,7 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message:$`, ClusterExtensionReportsCondition) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message includes:$`, ClusterExtensionReportsConditionWithMessageFragment) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+)$`, ClusterExtensionReportsConditionWithoutMsg) + sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) or has progressed past it$`, ClusterExtensionReportsConditionOrProgressed) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+)$`, ClusterExtensionReportsConditionWithoutReason) sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+)$`, ClusterExtensionRevisionReportsConditionWithoutMsg) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) transition between (\d+) and (\d+) minutes since its creation$`, ClusterExtensionReportsConditionTransitionTime) @@ -422,6 +423,57 @@ func ClusterExtensionReportsConditionWithoutReason(ctx context.Context, conditio return waitForExtensionCondition(ctx, conditionType, conditionStatus, nil, nil) } +func ClusterExtensionReportsConditionOrProgressed(ctx context.Context, conditionType, conditionStatus, conditionReason string) error { + sc := scenarioCtx(ctx) + // Define what "progressed past" means for each transient reason + progressedReasons := map[string][]string{ + "Installing": {"Succeeded"}, // Installing -> Succeeded + "Upgrading": {"Succeeded"}, // Upgrading -> Succeeded + } + + acceptableReasons, hasProgressedStates := progressedReasons[conditionReason] + if !hasProgressedStates { + // If no progressed states defined, use normal wait + return waitForExtensionCondition(ctx, conditionType, conditionStatus, &conditionReason, nil) + } + + // Check if we find either the target reason OR any of the progressed reasons + require.Eventually(godog.T(ctx), func() bool { + v, err := k8sClient("get", "clusterextension", sc.clusterExtensionName, "-o", fmt.Sprintf("jsonpath={.status.conditions[?(@.type==\"%s\")]}", conditionType)) + if err != nil { + return false + } + + var condition metav1.Condition + if err := json.Unmarshal([]byte(v), &condition); err != nil { + return false + } + + if condition.Status != metav1.ConditionStatus(conditionStatus) { + return false + } + + // Accept if we have the target reason + if condition.Reason == conditionReason { + return true + } + + // Accept if we've progressed past it + for _, acceptableReason := range acceptableReasons { + if condition.Reason == acceptableReason { + logger.V(1).Info("Condition has progressed past transient state", + "expectedReason", conditionReason, + "actualReason", condition.Reason, + "conditionType", conditionType) + return true + } + } + + return false + }, timeout, tick) + return nil +} + func ClusterExtensionReportsConditionTransitionTime(ctx context.Context, conditionType string, minMinutes, maxMinutes int) error { sc := scenarioCtx(ctx) t := godog.T(ctx)