Skip to content

🐛 Fix deprecation conditions leaking install errors #2296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 Fix deprecation conditions leaking install errors #2296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions api/v1/clusterextension_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/api-reference/olmv1-api-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br /><br />The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.<br />When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />When Installed is False and the Reason is Failed, the bundle has failed to install.<br /><br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br /><br />When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.<br />ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.<br />PackageDeprecated is set if the requested package is marked deprecated in the catalog.<br />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.<br /><br />The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.<br />When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />When Installed is False and the Reason is Failed, the bundle has failed to install.<br /><br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br /><br />When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False.<br />ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False.<br />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.<br />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. | | |


Expand Down
10 changes: 5 additions & 5 deletions ...base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions ...mv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 18 additions & 10 deletions internal/operator-controller/controllers/clusterextension_admission_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ 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
testCases := []struct {
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()
Expand Down Expand Up @@ -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)
})
}
}
Expand Down
Loading
Loading