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..4e85232fa8 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ 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/cert-manager/cert-manager v1.20.2 github.com/containerd/containerd v1.7.30 github.com/cucumber/godog v0.15.1 github.com/evanphx/json-patch v5.9.11+incompatible @@ -14,7 +14,7 @@ require ( 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 @@ -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..d7d18fdc64 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= @@ -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= 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..c29b5742f7 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 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/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/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/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/steps/ha_steps.go b/test/e2e/steps/ha_steps.go new file mode 100644 index 0000000000..2859178504 --- /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", olmNamespace, + "--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", olmNamespace, + "-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..5325a79f21 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 ) @@ -131,6 +133,14 @@ func BeforeSuite() { logger = textlogger.NewLogger(textlogger.NewConfig()) } + // 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, err := detectOLMDeployment() if err != nil { logger.Info("OLM deployments not found; skipping feature gate detection (upgrade scenarios will install OLM in Background)") @@ -152,6 +162,7 @@ func BeforeSuite() { } } } + logger.Info(fmt.Sprintf("Enabled feature gates: %v", featureGates)) } @@ -233,20 +244,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..5466e2fc0c 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -194,6 +194,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() { @@ -1568,7 +1571,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 +1661,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 +1692,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 +1708,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/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/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..a1856a828b 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 @@ -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