From c15272d98c3f94d86e4ffbee0e392921c14b765f Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Fri, 31 Oct 2025 18:03:34 +0000 Subject: [PATCH] Fix deprecation conditions leaking install errors - stop copying install/validation errors into deprecation conditions - base bundle deprecation on the installed bundle (Unknown when none) - extend unit tests to cover resolver failures with catalog deprecations - update docs to provide a more compreehnesinve info Assisted-by: Cursor --- api/v1/clusterextension_types.go | 10 +- docs/api-reference/olmv1-api-reference.md | 2 +- ...peratorframework.io_clusterextensions.yaml | 10 +- ...peratorframework.io_clusterextensions.yaml | 10 +- .../clusterextension_admission_test.go | 28 +- .../clusterextension_controller.go | 241 ++++++++++++------ .../clusterextension_controller_test.go | 207 ++++++++++++++- .../controllers/common_controller_test.go | 2 +- manifests/experimental-e2e.yaml | 10 +- manifests/experimental.yaml | 10 +- manifests/standard-e2e.yaml | 10 +- manifests/standard.yaml | 10 +- test/e2e/cluster_extension_install_test.go | 24 +- 13 files changed, 433 insertions(+), 141 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6de62b0e12..57415294ab 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -483,12 +483,12 @@ type ClusterExtensionStatus struct { // 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, if may also communicate a deprecation condition. + // 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. - // 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. + // PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + // ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + // BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + // Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. // // +listType=map // +listMapKey=type diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index b21e404520..38c513c0f1 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_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.

The Installed condition represents whether or not 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 the ClusterExtension is sourced from a catalog, if 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_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.

The Installed condition represents whether or not 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 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.
PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False.
ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False.
BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated.
Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. | | | | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | | 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 1038b7fdf0..51b341655a 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 @@ -518,12 +518,12 @@ 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, if may also communicate a deprecation condition. + 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. - 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. + PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. 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 a0983e41f9..1f004ad1d9 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 @@ -479,12 +479,12 @@ 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, if may also communicate a deprecation condition. + 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. - 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. + PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index 38c6c60d41..8b572dbe8c 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 7bcedde656..91e2b60fd3 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -25,6 +25,7 @@ import ( "slices" "strings" + bsemver "github.com/blang/semver/v4" "github.com/go-logr/logr" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" @@ -139,13 +140,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, @@ -157,6 +165,17 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C } } +// 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 + } +} + // Compare resources - ignoring status & metadata.finalizers func checkForUnexpectedClusterExtensionFieldChange(a, b ocv1.ClusterExtension) bool { a.Status, b.Status = ocv1.ClusterExtensionStatus{}, ocv1.ClusterExtensionStatus{} @@ -229,6 +248,23 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl return ctrl.Result{}, err } + // Hold deprecation updates until the end. That way: + // * if nothing installs, BundleDeprecated stays Unknown/Absent + // * if a bundle installs, we report its real deprecation status + // * install errors never leak into the deprecation conditions + var resolvedDeprecation *declcfg.Deprecation + defer func(resolvedDeprecationPtr **declcfg.Deprecation, revisionStatesPtr **RevisionStates) { + installedBundleName := "" + if revisionStatesPtr != nil && *revisionStatesPtr != nil && (*revisionStatesPtr).Installed != nil { + installedBundleName = (*revisionStatesPtr).Installed.Name + } + var resolvedDeprecationVal *declcfg.Deprecation + if resolvedDeprecationPtr != nil { + resolvedDeprecationVal = *resolvedDeprecationPtr + } + SetDeprecationStatus(ext, installedBundleName, resolvedDeprecationVal) + }(&resolvedDeprecation, &revisionStates) + var resolvedRevisionMetadata *RevisionMetadata if len(revisionStates.RollingOut) == 0 { l.Info("resolving bundle") @@ -236,30 +272,21 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl if revisionStates.Installed != nil { bm = &revisionStates.Installed.BundleMetadata } - resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, bm) + var resolvedBundle *declcfg.Bundle + var resolvedBundleVersion *bsemver.Version + resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err = r.Resolver.Resolve(ctx, ext, bm) + // Keep any deprecation data the resolver returned. The deferred update will use it + // even if installation later fails or never begins. if err != nil { // Note: We don't distinguish between resolution-specific errors and generic errors setStatusProgressing(ext, err) setInstalledStatusFromRevisionStates(ext, revisionStates) - ensureAllConditionsWithReason(ext, ocv1.ReasonFailed, err.Error()) + // Ensure non-deprecation conditions capture the failure immediately. The deferred + // SetDeprecationStatus call is responsible for updating the deprecation conditions + // based on any catalog data returned by the resolver. + ensureFailureConditionsWithReason(ext, ocv1.ReasonFailed, err.Error()) return ctrl.Result{}, err } - - // 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, @@ -326,83 +353,141 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl return ctrl.Result{}, nil } -// 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{} - channelSet := sets.New[string]() +// 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 +} + +// SetDeprecationStatus updates the ClusterExtension deprecation conditions using the +// catalog data from resolve plus the name of the bundle that actually landed. Examples: +// - no bundle installed -> bundle status stays Unknown/Absent +// - installed bundle marked deprecated -> bundle status True/Deprecated +// - installed bundle not deprecated -> bundle status False/Deprecated +// +// This keeps the deprecation conditions focused on catalog information: +// - PackageDeprecated: true if the catalog marks the package deprecated +// - ChannelDeprecated: true if any requested channel is marked deprecated +// - BundleDeprecated: reflects the installed bundle (Unknown/Absent when nothing installed) +// - Deprecated (rollup): true if any of the above signals a deprecation +// +// Install or validation errors never appear here because they belong on the +// Progressing/Installed conditions instead. Callers should invoke this after reconcile +// finishes (for example via a defer) so catalog data replaces any transient error messages. +func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, deprecation *declcfg.Deprecation) { + info := buildDeprecationInfo(ext, installedBundleName, deprecation) + + packageMessages := collectDeprecationMessages(info.PackageEntries) + channelMessages := collectDeprecationMessages(info.ChannelEntries) + bundleMessages := collectDeprecationMessages(info.BundleEntries) + + messages := slices.Concat(packageMessages, channelMessages, bundleMessages) + + status := metav1.ConditionFalse + if len(messages) > 0 { + status = metav1.ConditionTrue + } + + SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypeDeprecated, + Status: status, + Reason: ocv1.ReasonDeprecated, + Message: strings.Join(messages, "\n"), + ObservedGeneration: ext.GetGeneration(), + }) + + SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypePackageDeprecated, + Status: conditionStatus(len(packageMessages) > 0), + Reason: ocv1.ReasonDeprecated, + Message: strings.Join(packageMessages, "\n"), + ObservedGeneration: ext.GetGeneration(), + }) + + SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypeChannelDeprecated, + Status: conditionStatus(len(channelMessages) > 0), + Reason: ocv1.ReasonDeprecated, + Message: strings.Join(channelMessages, "\n"), + ObservedGeneration: ext.GetGeneration(), + }) + + bundleReason := ocv1.ReasonDeprecated + bundleMessage := strings.Join(bundleMessages, "\n") + if info.BundleStatus == metav1.ConditionUnknown { + bundleReason = ocv1.ReasonAbsent + bundleMessage = "" + } + + SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypeBundleDeprecated, + Status: info.BundleStatus, + Reason: bundleReason, + Message: bundleMessage, + ObservedGeneration: ext.GetGeneration(), + }) +} + +// 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} + var channelSet sets.Set[string] if ext.Spec.Source.Catalog != nil { - for _, channel := range ext.Spec.Source.Catalog.Channels { - channelSet.Insert(channel) - } + channelSet = sets.New(ext.Spec.Source.Catalog.Channels...) + } else { + channelSet = sets.New[string]() } + 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, - }) + return info +} - // 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) - } +// 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 +} + +func conditionStatus(ok bool) metav1.ConditionStatus { + if ok { + return metav1.ConditionTrue + } + return metav1.ConditionFalse } 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 437f62dcec..a84f7329d4 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -31,6 +31,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" "github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" + "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" @@ -172,6 +173,58 @@ func TestClusterExtensionResolutionFails(t *testing.T) { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) } +func TestClusterExtensionResolutionFailsWithDeprecationData(t *testing.T) { + ctx := context.Background() + cl, reconciler := newClientAndReconciler(t) + deprecationMessage := "package marked deprecated in catalog" + pkgName := fmt.Sprintf("deprecated-%s", rand.String(6)) + reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *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{})) +} + func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { type testCase struct { name string @@ -264,6 +317,24 @@ 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") + deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated) + require.NotNil(t, deprecatedCond) + require.Equal(t, metav1.ConditionFalse, deprecatedCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, deprecatedCond.Reason) + pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated) + require.NotNil(t, pkgCond) + require.Equal(t, metav1.ConditionFalse, pkgCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, pkgCond.Reason) + chanCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated) + require.NotNil(t, chanCond) + require.Equal(t, metav1.ConditionFalse, chanCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, chanCond.Reason) + 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{})) }) } @@ -343,6 +414,118 @@ 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") + deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated) + require.NotNil(t, deprecatedCond) + require.Equal(t, metav1.ConditionFalse, deprecatedCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, deprecatedCond.Reason) + pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated) + require.NotNil(t, pkgCond) + require.Equal(t, metav1.ConditionFalse, pkgCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, pkgCond.Reason) + chanCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated) + require.NotNil(t, chanCond) + require.Equal(t, metav1.ConditionFalse, chanCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, chanCond.Reason) + 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{})) +} + +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) + // Boxcutter keeps a rolling revision when apply fails. We mirror that state so the test uses + // the same inputs the runtime would see. + reconciler.RevisionStatesGetter = &MockRevisionStatesGetter{ + RevisionStates: &controllers.RevisionStates{ + RollingOut: []*controllers.RevisionMetadata{{}}, + }, + } + reconciler.ImagePuller = &imageutil.MockPuller{ImageFS: fstest.MapFS{}} + + 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)) + + reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := 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 + }) + reconciler.Applier = &MockApplier{err: errors.New("boxcutter apply failure")} + + 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.ConditionFalse, deprecatedCond.Status) + require.Equal(t, ocv1.ReasonDeprecated, deprecatedCond.Reason) + require.Empty(t, deprecatedCond.Message) + + packageCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated) + require.NotNil(t, packageCond) + require.Equal(t, metav1.ConditionFalse, packageCond.Status, "catalog said nothing about the package, so stay False") + require.Equal(t, ocv1.ReasonDeprecated, packageCond.Reason) + require.Empty(t, packageCond.Message) + + channelCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated) + require.NotNil(t, channelCond) + require.Equal(t, metav1.ConditionFalse, channelCond.Status, "channel also has no deprecation info") + require.Equal(t, ocv1.ReasonDeprecated, channelCond.Reason) + require.Empty(t, 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.Empty(t, bundleCond.Message) + require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) } @@ -887,8 +1070,8 @@ func TestSetDeprecationStatus(t *testing.T) { }, { Type: ocv1.TypeBundleDeprecated, - Reason: ocv1.ReasonDeprecated, - Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + Status: metav1.ConditionUnknown, ObservedGeneration: 1, }, }, @@ -945,8 +1128,8 @@ func TestSetDeprecationStatus(t *testing.T) { }, { Type: ocv1.TypeBundleDeprecated, - Reason: ocv1.ReasonDeprecated, - Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + Status: metav1.ConditionUnknown, ObservedGeneration: 1, }, }, @@ -1014,8 +1197,8 @@ func TestSetDeprecationStatus(t *testing.T) { }, { Type: ocv1.TypeBundleDeprecated, - Reason: ocv1.ReasonDeprecated, - Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + Status: metav1.ConditionUnknown, ObservedGeneration: 1, }, }, @@ -1085,8 +1268,8 @@ func TestSetDeprecationStatus(t *testing.T) { }, { Type: ocv1.TypeBundleDeprecated, - Reason: ocv1.ReasonDeprecated, - Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + Status: metav1.ConditionUnknown, ObservedGeneration: 1, }, }, @@ -1321,8 +1504,8 @@ func TestSetDeprecationStatus(t *testing.T) { }, { Type: ocv1.TypeBundleDeprecated, - Reason: ocv1.ReasonDeprecated, - Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + Status: metav1.ConditionUnknown, ObservedGeneration: 1, }, }, @@ -1399,8 +1582,8 @@ func TestSetDeprecationStatus(t *testing.T) { }, { Type: ocv1.TypeBundleDeprecated, - Reason: ocv1.ReasonDeprecated, - Status: metav1.ConditionFalse, + Reason: ocv1.ReasonAbsent, + Status: metav1.ConditionUnknown, ObservedGeneration: 1, }, }, diff --git a/internal/operator-controller/controllers/common_controller_test.go b/internal/operator-controller/controllers/common_controller_test.go index 4d0a0536d1..93fad962e8 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 1efa8b8d99..81cea096a4 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1323,12 +1323,12 @@ 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, if may also communicate a deprecation condition. + 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. - 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. + PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. 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 664f8599cc..0e3f0cee2c 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1288,12 +1288,12 @@ 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, if may also communicate a deprecation condition. + 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. - 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. + PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. 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 783beec515..6bdc2ed5d5 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -1070,12 +1070,12 @@ 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, if may also communicate a deprecation condition. + 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. - 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. + PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. 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 95e400c264..9eb0341ba2 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -1035,12 +1035,12 @@ 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, if may also communicate a deprecation condition. + 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. - 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. + PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False. + ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False. + BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated. + Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index ab0bf48b1c..6ac6087b9f 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "slices" "testing" "time" @@ -656,6 +657,12 @@ func TestClusterExtensionRecoversFromNoNamespaceWhenFailureFixed(t *testing.T) { require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) require.Contains(ct, cond.Message, "Installed bundle") require.NotEmpty(ct, clusterExtension.Status.Install) + + bundleDeprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated) + require.NotNil(ct, bundleDeprecatedCond) + require.Equal(ct, metav1.ConditionFalse, bundleDeprecatedCond.Status) + require.Equal(ct, ocv1.ReasonDeprecated, bundleDeprecatedCond.Reason) + require.Empty(ct, bundleDeprecatedCond.Message) }, pollDuration, pollInterval) t.Log("By eventually reporting Progressing == True with Reason Success") @@ -755,11 +762,20 @@ func TestClusterExtensionRecoversFromExistingDeploymentWhenFailureFixed(t *testi cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) require.NotNil(ct, cond) require.Equal(ct, metav1.ConditionFalse, cond.Status) - // TODO: We probably _should_ be testing the reason here, but helm and boxcutter applier have different reasons. - // Maybe we change helm to use "Absent" rather than "Failed" since the Progressing condition already captures - // the failure? - //require.Equal(ct, ocv1.ReasonFailed, cond.Reason) + // Helm uses Failed, Boxcutter uses Absent; both are fine here. + require.True(ct, slices.Contains([]string{ocv1.ReasonFailed, ocv1.ReasonAbsent}, cond.Reason)) require.Contains(ct, cond.Message, "No bundle installed") + + deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated) + require.NotNil(ct, deprecatedCond) + require.Equal(ct, metav1.ConditionFalse, deprecatedCond.Status) + require.Empty(ct, deprecatedCond.Message) + + bundleDeprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated) + require.NotNil(ct, bundleDeprecatedCond) + require.Equal(ct, metav1.ConditionUnknown, bundleDeprecatedCond.Status) + require.Equal(ct, ocv1.ReasonAbsent, bundleDeprecatedCond.Reason) + require.Empty(ct, bundleDeprecatedCond.Message) }, pollDuration, pollInterval) t.Log("By deleting the new Deployment")