Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api/v1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ const (
TypeProgressing = "Progressing"

// Installed reasons
ReasonAbsent = "Absent"
ReasonAbsent = "Absent"
ReasonInstalling = "Installing"
ReasonUpgrading = "Upgrading"

// Progressing reasons
ReasonRollingOut = "RollingOut"
Expand Down
2 changes: 2 additions & 0 deletions internal/operator-controller/conditionsets/conditionsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ var ConditionReasons = []string{
ocv1.ReasonInvalidConfiguration,
ocv1.ReasonRetrying,
ocv1.ReasonAbsent,
ocv1.ReasonInstalling,
ocv1.ReasonUpgrading,
ocv1.ReasonRollingOut,
ocv1.ReasonProgressDeadlineExceeded,
}
64 changes: 50 additions & 14 deletions internal/operator-controller/controllers/common_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check for upgrade status on line 75 only verifies that progressingCond.Reason equals RollingOut, but doesn't verify the condition Status. This could potentially report Upgrading even if the condition status is False or Unknown. Consider adding a check for progressingCond.Status == metav1.ConditionTrue to ensure the upgrade is actually progressing successfully before reporting the Upgrading status.

Suggested change
if progressingCond != nil && progressingCond.Reason == string(ocv1.ReasonRollingOut) {
if progressingCond != nil && progressingCond.Status == metav1.ConditionTrue && progressingCond.Reason == string(ocv1.ReasonRollingOut) {

Copilot uses AI. Check for mistakes.
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
}
Expand All @@ -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
}

Expand All @@ -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{
Expand Down
148 changes: 145 additions & 3 deletions internal/operator-controller/controllers/common_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Comment on lines +256 to +355
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TestSetInstalledStatusFromRevisionStates_Upgrade test is missing a test case for when an installed bundle has a rolling revision with Reason: Retrying (indicating an upgrade failure). According to the logic in setInstalledStatusFromRevisionStates (lines 71-79), this should result in Installed: True with Reason: Succeeded (showing the current bundle is still installed despite the upgrade failing). Add a test case to verify this behavior is correct and intentional.

Copilot uses AI. Check for mistakes.

func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T) {
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test function name "TestSetInstalledStatusFromRevisionStates_ConfigValidationError" is misleading. The test cases now include scenarios beyond config validation errors, such as "rolling revision with RollingOut reason - uses Installing", "rolling revision with no conditions set - uses Absent", and "rolling revision with unknown reason - uses Absent". Consider renaming this test to something more general like "TestSetInstalledStatusFromRevisionStates_NoInstalledBundle" or "TestSetInstalledStatusFromRevisionStates_InstallationInProgress" to better reflect its broader scope.

Suggested change
func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T) {
func TestSetInstalledStatusFromRevisionStates_NoInstalledBundle(t *testing.T) {

Copilot uses AI. Check for mistakes.
tests := []struct {
name string
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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,
Expand Down
24 changes: 24 additions & 0 deletions test/e2e/features/install.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/features/update.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Loading
Loading