From c9df78c5a012567986535b6ce2612e1c155d368e Mon Sep 17 00:00:00 2001
From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Date: Fri, 7 Nov 2025 16:54:52 +0000
Subject: [PATCH 1/2] Fix deprecation conditions and make YAML output cleaner
Deprecation conditions now only show what matters:
- When deprecated: condition shows "True" with deprecation message
- When not deprecated: condition is absent (no clutter!)
- When we can't check: condition shows "Unknown"
This fixes three problems:
1. Install errors were leaking into deprecation conditions
2. Catalog unavailable showed "False" instead of "Unknown"
3. BundleDeprecated was checking the wrong bundle (resolved vs installed)
Also improved the code:
- Simpler logic (clear all, then add only what's needed)
- Better reason values (Deprecated, DeprecationStatusUnknown, Absent)
- Comprehensive test coverage for all scenarios
Assisted-by: Cursor
---
api/v1/clusterextension_types.go | 13 +-
api/v1/common_types.go | 3 +-
docs/api-reference/olmv1-api-reference.md | 2 +-
...peratorframework.io_clusterextensions.yaml | 13 +-
...peratorframework.io_clusterextensions.yaml | 13 +-
.../conditionsets/conditionsets.go | 1 +
.../clusterextension_admission_test.go | 28 +-
.../clusterextension_controller.go | 244 +++++--
.../clusterextension_controller_test.go | 682 +++++++++++++++---
.../clusterextension_reconcile_steps.go | 93 ++-
.../controllers/common_controller_test.go | 2 +-
manifests/experimental-e2e.yaml | 13 +-
manifests/experimental.yaml | 13 +-
manifests/standard-e2e.yaml | 13 +-
manifests/standard.yaml | 13 +-
15 files changed, 893 insertions(+), 253 deletions(-)
diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go
index 2846b24c6..ecb7544f8 100644
--- a/api/v1/clusterextension_types.go
+++ b/api/v1/clusterextension_types.go
@@ -489,12 +489,13 @@ type ClusterExtensionStatus struct {
// When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
//
//
- // When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- // These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- // - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- // - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- // - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- // - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ // When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ // These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ // Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ // - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ // - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ // - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ // - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
//
// +listType=map
// +listMapKey=type
diff --git a/api/v1/common_types.go b/api/v1/common_types.go
index 115836b10..b63ca9a95 100644
--- a/api/v1/common_types.go
+++ b/api/v1/common_types.go
@@ -29,7 +29,8 @@ const (
ReasonBlocked = "Blocked"
// Deprecation reasons
- ReasonDeprecated = "Deprecated"
+ ReasonDeprecated = "Deprecated"
+ ReasonDeprecationStatusUnknown = "DeprecationStatusUnknown"
// Common reasons
ReasonSucceeded = "Succeeded"
diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md
index 4feb0d632..2c3de364c 100644
--- a/docs/api-reference/olmv1-api-reference.md
+++ b/docs/api-reference/olmv1-api-reference.md
@@ -359,7 +359,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
-| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions represents the current state of the ClusterExtension.
The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether the bundle has been installed for this ClusterExtension:
- When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
- When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- Deprecated is a rollup condition that is present when any of the deprecated conditions are present. | | |
+| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions represents the current state of the ClusterExtension.
The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether the bundle has been installed for this ClusterExtension:
- When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
- When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
Deprecation conditions are only present when there's something to report - absence means "not deprecated".
- BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
- ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
- PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
- Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown). | | |
| `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | |
| `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | activeRevisions holds a list of currently active (non-archived) ClusterExtensionRevisions,
including both installed and rolling out revisions.
| | |
diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml
index 7194392b6..ab68991c1 100644
--- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml
+++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml
@@ -591,12 +591,13 @@ spec:
When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
- When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml
index ca316f7e8..0f000df4f 100644
--- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml
+++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml
@@ -507,12 +507,13 @@ spec:
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
- When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/internal/operator-controller/conditionsets/conditionsets.go b/internal/operator-controller/conditionsets/conditionsets.go
index 6c33b1c8f..77b11fe46 100644
--- a/internal/operator-controller/conditionsets/conditionsets.go
+++ b/internal/operator-controller/conditionsets/conditionsets.go
@@ -36,6 +36,7 @@ var ConditionTypes = []string{
var ConditionReasons = []string{
ocv1.ReasonSucceeded,
ocv1.ReasonDeprecated,
+ ocv1.ReasonDeprecationStatusUnknown,
ocv1.ReasonFailed,
ocv1.ReasonBlocked,
ocv1.ReasonRetrying,
diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go
index 2f8791999..d98d9bf4f 100644
--- a/internal/operator-controller/controllers/clusterextension_admission_test.go
+++ b/internal/operator-controller/controllers/clusterextension_admission_test.go
@@ -13,7 +13,7 @@ import (
)
func TestClusterExtensionSourceConfig(t *testing.T) {
- sourceTypeEmptyError := "Invalid value: null"
+ sourceTypeEmptyErrors := []string{"Invalid value: \"null\"", "Invalid value: null"}
sourceTypeMismatchError := "spec.source.sourceType: Unsupported value"
sourceConfigInvalidError := "spec.source: Invalid value"
// unionField represents the required Catalog or (future) Bundle field required by SourceConfig
@@ -21,12 +21,12 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
name string
sourceType string
unionField string
- errMsg string
+ errMsgs []string
}{
- {"sourceType is null", "", "Catalog", sourceTypeEmptyError},
- {"sourceType is invalid", "Invalid", "Catalog", sourceTypeMismatchError},
- {"catalog field does not exist", "Catalog", "", sourceConfigInvalidError},
- {"sourceConfig has required fields", "Catalog", "Catalog", ""},
+ {"sourceType is null", "", "Catalog", sourceTypeEmptyErrors},
+ {"sourceType is invalid", "Invalid", "Catalog", []string{sourceTypeMismatchError}},
+ {"catalog field does not exist", "Catalog", "", []string{sourceConfigInvalidError}},
+ {"sourceConfig has required fields", "Catalog", "Catalog", nil},
}
t.Parallel()
@@ -62,12 +62,20 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
}))
}
- if tc.errMsg == "" {
+ if len(tc.errMsgs) == 0 {
require.NoError(t, err, "unexpected error for sourceType %q: %w", tc.sourceType, err)
- } else {
- require.Error(t, err)
- require.Contains(t, err.Error(), tc.errMsg)
+ return
+ }
+
+ require.Error(t, err)
+ matched := false
+ for _, msg := range tc.errMsgs {
+ if strings.Contains(err.Error(), msg) {
+ matched = true
+ break
+ }
}
+ require.True(t, matched, "expected one of %v in error %q", tc.errMsgs, err)
})
}
}
diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go
index 7f3192b0f..c079329d6 100644
--- a/internal/operator-controller/controllers/clusterextension_controller.go
+++ b/internal/operator-controller/controllers/clusterextension_controller.go
@@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"io/fs"
+ "slices"
"strings"
"github.com/go-logr/logr"
@@ -166,13 +167,20 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
return res, reconcileErr
}
-// ensureAllConditionsWithReason checks that all defined condition types exist in the given ClusterExtension,
-// and assigns a specified reason and custom message to any missing condition.
-func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.ConditionReason, message string) {
+// ensureFailureConditionsWithReason keeps every non-deprecation condition present.
+// If one is missing, we add it with the given reason and message so users see why
+// reconcile failed. Deprecation conditions are handled later by SetDeprecationStatus.
+func ensureFailureConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.ConditionReason, message string) {
for _, condType := range conditionsets.ConditionTypes {
+ if isDeprecationCondition(condType) {
+ continue
+ }
cond := apimeta.FindStatusCondition(ext.Status.Conditions, condType)
+ // Guard so we only fill empty slots. Without it, we would overwrite the detailed status that
+ // helpers (setStatusProgressing, setInstalledStatusCondition*, SetDeprecationStatus) already set.
if cond == nil {
- // Create a new condition with a valid reason and add it
+ // No condition exists yet, so add a fallback with the failure reason. Specific helpers replace it
+ // with the real progressing/bundle/package/channel message during reconciliation.
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: condType,
Status: metav1.ConditionFalse,
@@ -184,83 +192,199 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C
}
}
-// SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension
-// based on the provided bundle
-func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) {
- deprecations := map[string][]declcfg.DeprecationEntry{}
+// SetDeprecationStatus updates deprecation conditions based on catalog metadata.
+//
+// Behavior:
+// - IS deprecated → condition True with Reason: Deprecated
+// - NOT deprecated → condition absent (clean YAML)
+// - Can't check (no catalog) → condition Unknown with Reason: DeprecationStatusUnknown
+// - No bundle installed → BundleDeprecated Unknown with Reason: Absent
+//
+// This keeps deprecation conditions focused on catalog data. Install/validation errors
+// never appear here - they belong in Progressing/Installed conditions.
+//
+// TODO (out of scope): What if different catalogs have conflicting deprecation data?
+//
+// Example scenario:
+// Catalog A: package "foo" marked deprecated
+// Catalog B: package "foo" NOT deprecated
+// Problem: Resolver picks one catalog arbitrarily when resolution fails
+// Question: Should we mark Unknown? Combine all? Pick by priority?
+// This needs follow-up discussion and PR.
+func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, deprecation *declcfg.Deprecation, hasCatalogData bool) {
+ info := buildDeprecationInfo(ext, installedBundleName, deprecation)
+ packageMessages := collectDeprecationMessages(info.PackageEntries)
+ channelMessages := collectDeprecationMessages(info.ChannelEntries)
+ bundleMessages := collectDeprecationMessages(info.BundleEntries)
+
+ // Clear all deprecation conditions first, then only add the ones we need.
+ // Absence of a deprecation condition means "not deprecated" - keeps output clean.
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeDeprecated)
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypePackageDeprecated)
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeChannelDeprecated)
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeBundleDeprecated)
+
+ if !hasCatalogData {
+ // When catalog is unavailable, set all to Unknown.
+ // BundleDeprecated uses Absent only when no bundle installed.
+ bundleReason := ocv1.ReasonAbsent
+ bundleMessage := "no bundle installed yet"
+ if installedBundleName != "" {
+ bundleReason = ocv1.ReasonDeprecationStatusUnknown
+ bundleMessage = "deprecation status unknown: catalog data unavailable"
+ }
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeDeprecated,
+ Status: metav1.ConditionUnknown,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypePackageDeprecated,
+ Status: metav1.ConditionUnknown,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeChannelDeprecated,
+ Status: metav1.ConditionUnknown,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeBundleDeprecated,
+ Status: metav1.ConditionUnknown,
+ Reason: bundleReason,
+ Message: bundleMessage,
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ return
+ }
+
+ // Only add conditions when there's something to report (True or Unknown states).
+ // False (not deprecated) is represented by absence of the condition.
+ messages := slices.Concat(packageMessages, channelMessages, bundleMessages)
+ if len(messages) > 0 {
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeDeprecated,
+ Status: metav1.ConditionTrue,
+ Reason: ocv1.ReasonDeprecated,
+ Message: strings.Join(messages, "\n"),
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ }
+
+ if len(packageMessages) > 0 {
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypePackageDeprecated,
+ Status: metav1.ConditionTrue,
+ Reason: ocv1.ReasonDeprecated,
+ Message: strings.Join(packageMessages, "\n"),
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ }
+
+ if len(channelMessages) > 0 {
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeChannelDeprecated,
+ Status: metav1.ConditionTrue,
+ Reason: ocv1.ReasonDeprecated,
+ Message: strings.Join(channelMessages, "\n"),
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ }
+
+ // BundleDeprecated: Unknown when no bundle installed, True when deprecated, absent otherwise
+ if info.BundleStatus == metav1.ConditionUnknown {
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeBundleDeprecated,
+ Status: metav1.ConditionUnknown,
+ Reason: ocv1.ReasonAbsent,
+ Message: "no bundle installed yet",
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ } else if len(bundleMessages) > 0 {
+ SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeBundleDeprecated,
+ Status: metav1.ConditionTrue,
+ Reason: ocv1.ReasonDeprecated,
+ Message: strings.Join(bundleMessages, "\n"),
+ ObservedGeneration: ext.GetGeneration(),
+ })
+ }
+}
+
+// isDeprecationCondition reports whether the given type is one of the deprecation
+// conditions we manage separately.
+func isDeprecationCondition(condType string) bool {
+ switch condType {
+ case ocv1.TypeDeprecated, ocv1.TypePackageDeprecated, ocv1.TypeChannelDeprecated, ocv1.TypeBundleDeprecated:
+ return true
+ default:
+ return false
+ }
+}
+
+// deprecationInfo captures the deprecation data needed to update condition status.
+type deprecationInfo struct {
+ PackageEntries []declcfg.DeprecationEntry
+ ChannelEntries []declcfg.DeprecationEntry
+ BundleEntries []declcfg.DeprecationEntry
+ BundleStatus metav1.ConditionStatus
+}
+
+// buildDeprecationInfo filters the catalog deprecation data down to the package, channel,
+// and bundle entries that matter for this ClusterExtension. An empty bundle name means
+// nothing is installed yet, so we leave bundle status Unknown/Absent.
+func buildDeprecationInfo(ext *ocv1.ClusterExtension, installedBundleName string, deprecation *declcfg.Deprecation) deprecationInfo {
+ info := deprecationInfo{BundleStatus: metav1.ConditionUnknown}
channelSet := sets.New[string]()
if ext.Spec.Source.Catalog != nil {
- for _, channel := range ext.Spec.Source.Catalog.Channels {
- channelSet.Insert(channel)
- }
+ channelSet.Insert(ext.Spec.Source.Catalog.Channels...)
}
+
if deprecation != nil {
for _, entry := range deprecation.Entries {
switch entry.Reference.Schema {
case declcfg.SchemaPackage:
- deprecations[ocv1.TypePackageDeprecated] = []declcfg.DeprecationEntry{entry}
+ info.PackageEntries = append(info.PackageEntries, entry)
case declcfg.SchemaChannel:
if channelSet.Has(entry.Reference.Name) {
- deprecations[ocv1.TypeChannelDeprecated] = append(deprecations[ocv1.TypeChannelDeprecated], entry)
+ info.ChannelEntries = append(info.ChannelEntries, entry)
}
case declcfg.SchemaBundle:
- if bundleName != entry.Reference.Name {
- continue
+ if installedBundleName != "" && entry.Reference.Name == installedBundleName {
+ info.BundleEntries = append(info.BundleEntries, entry)
}
- deprecations[ocv1.TypeBundleDeprecated] = []declcfg.DeprecationEntry{entry}
}
}
}
- // first get ordered deprecation messages that we'll join in the Deprecated condition message
- var deprecationMessages []string
- for _, conditionType := range []string{
- ocv1.TypePackageDeprecated,
- ocv1.TypeChannelDeprecated,
- ocv1.TypeBundleDeprecated,
- } {
- if entries, ok := deprecations[conditionType]; ok {
- for _, entry := range entries {
- deprecationMessages = append(deprecationMessages, entry.Message)
- }
+ // installedBundleName is empty when nothing is installed. In that case we want
+ // to report the bundle deprecation condition as Unknown/Absent.
+ if installedBundleName != "" {
+ if len(info.BundleEntries) > 0 {
+ info.BundleStatus = metav1.ConditionTrue
+ } else {
+ info.BundleStatus = metav1.ConditionFalse
}
}
- // next, set the Deprecated condition
- status, reason, message := metav1.ConditionFalse, ocv1.ReasonDeprecated, ""
- if len(deprecationMessages) > 0 {
- status, reason, message = metav1.ConditionTrue, ocv1.ReasonDeprecated, strings.Join(deprecationMessages, ";")
- }
- SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
- Type: ocv1.TypeDeprecated,
- Reason: reason,
- Status: status,
- Message: message,
- ObservedGeneration: ext.Generation,
- })
-
- // finally, set the individual deprecation conditions for package, channel, and bundle
- for _, conditionType := range []string{
- ocv1.TypePackageDeprecated,
- ocv1.TypeChannelDeprecated,
- ocv1.TypeBundleDeprecated,
- } {
- entries, ok := deprecations[conditionType]
- status, reason, message := metav1.ConditionFalse, ocv1.ReasonDeprecated, ""
- if ok {
- status, reason = metav1.ConditionTrue, ocv1.ReasonDeprecated
- for _, entry := range entries {
- message = fmt.Sprintf("%s\n%s", message, entry.Message)
- }
+ return info
+}
+
+// collectDeprecationMessages collects the non-empty deprecation messages from the provided entries.
+func collectDeprecationMessages(entries []declcfg.DeprecationEntry) []string {
+ messages := make([]string, 0, len(entries))
+ for _, entry := range entries {
+ if entry.Message != "" {
+ messages = append(messages, entry.Message)
}
- SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
- Type: conditionType,
- Reason: reason,
- Status: status,
- Message: message,
- ObservedGeneration: ext.Generation,
- })
}
+ return messages
}
type ControllerBuilderOption func(builder *ctrl.Builder)
diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go
index 1b09a43ad..ef12048b7 100644
--- a/internal/operator-controller/controllers/clusterextension_controller_test.go
+++ b/internal/operator-controller/controllers/clusterextension_controller_test.go
@@ -32,7 +32,8 @@ import (
"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
- finalizers "github.com/operator-framework/operator-controller/internal/operator-controller/finalizers"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/features"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/finalizers"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
"github.com/operator-framework/operator-controller/internal/operator-controller/resolve"
imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
@@ -127,7 +128,7 @@ func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) {
func TestClusterExtensionResolutionFails(t *testing.T) {
pkgName := fmt.Sprintf("non-existent-%s", rand.String(6))
cl, reconciler := newClientAndReconciler(t, func(d *deps) {
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
return nil, nil, nil, fmt.Errorf("no package %q found", pkgName)
})
})
@@ -177,6 +178,242 @@ func TestClusterExtensionResolutionFails(t *testing.T) {
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
}
+// TestClusterExtensionResolutionFailsWithDeprecationData verifies that deprecation warnings are shown even when resolution fails.
+//
+// Scenario:
+// - Resolution fails (package not found or version not available)
+// - Resolver returns deprecation data along with the error
+// - Catalog has marked the package as deprecated
+// - PackageDeprecated and Deprecated conditions show True with the deprecation message
+// - BundleDeprecated stays Unknown/Absent because no bundle is installed yet
+//
+// This ensures deprecation warnings reach users even when installation cannot proceed.
+func TestClusterExtensionResolutionFailsWithDeprecationData(t *testing.T) {
+ ctx := context.Background()
+ pkgName := fmt.Sprintf("deprecated-%s", rand.String(6))
+ deprecationMessage := "package marked deprecated in catalog"
+ cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ return nil, nil, &declcfg.Deprecation{
+ Entries: []declcfg.DeprecationEntry{{
+ Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage},
+ Message: deprecationMessage,
+ }},
+ }, fmt.Errorf("no package %q found", pkgName)
+ })
+ })
+
+ extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
+ clusterExtension := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+ Spec: ocv1.ClusterExtensionSpec{
+ Source: ocv1.SourceConfig{
+ SourceType: "Catalog",
+ Catalog: &ocv1.CatalogFilter{PackageName: pkgName},
+ },
+ Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
+ },
+ }
+ require.NoError(t, cl.Create(ctx, clusterExtension))
+
+ res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+ require.Equal(t, ctrl.Result{}, res)
+ require.EqualError(t, err, fmt.Sprintf("no package %q found", pkgName))
+
+ require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+
+ pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+ require.NotNil(t, pkgCond)
+ require.Equal(t, metav1.ConditionTrue, pkgCond.Status)
+ require.Equal(t, deprecationMessage, pkgCond.Message)
+
+ deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+ require.NotNil(t, deprecatedCond)
+ require.Equal(t, metav1.ConditionTrue, deprecatedCond.Status)
+
+ bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+ require.NotNil(t, bundleCond)
+ require.Equal(t, metav1.ConditionUnknown, bundleCond.Status, "no bundle installed yet, so keep it Unknown/Absent")
+ require.Equal(t, ocv1.ReasonAbsent, bundleCond.Reason)
+
+ verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
+ require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+}
+
+// TestClusterExtensionUpgradeShowsInstalledBundleDeprecation verifies that deprecation status
+// reflects the INSTALLED bundle, not the RESOLVED bundle during upgrades.
+//
+// Scenario:
+// - Bundle v1.0.0 is installed and deprecated in the catalog
+// - Bundle v2.0.0 is available (resolved) and NOT deprecated
+// - BundleDeprecated should show True with v1.0.0's deprecation message
+//
+// This demonstrates the key fix: status shows actual state (installed), not desired state (resolved).
+// Users need to know what's currently running is deprecated, even if the upgrade target is fine.
+func TestClusterExtensionUpgradeShowsInstalledBundleDeprecation(t *testing.T) {
+ ctx := context.Background()
+ pkgName := fmt.Sprintf("upgrade-%s", rand.String(6))
+ installedBundleName := fmt.Sprintf("%s.v1.0.0", pkgName)
+ resolvedBundleName := fmt.Sprintf("%s.v2.0.0", pkgName)
+ deprecationMessage := "v1.0.0 is deprecated, please upgrade to v2.0.0"
+
+ cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ v := bundle.VersionRelease{
+ Version: bsemver.MustParse("2.0.0"),
+ }
+ // Catalog has deprecation for v1.0.0 (installed), but v2.0.0 (resolved) is NOT deprecated
+ return &declcfg.Bundle{
+ Name: resolvedBundleName,
+ Package: pkgName,
+ Image: fmt.Sprintf("quay.io/example/%s@sha256:resolved200", pkgName),
+ }, &v, &declcfg.Deprecation{
+ Entries: []declcfg.DeprecationEntry{{
+ Reference: declcfg.PackageScopedReference{
+ Schema: declcfg.SchemaBundle,
+ Name: installedBundleName, // v1.0.0 is deprecated
+ },
+ Message: deprecationMessage,
+ }},
+ }, nil
+ })
+ d.RevisionStatesGetter = &MockRevisionStatesGetter{
+ RevisionStates: &controllers.RevisionStates{
+ Installed: &controllers.RevisionMetadata{
+ Package: pkgName,
+ BundleMetadata: ocv1.BundleMetadata{
+ Name: installedBundleName, // v1.0.0 installed
+ Version: "1.0.0",
+ },
+ Image: fmt.Sprintf("quay.io/example/%s@sha256:installed100", pkgName),
+ },
+ },
+ }
+ d.ImagePuller = &imageutil.MockPuller{ImageFS: fstest.MapFS{}}
+ d.Applier = &MockApplier{}
+ })
+
+ extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
+ clusterExtension := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+ Spec: ocv1.ClusterExtensionSpec{
+ Source: ocv1.SourceConfig{
+ SourceType: "Catalog",
+ Catalog: &ocv1.CatalogFilter{PackageName: pkgName},
+ },
+ Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
+ },
+ }
+ require.NoError(t, cl.Create(ctx, clusterExtension))
+
+ res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+ require.Equal(t, ctrl.Result{}, res)
+ require.NoError(t, err)
+
+ require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+
+ // BundleDeprecated should reflect the INSTALLED bundle (v1.0.0), not the RESOLVED bundle (v2.0.0)
+ bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+ require.NotNil(t, bundleCond)
+ require.Equal(t, metav1.ConditionTrue, bundleCond.Status, "installed bundle v1.0.0 is deprecated")
+ require.Equal(t, ocv1.ReasonDeprecated, bundleCond.Reason)
+ require.Equal(t, deprecationMessage, bundleCond.Message)
+
+ // Deprecated condition should also be True (combines all deprecation types)
+ deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+ require.NotNil(t, deprecatedCond)
+ require.Equal(t, metav1.ConditionTrue, deprecatedCond.Status)
+ require.Contains(t, deprecatedCond.Message, deprecationMessage)
+
+ // Package and Channel should NOT be deprecated (not in deprecation data)
+ pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+ require.Nil(t, pkgCond, "package is not deprecated, condition should be absent")
+
+ channelCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated)
+ require.Nil(t, channelCond, "channel is not deprecated, condition should be absent")
+
+ verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
+ require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+}
+
+// TestClusterExtensionResolutionFailsWithoutCatalogDeprecationData verifies deprecation status handling when catalog data is unavailable.
+//
+// Scenario:
+// - A bundle is already installed (v1.0.0)
+// - Catalog is removed or resolution fails (no catalog data available)
+// - Resolution error is returned with no deprecation data
+// - All deprecation conditions must be set to Unknown (not False)
+// - BundleDeprecated uses reason Deprecated (not Absent) because a bundle exists
+//
+// This ensures users see "we don't know the deprecation status" rather than "definitely not deprecated"
+// when the catalog source of truth is unavailable.
+func TestClusterExtensionResolutionFailsWithoutCatalogDeprecationData(t *testing.T) {
+ ctx := context.Background()
+ pkgName := fmt.Sprintf("missing-%s", rand.String(6))
+ installedBundleName := fmt.Sprintf("%s.v1.0.0", pkgName)
+ cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ return nil, nil, nil, fmt.Errorf("no bundles found for package %q", pkgName)
+ })
+
+ d.RevisionStatesGetter = &MockRevisionStatesGetter{
+ RevisionStates: &controllers.RevisionStates{
+ Installed: &controllers.RevisionMetadata{
+ Package: pkgName,
+ BundleMetadata: ocv1.BundleMetadata{
+ Name: installedBundleName,
+ Version: "1.0.0",
+ },
+ Image: "example.com/installed@sha256:deadbeef",
+ },
+ },
+ }
+ })
+
+ extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
+ clusterExtension := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+ Spec: ocv1.ClusterExtensionSpec{
+ Source: ocv1.SourceConfig{
+ SourceType: "Catalog",
+ Catalog: &ocv1.CatalogFilter{PackageName: pkgName},
+ },
+ Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
+ },
+ }
+ require.NoError(t, cl.Create(ctx, clusterExtension))
+
+ res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+ require.Equal(t, ctrl.Result{}, res)
+ require.EqualError(t, err, fmt.Sprintf("no bundles found for package %q", pkgName))
+
+ require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+
+ packageCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+ require.NotNil(t, packageCond)
+ require.Equal(t, metav1.ConditionUnknown, packageCond.Status)
+ require.Equal(t, ocv1.ReasonDeprecationStatusUnknown, packageCond.Reason)
+ require.Equal(t, "deprecation status unknown: catalog data unavailable", packageCond.Message)
+
+ deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+ require.NotNil(t, deprecatedCond)
+ require.Equal(t, metav1.ConditionUnknown, deprecatedCond.Status)
+ require.Equal(t, ocv1.ReasonDeprecationStatusUnknown, deprecatedCond.Reason)
+ require.Equal(t, "deprecation status unknown: catalog data unavailable", deprecatedCond.Message)
+
+ bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+ require.NotNil(t, bundleCond)
+ require.Equal(t, metav1.ConditionUnknown, bundleCond.Status)
+ require.Equal(t, ocv1.ReasonDeprecationStatusUnknown, bundleCond.Reason)
+ require.Equal(t, "deprecation status unknown: catalog data unavailable", bundleCond.Message)
+
+ verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
+ require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+}
+
func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
type testCase struct {
name string
@@ -230,7 +467,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
}
},
func(d *deps) {
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -277,6 +514,19 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
require.Equal(t, expectReason, progressingCond.Reason)
require.Contains(t, progressingCond.Message, fmt.Sprintf("for resolved bundle %q with version %q", expectedBundleMetadata.Name, expectedBundleMetadata.Version))
+ t.Log("By checking deprecation conditions remain neutral and bundle is Unknown when not installed")
+ // When not deprecated, conditions are absent (cleaner output)
+ deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+ require.Nil(t, deprecatedCond, "Deprecated condition should be absent when not deprecated")
+ pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+ require.Nil(t, pkgCond, "PackageDeprecated condition should be absent when not deprecated")
+ chanCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated)
+ require.Nil(t, chanCond, "ChannelDeprecated condition should be absent when not deprecated")
+ bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+ require.NotNil(t, bundleCond)
+ require.Equal(t, metav1.ConditionUnknown, bundleCond.Status)
+ require.Equal(t, ocv1.ReasonAbsent, bundleCond.Reason)
+
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
})
}
@@ -288,7 +538,7 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
d.ImagePuller = &imageutil.MockPuller{
ImageFS: fstest.MapFS{},
}
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -361,6 +611,126 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason)
require.Contains(t, progressingCond.Message, fmt.Sprintf("for resolved bundle %q with version %q", expectedBundleMetadata.Name, expectedBundleMetadata.Version))
+ t.Log("By checking deprecation conditions remain neutral and bundle is Unknown when not installed")
+ // When not deprecated, conditions are absent (cleaner output)
+ deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+ require.Nil(t, deprecatedCond, "Deprecated condition should be absent when not deprecated")
+ pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+ require.Nil(t, pkgCond, "PackageDeprecated condition should be absent when not deprecated")
+ chanCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated)
+ require.Nil(t, chanCond, "ChannelDeprecated condition should be absent when not deprecated")
+ bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+ require.NotNil(t, bundleCond)
+ require.Equal(t, metav1.ConditionUnknown, bundleCond.Status)
+ require.Equal(t, ocv1.ReasonAbsent, bundleCond.Reason)
+
+ require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+}
+
+// TestClusterExtensionBoxcutterApplierFailsDoesNotLeakDeprecationErrors verifies deprecation status when apply fails.
+//
+// Scenario:
+// - Resolution succeeds and returns a valid bundle (prometheus.v1.0.0)
+// - Boxcutter applier fails during rollout (simulates apply failure)
+// - A rolling revision exists but nothing is installed yet
+// - Progressing condition shows the apply error (Retrying)
+// - Deprecation conditions reflect catalog data (all False since nothing deprecated)
+// - BundleDeprecated stays Unknown/Absent because apply failed before install
+//
+// This ensures apply errors appear in Progressing condition, not in deprecation conditions.
+func TestClusterExtensionBoxcutterApplierFailsDoesNotLeakDeprecationErrors(t *testing.T) {
+ require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=true", features.BoxcutterRuntime)))
+ t.Cleanup(func() {
+ require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=false", features.BoxcutterRuntime)))
+ })
+
+ cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+ // Boxcutter keeps a rolling revision when apply fails. We mirror that state so the test uses
+ // the same inputs the runtime would see.
+ d.RevisionStatesGetter = &MockRevisionStatesGetter{
+ RevisionStates: &controllers.RevisionStates{
+ RollingOut: []*controllers.RevisionMetadata{{}},
+ },
+ }
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ v := bundle.VersionRelease{
+ Version: bsemver.MustParse("1.0.0"),
+ }
+ return &declcfg.Bundle{
+ Name: "prometheus.v1.0.0",
+ Package: "prometheus",
+ Image: "quay.io/operatorhubio/prometheus@fake1.0.0",
+ }, &v, nil, nil
+ })
+ d.ImagePuller = &imageutil.MockPuller{ImageFS: fstest.MapFS{}}
+ d.Applier = &MockApplier{err: errors.New("boxcutter apply failure")}
+ })
+
+ ctx := context.Background()
+ extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
+
+ t.Log("When the Boxcutter Feature Flag is enabled and apply fails")
+ clusterExtension := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+ Spec: ocv1.ClusterExtensionSpec{
+ Source: ocv1.SourceConfig{
+ SourceType: "Catalog",
+ Catalog: &ocv1.CatalogFilter{
+ PackageName: "prometheus",
+ Version: "1.0.0",
+ Channels: []string{"beta"},
+ },
+ },
+ Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
+ },
+ }
+ require.NoError(t, cl.Create(ctx, clusterExtension))
+
+ res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+ require.Equal(t, ctrl.Result{}, res)
+ require.Error(t, err)
+
+ require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+
+ installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled)
+ require.NotNil(t, installedCond)
+ require.Equal(t, metav1.ConditionFalse, installedCond.Status)
+ require.Equal(t, ocv1.ReasonAbsent, installedCond.Reason)
+ require.Contains(t, installedCond.Message, "No bundle installed")
+
+ progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing)
+ require.NotNil(t, progressingCond)
+ require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
+ require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason)
+ require.Contains(t, progressingCond.Message, "boxcutter apply failure")
+
+ deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+ require.NotNil(t, deprecatedCond)
+ require.Equal(t, metav1.ConditionUnknown, deprecatedCond.Status, "no catalog data during rollout, so Unknown")
+ require.Equal(t, ocv1.ReasonDeprecationStatusUnknown, deprecatedCond.Reason)
+ require.Equal(t, "deprecation status unknown: catalog data unavailable", deprecatedCond.Message)
+
+ packageCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+ require.NotNil(t, packageCond)
+ require.Equal(t, metav1.ConditionUnknown, packageCond.Status, "no catalog data during rollout, so Unknown")
+ require.Equal(t, ocv1.ReasonDeprecationStatusUnknown, packageCond.Reason)
+ require.Equal(t, "deprecation status unknown: catalog data unavailable", packageCond.Message)
+
+ channelCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated)
+ require.NotNil(t, channelCond)
+ require.Equal(t, metav1.ConditionUnknown, channelCond.Status, "no catalog data during rollout, so Unknown")
+ require.Equal(t, ocv1.ReasonDeprecationStatusUnknown, channelCond.Reason)
+ require.Equal(t, "deprecation status unknown: catalog data unavailable", channelCond.Message)
+
+ bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+ require.NotNil(t, bundleCond)
+ require.Equal(t, metav1.ConditionUnknown, bundleCond.Status, "apply failed before install, so bundle status stays Unknown/Absent")
+ require.Equal(t, ocv1.ReasonAbsent, bundleCond.Reason)
+ require.Equal(t, "no bundle installed yet", bundleCond.Message)
+
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
}
@@ -428,7 +798,7 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
d.ImagePuller = &imageutil.MockPuller{
ImageFS: fstest.MapFS{},
}
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -523,7 +893,7 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
d.ImagePuller = &imageutil.MockPuller{
ImageFS: fstest.MapFS{},
}
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -602,7 +972,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
d.ImagePuller = &imageutil.MockPuller{
ImageFS: fstest.MapFS{},
}
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -683,7 +1053,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
d.ImagePuller = &imageutil.MockPuller{
ImageFS: fstest.MapFS{},
}
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -764,7 +1134,7 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
d.ImagePuller = &imageutil.MockPuller{
ImageFS: fstest.MapFS{},
}
- d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+ d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
v := bundle.VersionRelease{
Version: bsemver.MustParse("1.0.0"),
}
@@ -867,30 +1237,55 @@ func verifyInvariants(ctx context.Context, t *testing.T, c client.Client, ext *o
}
func verifyConditionsInvariants(t *testing.T, ext *ocv1.ClusterExtension) {
- // Expect that the cluster extension's set of conditions contains all defined
- // condition types for the ClusterExtension API. Every reconcile should always
- // ensure every condition type's status/reason/message reflects the state
- // read during _this_ reconcile call.
- require.Len(t, ext.Status.Conditions, len(conditionsets.ConditionTypes))
- for _, tt := range conditionsets.ConditionTypes {
+ // Core conditions (Installed, Progressing) must always be present.
+ // Deprecation conditions are optional - absence means "not deprecated".
+ coreConditions := []string{ocv1.TypeInstalled, ocv1.TypeProgressing}
+ deprecationConditions := []string{ocv1.TypeDeprecated, ocv1.TypePackageDeprecated, ocv1.TypeChannelDeprecated, ocv1.TypeBundleDeprecated}
+
+ for _, tt := range coreConditions {
cond := apimeta.FindStatusCondition(ext.Status.Conditions, tt)
- require.NotNil(t, cond)
+ require.NotNil(t, cond, "core condition %s must be present", tt)
require.NotEmpty(t, cond.Status)
require.Contains(t, conditionsets.ConditionReasons, cond.Reason)
require.Equal(t, ext.GetGeneration(), cond.ObservedGeneration)
}
+
+ // Deprecation conditions are optional, but if present must be valid
+ for _, tt := range deprecationConditions {
+ cond := apimeta.FindStatusCondition(ext.Status.Conditions, tt)
+ if cond != nil {
+ require.NotEmpty(t, cond.Status)
+ require.Contains(t, conditionsets.ConditionReasons, cond.Reason)
+ require.Equal(t, ext.GetGeneration(), cond.ObservedGeneration)
+ }
+ }
}
func TestSetDeprecationStatus(t *testing.T) {
+ // The catalogDataProvided/hasCatalogData pair lets each test express whether the catalog
+ // answered during reconciliation and, if it did, whether it marked anything as deprecated.
+ // This helps us cover three distinct user-facing states: "no catalog response" (everything
+ // stays Unknown), "catalog answered with no deprecations" (conditions absent, except
+ // BundleDeprecated which remains Unknown when no bundle is installed), and
+ // "catalog answered with explicit deprecations" (conditions go True).
+ //
+ // Key scenarios tested:
+ // 1. No catalog data + no bundle → all Unknown, BundleDeprecated uses reason Absent
+ // 2. No catalog data + bundle installed → all Unknown, BundleDeprecated uses reason DeprecationStatusUnknown
+ // 3. Catalog data provided + no deprecations → deprecation conditions absent except
+ // BundleDeprecated remains Unknown when no bundle is installed
+ // 4. Catalog data provided + explicit deprecations → relevant conditions True
for _, tc := range []struct {
name string
clusterExtension *ocv1.ClusterExtension
expectedClusterExtension *ocv1.ClusterExtension
bundle *declcfg.Bundle
deprecation *declcfg.Deprecation
+ catalogDataProvided bool
+ hasCatalogData bool
}{
{
- name: "no deprecations, all deprecation statuses set to False",
+ name: "no catalog data, all deprecation statuses set to Unknown",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
@@ -907,45 +1302,79 @@ func TestSetDeprecationStatus(t *testing.T) {
Conditions: []metav1.Condition{
{
Type: ocv1.TypeDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
ObservedGeneration: 1,
},
{
Type: ocv1.TypePackageDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
ObservedGeneration: 1,
},
{
Type: ocv1.TypeChannelDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
ObservedGeneration: 1,
},
{
Type: ocv1.TypeBundleDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
+ Message: "no bundle installed yet",
ObservedGeneration: 1,
},
},
},
},
- bundle: &declcfg.Bundle{},
- deprecation: nil,
+ bundle: &declcfg.Bundle{},
+ deprecation: nil,
+ catalogDataProvided: false,
+ hasCatalogData: false,
},
{
- name: "deprecated channel, but no channel specified, all deprecation statuses set to False",
+ // Scenario:
+ // - A bundle is installed (v1.0.0)
+ // - Catalog becomes unavailable (removed or network failure)
+ // - No catalog data can be retrieved
+ // - BundleDeprecated must show Unknown/DeprecationStatusUnknown (not Absent)
+ // - Reason is DeprecationStatusUnknown because catalog data is unavailable; Absent is only for no bundle
+ name: "no catalog data with installed bundle keeps bundle condition Unknown",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
},
- Spec: ocv1.ClusterExtensionSpec{
- Source: ocv1.SourceConfig{
- SourceType: "Catalog",
- Catalog: &ocv1.CatalogFilter{},
- },
+ Status: ocv1.ClusterExtensionStatus{Conditions: []metav1.Condition{}},
+ },
+ expectedClusterExtension: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{Generation: 1},
+ Status: ocv1.ClusterExtensionStatus{Conditions: []metav1.Condition{
+ {Type: ocv1.TypeDeprecated, Reason: ocv1.ReasonDeprecationStatusUnknown, Status: metav1.ConditionUnknown, Message: "deprecation status unknown: catalog data unavailable", ObservedGeneration: 1},
+ {Type: ocv1.TypePackageDeprecated, Reason: ocv1.ReasonDeprecationStatusUnknown, Status: metav1.ConditionUnknown, Message: "deprecation status unknown: catalog data unavailable", ObservedGeneration: 1},
+ {Type: ocv1.TypeChannelDeprecated, Reason: ocv1.ReasonDeprecationStatusUnknown, Status: metav1.ConditionUnknown, Message: "deprecation status unknown: catalog data unavailable", ObservedGeneration: 1},
+ {Type: ocv1.TypeBundleDeprecated, Reason: ocv1.ReasonDeprecationStatusUnknown, Status: metav1.ConditionUnknown, Message: "deprecation status unknown: catalog data unavailable", ObservedGeneration: 1},
+ }},
+ },
+ bundle: &declcfg.Bundle{Name: "installed.v1.0.0"},
+ deprecation: nil,
+ catalogDataProvided: false,
+ hasCatalogData: false,
+ },
+ {
+ // Scenario:
+ // - A bundle is installed
+ // - Catalog returns deprecation entries but catalogDataProvided=false
+ // - This tests that deprecation data is ignored when hasCatalogData is false
+ // - All conditions go to Unknown regardless of deprecation entries present
+ // - BundleDeprecated uses DeprecationStatusUnknown (not Absent) because bundle exists
+ name: "deprecation entries ignored when catalog data flag is false",
+ clusterExtension: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
},
Status: ocv1.ClusterExtensionStatus{
Conditions: []metav1.Condition{},
@@ -955,36 +1384,109 @@ func TestSetDeprecationStatus(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
},
- Spec: ocv1.ClusterExtensionSpec{
- Source: ocv1.SourceConfig{
- SourceType: "Catalog",
- Catalog: &ocv1.CatalogFilter{},
- },
- },
Status: ocv1.ClusterExtensionStatus{
Conditions: []metav1.Condition{
{
Type: ocv1.TypeDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
ObservedGeneration: 1,
},
{
Type: ocv1.TypePackageDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
ObservedGeneration: 1,
},
{
Type: ocv1.TypeChannelDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
ObservedGeneration: 1,
},
{
Type: ocv1.TypeBundleDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecationStatusUnknown,
+ Status: metav1.ConditionUnknown,
+ Message: "deprecation status unknown: catalog data unavailable",
+ ObservedGeneration: 1,
+ },
+ },
+ },
+ },
+ bundle: &declcfg.Bundle{Name: "ignored"},
+ deprecation: &declcfg.Deprecation{Entries: []declcfg.DeprecationEntry{{
+ Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage},
+ Message: "should not surface",
+ }}},
+ catalogDataProvided: true,
+ hasCatalogData: false,
+ },
+ {
+ name: "catalog consulted but no deprecations, conditions absent except BundleDeprecated Unknown when no bundle",
+ clusterExtension: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
+ },
+ Status: ocv1.ClusterExtensionStatus{
+ Conditions: []metav1.Condition{},
+ },
+ },
+ expectedClusterExtension: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
+ },
+ Status: ocv1.ClusterExtensionStatus{
+ Conditions: []metav1.Condition{
+ {
+ Type: ocv1.TypeBundleDeprecated,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
+ ObservedGeneration: 1,
+ },
+ },
+ },
+ },
+ bundle: &declcfg.Bundle{},
+ deprecation: nil,
+ catalogDataProvided: true,
+ hasCatalogData: true,
+ },
+ {
+ name: "deprecated channel exists but not used, conditions absent except BundleDeprecated Unknown",
+ clusterExtension: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
+ },
+ Spec: ocv1.ClusterExtensionSpec{
+ Source: ocv1.SourceConfig{
+ SourceType: "Catalog",
+ Catalog: &ocv1.CatalogFilter{},
+ },
+ },
+ Status: ocv1.ClusterExtensionStatus{
+ Conditions: []metav1.Condition{},
+ },
+ },
+ expectedClusterExtension: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
+ },
+ Spec: ocv1.ClusterExtensionSpec{
+ Source: ocv1.SourceConfig{
+ SourceType: "Catalog",
+ Catalog: &ocv1.CatalogFilter{},
+ },
+ },
+ Status: ocv1.ClusterExtensionStatus{
+ Conditions: []metav1.Condition{
+ {
+ Type: ocv1.TypeBundleDeprecated,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
ObservedGeneration: 1,
},
},
@@ -999,9 +1501,11 @@ func TestSetDeprecationStatus(t *testing.T) {
},
}},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
{
- name: "deprecated channel, but a non-deprecated channel specified, all deprecation statuses set to False",
+ name: "deprecated channel exists but non-deprecated channel specified; conditions absent except BundleDeprecated Unknown",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
@@ -1032,28 +1536,10 @@ func TestSetDeprecationStatus(t *testing.T) {
},
Status: ocv1.ClusterExtensionStatus{
Conditions: []metav1.Condition{
- {
- Type: ocv1.TypeDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
- ObservedGeneration: 1,
- },
- {
- Type: ocv1.TypePackageDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
- ObservedGeneration: 1,
- },
- {
- Type: ocv1.TypeChannelDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
- ObservedGeneration: 1,
- },
{
Type: ocv1.TypeBundleDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
ObservedGeneration: 1,
},
},
@@ -1070,9 +1556,11 @@ func TestSetDeprecationStatus(t *testing.T) {
},
},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
{
- name: "deprecated channel specified, ChannelDeprecated and Deprecated status set to true, others set to false",
+ name: "deprecated channel specified, ChannelDeprecated and Deprecated set to true, PackageDeprecated absent, BundleDeprecated Unknown",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
@@ -1109,12 +1597,6 @@ func TestSetDeprecationStatus(t *testing.T) {
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
},
- {
- Type: ocv1.TypePackageDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
- ObservedGeneration: 1,
- },
{
Type: ocv1.TypeChannelDeprecated,
Reason: ocv1.ReasonDeprecated,
@@ -1123,8 +1605,8 @@ func TestSetDeprecationStatus(t *testing.T) {
},
{
Type: ocv1.TypeBundleDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
ObservedGeneration: 1,
},
},
@@ -1142,6 +1624,8 @@ func TestSetDeprecationStatus(t *testing.T) {
},
},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
{
name: "deprecated package and channel specified, deprecated bundle, all deprecation statuses set to true",
@@ -1227,9 +1711,11 @@ func TestSetDeprecationStatus(t *testing.T) {
},
},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
{
- name: "deprecated channel specified, deprecated bundle, all deprecation statuses set to true, all deprecation statuses set to true except PackageDeprecated",
+ name: "deprecated channel and bundle specified, Deprecated/ChannelDeprecated/BundleDeprecated set to true, PackageDeprecated absent",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
@@ -1266,12 +1752,6 @@ func TestSetDeprecationStatus(t *testing.T) {
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
},
- {
- Type: ocv1.TypePackageDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
- ObservedGeneration: 1,
- },
{
Type: ocv1.TypeChannelDeprecated,
Reason: ocv1.ReasonDeprecated,
@@ -1306,9 +1786,11 @@ func TestSetDeprecationStatus(t *testing.T) {
},
},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
{
- name: "deprecated package and channel specified, all deprecation statuses set to true except BundleDeprecated",
+ name: "deprecated package and channel specified, Deprecated/PackageDeprecated/ChannelDeprecated set to true, BundleDeprecated Unknown/Absent (no bundle installed)",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
@@ -1359,8 +1841,8 @@ func TestSetDeprecationStatus(t *testing.T) {
},
{
Type: ocv1.TypeBundleDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
ObservedGeneration: 1,
},
},
@@ -1384,9 +1866,11 @@ func TestSetDeprecationStatus(t *testing.T) {
},
},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
{
- name: "deprecated channels specified, ChannelDeprecated and Deprecated status set to true, others set to false",
+ name: "deprecated channels specified, ChannelDeprecated and Deprecated set to true, PackageDeprecated absent, BundleDeprecated Unknown/Absent",
clusterExtension: &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
@@ -1423,12 +1907,6 @@ func TestSetDeprecationStatus(t *testing.T) {
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
},
- {
- Type: ocv1.TypePackageDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
- ObservedGeneration: 1,
- },
{
Type: ocv1.TypeChannelDeprecated,
Reason: ocv1.ReasonDeprecated,
@@ -1437,8 +1915,8 @@ func TestSetDeprecationStatus(t *testing.T) {
},
{
Type: ocv1.TypeBundleDeprecated,
- Reason: ocv1.ReasonDeprecated,
- Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonAbsent,
+ Status: metav1.ConditionUnknown,
ObservedGeneration: 1,
},
},
@@ -1459,14 +1937,22 @@ func TestSetDeprecationStatus(t *testing.T) {
Schema: declcfg.SchemaChannel,
Name: "anotherbadchannel",
},
- Message: "another bad channedl!",
+ Message: "another bad channel!",
},
},
},
+ catalogDataProvided: true,
+ hasCatalogData: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
- controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle.Name, tc.deprecation)
+ // When a test provides deprecation data it must also explicitly state that the catalog responded.
+ // This guard keeps future cases from silently falling back to the "catalog absent" branch.
+ if tc.deprecation != nil && !tc.catalogDataProvided {
+ require.Failf(t, "test case must set catalogDataProvided when deprecation is supplied", "test case %q", tc.name)
+ }
+ hasCatalogData := tc.catalogDataProvided && tc.hasCatalogData
+ controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle.Name, tc.deprecation, hasCatalogData)
// TODO: we should test for unexpected changes to lastTransitionTime. We only expect
// lastTransitionTime to change when the status of the condition changes.
assert.Empty(t, cmp.Diff(tc.expectedClusterExtension, tc.clusterExtension, cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime")))
diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go
index 09dd2ce7d..4dc4ba558 100644
--- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go
+++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go
@@ -86,50 +86,63 @@ func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc {
func ResolveBundle(r resolve.Resolver) ReconcileStepFunc {
return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
l := log.FromContext(ctx)
- var resolvedRevisionMetadata *RevisionMetadata
- if len(state.revisionStates.RollingOut) == 0 {
- l.Info("resolving bundle")
- var bm *ocv1.BundleMetadata
+
+ // If already rolling out, use existing revision and set deprecation to Unknown (no catalog check)
+ if len(state.revisionStates.RollingOut) > 0 {
+ installedBundleName := ""
if state.revisionStates.Installed != nil {
- bm = &state.revisionStates.Installed.BundleMetadata
- }
- resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolve(ctx, ext, bm)
- if err != nil {
- // Note: We don't distinguish between resolution-specific errors and generic errors
- setStatusProgressing(ext, err)
- setInstalledStatusFromRevisionStates(ext, state.revisionStates)
- ensureAllConditionsWithReason(ext, ocv1.ReasonFailed, err.Error())
- return nil, err
+ installedBundleName = state.revisionStates.Installed.Name
}
+ SetDeprecationStatus(ext, installedBundleName, nil, false)
+ state.resolvedRevisionMetadata = state.revisionStates.RollingOut[0]
+ return nil, nil
+ }
- // set deprecation status after _successful_ resolution
- // TODO:
- // 1. It seems like deprecation status should reflect the currently installed bundle, not the resolved
- // bundle. So perhaps we should set package and channel deprecations directly after resolution, but
- // defer setting the bundle deprecation until we successfully install the bundle.
- // 2. If resolution fails because it can't find a bundle, that doesn't mean we wouldn't be able to find
- // a deprecation for the ClusterExtension's spec.packageName. Perhaps we should check for a non-nil
- // resolvedDeprecation even if resolution returns an error. If present, we can still update some of
- // our deprecation status.
- // - Open question though: what if different catalogs have different opinions of what's deprecated.
- // If we can't resolve a bundle, how do we know which catalog to trust for deprecation information?
- // Perhaps if the package shows up in multiple catalogs and deprecations don't match, we can set
- // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from
- // all catalogs?
- SetDeprecationStatus(ext, resolvedBundle.Name, resolvedDeprecation)
- resolvedRevisionMetadata = &RevisionMetadata{
- Package: resolvedBundle.Package,
- Image: resolvedBundle.Image,
- // TODO: Right now, operator-controller only supports registry+v1 bundles and has no concept
- // of a "release" field. If/when we add a release field concept or a new bundle format
- // we need to re-evaluate use of `AsLegacyRegistryV1Version` so that we avoid propagating
- // registry+v1's semver spec violations of treating build metadata as orderable.
- BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, resolvedBundleVersion.AsLegacyRegistryV1Version()),
- }
- } else {
- resolvedRevisionMetadata = state.revisionStates.RollingOut[0]
+ // Resolve a new bundle from the catalog
+ l.V(1).Info("resolving bundle")
+ var bm *ocv1.BundleMetadata
+ if state.revisionStates.Installed != nil {
+ bm = &state.revisionStates.Installed.BundleMetadata
+ }
+ resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolve(ctx, ext, bm)
+
+ // Get the installed bundle name for deprecation status.
+ // BundleDeprecated should reflect what's currently running, not what we're trying to install.
+ installedBundleName := ""
+ if state.revisionStates.Installed != nil {
+ installedBundleName = state.revisionStates.Installed.Name
+ }
+
+ // Set deprecation status based on resolution results:
+ // - If resolution succeeds: hasCatalogData=true, deprecation shows catalog data (nil=not deprecated)
+ // - If resolution fails but returns deprecation: hasCatalogData=true, show package/channel deprecation warnings
+ // - If resolution fails with nil deprecation: hasCatalogData=false, all conditions go Unknown
+ //
+ // TODO: Open question - what if different catalogs have different opinions of what's deprecated?
+ // If we can't resolve a bundle, how do we know which catalog to trust for deprecation information?
+ // Perhaps if the package shows up in multiple catalogs and deprecations don't match, we can set
+ // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from
+ // all catalogs? This needs a follow-up discussion and PR.
+ hasCatalogData := err == nil || resolvedDeprecation != nil
+ SetDeprecationStatus(ext, installedBundleName, resolvedDeprecation, hasCatalogData)
+
+ if err != nil {
+ // Note: We don't distinguish between resolution-specific errors and generic errors
+ setStatusProgressing(ext, err)
+ setInstalledStatusFromRevisionStates(ext, state.revisionStates)
+ ensureFailureConditionsWithReason(ext, ocv1.ReasonFailed, err.Error())
+ return nil, err
+ }
+
+ state.resolvedRevisionMetadata = &RevisionMetadata{
+ Package: resolvedBundle.Package,
+ Image: resolvedBundle.Image,
+ // TODO: Right now, operator-controller only supports registry+v1 bundles and has no concept
+ // of a "release" field. If/when we add a release field concept or a new bundle format
+ // we need to re-evaluate use of `AsLegacyRegistryV1Version` so that we avoid propagating
+ // registry+v1's semver spec violations of treating build metadata as orderable.
+ BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, resolvedBundleVersion.AsLegacyRegistryV1Version()),
}
- state.resolvedRevisionMetadata = resolvedRevisionMetadata
return nil, nil
}
}
diff --git a/internal/operator-controller/controllers/common_controller_test.go b/internal/operator-controller/controllers/common_controller_test.go
index 4d0a0536d..93fad962e 100644
--- a/internal/operator-controller/controllers/common_controller_test.go
+++ b/internal/operator-controller/controllers/common_controller_test.go
@@ -146,7 +146,7 @@ func TestClusterExtensionDeprecationMessageTruncation(t *testing.T) {
deprecationMessages = append(deprecationMessages, fmt.Sprintf("API version 'v1beta1' of resource 'customresources%d.example.com' is deprecated, use 'v1' instead", i))
}
- longDeprecationMsg := strings.Join(deprecationMessages, "; ")
+ longDeprecationMsg := strings.Join(deprecationMessages, "\n")
setInstalledStatusConditionUnknown(ext, longDeprecationMsg)
cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled)
diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml
index 8d56b5ad4..7372fdfdb 100644
--- a/manifests/experimental-e2e.yaml
+++ b/manifests/experimental-e2e.yaml
@@ -1478,12 +1478,13 @@ spec:
When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
- When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml
index 324a0fe4c..e44ffc785 100644
--- a/manifests/experimental.yaml
+++ b/manifests/experimental.yaml
@@ -1439,12 +1439,13 @@ spec:
When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
- When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml
index 51c3b412c..677bd66af 100644
--- a/manifests/standard-e2e.yaml
+++ b/manifests/standard-e2e.yaml
@@ -1119,12 +1119,13 @@ spec:
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
- When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
diff --git a/manifests/standard.yaml b/manifests/standard.yaml
index 16c489f76..c58a16a37 100644
--- a/manifests/standard.yaml
+++ b/manifests/standard.yaml
@@ -1080,12 +1080,13 @@ spec:
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
- When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
- These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
- - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
- - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
- - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+ When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+ These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+ Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+ - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+ - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+ - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+ - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
From bb8fe7d16c15f3d4192948ae5ea4fbdfd782ccdd Mon Sep 17 00:00:00 2001
From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Date: Fri, 9 Jan 2026 17:38:32 +0000
Subject: [PATCH 2/2] Fix infinite reconcile loop in SetDeprecationStatus
Address review feedback from Per: change "remove all, then add" pattern
to "if adding then set, else remove" to preserve lastTransitionTime
when status unchanged. Prevents infinite reconciliation loops.
Add tests verifying multiple reconciles stabilize correctly.
---
.../clusterextension_controller.go | 22 +-
.../clusterextension_controller_test.go | 204 ++++++++++++++++++
2 files changed, 219 insertions(+), 7 deletions(-)
diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go
index c079329d6..52ab98b62 100644
--- a/internal/operator-controller/controllers/clusterextension_controller.go
+++ b/internal/operator-controller/controllers/clusterextension_controller.go
@@ -217,12 +217,11 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string
channelMessages := collectDeprecationMessages(info.ChannelEntries)
bundleMessages := collectDeprecationMessages(info.BundleEntries)
- // Clear all deprecation conditions first, then only add the ones we need.
+ // Strategy: Only remove conditions when we're NOT going to re-add them.
+ // If we're setting a condition, call SetStatusCondition directly - it preserves
+ // lastTransitionTime when status/reason/message haven't changed, preventing
+ // infinite reconciliation loops.
// Absence of a deprecation condition means "not deprecated" - keeps output clean.
- apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeDeprecated)
- apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypePackageDeprecated)
- apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeChannelDeprecated)
- apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeBundleDeprecated)
if !hasCatalogData {
// When catalog is unavailable, set all to Unknown.
@@ -264,8 +263,8 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string
return
}
- // Only add conditions when there's something to report (True or Unknown states).
- // False (not deprecated) is represented by absence of the condition.
+ // Handle catalog data available: set conditions to True when deprecated,
+ // or remove them when not deprecated (absence = not deprecated).
messages := slices.Concat(packageMessages, channelMessages, bundleMessages)
if len(messages) > 0 {
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
@@ -275,6 +274,9 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string
Message: strings.Join(messages, "\n"),
ObservedGeneration: ext.GetGeneration(),
})
+ } else {
+ // Only remove if we're not setting it - prevents unnecessary lastTransitionTime updates
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeDeprecated)
}
if len(packageMessages) > 0 {
@@ -285,6 +287,8 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string
Message: strings.Join(packageMessages, "\n"),
ObservedGeneration: ext.GetGeneration(),
})
+ } else {
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypePackageDeprecated)
}
if len(channelMessages) > 0 {
@@ -295,6 +299,8 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string
Message: strings.Join(channelMessages, "\n"),
ObservedGeneration: ext.GetGeneration(),
})
+ } else {
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeChannelDeprecated)
}
// BundleDeprecated: Unknown when no bundle installed, True when deprecated, absent otherwise
@@ -314,6 +320,8 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string
Message: strings.Join(bundleMessages, "\n"),
ObservedGeneration: ext.GetGeneration(),
})
+ } else {
+ apimeta.RemoveStatusCondition(&ext.Status.Conditions, ocv1.TypeBundleDeprecated)
}
}
diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go
index ef12048b7..2e0ea11b8 100644
--- a/internal/operator-controller/controllers/clusterextension_controller_test.go
+++ b/internal/operator-controller/controllers/clusterextension_controller_test.go
@@ -15,6 +15,7 @@ import (
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage/driver"
+ "k8s.io/apimachinery/pkg/api/equality"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
@@ -1960,6 +1961,209 @@ func TestSetDeprecationStatus(t *testing.T) {
}
}
+// TestSetDeprecationStatus_NoInfiniteReconcileLoop verifies that calling SetDeprecationStatus
+// multiple times with the same inputs does not cause infinite reconciliation loops.
+//
+// The issue: If we always remove and re-add conditions, lastTransitionTime updates every time,
+// which causes DeepEqual to fail, triggering another reconcile indefinitely.
+//
+// The fix: Only remove conditions when we're NOT re-adding them. When setting a condition,
+// call SetStatusCondition directly - it preserves lastTransitionTime when status/reason/message
+// haven't changed.
+func TestSetDeprecationStatus_NoInfiniteReconcileLoop(t *testing.T) {
+ tests := []struct {
+ name string
+ installedBundleName string
+ deprecation *declcfg.Deprecation
+ hasCatalogData bool
+ setupConditions func(*ocv1.ClusterExtension)
+ expectConditionsCount int
+ description string
+ }{
+ {
+ name: "deprecated package - should stabilize after first reconcile",
+ installedBundleName: "test.v1.0.0",
+ deprecation: &declcfg.Deprecation{
+ Entries: []declcfg.DeprecationEntry{
+ {
+ Reference: declcfg.PackageScopedReference{
+ Schema: declcfg.SchemaPackage,
+ },
+ Message: "package is deprecated",
+ },
+ },
+ },
+ hasCatalogData: true,
+ setupConditions: func(ext *ocv1.ClusterExtension) {
+ // No conditions initially
+ },
+ expectConditionsCount: 2, // Deprecated and PackageDeprecated
+ description: "First call adds conditions, second call preserves lastTransitionTime",
+ },
+ {
+ name: "not deprecated - migration from False to absent",
+ installedBundleName: "", // No bundle installed
+ deprecation: nil,
+ hasCatalogData: true,
+ setupConditions: func(ext *ocv1.ClusterExtension) {
+ // Simulate old behavior: False conditions present
+ apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypeDeprecated,
+ Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecated,
+ Message: "",
+ ObservedGeneration: 1,
+ })
+ apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+ Type: ocv1.TypePackageDeprecated,
+ Status: metav1.ConditionFalse,
+ Reason: ocv1.ReasonDeprecated,
+ Message: "",
+ ObservedGeneration: 1,
+ })
+ },
+ expectConditionsCount: 1, // Only BundleDeprecated Unknown (no bundle installed)
+ description: "Migrates from False to absent, then stabilizes",
+ },
+ {
+ name: "catalog unavailable - should stabilize with Unknown conditions",
+ installedBundleName: "test.v1.0.0",
+ deprecation: nil,
+ hasCatalogData: false,
+ setupConditions: func(ext *ocv1.ClusterExtension) {
+ // No conditions initially
+ },
+ expectConditionsCount: 4, // All four Unknown conditions
+ description: "Sets Unknown conditions, then preserves them",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ ext := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
+ },
+ Status: ocv1.ClusterExtensionStatus{
+ Conditions: []metav1.Condition{},
+ },
+ }
+
+ // Setup initial conditions if specified
+ if tt.setupConditions != nil {
+ tt.setupConditions(ext)
+ }
+
+ // First reconcile: should add/update conditions
+ controllers.SetDeprecationStatus(ext, tt.installedBundleName, tt.deprecation, tt.hasCatalogData)
+
+ firstReconcileConditions := make([]metav1.Condition, len(ext.Status.Conditions))
+ copy(firstReconcileConditions, ext.Status.Conditions)
+
+ // Verify expected number of conditions
+ deprecationConditions := filterDeprecationConditions(ext.Status.Conditions)
+ require.Len(t, deprecationConditions, tt.expectConditionsCount,
+ "First reconcile should have %d deprecation conditions", tt.expectConditionsCount)
+
+ // Second reconcile: should preserve lastTransitionTime (no changes)
+ controllers.SetDeprecationStatus(ext, tt.installedBundleName, tt.deprecation, tt.hasCatalogData)
+
+ secondReconcileConditions := ext.Status.Conditions
+
+ // Verify conditions are identical (including lastTransitionTime)
+ require.Len(t, secondReconcileConditions, len(firstReconcileConditions),
+ "Number of conditions should remain the same")
+
+ for i, firstCond := range firstReconcileConditions {
+ secondCond := secondReconcileConditions[i]
+ require.Equal(t, firstCond.Type, secondCond.Type, "Condition type should match")
+ require.Equal(t, firstCond.Status, secondCond.Status, "Condition status should match")
+ require.Equal(t, firstCond.Reason, secondCond.Reason, "Condition reason should match")
+ require.Equal(t, firstCond.Message, secondCond.Message, "Condition message should match")
+
+ // This is the critical check: lastTransitionTime should NOT change
+ require.Equal(t, firstCond.LastTransitionTime, secondCond.LastTransitionTime,
+ "lastTransitionTime should be preserved (prevents infinite reconcile loop)")
+ }
+
+ // Third reconcile: verify it remains stable
+ controllers.SetDeprecationStatus(ext, tt.installedBundleName, tt.deprecation, tt.hasCatalogData)
+
+ thirdReconcileConditions := ext.Status.Conditions
+ require.Len(t, thirdReconcileConditions, len(secondReconcileConditions),
+ "Conditions should remain stable after multiple reconciles")
+
+ for i, secondCond := range secondReconcileConditions {
+ thirdCond := thirdReconcileConditions[i]
+ require.Equal(t, secondCond.LastTransitionTime, thirdCond.LastTransitionTime,
+ "lastTransitionTime should remain stable across reconciles")
+ }
+ })
+ }
+}
+
+// TestSetDeprecationStatus_StatusChangesOnlyWhenNeeded verifies that calling SetDeprecationStatus
+// only modifies the status when actual deprecation state changes, not on every reconcile.
+func TestSetDeprecationStatus_StatusChangesOnlyWhenNeeded(t *testing.T) {
+ ext := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Generation: 1,
+ },
+ Status: ocv1.ClusterExtensionStatus{
+ Conditions: []metav1.Condition{},
+ },
+ }
+
+ // Scenario 1: Package becomes deprecated
+ deprecation := &declcfg.Deprecation{
+ Entries: []declcfg.DeprecationEntry{
+ {
+ Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage},
+ Message: "package is deprecated",
+ },
+ },
+ }
+
+ // First reconcile: add deprecation condition
+ controllers.SetDeprecationStatus(ext, "test.v1.0.0", deprecation, true)
+ statusAfterFirstReconcile := ext.Status.DeepCopy()
+
+ // Second reconcile: same deprecation state
+ controllers.SetDeprecationStatus(ext, "test.v1.0.0", deprecation, true)
+ statusAfterSecondReconcile := ext.Status.DeepCopy()
+
+ // Status should be semantically equal (DeepEqual would return true)
+ require.True(t, equality.Semantic.DeepEqual(statusAfterFirstReconcile, statusAfterSecondReconcile),
+ "Status should not change when deprecation state is unchanged")
+
+ // Scenario 2: Deprecation is resolved (package no longer deprecated)
+ controllers.SetDeprecationStatus(ext, "test.v1.0.0", nil, true)
+ statusAfterResolution := ext.Status.DeepCopy()
+
+ // Status should have changed (conditions removed)
+ require.False(t, equality.Semantic.DeepEqual(statusAfterSecondReconcile, statusAfterResolution),
+ "Status should change when deprecation is resolved")
+
+ // Scenario 3: Verify resolution is stable
+ controllers.SetDeprecationStatus(ext, "test.v1.0.0", nil, true)
+ statusAfterFourthReconcile := ext.Status.DeepCopy()
+
+ require.True(t, equality.Semantic.DeepEqual(statusAfterResolution, statusAfterFourthReconcile),
+ "Status should remain stable after deprecation is resolved")
+}
+
+// filterDeprecationConditions returns only the deprecation-related conditions
+func filterDeprecationConditions(conditions []metav1.Condition) []metav1.Condition {
+ var result []metav1.Condition
+ for _, cond := range conditions {
+ switch cond.Type {
+ case ocv1.TypeDeprecated, ocv1.TypePackageDeprecated, ocv1.TypeChannelDeprecated, ocv1.TypeBundleDeprecated:
+ result = append(result, cond)
+ }
+ }
+ return result
+}
+
type MockActionGetter struct {
description string
rels []*release.Release