diff --git a/Makefile b/Makefile index edb8950935..a645e17c2c 100644 --- a/Makefile +++ b/Makefile @@ -254,7 +254,7 @@ $(eval $(call install-sh,standard,operator-controller-standard.yaml)) .PHONY: test test: manifests generate fmt lint test-unit test-e2e test-regression #HELP Run all tests. -E2E_TIMEOUT ?= 15m +E2E_TIMEOUT ?= 20m GODOG_ARGS ?= .PHONY: e2e e2e: #EXHELP Run the e2e tests. @@ -316,7 +316,7 @@ test-experimental-e2e: COVERAGE_NAME := experimental-e2e test-experimental-e2e: export MANIFEST := $(EXPERIMENTAL_RELEASE_MANIFEST) test-experimental-e2e: export INSTALL_DEFAULT_CATALOGS := false test-experimental-e2e: PROMETHEUS_VALUES := helm/prom_experimental.yaml -test-experimental-e2e: E2E_TIMEOUT := 20m +test-experimental-e2e: E2E_TIMEOUT := 25m test-experimental-e2e: run-internal prometheus e2e e2e-coverage kind-clean #HELP Run experimental e2e test suite on local kind cluster .PHONY: prometheus diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index cf5946a408..12614d1a43 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -466,6 +466,29 @@ type BundleMetadata struct { // +required // +kubebuilder:validation:XValidation:rule="self.matches(\"^([0-9]+)(\\\\.[0-9]+)?(\\\\.[0-9]+)?(-([-0-9A-Za-z]+(\\\\.[-0-9A-Za-z]+)*))?(\\\\+([-0-9A-Za-z]+(-\\\\.[-0-9A-Za-z]+)*))?\")",message="version must be well-formed semver" Version string `json:"version"` + + // release is an optional field that identifies a specific release of this bundle's version. + // A release represents a re-publication of the same version, typically used to deliver + // packaging or metadata changes without changing the version number. When multiple + // releases exist for the same version, higher releases are preferred. An unset release + // is less preferred than all other release values. + // + // The value consists of dot-separated identifiers, where each identifier is either a + // numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + // "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + // compared as integers, alphanumeric identifiers are compared lexically, and numeric + // identifiers always sort before alphanumeric identifiers. + // + // For bundles with explicit pkg.Release metadata, this field contains that release value. + // For registry+v1 bundles lacking an explicit release value, this field contains the release + // extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + // This field is omitted when the bundle's release value is unset. + // + // +optional + // + // +kubebuilder:validation:MaxLength=20 + // +kubebuilder:validation:XValidation:rule="self.matches(\"^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$\")",message="release must be empty or consist of dot-separated identifiers (numeric without leading zeros, or alphanumeric)" + Release *string `json:"release,omitempty"` } // RevisionStatus defines the observed state of a ClusterObjectSet. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 384439787b..ef6f145b9e 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -46,6 +46,11 @@ func (in *Assertion) DeepCopy() *Assertion { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BundleMetadata) DeepCopyInto(out *BundleMetadata) { *out = *in + if in.Release != nil { + in, out := &in.Release, &out.Release + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BundleMetadata. @@ -312,7 +317,7 @@ func (in *ClusterExtensionInstallConfig) DeepCopy() *ClusterExtensionInstallConf // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterExtensionInstallStatus) DeepCopyInto(out *ClusterExtensionInstallStatus) { *out = *in - out.Bundle = in.Bundle + in.Bundle.DeepCopyInto(&out.Bundle) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionInstallStatus. @@ -397,7 +402,7 @@ func (in *ClusterExtensionStatus) DeepCopyInto(out *ClusterExtensionStatus) { if in.Install != nil { in, out := &in.Install, &out.Install *out = new(ClusterExtensionInstallStatus) - **out = **in + (*in).DeepCopyInto(*out) } if in.ActiveRevisions != nil { in, out := &in.ActiveRevisions, &out.ActiveRevisions diff --git a/applyconfigurations/api/v1/bundlemetadata.go b/applyconfigurations/api/v1/bundlemetadata.go index acf9d152f8..f6ba9dfdab 100644 --- a/applyconfigurations/api/v1/bundlemetadata.go +++ b/applyconfigurations/api/v1/bundlemetadata.go @@ -29,6 +29,25 @@ type BundleMetadataApplyConfiguration struct { // version is required and references the version that this bundle represents. // It follows the semantic versioning standard as defined in https://semver.org/. Version *string `json:"version,omitempty"` + // release is an optional field that identifies a specific release of this bundle's version. + // A release represents a re-publication of the same version, typically used to deliver + // packaging or metadata changes without changing the version number. When multiple + // releases exist for the same version, higher releases are preferred. An unset release + // is less preferred than all other release values. + // + // The value consists of dot-separated identifiers, where each identifier is either a + // numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + // "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + // compared as integers, alphanumeric identifiers are compared lexically, and numeric + // identifiers always sort before alphanumeric identifiers. + // + // For bundles with explicit pkg.Release metadata, this field contains that release value. + // For registry+v1 bundles lacking an explicit release value, this field contains the release + // extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + // This field is omitted when the bundle's release value is unset. + // + // + Release *string `json:"release,omitempty"` } // BundleMetadataApplyConfiguration constructs a declarative configuration of the BundleMetadata type for use with @@ -52,3 +71,11 @@ func (b *BundleMetadataApplyConfiguration) WithVersion(value string) *BundleMeta b.Version = &value return b } + +// WithRelease sets the Release field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Release field is set to the value of the last call. +func (b *BundleMetadataApplyConfiguration) WithRelease(value string) *BundleMetadataApplyConfiguration { + b.Release = &value + return b +} diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 5b5eeb2361..37be2e3ac3 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -67,6 +67,7 @@ _Appears in:_ | --- | --- | --- | --- | | `name` _string_ | name is required and follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.),
start and end with an alphanumeric character, and be no longer than 253 characters. | | Required: \{\}
| | `version` _string_ | version is required and references the version that this bundle represents.
It follows the semantic versioning standard as defined in https://semver.org/. | | Required: \{\}
| +| `release` _string_ | release is an optional field that identifies a specific release of this bundle's version.
A release represents a re-publication of the same version, typically used to deliver
packaging or metadata changes without changing the version number. When multiple
releases exist for the same version, higher releases are preferred. An unset release
is less preferred than all other release values.
The value consists of dot-separated identifiers, where each identifier is either a
numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
"3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
compared as integers, alphanumeric identifiers are compared lexically, and numeric
identifiers always sort before alphanumeric identifiers.
For bundles with explicit pkg.Release metadata, this field contains that release value.
For registry+v1 bundles lacking an explicit release value, this field contains the release
extracted from version's build metadata (e.g., '2' from '1.0.0+2').
This field is omitted when the bundle's release value is unset.
| | MaxLength: 20
Optional: \{\}
| #### CRDUpgradeSafetyEnforcement diff --git a/docs/designs/testing/2026-04-13-e2e-isolation/design.md b/docs/designs/testing/2026-04-13-e2e-isolation/design.md index b006d26a91..97e112c44b 100644 --- a/docs/designs/testing/2026-04-13-e2e-isolation/design.md +++ b/docs/designs/testing/2026-04-13-e2e-isolation/design.md @@ -12,7 +12,7 @@ Each scenario dynamically builds and pushes its own bundle and catalog OCI image parameterized by scenario ID. All cluster-scoped resource names include the scenario ID, making conflicts structurally impossible. -``` +```text Scenario starts -> Generate parameterized bundle manifests (CRD names, deployments, etc. include scenario ID) -> Build + push bundle OCI images to e2e registry via go-containerregistry diff --git a/go.mod b/go.mod index 9fa2070c1d..70ea48ea2a 100644 --- a/go.mod +++ b/go.mod @@ -6,15 +6,15 @@ require ( github.com/BurntSushi/toml v1.6.0 github.com/Masterminds/semver/v3 v3.4.0 github.com/blang/semver/v4 v4.0.0 - github.com/cert-manager/cert-manager v1.20.1 - github.com/containerd/containerd v1.7.30 + github.com/cert-manager/cert-manager v1.20.2 + github.com/containerd/containerd v1.7.31 github.com/cucumber/godog v0.15.1 github.com/evanphx/json-patch v5.9.11+incompatible github.com/fsnotify/fsnotify v1.9.0 github.com/go-logr/logr v1.4.3 github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/go-cmp v0.7.0 - github.com/google/go-containerregistry v0.21.4 + github.com/google/go-containerregistry v0.21.5 github.com/google/renameio/v2 v2.0.2 github.com/gorilla/handlers v1.5.2 github.com/klauspost/compress v1.18.5 @@ -49,7 +49,7 @@ require ( sigs.k8s.io/controller-runtime v0.23.3 sigs.k8s.io/controller-tools v0.20.1 sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 - sigs.k8s.io/structured-merge-diff/v6 v6.3.2 + sigs.k8s.io/structured-merge-diff/v6 v6.4.0 sigs.k8s.io/yaml v1.6.0 ) @@ -97,7 +97,7 @@ require ( github.com/cyphar/filepath-securejoin v0.6.1 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/distribution/reference v0.6.0 // indirect - github.com/docker/cli v29.3.1+incompatible // indirect + github.com/docker/cli v29.4.0+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect github.com/docker/docker v28.5.2+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.5 // indirect diff --git a/go.sum b/go.sum index a8bf64ba85..86a1c4d88a 100644 --- a/go.sum +++ b/go.sum @@ -47,8 +47,8 @@ github.com/bshuster-repo/logrus-logstash-hook v1.0.0/go.mod h1:zsTqEiSzDgAa/8GZR github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= -github.com/cert-manager/cert-manager v1.20.1 h1:99ExHJu5TPp1V92AvvE4oY6BkOSyJiWLxxMkbqbdGaY= -github.com/cert-manager/cert-manager v1.20.1/go.mod h1:ut67FnggYJJqAdDWLhSPnj10P06QwbNU88RYNh9MvMc= +github.com/cert-manager/cert-manager v1.20.2 h1:CimnY00nLqB2lmxhoSuEC4GDMFDK7JCXqyjwMM9ndIQ= +github.com/cert-manager/cert-manager v1.20.2/go.mod h1:1g/+a/WK5zWH/dXPZa3dMD3aJQJNRXQu+PN17C6WrOw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/chai2010/gettext-go v1.0.3 h1:9liNh8t+u26xl5ddmWLmsOsdNLwkdRTg5AG+JnTiM80= @@ -59,8 +59,8 @@ github.com/clipperhouse/uax29/v2 v2.6.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsV github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/containerd/cgroups/v3 v3.1.2 h1:OSosXMtkhI6Qove637tg1XgK4q+DhR0mX8Wi8EhrHa4= github.com/containerd/cgroups/v3 v3.1.2/go.mod h1:PKZ2AcWmSBsY/tJUVhtS/rluX0b1uq1GmPO1ElCmbOw= -github.com/containerd/containerd v1.7.30 h1:/2vezDpLDVGGmkUXmlNPLCCNKHJ5BbC5tJB5JNzQhqE= -github.com/containerd/containerd v1.7.30/go.mod h1:fek494vwJClULlTpExsmOyKCMUAbuVjlFsJQc4/j44M= +github.com/containerd/containerd v1.7.31 h1:jn3IMuTV4Bb1Uwb0MFPW2ASJAD3W1lh6QqqZHIZwDh4= +github.com/containerd/containerd v1.7.31/go.mod h1:jdwD6s/BhV4XVJGrvtziNPVA+83n66TwptVaPKprq4E= github.com/containerd/containerd/api v1.10.0 h1:5n0oHYVBwN4VhoX9fFykCV9dF1/BvAXeg2F8W6UYq1o= github.com/containerd/containerd/api v1.10.0/go.mod h1:NBm1OAk8ZL+LG8R0ceObGxT5hbUYj7CzTmR3xh0DlMM= github.com/containerd/continuity v0.4.5 h1:ZRoN1sXq9u7V6QoHMcVWGhOwDFqZ4B9i5H6un1Wh0x4= @@ -114,8 +114,8 @@ github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5Qvfr github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= -github.com/docker/cli v29.3.1+incompatible h1:M04FDj2TRehDacrosh7Vlkgc7AuQoWloQkf1PA5hmoI= -github.com/docker/cli v29.3.1+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/cli v29.4.0+incompatible h1:+IjXULMetlvWJiuSI0Nbor36lcJ5BTcVpUmB21KBoVM= +github.com/docker/cli v29.4.0+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker v28.5.2+incompatible h1:DBX0Y0zAjZbSrm1uzOkdr1onVghKaftjlSWt4AFexzM= @@ -260,8 +260,8 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/go-containerregistry v0.21.4 h1:VrhlIQtdhE6riZW//MjPrcJ1snAjPoCCpPHqGOygrv8= -github.com/google/go-containerregistry v0.21.4/go.mod h1:kxgc23zQ2qMY/hAKt0wCbB/7tkeovAP2mE2ienynJUw= +github.com/google/go-containerregistry v0.21.5 h1:KTJG9Pn/jC0VdZR6ctV3/jcN+q6/Iqlx0sTVz3ywZlM= +github.com/google/go-containerregistry v0.21.5/go.mod h1:ySvMuiWg+dOsRW0Hw8GYwfMwBlNRTmpYBFJPlkco5zU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -824,7 +824,7 @@ sigs.k8s.io/kustomize/kyaml v0.21.1 h1:IVlbmhC076nf6foyL6Taw4BkrLuEsXUXNpsE+ScX7 sigs.k8s.io/kustomize/kyaml v0.21.1/go.mod h1:hmxADesM3yUN2vbA5z1/YTBnzLJ1dajdqpQonwBL1FQ= sigs.k8s.io/randfill v1.0.0 h1:JfjMILfT8A6RbawdsK2JXGBR5AQVfd+9TbzrlneTyrU= sigs.k8s.io/randfill v1.0.0/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY= -sigs.k8s.io/structured-merge-diff/v6 v6.3.2 h1:kwVWMx5yS1CrnFWA/2QHyRVJ8jM6dBA80uLmm0wJkk8= -sigs.k8s.io/structured-merge-diff/v6 v6.3.2/go.mod h1:M3W8sfWvn2HhQDIbGWj3S099YozAsymCo/wrT5ohRUE= +sigs.k8s.io/structured-merge-diff/v6 v6.4.0 h1:qmp2e3ZfFi1/jJbDGpD4mt3wyp6PE1NfKHCYLqgNQJo= +sigs.k8s.io/structured-merge-diff/v6 v6.4.0/go.mod h1:M3W8sfWvn2HhQDIbGWj3S099YozAsymCo/wrT5ohRUE= sigs.k8s.io/yaml v1.6.0 h1:G8fkbMSAFqgEFgh4b1wmtzDnioxFCUgTZhlbj5P9QYs= sigs.k8s.io/yaml v1.6.0/go.mod h1:796bPqUfzR/0jLAl6XjHl3Ck7MiyVv8dbTdyT3/pMf4= diff --git a/helm/experimental.yaml b/helm/experimental.yaml index b158389d48..d5fe64b227 100644 --- a/helm/experimental.yaml +++ b/helm/experimental.yaml @@ -7,6 +7,8 @@ # to pull in resources or additions options: operatorController: + deployment: + replicas: 2 features: enabled: - SingleOwnNamespaceInstallSupport @@ -14,12 +16,15 @@ options: - HelmChartSupport - BoxcutterRuntime - DeploymentConfig + - BundleReleaseSupport disabled: - WebhookProviderOpenshiftServiceCA # List of enabled experimental features for catalogd # Use with {{- if has "FeatureGate" .Values.options.catalogd.features.enabled }} # to pull in resources or additions catalogd: + deployment: + replicas: 2 features: enabled: - APIV1MetasHandler 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 aebb9e72be..8e5f53f62c 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 @@ -688,6 +688,30 @@ spec: hyphens (-) or periods (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") + release: + description: |- + release is an optional field that identifies a specific release of this bundle's version. + A release represents a re-publication of the same version, typically used to deliver + packaging or metadata changes without changing the version number. When multiple + releases exist for the same version, higher releases are preferred. An unset release + is less preferred than all other release values. + + The value consists of dot-separated identifiers, where each identifier is either a + numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + compared as integers, alphanumeric identifiers are compared lexically, and numeric + identifiers always sort before alphanumeric identifiers. + + For bundles with explicit pkg.Release metadata, this field contains that release value. + For registry+v1 bundles lacking an explicit release value, this field contains the release + extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + This field is omitted when the bundle's release value is unset. + maxLength: 20 + type: string + x-kubernetes-validations: + - message: release must be empty or consist of dot-separated + identifiers (numeric without leading zeros, or alphanumeric) + rule: self.matches("^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$") version: description: |- version is required and references the version that this bundle represents. diff --git a/internal/catalogd/serverutil/serverutil.go b/internal/catalogd/serverutil/serverutil.go index 143d4c8763..72e1e5652f 100644 --- a/internal/catalogd/serverutil/serverutil.go +++ b/internal/catalogd/serverutil/serverutil.go @@ -1,7 +1,9 @@ package serverutil import ( + "context" "crypto/tls" + "errors" "fmt" "io" "net" @@ -13,7 +15,7 @@ import ( "github.com/klauspost/compress/gzhttp" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/certwatcher" - "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/healthz" catalogdmetrics "github.com/operator-framework/operator-controller/internal/catalogd/metrics" "github.com/operator-framework/operator-controller/internal/catalogd/storage" @@ -27,49 +29,116 @@ type CatalogServerConfig struct { LocalStorage storage.Instance } -func AddCatalogServerToManager(mgr ctrl.Manager, cfg CatalogServerConfig, tlsFileWatcher *certwatcher.CertWatcher) error { - listener, err := net.Listen("tcp", cfg.CatalogAddr) +// AddCatalogServerToManager adds the catalog HTTP server to the manager and registers +// a readiness check that passes once the server has started serving. Because +// NeedLeaderElection returns false, Start() is called on every pod immediately, so all +// replicas bind the catalog port and become ready. Non-leader pods serve requests but +// return 404 (empty local cache); callers are expected to retry. +func AddCatalogServerToManager(mgr ctrl.Manager, cfg CatalogServerConfig, cw *certwatcher.CertWatcher) error { + shutdownTimeout := 30 * time.Second + r := &catalogServerRunnable{ + cfg: cfg, + cw: cw, + server: &http.Server{ + Addr: cfg.CatalogAddr, + Handler: storageServerHandlerWrapped(mgr.GetLogger().WithName("catalogd-http-server"), cfg), + ReadTimeout: 5 * time.Second, + WriteTimeout: 5 * time.Minute, + }, + shutdownTimeout: shutdownTimeout, + ready: make(chan struct{}), + } + + if err := mgr.Add(r); err != nil { + return fmt.Errorf("error adding catalog server to manager: %w", err) + } + + // Register a readiness check that passes once Start() has been called and the + // server is actively serving. All pods reach Start() (NeedLeaderElection=false), + // so all replicas become ready and receive traffic; non-leaders return 404 until + // they win the leader lease and populate their local cache. + if err := mgr.AddReadyzCheck("catalog-server", r.readyzCheck()); err != nil { + return fmt.Errorf("error adding catalog server readiness check: %w", err) + } + + return nil +} + +// catalogServerRunnable is a Runnable that binds the catalog HTTP port on every pod. +// Because NeedLeaderElection returns false, Start() is called on all replicas immediately; +// non-leader pods serve the catalog port but return 404 (empty local cache). +type catalogServerRunnable struct { + cfg CatalogServerConfig + cw *certwatcher.CertWatcher + server *http.Server + shutdownTimeout time.Duration + // ready is closed by Start() once the server is about to begin serving. + ready chan struct{} +} + +// NeedLeaderElection returns false so the catalog server starts on every pod +// immediately, regardless of leadership. This is required for rolling updates: +// if Start() were gated on leadership, a new pod could not win the leader lease +// (held by the still-running old pod) and therefore could never pass the +// catalog-server readiness check, deadlocking the rollout. +// +// Non-leader pods serve the catalog HTTP port but have an empty local cache +// (only the leader's reconciler downloads catalog content), so requests to a +// non-leader return 404. Callers are expected to retry. +func (r *catalogServerRunnable) NeedLeaderElection() bool { return false } + +func (r *catalogServerRunnable) Start(ctx context.Context) error { + listener, err := net.Listen("tcp", r.cfg.CatalogAddr) if err != nil { return fmt.Errorf("error creating catalog server listener: %w", err) } - if cfg.CertFile != "" && cfg.KeyFile != "" { - // Use the passed certificate watcher instead of creating a new one + if r.cfg.CertFile != "" && r.cfg.KeyFile != "" { config := &tls.Config{ - GetCertificate: tlsFileWatcher.GetCertificate, + GetCertificate: r.cw.GetCertificate, MinVersion: tls.VersionTLS12, } listener = tls.NewListener(listener, config) } - shutdownTimeout := 30 * time.Second - catalogServer := manager.Server{ - Name: "catalogs", - OnlyServeWhenLeader: true, - Server: &http.Server{ - Addr: cfg.CatalogAddr, - Handler: storageServerHandlerWrapped(mgr.GetLogger().WithName("catalogd-http-server"), cfg), - ReadTimeout: 5 * time.Second, - // TODO: Revert this to 10 seconds if/when the API - // evolves to have significantly smaller responses - WriteTimeout: 5 * time.Minute, - }, - ShutdownTimeout: &shutdownTimeout, - Listener: listener, - } + // Signal readiness before blocking on Serve so the readiness probe passes promptly. + close(r.ready) - err = mgr.Add(&catalogServer) - if err != nil { - return fmt.Errorf("error adding catalog server to manager: %w", err) - } + go func() { + <-ctx.Done() + shutdownCtx := context.Background() + if r.shutdownTimeout > 0 { + var cancel context.CancelFunc + shutdownCtx, cancel = context.WithTimeout(shutdownCtx, r.shutdownTimeout) + defer cancel() + } + if err := r.server.Shutdown(shutdownCtx); err != nil { + // Shutdown errors (e.g. context deadline exceeded) are not actionable; + // the process is terminating regardless. + _ = err + } + }() + if err := r.server.Serve(listener); err != nil && !errors.Is(err, http.ErrServerClosed) { + return fmt.Errorf("catalog server on %q failed: %w", r.cfg.CatalogAddr, err) + } return nil } +// readyzCheck returns a healthz.Checker that passes once Start() has been called. +func (r *catalogServerRunnable) readyzCheck() healthz.Checker { + return func(_ *http.Request) error { + select { + case <-r.ready: + return nil + default: + return fmt.Errorf("catalog server not yet started") + } + } +} + func logrLoggingHandler(l logr.Logger, handler http.Handler) http.Handler { return handlers.CustomLoggingHandler(nil, handler, func(_ io.Writer, params handlers.LogFormatterParams) { - // extract parameters used in apache common log format, but then log using `logr` to remain consistent - // with other loggers used in this codebase. username := "-" if params.URL.User != nil { if name := params.URL.User.Username(); name != "" { diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 3c7f0c00a6..efd2c01a19 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -85,6 +85,9 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( if v := helmRelease.Labels[labels.BundleVersionKey]; v != "" { annotationUpdates[labels.BundleVersionKey] = v } + if v, ok := helmRelease.Labels[labels.BundleReleaseKey]; ok { + annotationUpdates[labels.BundleReleaseKey] = v + } if v := helmRelease.Labels[labels.PackageNameKey]; v != "" { annotationUpdates[labels.PackageNameKey] = v } @@ -96,12 +99,16 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( WithObject(obj)) } - rev := r.buildClusterObjectSet(objs, ext, map[string]string{ + revisionAnnotations := map[string]string{ labels.BundleNameKey: helmRelease.Labels[labels.BundleNameKey], labels.PackageNameKey: helmRelease.Labels[labels.PackageNameKey], labels.BundleVersionKey: helmRelease.Labels[labels.BundleVersionKey], labels.BundleReferenceKey: helmRelease.Labels[labels.BundleReferenceKey], - }) + } + if v, ok := helmRelease.Labels[labels.BundleReleaseKey]; ok { + revisionAnnotations[labels.BundleReleaseKey] = v + } + rev := r.buildClusterObjectSet(objs, ext, revisionAnnotations) rev.WithName(fmt.Sprintf("%s-1", ext.Name)) rev.Spec.WithRevision(1) rev.Spec.WithCollisionProtection(ocv1.CollisionProtectionNone) // allow to adopt objects from previous release diff --git a/internal/operator-controller/bundle/versionrelease.go b/internal/operator-controller/bundle/versionrelease.go index 03fbc8e507..48ab9d0a16 100644 --- a/internal/operator-controller/bundle/versionrelease.go +++ b/internal/operator-controller/bundle/versionrelease.go @@ -85,6 +85,19 @@ func (vr *VersionRelease) AsLegacyRegistryV1Version() bsemver.Version { type Release []bsemver.PRVersion +// String returns the string representation of the release. +// Returns an empty string if the release is nil or empty. +func (r Release) String() string { + if len(r) == 0 { + return "" + } + parts := make([]string, len(r)) + for i, pr := range r { + parts[i] = pr.String() + } + return strings.Join(parts, ".") +} + // Compare compares two Release values. It returns: // // -1 if r < other diff --git a/internal/operator-controller/bundleutil/bundle.go b/internal/operator-controller/bundleutil/bundle.go index 2771c52593..956feba539 100644 --- a/internal/operator-controller/bundleutil/bundle.go +++ b/internal/operator-controller/bundleutil/bundle.go @@ -11,34 +11,99 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" ) func GetVersionAndRelease(b declcfg.Bundle) (*bundle.VersionRelease, error) { for _, p := range b.Properties { if p.Type == property.TypePackage { - var pkg property.Package - if err := json.Unmarshal(p.Value, &pkg); err != nil { - return nil, fmt.Errorf("error unmarshalling package property: %w", err) - } - - // TODO: For now, we assume that all bundles are registry+v1 bundles. - // In the future, when we support other bundle formats, we should stop - // using the legacy mechanism (i.e. using build metadata in the version) - // to determine the bundle's release. - vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version) - if err != nil { - return nil, err - } - return vr, nil + return parseVersionRelease(p.Value) } } return nil, fmt.Errorf("no package property found in bundle %q", b.Name) } -// MetadataFor returns a BundleMetadata for the given bundle name and version. -func MetadataFor(bundleName string, bundleVersion bsemver.Version) ocv1.BundleMetadata { +func parseVersionRelease(pkgData json.RawMessage) (*bundle.VersionRelease, error) { + var pkg property.Package + if err := json.Unmarshal(pkgData, &pkg); err != nil { + return nil, fmt.Errorf("error unmarshalling package property: %w", err) + } + + // Check if release field is explicitly present in JSON (even if empty). + // property.Package has Release string, so we can't distinguish "field absent" from "field empty". + // We unmarshal again into a helper struct with Release *string to detect presence. + var releaseField struct { + Release *string `json:"release"` + } + if err := json.Unmarshal(pkgData, &releaseField); err != nil { + return nil, fmt.Errorf("error unmarshalling package release field: %w", err) + } + + // When BundleReleaseSupport is enabled and bundle has explicit release field, use it. + if features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) && releaseField.Release != nil { + return parseExplicitRelease(pkg.Version, *releaseField.Release) + } + + // Fall back to legacy registry+v1 behavior (release in build metadata) + // + // TODO: For now, we assume that all bundles are registry+v1 bundles. + // In the future, for supporting other bundle formats, we should not + // use the legacy registry+v1 mechanism (i.e. using build metadata in + // the version) to determine the bundle's release. + vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version) + if err != nil { + return nil, err + } + return vr, nil +} + +// parseExplicitRelease parses version and release from separate fields. +// Build metadata is preserved in the version because with an explicit release field, +// build metadata serves its proper semver purpose (e.g., git commit, build number). +// In contrast, NewLegacyRegistryV1VersionRelease clears build metadata because it +// interprets build metadata AS the release value for registry+v1 bundles. +func parseExplicitRelease(version, releaseStr string) (*bundle.VersionRelease, error) { + vers, err := bsemver.Parse(version) + if err != nil { + return nil, fmt.Errorf("error parsing version %q: %w", version, err) + } + + var rel bundle.Release + if releaseStr == "" { + // Explicit empty release: use empty slice (not nil) + rel = bundle.Release([]bsemver.PRVersion{}) + } else { + rel, err = bundle.NewRelease(releaseStr) + if err != nil { + return nil, fmt.Errorf("error parsing release %q: %w", releaseStr, err) + } + } + + return &bundle.VersionRelease{ + Version: vers, + Release: rel, + }, nil +} + +// MetadataFor returns a BundleMetadata for the given bundle name and version/release. +func MetadataFor(bundleName string, vr bundle.VersionRelease) ocv1.BundleMetadata { + if features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) { + // New behavior: separate Version and Release fields + bm := ocv1.BundleMetadata{ + Name: bundleName, + Version: vr.Version.String(), + } + if vr.Release != nil { + relStr := vr.Release.String() + bm.Release = &relStr + } + return bm + } + // Old behavior for backward compatibility: reconstitute build metadata in Version field + // This preserves release information (e.g., "1.0.0+2") for standard CRD users where + // the Release field is pruned by the API server. return ocv1.BundleMetadata{ Name: bundleName, - Version: bundleVersion.String(), + Version: vr.AsLegacyRegistryV1Version().String(), } } diff --git a/internal/operator-controller/bundleutil/bundle_test.go b/internal/operator-controller/bundleutil/bundle_test.go index 1cdabad054..925604a8bf 100644 --- a/internal/operator-controller/bundleutil/bundle_test.go +++ b/internal/operator-controller/bundleutil/bundle_test.go @@ -2,6 +2,7 @@ package bundleutil_test import ( "encoding/json" + "fmt" "testing" bsemver "github.com/blang/semver/v4" @@ -12,6 +13,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" ) func TestGetVersionAndRelease(t *testing.T) { @@ -83,12 +85,257 @@ func TestGetVersionAndRelease(t *testing.T) { Properties: properties, } - _, err := bundleutil.GetVersionAndRelease(bundle) + actual, err := bundleutil.GetVersionAndRelease(bundle) if tc.wantErr { require.Error(t, err) } else { require.NoError(t, err) + require.Equal(t, tc.wantVersionRelease, actual) } }) } } + +// TestGetVersionAndRelease_WithBundleReleaseSupport tests the feature-gated parsing behavior. +// Explicitly sets the gate to test both enabled and disabled paths. +func TestGetVersionAndRelease_WithBundleReleaseSupport(t *testing.T) { + t.Run("gate enabled - parses explicit release field", func(t *testing.T) { + // Enable the feature gate for this test + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=true")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + tests := []struct { + name string + pkgProperty *property.Property + wantVersionRelease *bundle.VersionRelease + wantErr bool + }{ + { + name: "explicit release field - takes precedence over build metadata", + pkgProperty: &property.Property{ + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "1.0.0+ignored", "release": "2"}`), + }, + wantVersionRelease: &bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0+ignored"), // Build metadata preserved - serves its proper semver purpose + Release: bundle.Release([]bsemver.PRVersion{ + {VersionNum: 2, IsNum: true}, + }), + }, + wantErr: false, + }, + { + name: "explicit release field - complex release", + pkgProperty: &property.Property{ + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "2.1.0", "release": "1.alpha.3"}`), + }, + wantVersionRelease: &bundle.VersionRelease{ + Version: bsemver.MustParse("2.1.0"), + Release: bundle.Release([]bsemver.PRVersion{ + {VersionNum: 1, IsNum: true}, + {VersionStr: "alpha"}, + {VersionNum: 3, IsNum: true}, + }), + }, + wantErr: false, + }, + { + name: "explicit release field - invalid release", + pkgProperty: &property.Property{ + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "1.0.0", "release": "001"}`), + }, + wantErr: true, + }, + { + name: "explicit empty release - preserves build metadata in version", + pkgProperty: &property.Property{ + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "1.0.0+2", "release": ""}`), + }, + wantVersionRelease: &bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0+2"), // Build metadata preserved (not extracted as release) + Release: bundle.Release([]bsemver.PRVersion{}), // Explicit empty release + }, + wantErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + bundle := declcfg.Bundle{ + Name: "test-bundle", + Properties: []property.Property{*tc.pkgProperty}, + } + + actual, err := bundleutil.GetVersionAndRelease(bundle) + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantVersionRelease, actual) + } + }) + } + }) + + t.Run("gate disabled - ignores explicit release field, uses build metadata", func(t *testing.T) { + // Disable the feature gate for this test + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=false")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + // When gate disabled, explicit release field is ignored and parsing falls back to legacy behavior + bundleWithExplicitRelease := declcfg.Bundle{ + Name: "test-bundle", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "1.0.0+2", "release": "999"}`), + }, + }, + } + + actual, err := bundleutil.GetVersionAndRelease(bundleWithExplicitRelease) + require.NoError(t, err) + + // Should parse build metadata (+2), not explicit release field (999) + expected := &bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: bundle.Release([]bsemver.PRVersion{ + {VersionNum: 2, IsNum: true}, + }), + } + require.Equal(t, expected, actual) + }) +} + +func TestMetadataFor(t *testing.T) { + t.Run("with feature gate enabled", func(t *testing.T) { + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=true")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + t.Run("with release", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: bundle.Release([]bsemver.PRVersion{{VersionNum: 2, IsNum: true}}), + } + result := bundleutil.MetadataFor("test-bundle", vr) + require.Equal(t, "test-bundle", result.Name) + require.Equal(t, "1.0.0", result.Version) + require.NotNil(t, result.Release) + require.Equal(t, "2", *result.Release) + }) + + t.Run("without release", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: nil, + } + result := bundleutil.MetadataFor("test-bundle", vr) + require.Equal(t, "test-bundle", result.Name) + require.Equal(t, "1.0.0", result.Version) + require.Nil(t, result.Release) + }) + + t.Run("with explicit empty release", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: bundle.Release([]bsemver.PRVersion{}), + } + result := bundleutil.MetadataFor("test-bundle", vr) + require.Equal(t, "test-bundle", result.Name) + require.Equal(t, "1.0.0", result.Version) + require.NotNil(t, result.Release) + require.Empty(t, *result.Release) + }) + }) + + t.Run("with feature gate disabled (legacy mode)", func(t *testing.T) { + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=false")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + t.Run("reconstitutes build metadata in version", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: bundle.Release([]bsemver.PRVersion{{VersionNum: 2, IsNum: true}}), + } + result := bundleutil.MetadataFor("test-bundle", vr) + require.Equal(t, "test-bundle", result.Name) + require.Equal(t, "1.0.0+2", result.Version) // Build metadata reconstituted + require.Nil(t, result.Release) // Release field not used in legacy mode + }) + + t.Run("preserves original build metadata when no release", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: nil, + } + result := bundleutil.MetadataFor("test-bundle", vr) + require.Equal(t, "test-bundle", result.Name) + require.Equal(t, "1.0.0", result.Version) + require.Nil(t, result.Release) + }) + }) +} + +func TestGetVersionAndRelease_Errors(t *testing.T) { + t.Run("invalid JSON for release field detection", func(t *testing.T) { + // Enable gate for this test + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=true")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + // Malformed JSON that can't be unmarshalled for release detection + bundle := declcfg.Bundle{ + Name: "test-bundle", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "1.0.0", "release": invalid}`), + }, + }, + } + + _, err := bundleutil.GetVersionAndRelease(bundle) + require.Error(t, err) + require.Contains(t, err.Error(), "error unmarshalling package") + }) + + t.Run("invalid version in explicit release path", func(t *testing.T) { + // Enable gate for this test + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=true")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + bundle := declcfg.Bundle{ + Name: "test-bundle", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "not-a-version", "release": "1"}`), + }, + }, + } + + _, err := bundleutil.GetVersionAndRelease(bundle) + require.Error(t, err) + require.Contains(t, err.Error(), "error parsing version") + }) +} diff --git a/internal/operator-controller/catalogmetadata/client/client.go b/internal/operator-controller/catalogmetadata/client/client.go index 0d08c40ef5..e79fdfbcc5 100644 --- a/internal/operator-controller/catalogmetadata/client/client.go +++ b/internal/operator-controller/catalogmetadata/client/client.go @@ -106,8 +106,10 @@ func (c *Client) PopulateCache(ctx context.Context, catalog *ocv1.ClusterCatalog defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - errToCache := fmt.Errorf("error: received unexpected response status code %d", resp.StatusCode) - return c.cache.Put(catalog.Name, catalog.Status.ResolvedSource.Image.Ref, nil, errToCache) + // Do not cache non-200 responses (e.g. 404 from a non-leader catalogd pod). + // Returning the error directly lets the next reconcile retry a fresh HTTP + // request and eventually hit the leader. + return nil, fmt.Errorf("error: received unexpected response status code %d", resp.StatusCode) } return c.cache.Put(catalog.Name, catalog.Status.ResolvedSource.Image.Ref, resp.Body, nil) diff --git a/internal/operator-controller/catalogmetadata/client/client_test.go b/internal/operator-controller/catalogmetadata/client/client_test.go index 00a226873e..66817c504b 100644 --- a/internal/operator-controller/catalogmetadata/client/client_test.go +++ b/internal/operator-controller/catalogmetadata/client/client_test.go @@ -234,13 +234,6 @@ func TestClientPopulateCache(t *testing.T) { }}, }, nil }, - putFuncConstructor: func(t *testing.T) func(source string, errToCache error) (fs.FS, error) { - return func(source string, errToCache error) (fs.FS, error) { - assert.Empty(t, source) - assert.Error(t, errToCache) - return nil, errToCache - } - }, assert: func(t *testing.T, fs fs.FS, err error) { assert.Nil(t, fs) assert.ErrorContains(t, err, "received unexpected response status code 500") diff --git a/internal/operator-controller/catalogmetadata/filter/successors.go b/internal/operator-controller/catalogmetadata/filter/successors.go index 975c8cb39f..969beea72d 100644 --- a/internal/operator-controller/catalogmetadata/filter/successors.go +++ b/internal/operator-controller/catalogmetadata/filter/successors.go @@ -12,15 +12,51 @@ import ( "github.com/operator-framework/operator-controller/internal/shared/util/filter" ) +// parseInstalledBundleVersionRelease constructs a VersionRelease from BundleMetadata. +// If the Release field is not nil (even if empty string), use it as the explicit release. +// If the Release field is nil, parse release from version build metadata (registry+v1 legacy format). +func parseInstalledBundleVersionRelease(installedBundle ocv1.BundleMetadata) (*bundle.VersionRelease, error) { + // Handle legacy registry+v1 format: release embedded in version's build metadata + if installedBundle.Release == nil { + vr, err := bundle.NewLegacyRegistryV1VersionRelease(installedBundle.Version) + if err != nil { + return nil, fmt.Errorf("failed to get version and release of installed bundle: %w", err) + } + return vr, nil + } + + // Bundle has explicit release field (or explicitly empty) - parse version and release from separate fields. + // Note: We can't use NewLegacyRegistryV1VersionRelease here because the version might + // already contain build metadata (e.g., "1.0.0+git.abc"), which serves its proper + // semver purpose when using explicit pkg.Release. Concatenating would create invalid + // semver like "1.0.0+git.abc+2". + version, err := bsemver.Parse(installedBundle.Version) + if err != nil { + return nil, fmt.Errorf("failed to parse installed bundle version %q: %w", installedBundle.Version, err) + } + + var release bundle.Release + if *installedBundle.Release == "" { + // Explicit empty release: use empty slice (not nil) to match catalog parsing behavior. + // NewRelease("") returns nil, but we need empty slice for roundtrip correctness. + release = bundle.Release([]bsemver.PRVersion{}) + } else { + release, err = bundle.NewRelease(*installedBundle.Release) + if err != nil { + return nil, fmt.Errorf("failed to parse installed bundle release %q: %w", *installedBundle.Release, err) + } + } + + return &bundle.VersionRelease{ + Version: version, + Release: release, + }, nil +} + func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) { - // TODO: We do not have an explicit field in our BundleMetadata for a bundle's release value. - // Legacy registry+v1 bundles embed the release value inside their versions as build metadata - // (in violation of the semver spec). If/when we add explicit release metadata to bundles and/or - // we support a new bundle format, we need to revisit the assumption that all bundles are - // registry+v1 and embed release in build metadata. - installedVersionRelease, err := bundle.NewLegacyRegistryV1VersionRelease(installedBundle.Version) + installedVersionRelease, err := parseInstalledBundleVersionRelease(installedBundle) if err != nil { - return nil, fmt.Errorf("failed to get version and release of installed bundle: %v", err) + return nil, err } successorsPredicate, err := legacySuccessor(installedBundle, channels...) diff --git a/internal/operator-controller/catalogmetadata/filter/successors_test.go b/internal/operator-controller/catalogmetadata/filter/successors_test.go index d22a1fdb2f..ed2508a7b5 100644 --- a/internal/operator-controller/catalogmetadata/filter/successors_test.go +++ b/internal/operator-controller/catalogmetadata/filter/successors_test.go @@ -4,7 +4,6 @@ import ( "slices" "testing" - bsemver "github.com/blang/semver/v4" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" @@ -14,11 +13,22 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/compare" "github.com/operator-framework/operator-controller/internal/shared/util/filter" ) +// mustVersionRelease is a test helper that parses a version string into a VersionRelease. +// For registry+v1 bundles, build metadata is interpreted as release (e.g., "1.0.0+2" -> Version: 1.0.0, Release: 2). +func mustVersionRelease(versionStr string) bundle.VersionRelease { + vr, err := bundle.NewLegacyRegistryV1VersionRelease(versionStr) + if err != nil { + panic(err) + } + return *vr +} + func TestSuccessorsPredicate(t *testing.T) { const testPackageName = "test-package" channelSet := map[string]declcfg.Channel{ @@ -122,7 +132,7 @@ func TestSuccessorsPredicate(t *testing.T) { }{ { name: "respect replaces directive from catalog", - installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", bsemver.MustParse("2.0.0")), + installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", mustVersionRelease("2.0.0")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which replaces the current version @@ -133,7 +143,7 @@ func TestSuccessorsPredicate(t *testing.T) { }, { name: "respect skips directive from catalog", - installedBundle: bundleutil.MetadataFor("test-package.v2.2.1", bsemver.MustParse("2.2.1")), + installedBundle: bundleutil.MetadataFor("test-package.v2.2.1", mustVersionRelease("2.2.1")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which skips the current version @@ -144,7 +154,7 @@ func TestSuccessorsPredicate(t *testing.T) { }, { name: "respect skipRange directive from catalog", - installedBundle: bundleutil.MetadataFor("test-package.v2.3.0", bsemver.MustParse("2.3.0")), + installedBundle: bundleutil.MetadataFor("test-package.v2.3.0", mustVersionRelease("2.3.0")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which is skipRanges the current version @@ -155,7 +165,7 @@ func TestSuccessorsPredicate(t *testing.T) { }, { name: "installed bundle matcher is exact", - installedBundle: bundleutil.MetadataFor("test-package.v2.0.0+1", bsemver.MustParse("2.0.0+1")), + installedBundle: bundleutil.MetadataFor("test-package.v2.0.0+1", mustVersionRelease("2.0.0+1")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which is skips the current version @@ -177,6 +187,28 @@ func TestSuccessorsPredicate(t *testing.T) { }, expectedResult: []declcfg.Bundle{}, }, + { + name: "explicit release field - non-empty", + installedBundle: ocv1.BundleMetadata{ + Name: "test-package.v1.0.0", + Version: "1.0.0", + Release: func() *string { s := "2"; return &s }(), + }, + // No matches expected: this bundle name doesn't exist in catalog. + // This test exercises the Release != nil code path in parseInstalledBundleVersionRelease. + expectedResult: []declcfg.Bundle{}, + }, + { + name: "explicit empty release with build metadata preserves build metadata", + installedBundle: ocv1.BundleMetadata{ + Name: "test-package.v1.0.0+git", + Version: "1.0.0+git", + Release: func() *string { s := ""; return &s }(), + }, + // No matches expected: this bundle name doesn't exist in catalog. + // This test exercises the empty string handling in parseInstalledBundleVersionRelease. + expectedResult: []declcfg.Bundle{}, + }, } { t.Run(tt.name, func(t *testing.T) { successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName]) @@ -240,3 +272,52 @@ func TestLegacySuccessor(t *testing.T) { assert.True(t, f(b5)) assert.False(t, f(emptyBundle)) } + +func stringPtr(s string) *string { + return &s +} + +func TestParseInstalledBundleVersionRelease_Errors(t *testing.T) { + t.Run("invalid version - legacy format", func(t *testing.T) { + installedBundle := ocv1.BundleMetadata{ + Name: "test", + Version: "invalid-version", + } + _, err := parseInstalledBundleVersionRelease(installedBundle) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to get version and release") + }) + + t.Run("invalid version - explicit release format", func(t *testing.T) { + installedBundle := ocv1.BundleMetadata{ + Name: "test", + Version: "invalid-version", + Release: stringPtr("1"), + } + _, err := parseInstalledBundleVersionRelease(installedBundle) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse installed bundle version") + }) + + t.Run("invalid release - explicit release format", func(t *testing.T) { + installedBundle := ocv1.BundleMetadata{ + Name: "test", + Version: "1.0.0", + Release: stringPtr("001"), + } + _, err := parseInstalledBundleVersionRelease(installedBundle) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse installed bundle release") + }) +} + +func TestSuccessorsOf_Errors(t *testing.T) { + t.Run("invalid installed bundle version", func(t *testing.T) { + installedBundle := ocv1.BundleMetadata{ + Name: "test", + Version: "invalid", + } + _, err := SuccessorsOf(installedBundle) + require.Error(t, err) + }) +} diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index 63b8c7ddb2..f340520fc7 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -71,6 +71,10 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e Version: rev.Annotations[labels.BundleVersionKey], }, } + // Only set Release if the annotation key exists (to distinguish "not set" from "explicitly empty") + if releaseValue, ok := rev.Annotations[labels.BundleReleaseKey]; ok { + rm.Release = &releaseValue + } if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { rs.Installed = rm @@ -105,6 +109,9 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, } + if state.resolvedRevisionMetadata.Release != nil { + revisionAnnotations[labels.BundleReleaseKey] = *state.resolvedRevisionMetadata.Release + } objLbls := map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, labels.OwnerNameKey: ext.GetName(), diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 22b6768514..6645d392f3 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -542,6 +542,10 @@ func (d *HelmRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *o Version: rel.Labels[labels.BundleVersionKey], }, } + // Only set Release if the label key exists (to distinguish "not set" from "explicitly empty") + if releaseValue, ok := rel.Labels[labels.BundleReleaseKey]; ok { + rs.Installed.Release = &releaseValue + } break } } diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index b86c500764..2726fe7c8c 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" @@ -2596,6 +2597,7 @@ func TestGetInstalledBundleHistory(t *testing.T) { labels.BundleNameKey: "test-ext", labels.BundleVersionKey: "1.0", labels.BundleReferenceKey: "bundle-ref", + labels.BundleReleaseKey: "2", }, }, }, @@ -2604,6 +2606,7 @@ func TestGetInstalledBundleHistory(t *testing.T) { BundleMetadata: ocv1.BundleMetadata{ Name: "test-ext", Version: "1.0", + Release: ptr.To("2"), }, Image: "bundle-ref", }, nil, diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 48507a2b7c..1c6bfcb29c 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -191,11 +191,13 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { 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()), + // MetadataFor accepts VersionRelease input and normalizes bundle metadata. + // - With BundleReleaseSupport enabled, it stores release information in BundleMetadata.Release + // (whether provided explicitly via pkg.Release or extracted from registry+v1 build metadata, + // e.g. "1.0.0+2" -> Release "2"). + // - Without BundleReleaseSupport, it preserves legacy behavior by embedding release information + // back into BundleMetadata.Version instead of emitting a separate Release field. + BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), } return nil, nil } @@ -414,6 +416,9 @@ func ApplyBundle(a Applier) ReconcileStepFunc { labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, } + if state.resolvedRevisionMetadata.Release != nil { + revisionAnnotations[labels.BundleReleaseKey] = *state.resolvedRevisionMetadata.Release + } objLbls := map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, labels.OwnerNameKey: ext.GetName(), diff --git a/internal/operator-controller/controllers/clusterobjectset_controller.go b/internal/operator-controller/controllers/clusterobjectset_controller.go index 2d81b71c92..bc081b6765 100644 --- a/internal/operator-controller/controllers/clusterobjectset_controller.go +++ b/internal/operator-controller/controllers/clusterobjectset_controller.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -23,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/util/workqueue" "k8s.io/utils/clock" "pkg.package-operator.run/boxcutter" "pkg.package-operator.run/boxcutter/machinery" @@ -32,8 +34,8 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -83,29 +85,6 @@ func (c *ClusterObjectSetReconciler) Reconcile(ctx context.Context, req ctrl.Req reconciledRev := existingRev.DeepCopy() res, reconcileErr := c.reconcile(ctx, reconciledRev) - if pd := existingRev.Spec.ProgressDeadlineMinutes; pd > 0 { - cnd := meta.FindStatusCondition(reconciledRev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) - isStillProgressing := cnd != nil && cnd.Status == metav1.ConditionTrue && cnd.Reason != ocv1.ReasonSucceeded - succeeded := meta.IsStatusConditionTrue(reconciledRev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) - // check if we reached the progress deadline only if the revision is still progressing and has not succeeded yet - if isStillProgressing && !succeeded { - timeout := time.Duration(pd) * time.Minute - if c.Clock.Since(existingRev.CreationTimestamp.Time) > timeout { - // progress deadline reached, reset any errors and stop reconciling this revision - markAsNotProgressing(reconciledRev, ocv1.ReasonProgressDeadlineExceeded, fmt.Sprintf("Revision has not rolled out for %d minute(s).", pd)) - reconcileErr = nil - res = ctrl.Result{} - } else if reconcileErr == nil { - // We want to requeue so far in the future that the next reconciliation - // can detect if the revision did not progress within the given timeout. - // Thus, we plan the next reconcile slightly after (+2secs) the timeout is passed. - drift := 2 * time.Second - requeueAfter := existingRev.CreationTimestamp.Time.Add(timeout).Add(drift).Sub(c.Clock.Now()).Round(time.Second) - l.Info(fmt.Sprintf("ProgressDeadline not exceeded, requeue after ~%v to check again.", requeueAfter)) - res = ctrl.Result{RequeueAfter: requeueAfter} - } - } - } // Do checks before any Update()s, as Update() may modify the resource structure! updateStatus := !equality.Semantic.DeepEqual(existingRev.Status, reconciledRev.Status) @@ -146,6 +125,10 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl return c.delete(ctx, cos) } + remaining, hasDeadline := durationUntilDeadline(c.Clock, cos) + isDeadlineExceeded := hasDeadline && remaining <= 0 + + // Blocked takes precedence over ProgressDeadlineExceeded: it is more actionable for the user. if err := c.verifyReferencedSecretsImmutable(ctx, cos); err != nil { l.Error(err, "referenced Secret verification failed, blocking reconciliation") markAsNotProgressing(cos, ocv1.ClusterObjectSetReasonBlocked, err.Error()) @@ -154,7 +137,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl phases, currentPhases, opts, err := c.buildBoxcutterPhases(ctx, cos) if err != nil { - setRetryingConditions(cos, err.Error()) + setRetryingConditions(l, cos, err.Error(), isDeadlineExceeded) return ctrl.Result{}, fmt.Errorf("converting to boxcutter revision: %v", err) } @@ -168,7 +151,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, cos) if err != nil { - setRetryingConditions(cos, err.Error()) + setRetryingConditions(l, cos, err.Error(), isDeadlineExceeded) return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err) } @@ -194,7 +177,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl if err := c.establishWatch(ctx, cos, revision); err != nil { werr := fmt.Errorf("establish watch: %v", err) - setRetryingConditions(cos, werr.Error()) + setRetryingConditions(l, cos, werr.Error(), isDeadlineExceeded) return ctrl.Result{}, werr } @@ -204,7 +187,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl // Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity. l.V(1).Info("reconcile report", "report", rres.String()) } - setRetryingConditions(cos, err.Error()) + setRetryingConditions(l, cos, err.Error(), isDeadlineExceeded) return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err) } @@ -212,14 +195,14 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl // TODO: report status, backoff? if verr := rres.GetValidationError(); verr != nil { l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s") - setRetryingConditions(cos, fmt.Sprintf("revision validation error: %s", verr)) + setRetryingConditions(l, cos, fmt.Sprintf("revision validation error: %s", verr), isDeadlineExceeded) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } for i, pres := range rres.GetPhases() { if verr := pres.GetValidationError(); verr != nil { l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", i) - setRetryingConditions(cos, fmt.Sprintf("phase %d validation error: %s", i, verr)) + setRetryingConditions(l, cos, fmt.Sprintf("phase %d validation error: %s", i, verr), isDeadlineExceeded) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } @@ -232,14 +215,14 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl if len(collidingObjs) > 0 { l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs) - setRetryingConditions(cos, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n"))) + setRetryingConditions(l, cos, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), isDeadlineExceeded) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } } revVersion := cos.GetAnnotations()[labels.BundleVersionKey] if rres.InTransition() { - markAsProgressing(cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion)) + markAsProgressing(l, cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion), isDeadlineExceeded) } //nolint:nestif @@ -259,7 +242,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl } } - markAsProgressing(cos, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion)) + markAsProgressing(l, cos, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion), isDeadlineExceeded) markAsAvailable(cos, ocv1.ClusterObjectSetReasonProbesSucceeded, "Objects are available and pass all probes.") // We'll probably only want to remove this once we are done updating the ClusterExtension conditions @@ -304,8 +287,9 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl } else { markAsUnavailable(cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion)) } - if meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) == nil { - markAsProgressing(cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion)) + markAsProgressing(l, cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion), isDeadlineExceeded) + if hasDeadline && !isDeadlineExceeded { + return ctrl.Result{RequeueAfter: remaining}, nil } } @@ -324,14 +308,15 @@ func (c *ClusterObjectSetReconciler) delete(ctx context.Context, cos *ocv1.Clust } func (c *ClusterObjectSetReconciler) archive(ctx context.Context, revisionEngine RevisionEngine, cos *ocv1.ClusterObjectSet, revision boxcutter.RevisionBuilder) (ctrl.Result, error) { + l := log.FromContext(ctx) tdres, err := revisionEngine.Teardown(ctx, revision) if err != nil { err = fmt.Errorf("error archiving revision: %v", err) - setRetryingConditions(cos, err.Error()) + setRetryingConditions(l, cos, err.Error(), false) return ctrl.Result{}, err } if tdres != nil && !tdres.IsComplete() { - setRetryingConditions(cos, "removing revision resources that are not owned by another revision") + setRetryingConditions(l, cos, "removing revision resources that are not owned by another revision", false) return ctrl.Result{RequeueAfter: 5 * time.Second}, nil } // Ensure conditions are set before removing the finalizer when archiving @@ -349,29 +334,19 @@ type Sourcoser interface { } func (c *ClusterObjectSetReconciler) SetupWithManager(mgr ctrl.Manager) error { - skipProgressDeadlineExceededPredicate := predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - rev, ok := e.ObjectNew.(*ocv1.ClusterObjectSet) - if !ok { - return true - } - // allow deletions to happen - if !rev.DeletionTimestamp.IsZero() { - return true - } - if cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing); cnd != nil && cnd.Status == metav1.ConditionFalse && cnd.Reason == ocv1.ReasonProgressDeadlineExceeded { - return false - } - return true - }, - } c.Clock = clock.RealClock{} return ctrl.NewControllerManagedBy(mgr). + WithOptions(controller.Options{ + RateLimiter: newDeadlineAwareRateLimiter( + workqueue.DefaultTypedControllerRateLimiter[ctrl.Request](), + mgr.GetClient(), + c.Clock, + ), + }). For( &ocv1.ClusterObjectSet{}, builder.WithPredicates( predicate.ResourceVersionChangedPredicate{}, - skipProgressDeadlineExceededPredicate, ), ). WatchesRawSource( @@ -659,14 +634,32 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing. return userProbes, nil } -func setRetryingConditions(cos *ocv1.ClusterObjectSet, message string) { - markAsProgressing(cos, ocv1.ClusterObjectSetReasonRetrying, message) +func setRetryingConditions(l logr.Logger, cos *ocv1.ClusterObjectSet, message string, isDeadlineExceeded bool) { + markAsProgressing(l, cos, ocv1.ClusterObjectSetReasonRetrying, message, isDeadlineExceeded) if meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) != nil { markAsAvailableUnknown(cos, ocv1.ClusterObjectSetReasonReconciling, message) } } -func markAsProgressing(cos *ocv1.ClusterObjectSet, reason, message string) { +var nonTerminalProgressingReasons = map[string]struct{}{ + ocv1.ReasonRollingOut: {}, + ocv1.ClusterObjectSetReasonRetrying: {}, +} + +func markAsProgressing(l logr.Logger, cos *ocv1.ClusterObjectSet, reason, message string, isDeadlineExceeded bool) { + switch reason { + case ocv1.ReasonSucceeded: + // Terminal — always apply. + default: + if _, known := nonTerminalProgressingReasons[reason]; !known { + l.Error(fmt.Errorf("unregistered progressing reason: %q", reason), "treating as non-terminal for deadline enforcement") + } + if isDeadlineExceeded { + markAsNotProgressing(cos, ocv1.ReasonProgressDeadlineExceeded, + fmt.Sprintf("Revision has not rolled out for %d minute(s). Last status: %s", cos.Spec.ProgressDeadlineMinutes, message)) + return + } + } meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ Type: ocv1.ClusterObjectSetTypeProgressing, Status: metav1.ConditionTrue, diff --git a/internal/operator-controller/controllers/clusterobjectset_controller_test.go b/internal/operator-controller/controllers/clusterobjectset_controller_test.go index b3b10f575a..75e7a2f8e4 100644 --- a/internal/operator-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/operator-controller/controllers/clusterobjectset_controller_test.go @@ -988,7 +988,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { revisionResult: &mockRevisionResult{ inTransition: true, }, - reconcileResult: ctrl.Result{RequeueAfter: 62 * time.Second}, + reconcileResult: ctrl.Result{RequeueAfter: 60 * time.Second}, validate: func(t *testing.T, c client.Client) { rev := &ocv1.ClusterObjectSet{} err := c.Get(t.Context(), client.ObjectKey{ @@ -1000,6 +1000,39 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { require.Equal(t, ocv1.ReasonRollingOut, cnd.Reason) }, }, + { + name: "recovery from ProgressDeadlineExceeded to Succeeded when revision completes", + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + rev1.Spec.ProgressDeadlineMinutes = 1 + rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) + meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterObjectSetTypeProgressing, + Status: metav1.ConditionFalse, + Reason: ocv1.ReasonProgressDeadlineExceeded, + Message: "Revision has not rolled out for 1 minute(s). Last status: Revision 1.0.0 is rolling out.", + ObservedGeneration: rev1.Generation, + }) + return []client.Object{rev1, ext} + }, + clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 5, 0, 0, time.UTC)), + revisionResult: &mockRevisionResult{ + isComplete: true, + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterObjectSet{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterObjectSetName, + }, rev) + require.NoError(t, err) + cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + require.NotNil(t, cnd) + require.Equal(t, metav1.ConditionTrue, cnd.Status) + require.Equal(t, ocv1.ReasonSucceeded, cnd.Reason) + require.Equal(t, "Revision 1.0.0 has rolled out.", cnd.Message) + }, + }, { name: "no progression deadline checks on revision recovery", existingObjs: func() []client.Object { diff --git a/internal/operator-controller/controllers/progress_deadline.go b/internal/operator-controller/controllers/progress_deadline.go new file mode 100644 index 0000000000..5bce123070 --- /dev/null +++ b/internal/operator-controller/controllers/progress_deadline.go @@ -0,0 +1,110 @@ +//go:build !standard + +package controllers + +import ( + "context" + "sync" + "time" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/util/workqueue" + "k8s.io/utils/clock" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +// deadlineAwareRateLimiter wraps a delegate rate limiter and caps the backoff +// duration to the time remaining until the COS progress deadline, ensuring +// that ProgressDeadlineExceeded is set promptly even during exponential backoff. +// +// After the deadline passes, it allows one immediate requeue (returning 0) so +// the reconciler can set the ProgressDeadlineExceeded condition, then falls +// back to the delegate's normal backoff. This avoids both tight-looping and +// coupling to the COS's status conditions. +type deadlineAwareRateLimiter struct { + delegate workqueue.TypedRateLimiter[ctrl.Request] + client client.Reader + clock clock.Clock + pastDeadline sync.Map +} + +func newDeadlineAwareRateLimiter( + delegate workqueue.TypedRateLimiter[ctrl.Request], + c client.Reader, + clk clock.Clock, +) *deadlineAwareRateLimiter { + return &deadlineAwareRateLimiter{delegate: delegate, client: c, clock: clk} +} + +func (r *deadlineAwareRateLimiter) When(item ctrl.Request) time.Duration { + backoff := r.delegate.When(item) + + cos := &ocv1.ClusterObjectSet{} + if err := r.client.Get(context.Background(), item.NamespacedName, cos); err != nil { + return backoff + } + + remaining, hasDeadline := durationUntilDeadline(r.clock, cos) + if !hasDeadline { + return backoff + } + if remaining > 0 { + if remaining < backoff { + return remaining + } + return backoff + } + + // Deadline has passed — allow one immediate requeue, then delegate. + if _, already := r.pastDeadline.LoadOrStore(item, struct{}{}); !already { + return 0 + } + return backoff +} + +func (r *deadlineAwareRateLimiter) Forget(item ctrl.Request) { + r.delegate.Forget(item) + r.pastDeadline.Delete(item) +} + +func (r *deadlineAwareRateLimiter) NumRequeues(item ctrl.Request) int { + return r.delegate.NumRequeues(item) +} + +// durationUntilDeadline returns how much time remains before the progress deadline +// expires. A negative duration means the deadline has already passed. +// +// It derives the deadline from spec and metadata only, with one exception: +// it checks the Succeeded status condition so that a revision recovering +// from drift is not penalised by the original deadline. +// +// Succeeded is a latch: there is no way to deduce from current cluster state +// alone that a COS succeeded in the past. If Succeeded is removed or set to +// False, this function will return a deadline and the reconciler will set +// ProgressDeadlineExceeded even though the revision previously succeeded. +// +// Returns (0, false) when there is no active deadline: +// - progressDeadlineMinutes is 0 +// - the revision has already succeeded +// - the revision is archived (deadline is irrelevant) +// - the revision is being deleted +func durationUntilDeadline(clk clock.Clock, cos *ocv1.ClusterObjectSet) (time.Duration, bool) { + pd := cos.Spec.ProgressDeadlineMinutes + if pd <= 0 { + return 0, false + } + if meta.IsStatusConditionTrue(cos.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { + return 0, false + } + if cos.Spec.LifecycleState == ocv1.ClusterObjectSetLifecycleStateArchived { + return 0, false + } + if !cos.DeletionTimestamp.IsZero() { + return 0, false + } + deadline := cos.CreationTimestamp.Add(time.Duration(pd) * time.Minute) + return deadline.Sub(clk.Now()), true +} diff --git a/internal/operator-controller/controllers/progress_deadline_test.go b/internal/operator-controller/controllers/progress_deadline_test.go new file mode 100644 index 0000000000..73102e9ab1 --- /dev/null +++ b/internal/operator-controller/controllers/progress_deadline_test.go @@ -0,0 +1,266 @@ +//go:build !standard + +package controllers + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clocktesting "k8s.io/utils/clock/testing" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +func TestDurationUntilDeadline(t *testing.T) { + creation := time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC) + now := creation.Add(30 * time.Second) + clk := clocktesting.NewFakeClock(now) + + for _, tc := range []struct { + name string + cos ocv1.ClusterObjectSet + expectDuration time.Duration + expectHasDeadline bool + }{ + { + name: "progressDeadlineMinutes is 0 — no deadline", + cos: ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, + Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 0, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, + }, + expectDuration: 0, + expectHasDeadline: false, + }, + { + name: "Succeeded is true — no deadline", + cos: ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, + Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, + Status: ocv1.ClusterObjectSetStatus{ + Conditions: []metav1.Condition{{ + Type: ocv1.ClusterObjectSetTypeSucceeded, + Status: metav1.ConditionTrue, + }}, + }, + }, + expectDuration: 0, + expectHasDeadline: false, + }, + { + name: "lifecycleState is Archived — no deadline", + cos: ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, + Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived}, + }, + expectDuration: 0, + expectHasDeadline: false, + }, + { + name: "DeletionTimestamp is set — no deadline", + cos: ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: metav1.NewTime(creation), + DeletionTimestamp: &metav1.Time{Time: now}, + }, + Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, + }, + expectDuration: 0, + expectHasDeadline: false, + }, + { + name: "deadline not yet exceeded — returns positive remaining", + cos: ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, + Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, + }, + expectDuration: 30 * time.Second, + expectHasDeadline: true, + }, + { + name: "deadline already exceeded — returns negative remaining", + cos: ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation.Add(-2 * time.Minute))}, + Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, + }, + expectDuration: -90 * time.Second, + expectHasDeadline: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + duration, hasDeadline := durationUntilDeadline(clk, &tc.cos) + require.Equal(t, tc.expectHasDeadline, hasDeadline) + require.Equal(t, tc.expectDuration, duration) + }) + } +} + +type fixedRateLimiter struct { + duration time.Duration +} + +func (f *fixedRateLimiter) When(_ ctrl.Request) time.Duration { return f.duration } +func (f *fixedRateLimiter) Forget(_ ctrl.Request) {} +func (f *fixedRateLimiter) NumRequeues(_ ctrl.Request) int { return 0 } + +func TestDeadlineAwareRateLimiter(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(scheme)) + + creation := time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC) + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "test-cos"}} + + for _, tc := range []struct { + name string + backoff time.Duration + cos *ocv1.ClusterObjectSet + clockTime time.Time + expectDuration time.Duration + }{ + { + name: "no deadline configured — uses delegate backoff", + backoff: 30 * time.Second, + clockTime: creation, + cos: &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cos", + CreationTimestamp: metav1.NewTime(creation), + }, + Spec: ocv1.ClusterObjectSetSpec{ + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + }, + }, + expectDuration: 30 * time.Second, + }, + { + name: "deadline not exceeded and backoff is shorter — uses delegate backoff", + backoff: 5 * time.Second, + clockTime: creation, + cos: &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cos", + CreationTimestamp: metav1.NewTime(creation), + }, + Spec: ocv1.ClusterObjectSetSpec{ + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + ProgressDeadlineMinutes: 1, + }, + }, + expectDuration: 5 * time.Second, + }, + { + name: "deadline not exceeded and backoff is longer — caps at deadline", + backoff: 5 * time.Minute, + clockTime: creation, + cos: &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cos", + CreationTimestamp: metav1.NewTime(creation), + }, + Spec: ocv1.ClusterObjectSetSpec{ + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + ProgressDeadlineMinutes: 1, + }, + }, + expectDuration: 60 * time.Second, + }, + { + name: "deadline exceeded — first call returns immediate requeue", + backoff: 30 * time.Second, + clockTime: creation.Add(61 * time.Second), + cos: &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cos", + CreationTimestamp: metav1.NewTime(creation), + }, + Spec: ocv1.ClusterObjectSetSpec{ + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + ProgressDeadlineMinutes: 1, + }, + }, + expectDuration: 0, + }, + { + name: "COS not found — uses delegate backoff", + backoff: 30 * time.Second, + clockTime: creation, + cos: nil, + expectDuration: 30 * time.Second, + }, + } { + t.Run(tc.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme) + if tc.cos != nil { + builder = builder.WithObjects(tc.cos) + } + + limiter := newDeadlineAwareRateLimiter( + &fixedRateLimiter{duration: tc.backoff}, + builder.Build(), + clocktesting.NewFakeClock(tc.clockTime), + ) + + testReq := req + if tc.cos == nil { + testReq.Name = "nonexistent" + } + + result := limiter.When(testReq) + require.Equal(t, tc.expectDuration, result) + }) + } + + t.Run("deadline exceeded — second call uses delegate backoff (one-shot)", func(t *testing.T) { + cos := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cos", + CreationTimestamp: metav1.NewTime(creation), + }, + Spec: ocv1.ClusterObjectSetSpec{ + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + ProgressDeadlineMinutes: 1, + }, + } + limiter := newDeadlineAwareRateLimiter( + &fixedRateLimiter{duration: 30 * time.Second}, + fake.NewClientBuilder().WithScheme(scheme).WithObjects(cos).Build(), + clocktesting.NewFakeClock(creation.Add(61*time.Second)), + ) + + first := limiter.When(req) + require.Equal(t, time.Duration(0), first) + + second := limiter.When(req) + require.Equal(t, 30*time.Second, second) + }) + + t.Run("Forget resets the one-shot flag", func(t *testing.T) { + cos := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cos", + CreationTimestamp: metav1.NewTime(creation), + }, + Spec: ocv1.ClusterObjectSetSpec{ + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + ProgressDeadlineMinutes: 1, + }, + } + limiter := newDeadlineAwareRateLimiter( + &fixedRateLimiter{duration: 30 * time.Second}, + fake.NewClientBuilder().WithScheme(scheme).WithObjects(cos).Build(), + clocktesting.NewFakeClock(creation.Add(61*time.Second)), + ) + + require.Equal(t, time.Duration(0), limiter.When(req)) + require.Equal(t, 30*time.Second, limiter.When(req)) + + limiter.Forget(req) + + require.Equal(t, time.Duration(0), limiter.When(req)) + }) +} diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index 0f99c1b28e..01e4fe4486 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -19,6 +19,7 @@ const ( HelmChartSupport featuregate.Feature = "HelmChartSupport" BoxcutterRuntime featuregate.Feature = "BoxcutterRuntime" DeploymentConfig featuregate.Feature = "DeploymentConfig" + BundleReleaseSupport featuregate.Feature = "BundleReleaseSupport" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -89,6 +90,17 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature PreRelease: featuregate.Alpha, LockToDefault: false, }, + + // BundleReleaseSupport enables parsing of the explicit pkg.Release field + // from the olm.package property. When enabled, bundles with an explicit + // pkg.Release field have their release value parsed separately from the version, + // allowing build metadata to serve its proper semver purpose (e.g., git commit). + // When disabled, release values are parsed from version build metadata (registry+v1 legacy). + BundleReleaseSupport: { + Default: false, + PreRelease: featuregate.Alpha, + LockToDefault: false, + }, } var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() diff --git a/internal/operator-controller/labels/labels.go b/internal/operator-controller/labels/labels.go index 805c34c3b0..02234dce49 100644 --- a/internal/operator-controller/labels/labels.go +++ b/internal/operator-controller/labels/labels.go @@ -28,6 +28,14 @@ const ( // associated with a ClusterObjectSet. BundleVersionKey = "olm.operatorframework.io/bundle-version" + // BundleReleaseKey is the storage key used to record the bundle release value + // across supported metadata backends, including Helm release labels and + // ClusterObjectSet annotations. For bundles with explicit pkg.Release metadata, + // this field contains that release value. For registry+v1 bundles, this field contains + // a release derived from the version's build metadata only when that metadata is + // parseable as a release value (e.g., '2' from '1.0.0+2'). + BundleReleaseKey = "olm.operatorframework.io/bundle-release" + // BundleReferenceKey is the label key used to record an external reference // (such as an image or catalog reference) to the bundle for a // ClusterObjectSet. diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 4e19ea3f44..fcd40e2d4e 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1300,6 +1300,30 @@ spec: hyphens (-) or periods (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") + release: + description: |- + release is an optional field that identifies a specific release of this bundle's version. + A release represents a re-publication of the same version, typically used to deliver + packaging or metadata changes without changing the version number. When multiple + releases exist for the same version, higher releases are preferred. An unset release + is less preferred than all other release values. + + The value consists of dot-separated identifiers, where each identifier is either a + numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + compared as integers, alphanumeric identifiers are compared lexically, and numeric + identifiers always sort before alphanumeric identifiers. + + For bundles with explicit pkg.Release metadata, this field contains that release value. + For registry+v1 bundles lacking an explicit release value, this field contains the release + extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + This field is omitted when the bundle's release value is unset. + maxLength: 20 + type: string + x-kubernetes-validations: + - message: release must be empty or consist of dot-separated + identifiers (numeric without leading zeros, or alphanumeric) + rule: self.matches("^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$") version: description: |- version is required and references the version that this bundle represents. @@ -2621,7 +2645,7 @@ metadata: namespace: olmv1-system spec: minReadySeconds: 5 - replicas: 1 + replicas: 2 strategy: type: RollingUpdate rollingUpdate: @@ -2772,7 +2796,7 @@ metadata: name: operator-controller-controller-manager namespace: olmv1-system spec: - replicas: 1 + replicas: 2 strategy: type: RollingUpdate rollingUpdate: @@ -2802,6 +2826,7 @@ spec: - --feature-gates=HelmChartSupport=true - --feature-gates=BoxcutterRuntime=true - --feature-gates=DeploymentConfig=true + - --feature-gates=BundleReleaseSupport=true - --feature-gates=WebhookProviderOpenshiftServiceCA=false - --tls-cert=/var/certs/tls.crt - --tls-key=/var/certs/tls.key diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index c49ab79a0f..0561ea4ac1 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1261,6 +1261,30 @@ spec: hyphens (-) or periods (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") + release: + description: |- + release is an optional field that identifies a specific release of this bundle's version. + A release represents a re-publication of the same version, typically used to deliver + packaging or metadata changes without changing the version number. When multiple + releases exist for the same version, higher releases are preferred. An unset release + is less preferred than all other release values. + + The value consists of dot-separated identifiers, where each identifier is either a + numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + compared as integers, alphanumeric identifiers are compared lexically, and numeric + identifiers always sort before alphanumeric identifiers. + + For bundles with explicit pkg.Release metadata, this field contains that release value. + For registry+v1 bundles lacking an explicit release value, this field contains the release + extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + This field is omitted when the bundle's release value is unset. + maxLength: 20 + type: string + x-kubernetes-validations: + - message: release must be empty or consist of dot-separated + identifiers (numeric without leading zeros, or alphanumeric) + rule: self.matches("^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$") version: description: |- version is required and references the version that this bundle represents. @@ -2541,7 +2565,7 @@ metadata: namespace: olmv1-system spec: minReadySeconds: 5 - replicas: 1 + replicas: 2 strategy: type: RollingUpdate rollingUpdate: @@ -2679,7 +2703,7 @@ metadata: name: operator-controller-controller-manager namespace: olmv1-system spec: - replicas: 1 + replicas: 2 strategy: type: RollingUpdate rollingUpdate: @@ -2708,6 +2732,7 @@ spec: - --feature-gates=HelmChartSupport=true - --feature-gates=BoxcutterRuntime=true - --feature-gates=DeploymentConfig=true + - --feature-gates=BundleReleaseSupport=true - --feature-gates=WebhookProviderOpenshiftServiceCA=false - --tls-cert=/var/certs/tls.crt - --tls-key=/var/certs/tls.key diff --git a/openshift/operator-controller/manifests-experimental.yaml b/openshift/operator-controller/manifests-experimental.yaml index 47fe2af589..1a97864aa4 100644 --- a/openshift/operator-controller/manifests-experimental.yaml +++ b/openshift/operator-controller/manifests-experimental.yaml @@ -753,6 +753,29 @@ spec: x-kubernetes-validations: - message: packageName must be a valid DNS1123 subdomain. It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") + release: + description: |- + release is an optional field that identifies a specific release of this bundle's version. + A release represents a re-publication of the same version, typically used to deliver + packaging or metadata changes without changing the version number. When multiple + releases exist for the same version, higher releases are preferred. An unset release + is less preferred than all other release values. + + The value consists of dot-separated identifiers, where each identifier is either a + numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + compared as integers, alphanumeric identifiers are compared lexically, and numeric + identifiers always sort before alphanumeric identifiers. + + For bundles with explicit pkg.Release metadata, this field contains that release value. + For registry+v1 bundles lacking an explicit release value, this field contains the release + extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + This field is omitted when the bundle's release value is unset. + maxLength: 20 + type: string + x-kubernetes-validations: + - message: release must be empty or consist of dot-separated identifiers (numeric without leading zeros, or alphanumeric) + rule: self.matches("^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$") version: description: |- version is required and references the version that this bundle represents. diff --git a/openshift/tests-extension/go.mod b/openshift/tests-extension/go.mod index 64f1583510..6c37e0b648 100644 --- a/openshift/tests-extension/go.mod +++ b/openshift/tests-extension/go.mod @@ -121,7 +121,7 @@ require ( sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.34.0 // indirect sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect - sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect + sigs.k8s.io/structured-merge-diff/v6 v6.4.0 // indirect sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/openshift/tests-extension/go.sum b/openshift/tests-extension/go.sum index f9b93ba5af..eefb23ab00 100644 --- a/openshift/tests-extension/go.sum +++ b/openshift/tests-extension/go.sum @@ -307,7 +307,7 @@ sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5E sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg= sigs.k8s.io/randfill v1.0.0 h1:JfjMILfT8A6RbawdsK2JXGBR5AQVfd+9TbzrlneTyrU= sigs.k8s.io/randfill v1.0.0/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY= -sigs.k8s.io/structured-merge-diff/v6 v6.3.2 h1:kwVWMx5yS1CrnFWA/2QHyRVJ8jM6dBA80uLmm0wJkk8= -sigs.k8s.io/structured-merge-diff/v6 v6.3.2/go.mod h1:M3W8sfWvn2HhQDIbGWj3S099YozAsymCo/wrT5ohRUE= +sigs.k8s.io/structured-merge-diff/v6 v6.4.0 h1:qmp2e3ZfFi1/jJbDGpD4mt3wyp6PE1NfKHCYLqgNQJo= +sigs.k8s.io/structured-merge-diff/v6 v6.4.0/go.mod h1:M3W8sfWvn2HhQDIbGWj3S099YozAsymCo/wrT5ohRUE= sigs.k8s.io/yaml v1.6.0 h1:G8fkbMSAFqgEFgh4b1wmtzDnioxFCUgTZhlbj5P9QYs= sigs.k8s.io/yaml v1.6.0/go.mod h1:796bPqUfzR/0jLAl6XjHl3Ck7MiyVv8dbTdyT3/pMf4= diff --git a/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/clusterextension_types.go b/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/clusterextension_types.go index cf5946a408..12614d1a43 100644 --- a/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/clusterextension_types.go +++ b/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/clusterextension_types.go @@ -466,6 +466,29 @@ type BundleMetadata struct { // +required // +kubebuilder:validation:XValidation:rule="self.matches(\"^([0-9]+)(\\\\.[0-9]+)?(\\\\.[0-9]+)?(-([-0-9A-Za-z]+(\\\\.[-0-9A-Za-z]+)*))?(\\\\+([-0-9A-Za-z]+(-\\\\.[-0-9A-Za-z]+)*))?\")",message="version must be well-formed semver" Version string `json:"version"` + + // release is an optional field that identifies a specific release of this bundle's version. + // A release represents a re-publication of the same version, typically used to deliver + // packaging or metadata changes without changing the version number. When multiple + // releases exist for the same version, higher releases are preferred. An unset release + // is less preferred than all other release values. + // + // The value consists of dot-separated identifiers, where each identifier is either a + // numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9", + // "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are + // compared as integers, alphanumeric identifiers are compared lexically, and numeric + // identifiers always sort before alphanumeric identifiers. + // + // For bundles with explicit pkg.Release metadata, this field contains that release value. + // For registry+v1 bundles lacking an explicit release value, this field contains the release + // extracted from version's build metadata (e.g., '2' from '1.0.0+2'). + // This field is omitted when the bundle's release value is unset. + // + // +optional + // + // +kubebuilder:validation:MaxLength=20 + // +kubebuilder:validation:XValidation:rule="self.matches(\"^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$\")",message="release must be empty or consist of dot-separated identifiers (numeric without leading zeros, or alphanumeric)" + Release *string `json:"release,omitempty"` } // RevisionStatus defines the observed state of a ClusterObjectSet. diff --git a/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/zz_generated.deepcopy.go b/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/zz_generated.deepcopy.go index 384439787b..ef6f145b9e 100644 --- a/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/zz_generated.deepcopy.go +++ b/openshift/tests-extension/vendor/github.com/operator-framework/operator-controller/api/v1/zz_generated.deepcopy.go @@ -46,6 +46,11 @@ func (in *Assertion) DeepCopy() *Assertion { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BundleMetadata) DeepCopyInto(out *BundleMetadata) { *out = *in + if in.Release != nil { + in, out := &in.Release, &out.Release + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BundleMetadata. @@ -312,7 +317,7 @@ func (in *ClusterExtensionInstallConfig) DeepCopy() *ClusterExtensionInstallConf // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterExtensionInstallStatus) DeepCopyInto(out *ClusterExtensionInstallStatus) { *out = *in - out.Bundle = in.Bundle + in.Bundle.DeepCopyInto(&out.Bundle) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionInstallStatus. @@ -397,7 +402,7 @@ func (in *ClusterExtensionStatus) DeepCopyInto(out *ClusterExtensionStatus) { if in.Install != nil { in, out := &in.Install, &out.Install *out = new(ClusterExtensionInstallStatus) - **out = **in + (*in).DeepCopyInto(*out) } if in.ActiveRevisions != nil { in, out := &in.ActiveRevisions, &out.ActiveRevisions diff --git a/openshift/tests-extension/vendor/modules.txt b/openshift/tests-extension/vendor/modules.txt index e754b3b0ee..dd5e728dcc 100644 --- a/openshift/tests-extension/vendor/modules.txt +++ b/openshift/tests-extension/vendor/modules.txt @@ -1255,7 +1255,7 @@ sigs.k8s.io/json/internal/golang/encoding/json ## explicit; go 1.18 sigs.k8s.io/randfill sigs.k8s.io/randfill/bytesource -# sigs.k8s.io/structured-merge-diff/v6 v6.3.2 +# sigs.k8s.io/structured-merge-diff/v6 v6.4.0 ## explicit; go 1.23 sigs.k8s.io/structured-merge-diff/v6/fieldpath sigs.k8s.io/structured-merge-diff/v6/merge diff --git a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go index 73436912cd..f1607601c4 100644 --- a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go +++ b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go @@ -19,7 +19,7 @@ package fieldpath import ( "fmt" "iter" - "sort" + "slices" "strings" "sigs.k8s.io/structured-merge-diff/v6/value" @@ -255,19 +255,13 @@ func (s PathElementSet) Copy() PathElementSet { // Insert adds pe to the set. func (s *PathElementSet) Insert(pe PathElement) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a, b PathElement) int { + return a.Compare(b) }) - if loc == len(s.members) { - s.members = append(s.members, pe) + if found { return } - if s.members[loc].Equals(pe) { - return - } - s.members = append(s.members, PathElement{}) - copy(s.members[loc+1:], s.members[loc:]) - s.members[loc] = pe + s.members = slices.Insert(s.members, loc, pe) } // Union returns a set containing elements that appear in either s or s2. @@ -344,16 +338,10 @@ func (s *PathElementSet) Size() int { return len(s.members) } // Has returns true if pe is a member of the set. func (s *PathElementSet) Has(pe PathElement) bool { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].Less(pe) + _, found := slices.BinarySearchFunc(s.members, pe, func(a, b PathElement) int { + return a.Compare(b) }) - if loc == len(s.members) { - return false - } - if s.members[loc].Equals(pe) { - return true - } - return false + return found } // Equals returns true if s and s2 have exactly the same members. diff --git a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go index ff7ee510c2..ca50b84e99 100644 --- a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go +++ b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go @@ -17,7 +17,7 @@ limitations under the License. package fieldpath import ( - "sort" + "slices" "sigs.k8s.io/structured-merge-diff/v6/value" ) @@ -82,32 +82,23 @@ func MakePathElementMap(size int) PathElementMap { // Insert adds the pathelement and associated value in the map. // If insert is called twice with the same PathElement, the value is replaced. func (s *PathElementMap) Insert(pe PathElement, v interface{}) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].PathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a pathElementValue, b PathElement) int { + return a.PathElement.Compare(b) }) - if loc == len(s.members) { - s.members = append(s.members, pathElementValue{pe, v}) - return - } - if s.members[loc].PathElement.Equals(pe) { + if found { s.members[loc].Value = v return } - s.members = append(s.members, pathElementValue{}) - copy(s.members[loc+1:], s.members[loc:]) - s.members[loc] = pathElementValue{pe, v} + s.members = slices.Insert(s.members, loc, pathElementValue{PathElement: pe, Value: v}) } // Get retrieves the value associated with the given PathElement from the map. // (nil, false) is returned if there is no such PathElement. func (s *PathElementMap) Get(pe PathElement) (interface{}, bool) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].PathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a pathElementValue, b PathElement) int { + return a.PathElement.Compare(b) }) - if loc == len(s.members) { - return nil, false - } - if s.members[loc].PathElement.Equals(pe) { + if found { return s.members[loc].Value, true } return nil, false diff --git a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go index d2d8c8a42b..d7fe643be2 100644 --- a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go +++ b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go @@ -19,6 +19,7 @@ package fieldpath import ( "fmt" "iter" + "slices" "sort" "strings" @@ -486,19 +487,13 @@ func (s *SetNodeMap) Copy() SetNodeMap { // Descend adds pe to the set if necessary, returning the associated subset. func (s *SetNodeMap) Descend(pe PathElement) *Set { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].pathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a setNode, b PathElement) int { + return a.pathElement.Compare(b) }) - if loc == len(s.members) { - s.members = append(s.members, setNode{pathElement: pe, set: &Set{}}) + if found { return s.members[loc].set } - if s.members[loc].pathElement.Equals(pe) { - return s.members[loc].set - } - s.members = append(s.members, setNode{}) - copy(s.members[loc+1:], s.members[loc:]) - s.members[loc] = setNode{pathElement: pe, set: &Set{}} + s.members = slices.Insert(s.members, loc, setNode{pathElement: pe, set: &Set{}}) return s.members[loc].set } @@ -523,13 +518,10 @@ func (s *SetNodeMap) Empty() bool { // Get returns (the associated set, true) or (nil, false) if there is none. func (s *SetNodeMap) Get(pe PathElement) (*Set, bool) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].pathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a setNode, b PathElement) int { + return a.pathElement.Compare(b) }) - if loc == len(s.members) { - return nil, false - } - if s.members[loc].pathElement.Equals(pe) { + if found { return s.members[loc].set, true } return nil, false diff --git a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go index f70cd41674..acbfe8ceba 100644 --- a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go +++ b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go @@ -16,6 +16,8 @@ limitations under the License. package value +import "reflect" + // Allocator provides a value object allocation strategy. // Value objects can be allocated by passing an allocator to the "Using" // receiver functions on the value interfaces, e.g. Map.ZipUsing(allocator, ...). @@ -24,8 +26,8 @@ package value type Allocator interface { // Free gives the allocator back any value objects returned by the "Using" // receiver functions on the value interfaces. - // interface{} may be any of: Value, Map, List or Range. - Free(interface{}) + // any may be any of: Value, Map, List or Range. + Free(any) // The unexported functions are for "Using" receiver functions of the value types // to request what they need from the allocator. @@ -74,7 +76,7 @@ func (p *heapAllocator) allocListReflectRange() *listReflectRange { return &listReflectRange{vr: &valueReflect{}} } -func (p *heapAllocator) Free(_ interface{}) {} +func (p *heapAllocator) Free(_ any) {} // NewFreelistAllocator creates freelist based allocator. // This allocator provides fast allocation and freeing of short lived value objects. @@ -89,25 +91,25 @@ func (p *heapAllocator) Free(_ interface{}) {} // for all temporary value access. func NewFreelistAllocator() Allocator { return &freelistAllocator{ - valueUnstructured: &freelist{new: func() interface{} { + valueUnstructured: &freelist[*valueUnstructured]{new: func() *valueUnstructured { return &valueUnstructured{} }}, - listUnstructuredRange: &freelist{new: func() interface{} { + listUnstructuredRange: &freelist[*listUnstructuredRange]{new: func() *listUnstructuredRange { return &listUnstructuredRange{vv: &valueUnstructured{}} }}, - valueReflect: &freelist{new: func() interface{} { + valueReflect: &freelist[*valueReflect]{new: func() *valueReflect { return &valueReflect{} }}, - mapReflect: &freelist{new: func() interface{} { + mapReflect: &freelist[*mapReflect]{new: func() *mapReflect { return &mapReflect{} }}, - structReflect: &freelist{new: func() interface{} { + structReflect: &freelist[*structReflect]{new: func() *structReflect { return &structReflect{} }}, - listReflect: &freelist{new: func() interface{} { + listReflect: &freelist[*listReflect]{new: func() *listReflect { return &listReflect{} }}, - listReflectRange: &freelist{new: func() interface{} { + listReflectRange: &freelist[*listReflectRange]{new: func() *listReflectRange { return &listReflectRange{vr: &valueReflect{}} }}, } @@ -119,22 +121,22 @@ func NewFreelistAllocator() Allocator { const freelistMaxSize = 1000 type freelistAllocator struct { - valueUnstructured *freelist - listUnstructuredRange *freelist - valueReflect *freelist - mapReflect *freelist - structReflect *freelist - listReflect *freelist - listReflectRange *freelist + valueUnstructured *freelist[*valueUnstructured] + listUnstructuredRange *freelist[*listUnstructuredRange] + valueReflect *freelist[*valueReflect] + mapReflect *freelist[*mapReflect] + structReflect *freelist[*structReflect] + listReflect *freelist[*listReflect] + listReflectRange *freelist[*listReflectRange] } -type freelist struct { - list []interface{} - new func() interface{} +type freelist[T any] struct { + list []T + new func() T } -func (f *freelist) allocate() interface{} { - var w2 interface{} +func (f *freelist[T]) allocate() T { + var w2 T if n := len(f.list); n > 0 { w2, f.list = f.list[n-1], f.list[:n-1] } else { @@ -143,61 +145,73 @@ func (f *freelist) allocate() interface{} { return w2 } -func (f *freelist) free(v interface{}) { +func (f *freelist[T]) free(v T) { if len(f.list) < freelistMaxSize { f.list = append(f.list, v) } } -func (w *freelistAllocator) Free(value interface{}) { +func (w *freelistAllocator) Free(value any) { switch v := value.(type) { case *valueUnstructured: v.Value = nil // don't hold references to unstructured objects w.valueUnstructured.free(v) case *listUnstructuredRange: + v.list = nil // don't hold references to unstructured objects v.vv.Value = nil // don't hold references to unstructured objects w.listUnstructuredRange.free(v) case *valueReflect: - v.ParentMapKey = nil v.ParentMap = nil + v.ParentMapKey = nil + v.Value = reflect.Value{} // don't hold references to reflected objects w.valueReflect.free(v) case *mapReflect: + v.valueReflect.ParentMap = nil + v.valueReflect.ParentMapKey = nil + v.valueReflect.Value = reflect.Value{} // don't hold references to reflected objects w.mapReflect.free(v) case *structReflect: + v.valueReflect.ParentMap = nil + v.valueReflect.ParentMapKey = nil + v.valueReflect.Value = reflect.Value{} // don't hold references to reflected objects w.structReflect.free(v) case *listReflect: + v.Value = reflect.Value{} // don't hold references to reflected objects w.listReflect.free(v) case *listReflectRange: - v.vr.ParentMapKey = nil + v.list = reflect.Value{} // don't hold references to reflected objects v.vr.ParentMap = nil + v.vr.ParentMapKey = nil + v.vr.Value = reflect.Value{} // don't hold references to reflected objects + v.entry = nil w.listReflectRange.free(v) } } func (w *freelistAllocator) allocValueUnstructured() *valueUnstructured { - return w.valueUnstructured.allocate().(*valueUnstructured) + return w.valueUnstructured.allocate() } func (w *freelistAllocator) allocListUnstructuredRange() *listUnstructuredRange { - return w.listUnstructuredRange.allocate().(*listUnstructuredRange) + return w.listUnstructuredRange.allocate() } func (w *freelistAllocator) allocValueReflect() *valueReflect { - return w.valueReflect.allocate().(*valueReflect) + return w.valueReflect.allocate() } func (w *freelistAllocator) allocStructReflect() *structReflect { - return w.structReflect.allocate().(*structReflect) + return w.structReflect.allocate() } func (w *freelistAllocator) allocMapReflect() *mapReflect { - return w.mapReflect.allocate().(*mapReflect) + return w.mapReflect.allocate() } func (w *freelistAllocator) allocListReflect() *listReflect { - return w.listReflect.allocate().(*listReflect) + return w.listReflect.allocate() } func (w *freelistAllocator) allocListReflectRange() *listReflectRange { - return w.listReflectRange.allocate().(*listReflectRange) + return w.listReflectRange.allocate() } diff --git a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go index 3aadceb222..db25613734 100644 --- a/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go +++ b/openshift/tests-extension/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go @@ -76,11 +76,16 @@ func OmitZeroFunc(t reflect.Type) func(reflect.Value) bool { // but is based on the highly efficient approach from https://golang.org/src/encoding/json/encode.go func lookupJsonTags(f reflect.StructField) (name string, omit bool, inline bool, omitempty bool, omitzero func(reflect.Value) bool) { + // Skip unexported non-embedded fields, matching encoding/json behavior. + if f.PkgPath != "" && !f.Anonymous { + return "", true, false, false, nil + } tag := f.Tag.Get("json") if tag == "-" { return "", true, false, false, nil } name, opts := parseTag(tag) + inline = f.Anonymous && name == "" if name == "" { name = f.Name } @@ -89,7 +94,7 @@ func lookupJsonTags(f reflect.StructField) (name string, omit bool, inline bool, omitzero = OmitZeroFunc(f.Type) } - return name, false, opts.Contains("inline"), opts.Contains("omitempty"), omitzero + return name, false, inline, opts.Contains("omitempty"), omitzero } func isEmpty(v reflect.Value) bool { diff --git a/requirements.txt b/requirements.txt index cfbd969b33..35100e407a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,7 @@ mergedeep==1.3.4 mkdocs==1.6.1 mkdocs-material==9.7.6 mkdocs-material-extensions==1.3.1 -packaging==26.0 +packaging==26.1 paginate==0.5.7 pathspec==1.0.4 platformdirs==4.9.6 diff --git a/test/e2e/README.md b/test/e2e/README.md index fa47f47c7b..98b99b9caa 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -292,7 +292,7 @@ Each scenario runs in its own namespace with unique resource names, ensuring com The `ScenarioCleanup` hook ensures all resources are deleted after each scenario: -- Deletes ClusterExtensions and ClusterObjectSets +- Deletes ClusterExtensions and (when BoxcutterRuntime gate is enabled) ClusterObjectSets - Deletes ClusterCatalogs - Deletes namespaces - Deletes added resources diff --git a/test/e2e/features/ha.feature b/test/e2e/features/ha.feature new file mode 100644 index 0000000000..6b889c99bb --- /dev/null +++ b/test/e2e/features/ha.feature @@ -0,0 +1,19 @@ +Feature: HA failover for catalogd + + When catalogd is deployed with multiple replicas, the remaining pods must + elect a new leader and resume serving catalogs if the leader pod is lost. + + Background: + Given OLM is available + And an image registry is available + + @CatalogdHA + Scenario: Catalogd resumes serving catalogs after leader pod failure + Given a catalog "test" with packages: + | package | version | channel | replaces | contents | + | test | 1.0.0 | stable | | CRD, Deployment, ConfigMap | + And catalogd is ready to reconcile resources + And catalog "test" is reconciled + When the catalogd leader pod is force-deleted + Then a new catalogd leader is elected + And catalog "test" reports Serving as True with Reason Available diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index ceb5e32c23..6265a8eff8 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -435,7 +435,7 @@ Feature: Install ClusterExtension Then ClusterObjectSet "${NAME}-1" reports Progressing as False with Reason ProgressDeadlineExceeded And ClusterExtension reports Progressing as False with Reason ProgressDeadlineExceeded and Message: """ - Revision has not rolled out for 1 minute(s). + Revision has not rolled out for 1 minute(s). Last status: Revision 1.0.2 is rolling out. """ And ClusterExtension reports Progressing transition between 1 and 2 minutes since its creation @@ -471,7 +471,7 @@ Feature: Install ClusterExtension Then ClusterObjectSet "${NAME}-1" reports Progressing as False with Reason ProgressDeadlineExceeded And ClusterExtension reports Progressing as False with Reason ProgressDeadlineExceeded and Message: """ - Revision has not rolled out for 1 minute(s). + Revision has not rolled out for 1 minute(s). Last status: Revision 1.0.3 is rolling out. """ And ClusterExtension reports Progressing transition between 1 and 2 minutes since its creation diff --git a/test/e2e/features/revision.feature b/test/e2e/features/revision.feature index 38ed160279..866e8195f4 100644 --- a/test/e2e/features/revision.feature +++ b/test/e2e/features/revision.feature @@ -601,3 +601,141 @@ Feature: Install ClusterObjectSet """ And ClusterObjectSet "${COS_NAME}" reconciliation is triggered Then ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded + + + @ProgressDeadline + Scenario: Archiving a COS with ProgressDeadlineExceeded cleans up its resources + Given min value for ClusterObjectSet .spec.progressDeadlineMinutes is set to 1 + And ServiceAccount "olm-sa" with needed permissions is available in test namespace + When ClusterObjectSet is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + annotations: + olm.operatorframework.io/service-account-name: olm-sa + olm.operatorframework.io/service-account-namespace: ${TEST_NAMESPACE} + name: ${COS_NAME} + spec: + lifecycleState: Active + collisionProtection: Prevent + progressDeadlineMinutes: 1 + progressionProbes: + - selector: + type: GroupKind + groupKind: + group: apps + kind: Deployment + assertions: + - type: ConditionEqual + conditionEqual: + type: Available + status: "True" + phases: + - name: resources + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: test-configmap + namespace: ${TEST_NAMESPACE} + data: + foo: bar + - object: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: test-deployment + namespace: ${TEST_NAMESPACE} + spec: + replicas: 1 + selector: + matchLabels: + app: never-ready + template: + metadata: + labels: + app: never-ready + spec: + containers: + - name: never-ready + image: does-not-exist:latest + revision: 1 + """ + Then resource "configmap/test-configmap" is installed + And resource "deployment/test-deployment" is installed + And ClusterObjectSet "${COS_NAME}" reports Progressing as False with Reason ProgressDeadlineExceeded + When ClusterObjectSet "${COS_NAME}" lifecycle is set to "Archived" + Then ClusterObjectSet "${COS_NAME}" is archived + And resource "configmap/test-configmap" is eventually not found + And resource "deployment/test-deployment" is eventually not found + + @ProgressDeadline + Scenario: COS recovers from ProgressDeadlineExceeded to Succeeded when probes pass + Given min value for ClusterObjectSet .spec.progressDeadlineMinutes is set to 1 + And ServiceAccount "olm-sa" with needed permissions is available in test namespace + When ClusterObjectSet is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + annotations: + olm.operatorframework.io/service-account-name: olm-sa + olm.operatorframework.io/service-account-namespace: ${TEST_NAMESPACE} + name: ${COS_NAME} + spec: + lifecycleState: Active + collisionProtection: Prevent + progressDeadlineMinutes: 1 + progressionProbes: + - selector: + type: GroupKind + groupKind: + group: apps + kind: Deployment + assertions: + - type: ConditionEqual + conditionEqual: + type: Available + status: "True" + phases: + - name: resources + objects: + - object: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: test-deployment + namespace: ${TEST_NAMESPACE} + spec: + replicas: 1 + selector: + matchLabels: + app: delayed-ready + template: + metadata: + labels: + app: delayed-ready + spec: + containers: + - name: delayed-ready + image: busybox:1.36 + command: ["sleep", "1000"] + readinessProbe: + exec: + command: ["true"] + initialDelaySeconds: 65 + securityContext: + runAsNonRoot: true + runAsUser: 1000 + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + seccompProfile: + type: RuntimeDefault + revision: 1 + """ + Then ClusterObjectSet "${COS_NAME}" reports Progressing as False with Reason ProgressDeadlineExceeded + And ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded diff --git a/test/e2e/steps/ha_steps.go b/test/e2e/steps/ha_steps.go new file mode 100644 index 0000000000..1d5d9844cb --- /dev/null +++ b/test/e2e/steps/ha_steps.go @@ -0,0 +1,63 @@ +package steps + +import ( + "context" + "fmt" + "strings" + + "k8s.io/component-base/featuregate" +) + +// catalogdHAFeature gates scenarios that require a multi-node cluster. +// It is set to true in BeforeSuite when the cluster has at least 2 nodes, +// which is the case for the experimental e2e suite (kind-config-2node.yaml) +// but not the standard suite. +const catalogdHAFeature featuregate.Feature = "CatalogdHA" + +// CatalogdLeaderPodIsForceDeleted force-deletes the catalogd leader pod to simulate leader loss. +// The pod is identified from sc.leaderPods["catalogd"] (populated by a prior +// "catalogd is ready to reconcile resources" step). Force-deletion is equivalent to +// an abrupt process crash: the lease is no longer renewed and the surviving pod +// acquires leadership after the lease expires. +// +// Note: stopping the kind node container is not used here because both nodes in the +// experimental 2-node cluster are control-plane nodes that run etcd — stopping either +// would break etcd quorum and make the API server unreachable for the rest of the test. +func CatalogdLeaderPodIsForceDeleted(ctx context.Context) error { + sc := scenarioCtx(ctx) + leaderPod := sc.leaderPods["catalogd"] + if leaderPod == "" { + return fmt.Errorf("catalogd leader pod not found in scenario context; run 'catalogd is ready to reconcile resources' first") + } + + logger.Info("Force-deleting catalogd leader pod", "pod", leaderPod) + if _, err := k8sClient("delete", "pod", leaderPod, "-n", namespaceForComponent("catalogd"), + "--force", "--grace-period=0"); err != nil { + return fmt.Errorf("failed to force-delete catalogd leader pod %q: %w", leaderPod, err) + } + return nil +} + +// NewCatalogdLeaderIsElected polls the catalogd leader election lease until the holder +// identity changes to a pod other than the deleted leader. It updates +// sc.leaderPods["catalogd"] with the new leader pod name. +func NewCatalogdLeaderIsElected(ctx context.Context) error { + sc := scenarioCtx(ctx) + oldLeader := sc.leaderPods["catalogd"] + + waitFor(ctx, func() bool { + holder, err := k8sClient("get", "lease", leaseNames["catalogd"], "-n", namespaceForComponent("catalogd"), + "-o", "jsonpath={.spec.holderIdentity}") + if err != nil || holder == "" { + return false + } + newPod := strings.Split(strings.TrimSpace(holder), "_")[0] + if newPod == oldLeader { + return false + } + sc.leaderPods["catalogd"] = newPod + logger.Info("New catalogd leader elected", "pod", newPod) + return true + }) + return nil +} diff --git a/test/e2e/steps/hooks.go b/test/e2e/steps/hooks.go index 7b7762d8fd..d34ccd31e2 100644 --- a/test/e2e/steps/hooks.go +++ b/test/e2e/steps/hooks.go @@ -8,11 +8,12 @@ import ( "os/exec" "regexp" "strconv" - "sync" + "strings" "github.com/cucumber/godog" "github.com/go-logr/logr" "github.com/spf13/pflag" + "golang.org/x/sync/errgroup" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/component-base/featuregate" @@ -90,6 +91,7 @@ var ( features.HelmChartSupport: false, features.BoxcutterRuntime: false, features.DeploymentConfig: false, + catalogdHAFeature: false, } logger logr.Logger ) @@ -106,22 +108,33 @@ func RegisterHooks(sc *godog.ScenarioContext) { sc.After(ScenarioCleanup) } -func detectOLMDeployment() (*appsv1.Deployment, error) { +// detectOLMDeployments returns the operator-controller deployment (first) and the catalogd +// deployment (second) found via the app.kubernetes.io/part-of=olm label across all namespaces. +// The catalogd return value may be nil when OLM is not yet installed (upgrade scenarios +// install it in a Background step). +func detectOLMDeployments() (*appsv1.Deployment, *appsv1.Deployment, error) { raw, err := k8sClient("get", "deployments", "-A", "-l", "app.kubernetes.io/part-of=olm", "-o", "jsonpath={.items}") if err != nil { - return nil, err + return nil, nil, err } dl := []appsv1.Deployment{} if err := json.Unmarshal([]byte(raw), &dl); err != nil { - return nil, fmt.Errorf("failed to unmarshal OLM deployments: %v", err) + return nil, nil, fmt.Errorf("failed to unmarshal OLM deployments: %v", err) } - for _, d := range dl { - if d.Name == olmDeploymentName { - return &d, nil + var operatorController, catalogd *appsv1.Deployment + for i := range dl { + switch dl[i].Name { + case olmDeploymentName: + operatorController = &dl[i] + case catalogdDeploymentName: + catalogd = &dl[i] } } - return nil, fmt.Errorf("failed to detect OLM Deployment") + if operatorController == nil { + return nil, nil, fmt.Errorf("failed to detect OLM Deployment") + } + return operatorController, catalogd, nil } func BeforeSuite() { @@ -131,12 +144,39 @@ func BeforeSuite() { logger = textlogger.NewLogger(textlogger.NewConfig()) } - olm, err := detectOLMDeployment() + // Enable HA scenarios when the cluster has at least 2 nodes. This runs + // unconditionally so that upgrade scenarios (which install OLM in a Background + // step and return early below) still get the gate set correctly. + if out, err := k8sClient("get", "nodes", "--no-headers", "-o", "name"); err == nil && + len(strings.Fields(strings.TrimSpace(out))) >= 2 { + featureGates[catalogdHAFeature] = true + } + + olm, catalogdDep, err := detectOLMDeployments() if err != nil { logger.Info("OLM deployments not found; skipping feature gate detection (upgrade scenarios will install OLM in Background)") return } olmNamespace = olm.Namespace + componentNamespaces["operator-controller"] = olmNamespace + + // Catalogd may be in a different namespace than operator-controller. + catalogdNS := olmNamespace + if catalogdDep != nil { + catalogdNS = catalogdDep.Namespace + } + componentNamespaces["catalogd"] = catalogdNS + + // Refine CatalogdHA based on actual catalogd replica count now that catalogdNS is + // known. The node-count check above can fire on any multi-node cluster even when + // catalogd runs with only 1 replica. Override the gate: HA scenarios require ≥2 + // catalogd replicas. Fall back to whatever the node-count check set when catalogd + // was not found or the replica count is not parseable. + if catalogdDep != nil { + if replicas := catalogdDep.Spec.Replicas; replicas != nil { + featureGates[catalogdHAFeature] = *replicas >= 2 + } + } featureGatePattern := regexp.MustCompile(`--feature-gates=([[:alnum:]]+)=(true|false)`) for _, c := range olm.Spec.Template.Spec.Containers { @@ -152,6 +192,7 @@ func BeforeSuite() { } } } + logger.Info(fmt.Sprintf("Enabled feature gates: %v", featureGates)) } @@ -233,20 +274,20 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context } forDeletion = append(forDeletion, resource{name: sc.namespace, kind: "namespace"}) - var wg sync.WaitGroup + g := new(errgroup.Group) + g.SetLimit(8) for _, r := range forDeletion { - wg.Add(1) - go func(res resource) { - defer wg.Done() - args := []string{"delete", res.kind, res.name, "--ignore-not-found=true"} - if res.namespace != "" { - args = append(args, "-n", res.namespace) + g.Go(func() error { + args := []string{"delete", r.kind, r.name, "--ignore-not-found=true"} + if r.namespace != "" { + args = append(args, "-n", r.namespace) } if _, err := k8sClient(args...); err != nil { - logger.Info("Error deleting resource", "name", res.name, "namespace", res.namespace, "stderr", stderrOutput(err)) + logger.Info("Error deleting resource", "name", r.name, "namespace", r.namespace, "stderr", stderrOutput(err)) } - }(r) + return nil + }) } - wg.Wait() + _ = g.Wait() return ctx, nil } diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index ccb1fc4a03..0a9bf21157 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -50,13 +50,15 @@ import ( ) const ( - olmDeploymentName = "operator-controller-controller-manager" - timeout = 5 * time.Minute - tick = 1 * time.Second + olmDeploymentName = "operator-controller-controller-manager" + catalogdDeploymentName = "catalogd-controller-manager" + timeout = 5 * time.Minute + tick = 1 * time.Second ) var ( olmNamespace = "olmv1-system" + componentNamespaces = map[string]string{} // keyed by component label (e.g. "catalogd"); falls back to olmNamespace kubeconfigPath string k8sCli string deployImageRegistry = sync.OnceValue(func() error { @@ -99,6 +101,7 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ClusterExtension is applied(?:\s+.*)?$`, ResourceIsApplied) sc.Step(`^(?i)ClusterExtension is updated to version "([^"]+)"$`, ClusterExtensionVersionUpdate) sc.Step(`^(?i)ClusterExtension is updated(?:\s+.*)?$`, ResourceIsApplied) + sc.Step(`^(?i)ClusterObjectSet "([^"]+)" lifecycle is set to "([^"]+)"$`, ClusterObjectSetLifecycleUpdate) sc.Step(`^(?i)ClusterExtension is available$`, ClusterExtensionIsAvailable) sc.Step(`^(?i)ClusterExtension is rolled out$`, ClusterExtensionIsRolledOut) sc.Step(`^(?i)ClusterExtension resources are created and labeled$`, ClusterExtensionResourcesCreatedAndAreLabeled) @@ -194,6 +197,9 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)the "([^"]+)" component is configured with HTTPS_PROXY "([^"]+)"$`, ConfigureDeploymentWithHTTPSProxy) sc.Step(`^(?i)the "([^"]+)" component is configured with HTTPS_PROXY pointing to a recording proxy$`, StartRecordingProxyAndConfigureDeployment) sc.Step(`^(?i)the recording proxy received a CONNECT request for the catalogd service$`, RecordingProxyReceivedCONNECTForCatalogd) + + sc.Step(`^(?i)the catalogd leader pod is force-deleted$`, CatalogdLeaderPodIsForceDeleted) + sc.Step(`^(?i)a new catalogd leader is elected$`, NewCatalogdLeaderIsElected) } func init() { @@ -210,6 +216,15 @@ func init() { } } +// namespaceForComponent returns the namespace for the named OLM component. +// Falls back to olmNamespace when the component has no dedicated entry (e.g. upstream single-namespace deployments). +func namespaceForComponent(component string) string { + if ns, ok := componentNamespaces[component]; ok { + return ns + } + return olmNamespace +} + func k8sClient(args ...string) (string, error) { cmd := exec.Command(k8sCli, args...) logger.V(1).Info("Running", "command", strings.Join(cmd.Args, " ")) @@ -374,6 +389,23 @@ func ClusterExtensionVersionUpdate(ctx context.Context, version string) error { return err } +// ClusterObjectSetLifecycleUpdate patches the ClusterObjectSet's lifecycleState to the specified value. +func ClusterObjectSetLifecycleUpdate(ctx context.Context, cosName, lifecycle string) error { + sc := scenarioCtx(ctx) + cosName = substituteScenarioVars(cosName, sc) + patch := map[string]any{ + "spec": map[string]any{ + "lifecycleState": lifecycle, + }, + } + pb, err := json.Marshal(patch) + if err != nil { + return err + } + _, err = k8sClient("patch", "clusterobjectset", cosName, "--type", "merge", "-p", string(pb)) + return err +} + // ResourceIsApplied applies the provided YAML resource to the cluster and in case of ClusterExtension or ClusterObjectSet it captures // its name in the test context so that it can be referred to in later steps with ${NAME} or ${COS_NAME}, respectively func ResourceIsApplied(ctx context.Context, yamlTemplate *godog.DocString) error { @@ -1568,7 +1600,10 @@ func parseCatalogTable(table *godog.Table) ([]catalog.PackageOption, error) { return nil, fmt.Errorf("duplicate bundle %s/%s: contents must be empty for repeated versions", pkg, version) } } else { - opts := parseContents(contents) + opts, err := parseContents(contents) + if err != nil { + return nil, fmt.Errorf("bundle %s/%s: %w", pkg, version, err) + } bundleDefs[bk] = &bundleEntry{opts: opts} bundleOrder = append(bundleOrder, bk) } @@ -1655,13 +1690,13 @@ spec: return nil } -func parseContents(contents string) []catalog.BundleOption { +func parseContents(contents string) ([]catalog.BundleOption, error) { contents = strings.TrimSpace(contents) if contents == "" { - return nil + return nil, nil } if strings.EqualFold(contents, "BadImage") { - return []catalog.BundleOption{catalog.BadImage()} + return []catalog.BundleOption{catalog.BadImage()}, nil } var opts []catalog.BundleOption for _, part := range strings.Split(contents, ",") { @@ -1686,10 +1721,11 @@ func parseContents(contents string) []catalog.BundleOption { case strings.HasPrefix(part, "LargeCRD(") && strings.HasSuffix(part, ")"): // LargeCRD(250) countStr := part[len("LargeCRD(") : len(part)-1] - count, err := strconv.Atoi(countStr) - if err == nil { - opts = append(opts, catalog.WithLargeCRD(count)) + count, err := strconv.ParseInt(countStr, 10, 0) + if err != nil || count <= 0 { + return nil, fmt.Errorf("invalid LargeCRD field count %q: must be a positive integer", countStr) } + opts = append(opts, catalog.WithLargeCRD(int(count))) case strings.HasPrefix(part, "ClusterRegistry(") && strings.HasSuffix(part, ")"): // ClusterRegistry(mirrored-registry.operator-controller-e2e.svc.cluster.local:5000) host := part[len("ClusterRegistry(") : len(part)-1] @@ -1701,7 +1737,7 @@ func parseContents(contents string) []catalog.BundleOption { opts = append(opts, catalog.StaticBundleDir(absDir)) } } - return opts + return opts, nil } // PrometheusMetricsAreReturned validates that each pod's stored metrics response is non-empty and parses as valid Prometheus text format. diff --git a/test/e2e/steps/upgrade_steps.go b/test/e2e/steps/upgrade_steps.go index 0c82437a2a..47aab69d2f 100644 --- a/test/e2e/steps/upgrade_steps.go +++ b/test/e2e/steps/upgrade_steps.go @@ -29,7 +29,7 @@ var ( if err != nil { return err } - olm, err := detectOLMDeployment() + olm, _, err := detectOLMDeployments() if err != nil { return err } @@ -82,19 +82,20 @@ func OLMIsUpgraded(ctx context.Context) error { // then checks the leader election lease and stores the leader pod name in the scenario context. func ComponentIsReadyToReconcile(ctx context.Context, component string) error { sc := scenarioCtx(ctx) + ns := namespaceForComponent(component) // Wait for deployment rollout to complete - depName, err := k8sClient("get", "deployments", "-n", olmNamespace, + depName, err := k8sClient("get", "deployments", "-n", ns, "-l", fmt.Sprintf("app.kubernetes.io/name=%s", component), "-o", "jsonpath={.items[0].metadata.name}") if err != nil { - return fmt.Errorf("failed to find deployment for component %s: %w", component, err) + return fmt.Errorf("failed to find deployment for component %s in namespace %s: %w", component, ns, err) } if depName == "" { - return fmt.Errorf("failed to find deployment for component %s: no matching deployments found", component) + return fmt.Errorf("failed to find deployment for component %s in namespace %s: no matching deployments found", component, ns) } if _, err := k8sClient("rollout", "status", fmt.Sprintf("deployment/%s", depName), - "-n", olmNamespace, fmt.Sprintf("--timeout=%s", timeout)); err != nil { + "-n", ns, fmt.Sprintf("--timeout=%s", timeout)); err != nil { return fmt.Errorf("deployment rollout failed for %s: %w", component, err) } @@ -106,7 +107,7 @@ func ComponentIsReadyToReconcile(ctx context.Context, component string) error { // Leader election can take up to LeaseDuration (137s) + RetryPeriod (26s) ≈ 163s in the worst case waitFor(ctx, func() bool { - output, err := k8sClient("get", "lease", leaseName, "-n", olmNamespace, + output, err := k8sClient("get", "lease", leaseName, "-n", ns, "-o", "jsonpath={.spec.holderIdentity}") if err != nil || output == "" { return false @@ -129,9 +130,9 @@ var resourceTypeToComponent = map[string]string{ // reconcileEndingCheck returns a function that checks whether the leader pod's logs // contain a "reconcile ending" entry for the given resource name. -func reconcileEndingCheck(leaderPod, resourceName string) func() bool { +func reconcileEndingCheck(namespace, leaderPod, resourceName string) func() bool { return func() bool { - logs, err := k8sClient("logs", leaderPod, "-n", olmNamespace, "--all-containers=true", "--tail=1000") + logs, err := k8sClient("logs", leaderPod, "-n", namespace, "--all-containers=true", "--tail=1000") if err != nil { return false } @@ -160,13 +161,13 @@ func ClusterExtensionIsReconciled(ctx context.Context) error { } leaderPod := sc.leaderPods[component] - waitFor(ctx, reconcileEndingCheck(leaderPod, resourceName)) + waitFor(ctx, reconcileEndingCheck(namespaceForComponent(component), leaderPod, resourceName)) return nil } // clusterCatalogUnpackedAfterPodCreation returns a check function that verifies the // ClusterCatalog is serving and its lastUnpacked timestamp is after the leader pod's creation. -func clusterCatalogUnpackedAfterPodCreation(resourceName, leaderPod string) func() bool { +func clusterCatalogUnpackedAfterPodCreation(namespace, resourceName, leaderPod string) func() bool { return func() bool { catalogJSON, err := k8sClient("get", "clustercatalog", resourceName, "-o", "json") if err != nil { @@ -190,7 +191,7 @@ func clusterCatalogUnpackedAfterPodCreation(resourceName, leaderPod string) func return false } - podJSON, err := k8sClient("get", "pod", leaderPod, "-n", olmNamespace, "-o", "json") + podJSON, err := k8sClient("get", "pod", leaderPod, "-n", namespace, "-o", "json") if err != nil { return false } @@ -229,8 +230,9 @@ func allResourcesAreReconciled(ctx context.Context, resourceType string) error { return fmt.Errorf("no %s resources found", resourceType) } + ns := namespaceForComponent(component) for _, name := range resourceNames { - waitFor(ctx, reconcileEndingCheck(leaderPod, name)) + waitFor(ctx, reconcileEndingCheck(ns, leaderPod, name)) } return nil @@ -254,11 +256,12 @@ func ScenarioCatalogIsReconciled(ctx context.Context, catalogUserName string) er return fmt.Errorf("leader pod not found for component %s", component) } - waitFor(ctx, reconcileEndingCheck(leaderPod, catalogName)) + ns := namespaceForComponent(component) + waitFor(ctx, reconcileEndingCheck(ns, leaderPod, catalogName)) // Also verify that lastUnpacked is after the leader pod's creation. // This mitigates flakiness caused by https://github.com/operator-framework/operator-controller/issues/1626 - waitFor(ctx, clusterCatalogUnpackedAfterPodCreation(catalogName, leaderPod)) + waitFor(ctx, clusterCatalogUnpackedAfterPodCreation(ns, catalogName, leaderPod)) return nil } diff --git a/test/extension-developer-e2e/setup.sh b/test/extension-developer-e2e/setup.sh index 9293ef420c..a25e0ad375 100755 --- a/test/extension-developer-e2e/setup.sh +++ b/test/extension-developer-e2e/setup.sh @@ -92,7 +92,6 @@ reg_bundle_img="${cluster_registry_host}/bundles/registry-v1/registry-bundle:v0. # because docker push goes through the Docker daemon which # may be in a different network context (e.g. colima VM). - ############################### # Create the FBC that contains # the registry+v1 extensions diff --git a/test/internal/catalog/catalog.go b/test/internal/catalog/catalog.go index 705eab2fdf..80e38a7ad2 100644 --- a/test/internal/catalog/catalog.go +++ b/test/internal/catalog/catalog.go @@ -154,9 +154,9 @@ func (c *Catalog) Build(ctx context.Context, tag, localRegistry, clusterRegistry return nil, fmt.Errorf("failed to set bundle labels for %s:%s: %w", paramPkgName, bd.version, err) } - tag := fmt.Sprintf("%s/bundles/%s:v%s", localRegistry, paramPkgName, bd.version) - if err := crane.Push(img, tag, crane.Insecure, crane.WithContext(ctx)); err != nil { - return nil, fmt.Errorf("failed to push bundle image %s: %w", tag, err) + bundleTag := fmt.Sprintf("%s/bundles/%s:v%s", localRegistry, paramPkgName, bd.version) + if err := crane.Push(img, bundleTag, crane.Insecure, crane.WithContext(ctx)); err != nil { + return nil, fmt.Errorf("failed to push bundle image %s: %w", bundleTag, err) } bundleClusterRegistry := clusterRegistry if spec.clusterRegistryOverride != "" { diff --git a/test/internal/catalog/catalog_test.go b/test/internal/catalog/catalog_test.go index 70fd345dfa..ce6465e2b9 100644 --- a/test/internal/catalog/catalog_test.go +++ b/test/internal/catalog/catalog_test.go @@ -134,17 +134,29 @@ func TestCatalog_FBCGeneration(t *testing.T) { t.Errorf("expected 2 channels, got %d", len(pkg.channels)) } - // Verify alpha channel has 1 entry - alpha := pkg.channels[0] - if alpha.name != "alpha" { - t.Errorf("expected channel 'alpha', got %q", alpha.name) + // Verify channels by name rather than by index so the test is robust to ordering changes. + var alpha, beta *channelDef + for i := range pkg.channels { + switch pkg.channels[i].name { + case "alpha": + alpha = &pkg.channels[i] + case "beta": + beta = &pkg.channels[i] + } + } + if alpha == nil { + t.Fatal("alpha channel not found") + } + if beta == nil { + t.Fatal("beta channel not found") } + + // Verify alpha channel has 1 entry if len(alpha.entries) != 1 { t.Errorf("expected 1 alpha entry, got %d", len(alpha.entries)) } // Verify beta channel has replaces edge - beta := pkg.channels[1] if len(beta.entries) != 2 { t.Fatalf("expected 2 beta entries, got %d", len(beta.entries)) } diff --git a/vendor/github.com/containerd/containerd/archive/tar_unix.go b/vendor/github.com/containerd/containerd/archive/tar_unix.go index fa61100609..684ea5783d 100644 --- a/vendor/github.com/containerd/containerd/archive/tar_unix.go +++ b/vendor/github.com/containerd/containerd/archive/tar_unix.go @@ -80,7 +80,7 @@ func openFile(name string, flag int, perm os.FileMode) (*os.File, error) { return nil, err } // Call chmod to avoid permission mask - if err := os.Chmod(name, perm); err != nil { + if err := f.Chmod(perm); err != nil { f.Close() return nil, err } diff --git a/vendor/github.com/containerd/containerd/version/version.go b/vendor/github.com/containerd/containerd/version/version.go index f20a57a35f..6344da5983 100644 --- a/vendor/github.com/containerd/containerd/version/version.go +++ b/vendor/github.com/containerd/containerd/version/version.go @@ -23,7 +23,7 @@ var ( Package = "github.com/containerd/containerd" // Version holds the complete version number. Filled in at linking time. - Version = "1.7.30+unknown" + Version = "1.7.31+unknown" // Revision is filled with the VCS (e.g. git) revision being used to build // the program at linking time. diff --git a/vendor/github.com/docker/cli/AUTHORS b/vendor/github.com/docker/cli/AUTHORS index 57af08b204..accbf6c52b 100644 --- a/vendor/github.com/docker/cli/AUTHORS +++ b/vendor/github.com/docker/cli/AUTHORS @@ -2,6 +2,7 @@ # This file lists all contributors to the repository. # See scripts/docs/generate-authors.sh to make modifications. +4RH1T3CT0R7 A. Lester Buck III Aanand Prasad Aaron L. Xu @@ -42,6 +43,7 @@ Alexander Larsson Alexander Morozov Alexander Ryabov Alexandre González +Alexandre Vallières-Lagacé Alexey Igrychev Alexis Couvreur Alfred Landrum @@ -64,6 +66,7 @@ Andres G. Aragoneses Andres Leon Rangel Andrew France Andrew He +Andrew Hopp Andrew Hsu Andrew Macpherson Andrew McDonnell @@ -127,6 +130,7 @@ Brian Goff Brian Tracy Brian Wieder Bruno Sousa +Bruno Verachten Bryan Bess Bryan Boreham Bryan Murphy @@ -178,6 +182,7 @@ Christopher Svensson Christy Norman Chun Chen Clinton Kitson +Codex Coenraad Loubser Colin Hebert Collin Guarino @@ -234,6 +239,7 @@ David Sheets David Williamson David Xia David Young +Davlat Davydov Deng Guangxing Denis Defreyne Denis Gladkikh @@ -241,6 +247,7 @@ Denis Ollier Dennis Docter dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Derek McGowan +Derek Misler Des Preston Deshi Xiao Dharmit Shah @@ -260,6 +267,7 @@ Dominik Braun Don Kjer Dong Chen DongGeon Lee +Dorin Geman Doug Davis Drew Erny Ed Costello @@ -358,7 +366,7 @@ Hugo Gabriel Eyherabide huqun Huu Nguyen Hyzhou Zhy -Iain MacDonald +Iain MacDonald Iain Samuel McLean Elder Ian Campbell Ian Philpot @@ -471,6 +479,7 @@ Justyn Temme Jyrki Puttonen Jérémie Drouet Jérôme Petazzoni +Jörg Sommer Jörg Thalheim Kai Blin Kai Qiang Wu (Kennan) @@ -539,10 +548,12 @@ Lovekesh Kumar Luca Favatella Luca Marturana Lucas Chan +Ludovic Temgoua Abanda Luis Henrique Mulinari Luka Hartwig Lukas Heeren Lukasz Zajaczkowski +Luo Jiyin Lydell Manganti Lénaïc Huard Ma Shimiao @@ -603,6 +614,7 @@ Michael Spetsiotis Michael Steinert Michael Tews Michael West +Michael Zampani Michal Minář Michał Czeraszkiewicz Miguel Angel Alvarez Cabrerizo @@ -617,6 +629,7 @@ Mike Goelzer Mike MacCana mikelinjie <294893458@qq.com> Mikhail Vasin +Milas Bowman Milind Chawre Mindaugas Rukas Miroslav Gula @@ -887,6 +900,7 @@ Vincent Batts Vincent Bernat Vincent Demeester Vincent Woo +Vineet Kumar Vishnu Kannan Vivek Goyal Wang Jie @@ -916,6 +930,7 @@ Yanqiang Miao Yassine Tijani Yi EungJun Ying Li +Yoan Wainmann Yong Tang Yosef Fertel Yu Peng diff --git a/vendor/github.com/docker/cli/cli/config/configfile/file.go b/vendor/github.com/docker/cli/cli/config/configfile/file.go index 246f23e983..1a0e5b46ca 100644 --- a/vendor/github.com/docker/cli/cli/config/configfile/file.go +++ b/vendor/github.com/docker/cli/cli/config/configfile/file.go @@ -1,5 +1,5 @@ // FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16: -//go:build go1.24 +//go:build go1.25 package configfile diff --git a/vendor/github.com/docker/cli/cli/config/credentials/file_store.go b/vendor/github.com/docker/cli/cli/config/credentials/file_store.go index c69312b014..e3ef8e25ed 100644 --- a/vendor/github.com/docker/cli/cli/config/credentials/file_store.go +++ b/vendor/github.com/docker/cli/cli/config/credentials/file_store.go @@ -99,9 +99,14 @@ func (c *fileStore) Store(authConfig types.AuthConfig) error { return nil } -// ConvertToHostname converts a registry url which has http|https prepended -// to just an hostname. -// Copied from github.com/docker/docker/registry.ConvertToHostname to reduce dependencies. +// ConvertToHostname normalizes a registry URL which has http|https prepended +// to just its hostname. It is used to match credentials, which may be either +// stored as hostname or as hostname including scheme (in legacy configuration +// files). +// +// It's the equivalent to [registry.ConvertToHostname] in the daemon. +// +// [registry.ConvertToHostname]: https://pkg.go.dev/github.com/moby/moby/v2@v2.0.0-beta.7/daemon/pkg/registry#ConvertToHostname func ConvertToHostname(maybeURL string) string { stripped := maybeURL if strings.Contains(stripped, "://") { diff --git a/vendor/github.com/docker/cli/cli/config/memorystore/store.go b/vendor/github.com/docker/cli/cli/config/memorystore/store.go index f8ec62b95a..44523d392b 100644 --- a/vendor/github.com/docker/cli/cli/config/memorystore/store.go +++ b/vendor/github.com/docker/cli/cli/config/memorystore/store.go @@ -1,5 +1,5 @@ // FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16: -//go:build go1.24 +//go:build go1.25 package memorystore diff --git a/vendor/modules.txt b/vendor/modules.txt index b1e70d7e8d..c61e9e2395 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -89,7 +89,7 @@ github.com/blang/semver/v4 # github.com/cenkalti/backoff/v5 v5.0.3 ## explicit; go 1.23 github.com/cenkalti/backoff/v5 -# github.com/cert-manager/cert-manager v1.20.1 +# github.com/cert-manager/cert-manager v1.20.2 ## explicit; go 1.25.0 github.com/cert-manager/cert-manager/pkg/apis/acme github.com/cert-manager/cert-manager/pkg/apis/acme/v1 @@ -116,7 +116,7 @@ github.com/clipperhouse/uax29/v2/graphemes # github.com/containerd/cgroups/v3 v3.1.2 ## explicit; go 1.22.0 github.com/containerd/cgroups/v3/cgroup1/stats -# github.com/containerd/containerd v1.7.30 +# github.com/containerd/containerd v1.7.31 ## explicit; go 1.24.0 github.com/containerd/containerd/archive github.com/containerd/containerd/archive/compression @@ -241,7 +241,7 @@ github.com/davecgh/go-spew/spew # github.com/distribution/reference v0.6.0 ## explicit; go 1.20 github.com/distribution/reference -# github.com/docker/cli v29.3.1+incompatible +# github.com/docker/cli v29.4.0+incompatible ## explicit github.com/docker/cli/cli/config github.com/docker/cli/cli/config/configfile @@ -444,7 +444,7 @@ github.com/google/go-cmp/cmp/internal/diff github.com/google/go-cmp/cmp/internal/flags github.com/google/go-cmp/cmp/internal/function github.com/google/go-cmp/cmp/internal/value -# github.com/google/go-containerregistry v0.21.4 +# github.com/google/go-containerregistry v0.21.5 ## explicit; go 1.25.0 github.com/google/go-containerregistry/internal/and github.com/google/go-containerregistry/internal/compression @@ -2185,7 +2185,7 @@ sigs.k8s.io/kustomize/kyaml/yaml/walk ## explicit; go 1.18 sigs.k8s.io/randfill sigs.k8s.io/randfill/bytesource -# sigs.k8s.io/structured-merge-diff/v6 v6.3.2 +# sigs.k8s.io/structured-merge-diff/v6 v6.4.0 ## explicit; go 1.23 sigs.k8s.io/structured-merge-diff/v6/fieldpath sigs.k8s.io/structured-merge-diff/v6/merge diff --git a/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go b/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go index 73436912cd..f1607601c4 100644 --- a/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go +++ b/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/element.go @@ -19,7 +19,7 @@ package fieldpath import ( "fmt" "iter" - "sort" + "slices" "strings" "sigs.k8s.io/structured-merge-diff/v6/value" @@ -255,19 +255,13 @@ func (s PathElementSet) Copy() PathElementSet { // Insert adds pe to the set. func (s *PathElementSet) Insert(pe PathElement) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a, b PathElement) int { + return a.Compare(b) }) - if loc == len(s.members) { - s.members = append(s.members, pe) + if found { return } - if s.members[loc].Equals(pe) { - return - } - s.members = append(s.members, PathElement{}) - copy(s.members[loc+1:], s.members[loc:]) - s.members[loc] = pe + s.members = slices.Insert(s.members, loc, pe) } // Union returns a set containing elements that appear in either s or s2. @@ -344,16 +338,10 @@ func (s *PathElementSet) Size() int { return len(s.members) } // Has returns true if pe is a member of the set. func (s *PathElementSet) Has(pe PathElement) bool { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].Less(pe) + _, found := slices.BinarySearchFunc(s.members, pe, func(a, b PathElement) int { + return a.Compare(b) }) - if loc == len(s.members) { - return false - } - if s.members[loc].Equals(pe) { - return true - } - return false + return found } // Equals returns true if s and s2 have exactly the same members. diff --git a/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go b/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go index ff7ee510c2..ca50b84e99 100644 --- a/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go +++ b/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/pathelementmap.go @@ -17,7 +17,7 @@ limitations under the License. package fieldpath import ( - "sort" + "slices" "sigs.k8s.io/structured-merge-diff/v6/value" ) @@ -82,32 +82,23 @@ func MakePathElementMap(size int) PathElementMap { // Insert adds the pathelement and associated value in the map. // If insert is called twice with the same PathElement, the value is replaced. func (s *PathElementMap) Insert(pe PathElement, v interface{}) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].PathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a pathElementValue, b PathElement) int { + return a.PathElement.Compare(b) }) - if loc == len(s.members) { - s.members = append(s.members, pathElementValue{pe, v}) - return - } - if s.members[loc].PathElement.Equals(pe) { + if found { s.members[loc].Value = v return } - s.members = append(s.members, pathElementValue{}) - copy(s.members[loc+1:], s.members[loc:]) - s.members[loc] = pathElementValue{pe, v} + s.members = slices.Insert(s.members, loc, pathElementValue{PathElement: pe, Value: v}) } // Get retrieves the value associated with the given PathElement from the map. // (nil, false) is returned if there is no such PathElement. func (s *PathElementMap) Get(pe PathElement) (interface{}, bool) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].PathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a pathElementValue, b PathElement) int { + return a.PathElement.Compare(b) }) - if loc == len(s.members) { - return nil, false - } - if s.members[loc].PathElement.Equals(pe) { + if found { return s.members[loc].Value, true } return nil, false diff --git a/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go b/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go index d2d8c8a42b..d7fe643be2 100644 --- a/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go +++ b/vendor/sigs.k8s.io/structured-merge-diff/v6/fieldpath/set.go @@ -19,6 +19,7 @@ package fieldpath import ( "fmt" "iter" + "slices" "sort" "strings" @@ -486,19 +487,13 @@ func (s *SetNodeMap) Copy() SetNodeMap { // Descend adds pe to the set if necessary, returning the associated subset. func (s *SetNodeMap) Descend(pe PathElement) *Set { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].pathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a setNode, b PathElement) int { + return a.pathElement.Compare(b) }) - if loc == len(s.members) { - s.members = append(s.members, setNode{pathElement: pe, set: &Set{}}) + if found { return s.members[loc].set } - if s.members[loc].pathElement.Equals(pe) { - return s.members[loc].set - } - s.members = append(s.members, setNode{}) - copy(s.members[loc+1:], s.members[loc:]) - s.members[loc] = setNode{pathElement: pe, set: &Set{}} + s.members = slices.Insert(s.members, loc, setNode{pathElement: pe, set: &Set{}}) return s.members[loc].set } @@ -523,13 +518,10 @@ func (s *SetNodeMap) Empty() bool { // Get returns (the associated set, true) or (nil, false) if there is none. func (s *SetNodeMap) Get(pe PathElement) (*Set, bool) { - loc := sort.Search(len(s.members), func(i int) bool { - return !s.members[i].pathElement.Less(pe) + loc, found := slices.BinarySearchFunc(s.members, pe, func(a setNode, b PathElement) int { + return a.pathElement.Compare(b) }) - if loc == len(s.members) { - return nil, false - } - if s.members[loc].pathElement.Equals(pe) { + if found { return s.members[loc].set, true } return nil, false diff --git a/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go b/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go index f70cd41674..acbfe8ceba 100644 --- a/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go +++ b/vendor/sigs.k8s.io/structured-merge-diff/v6/value/allocator.go @@ -16,6 +16,8 @@ limitations under the License. package value +import "reflect" + // Allocator provides a value object allocation strategy. // Value objects can be allocated by passing an allocator to the "Using" // receiver functions on the value interfaces, e.g. Map.ZipUsing(allocator, ...). @@ -24,8 +26,8 @@ package value type Allocator interface { // Free gives the allocator back any value objects returned by the "Using" // receiver functions on the value interfaces. - // interface{} may be any of: Value, Map, List or Range. - Free(interface{}) + // any may be any of: Value, Map, List or Range. + Free(any) // The unexported functions are for "Using" receiver functions of the value types // to request what they need from the allocator. @@ -74,7 +76,7 @@ func (p *heapAllocator) allocListReflectRange() *listReflectRange { return &listReflectRange{vr: &valueReflect{}} } -func (p *heapAllocator) Free(_ interface{}) {} +func (p *heapAllocator) Free(_ any) {} // NewFreelistAllocator creates freelist based allocator. // This allocator provides fast allocation and freeing of short lived value objects. @@ -89,25 +91,25 @@ func (p *heapAllocator) Free(_ interface{}) {} // for all temporary value access. func NewFreelistAllocator() Allocator { return &freelistAllocator{ - valueUnstructured: &freelist{new: func() interface{} { + valueUnstructured: &freelist[*valueUnstructured]{new: func() *valueUnstructured { return &valueUnstructured{} }}, - listUnstructuredRange: &freelist{new: func() interface{} { + listUnstructuredRange: &freelist[*listUnstructuredRange]{new: func() *listUnstructuredRange { return &listUnstructuredRange{vv: &valueUnstructured{}} }}, - valueReflect: &freelist{new: func() interface{} { + valueReflect: &freelist[*valueReflect]{new: func() *valueReflect { return &valueReflect{} }}, - mapReflect: &freelist{new: func() interface{} { + mapReflect: &freelist[*mapReflect]{new: func() *mapReflect { return &mapReflect{} }}, - structReflect: &freelist{new: func() interface{} { + structReflect: &freelist[*structReflect]{new: func() *structReflect { return &structReflect{} }}, - listReflect: &freelist{new: func() interface{} { + listReflect: &freelist[*listReflect]{new: func() *listReflect { return &listReflect{} }}, - listReflectRange: &freelist{new: func() interface{} { + listReflectRange: &freelist[*listReflectRange]{new: func() *listReflectRange { return &listReflectRange{vr: &valueReflect{}} }}, } @@ -119,22 +121,22 @@ func NewFreelistAllocator() Allocator { const freelistMaxSize = 1000 type freelistAllocator struct { - valueUnstructured *freelist - listUnstructuredRange *freelist - valueReflect *freelist - mapReflect *freelist - structReflect *freelist - listReflect *freelist - listReflectRange *freelist + valueUnstructured *freelist[*valueUnstructured] + listUnstructuredRange *freelist[*listUnstructuredRange] + valueReflect *freelist[*valueReflect] + mapReflect *freelist[*mapReflect] + structReflect *freelist[*structReflect] + listReflect *freelist[*listReflect] + listReflectRange *freelist[*listReflectRange] } -type freelist struct { - list []interface{} - new func() interface{} +type freelist[T any] struct { + list []T + new func() T } -func (f *freelist) allocate() interface{} { - var w2 interface{} +func (f *freelist[T]) allocate() T { + var w2 T if n := len(f.list); n > 0 { w2, f.list = f.list[n-1], f.list[:n-1] } else { @@ -143,61 +145,73 @@ func (f *freelist) allocate() interface{} { return w2 } -func (f *freelist) free(v interface{}) { +func (f *freelist[T]) free(v T) { if len(f.list) < freelistMaxSize { f.list = append(f.list, v) } } -func (w *freelistAllocator) Free(value interface{}) { +func (w *freelistAllocator) Free(value any) { switch v := value.(type) { case *valueUnstructured: v.Value = nil // don't hold references to unstructured objects w.valueUnstructured.free(v) case *listUnstructuredRange: + v.list = nil // don't hold references to unstructured objects v.vv.Value = nil // don't hold references to unstructured objects w.listUnstructuredRange.free(v) case *valueReflect: - v.ParentMapKey = nil v.ParentMap = nil + v.ParentMapKey = nil + v.Value = reflect.Value{} // don't hold references to reflected objects w.valueReflect.free(v) case *mapReflect: + v.valueReflect.ParentMap = nil + v.valueReflect.ParentMapKey = nil + v.valueReflect.Value = reflect.Value{} // don't hold references to reflected objects w.mapReflect.free(v) case *structReflect: + v.valueReflect.ParentMap = nil + v.valueReflect.ParentMapKey = nil + v.valueReflect.Value = reflect.Value{} // don't hold references to reflected objects w.structReflect.free(v) case *listReflect: + v.Value = reflect.Value{} // don't hold references to reflected objects w.listReflect.free(v) case *listReflectRange: - v.vr.ParentMapKey = nil + v.list = reflect.Value{} // don't hold references to reflected objects v.vr.ParentMap = nil + v.vr.ParentMapKey = nil + v.vr.Value = reflect.Value{} // don't hold references to reflected objects + v.entry = nil w.listReflectRange.free(v) } } func (w *freelistAllocator) allocValueUnstructured() *valueUnstructured { - return w.valueUnstructured.allocate().(*valueUnstructured) + return w.valueUnstructured.allocate() } func (w *freelistAllocator) allocListUnstructuredRange() *listUnstructuredRange { - return w.listUnstructuredRange.allocate().(*listUnstructuredRange) + return w.listUnstructuredRange.allocate() } func (w *freelistAllocator) allocValueReflect() *valueReflect { - return w.valueReflect.allocate().(*valueReflect) + return w.valueReflect.allocate() } func (w *freelistAllocator) allocStructReflect() *structReflect { - return w.structReflect.allocate().(*structReflect) + return w.structReflect.allocate() } func (w *freelistAllocator) allocMapReflect() *mapReflect { - return w.mapReflect.allocate().(*mapReflect) + return w.mapReflect.allocate() } func (w *freelistAllocator) allocListReflect() *listReflect { - return w.listReflect.allocate().(*listReflect) + return w.listReflect.allocate() } func (w *freelistAllocator) allocListReflectRange() *listReflectRange { - return w.listReflectRange.allocate().(*listReflectRange) + return w.listReflectRange.allocate() } diff --git a/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go b/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go index 3aadceb222..db25613734 100644 --- a/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go +++ b/vendor/sigs.k8s.io/structured-merge-diff/v6/value/jsontagutil.go @@ -76,11 +76,16 @@ func OmitZeroFunc(t reflect.Type) func(reflect.Value) bool { // but is based on the highly efficient approach from https://golang.org/src/encoding/json/encode.go func lookupJsonTags(f reflect.StructField) (name string, omit bool, inline bool, omitempty bool, omitzero func(reflect.Value) bool) { + // Skip unexported non-embedded fields, matching encoding/json behavior. + if f.PkgPath != "" && !f.Anonymous { + return "", true, false, false, nil + } tag := f.Tag.Get("json") if tag == "-" { return "", true, false, false, nil } name, opts := parseTag(tag) + inline = f.Anonymous && name == "" if name == "" { name = f.Name } @@ -89,7 +94,7 @@ func lookupJsonTags(f reflect.StructField) (name string, omit bool, inline bool, omitzero = OmitZeroFunc(f.Type) } - return name, false, opts.Contains("inline"), opts.Contains("omitempty"), omitzero + return name, false, inline, opts.Contains("omitempty"), omitzero } func isEmpty(v reflect.Value) bool {