From f20817c23e4a8ab3ee8aefc34d1f50493677e881 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 29 May 2026 16:15:07 -0400 Subject: [PATCH] override catalog tag to 4.x when RELEASE_VERSION is 4.x and catalogd.yaml pins v5.0 Partially reverts commit 7a85fa8f: removes the ocp-release sentinel mechanism (GetCatalogImageTag, ClusterCatalogImageTag, applyCatalogImageTagOverride, and the associated Builder field and main.go changes). Replaces it with a targeted check in renderHelmTemplate: when openshift/helm/catalogd.yaml (operator-framework-operator-controller) pins options.openshift.catalogs.version to "v5.0" and RELEASE_VERSION is a 4.x stream (the 4.23/5.0 co-release), substitute the 4.x catalog tag so images match the release stream. For 5.x releases, including 5.1 running against a "v5.0" or "v5.1" catalog, the value in catalogd.yaml is left untouched. Assisted-by: Claude code Signed-off-by: Todd Short --- cmd/cluster-olm-operator/main.go | 20 ++++----- internal/versionutils/version_utils.go | 6 --- internal/versionutils/version_utils_test.go | 31 ------------- pkg/controller/builder.go | 13 +++--- pkg/controller/helm.go | 34 +++++++------- pkg/controller/helm_test.go | 50 ++++++++++++--------- 6 files changed, 60 insertions(+), 94 deletions(-) diff --git a/cmd/cluster-olm-operator/main.go b/cmd/cluster-olm-operator/main.go index 7d7ee35d..ce3bf140 100644 --- a/cmd/cluster-olm-operator/main.go +++ b/cmd/cluster-olm-operator/main.go @@ -230,13 +230,6 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error log := klog.FromContext(ctx) - operatorImageVersion := status.VersionForOperatorFromEnv() - currentOCPMinorVersion, err := versionutils.GetCurrentOCPMinorVersion(operatorImageVersion) - if err != nil { - return err - } - catalogImageTag := versionutils.GetCatalogImageTag(currentOCPMinorVersion) - fg, err := cl.ConfigClient.ConfigV1().FeatureGates().Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { return fmt.Errorf("unable to retrieve featureSet: %w", err) @@ -259,9 +252,8 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error Scope: meta.RESTScopeRoot, }, }, - FeatureGate: *fg, - Infrastructure: infra, - ClusterCatalogImageTag: catalogImageTag, + FeatureGate: *fg, + Infrastructure: infra, } staticResourceControllers, deploymentControllers, clusterCatalogControllers, relatedObjects, err := cb.BuildControllers("catalogd", "operator-controller") @@ -296,6 +288,12 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error clusterCatalogControllerList = append(clusterCatalogControllerList, controller) } + operatorImageVersion := status.VersionForOperatorFromEnv() + currentOCPMinorVersion, err := versionutils.GetCurrentOCPMinorVersion(operatorImageVersion) + if err != nil { + return err + } + upgradeableConditionController := controller.NewStaticUpgradeableConditionController( "OLMStaticUpgradeableConditionController", cl.OperatorClient, @@ -340,7 +338,7 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error ) versionGetter := status.NewVersionGetter() - versionGetter.SetVersion("operator", operatorImageVersion) + versionGetter.SetVersion("operator", status.VersionForOperatorFromEnv()) // Add all resources to relatedObjects to ensure that must-gather picks them up. // Note: These resources are also hard-coded in the ClusterOperator manifest. This way, diff --git a/internal/versionutils/version_utils.go b/internal/versionutils/version_utils.go index e04d4e1b..ec78a924 100644 --- a/internal/versionutils/version_utils.go +++ b/internal/versionutils/version_utils.go @@ -61,12 +61,6 @@ func ToAllowedSemver(data []byte) (*semver.Version, error) { return &version, nil } -// GetCatalogImageTag converts an OCP version to the catalog image tag format used by default catalogs. -// For example, version 4.22.0 returns "v4.22", and version 5.0.0 returns "v5.0". -func GetCatalogImageTag(version *semver.Version) string { - return fmt.Sprintf("v%d.%d", version.Major, version.Minor) -} - // ocpVersion500 is the semver representation of OCP 5.0, which is co-released with 4.23 as an // equivalent release. Neither upgrades to the other; both upgrade exclusively to 5.1. var ocpVersion500 = semver.Version{Major: 5, Minor: 0, Patch: 0} diff --git a/internal/versionutils/version_utils_test.go b/internal/versionutils/version_utils_test.go index 3cba69b8..b64141cb 100644 --- a/internal/versionutils/version_utils_test.go +++ b/internal/versionutils/version_utils_test.go @@ -95,37 +95,6 @@ func TestToAllowedSemver(t *testing.T) { } } -func TestGetCatalogImageTag(t *testing.T) { - tests := []struct { - name string - version semver.Version - want string - }{ - { - name: "4.22", - version: semver.Version{Major: 4, Minor: 22, Patch: 0}, - want: "v4.22", - }, - { - name: "5.0", - version: semver.Version{Major: 5, Minor: 0, Patch: 0}, - want: "v5.0", - }, - { - name: "patch ignored", - version: semver.Version{Major: 4, Minor: 22, Patch: 5}, - want: "v4.22", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := GetCatalogImageTag(&tt.version) - assert.Equal(t, tt.want, got, "unexpected catalog image tag") - }) - } -} - func TestIsOperatorMaxOCPVersionCompatibleWithCluster(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/builder.go b/pkg/controller/builder.go index 6252a4fb..2a229229 100644 --- a/pkg/controller/builder.go +++ b/pkg/controller/builder.go @@ -40,13 +40,12 @@ import ( ) type Builder struct { - Assets string - Clients *clients.Clients - ControllerContext *controllercmd.ControllerContext - KnownRESTMappings map[schema.GroupVersionKind]*meta.RESTMapping - FeatureGate configv1.FeatureGate - Infrastructure *configv1.Infrastructure - ClusterCatalogImageTag string + Assets string + Clients *clients.Clients + ControllerContext *controllercmd.ControllerContext + KnownRESTMappings map[schema.GroupVersionKind]*meta.RESTMapping + FeatureGate configv1.FeatureGate + Infrastructure *configv1.Infrastructure } func (b *Builder) BuildControllers(subDirectories ...string) (map[string]factory.Controller, map[string]factory.Controller, map[string]factory.Controller, []configv1.ObjectReference, error) { diff --git a/pkg/controller/helm.go b/pkg/controller/helm.go index ef3faf36..b5e9758b 100644 --- a/pkg/controller/helm.go +++ b/pkg/controller/helm.go @@ -10,6 +10,7 @@ import ( "strings" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/cluster-olm-operator/internal/versionutils" "github.com/openshift/cluster-olm-operator/pkg/helmvalues" yaml3 "gopkg.in/yaml.v3" @@ -20,11 +21,6 @@ import ( "k8s.io/klog/v2" ) -// catalogVersionSentinel is a placeholder value that openshift.yaml sets for -// options.openshift.catalogs.version to indicate the catalog tag should be -// resolved dynamically from the running cluster's OCP major.minor version. -const catalogVersionSentinel = "ocp-release" - // Expected path structure: // ${assets}/helm/${subDir}/olmv1/ = chart // ${assets}/helm/${subDir}/openshift.yaml = primary values file @@ -85,9 +81,10 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error { if err := values.SetStringValue("options.operatorController.deployment.image", os.Getenv("OPERATOR_CONTROLLER_IMAGE")); err != nil { return fmt.Errorf("error setting OPERATOR_CONTROLLER_IMAGE: %w", err) } - // When openshift.yaml sets options.openshift.catalogs.version to catalogVersionSentinel, - // replace it with the tag derived from the running cluster's OCP major.minor version. - if err := applyCatalogImageTagOverride(values, b.ClusterCatalogImageTag); err != nil { + // OCP 4.23 and 5.0 are co-released. If catalogd.yaml pins "v5.0" but RELEASE_VERSION is + // a 4.x stream, substitute the 4.x catalog tag so images match the release stream. + // For 5.x releases (including 5.1) the value in catalogd.yaml is left untouched. + if err := applyCatalogImageTagOverride(values, os.Getenv("RELEASE_VERSION")); err != nil { return fmt.Errorf("error setting catalog image tag: %w", err) } @@ -232,19 +229,22 @@ type DocumentInfo struct { Order int } -// applyCatalogImageTagOverride replaces options.openshift.catalogs.version in the -// Helm values when it equals catalogVersionSentinel, substituting the tag derived -// from the running cluster's OCP major.minor version. It is a no-op when clusterTag -// is empty or when the current value is not catalogVersionSentinel. -func applyCatalogImageTagOverride(values *helmvalues.HelmValues, clusterTag string) error { - if clusterTag == "" { +// applyCatalogImageTagOverride substitutes options.openshift.catalogs.version when +// catalogd.yaml pins it to "v5.0" and RELEASE_VERSION is a 4.x stream (the 4.23/5.0 +// co-release). For 5.x releases the value in catalogd.yaml is left untouched. +func applyCatalogImageTagOverride(values *helmvalues.HelmValues, releaseVersion string) error { + if !strings.HasPrefix(releaseVersion, "4.") { return nil } - currentTag, found := values.GetStringValue("options.openshift.catalogs.version") - if !found || currentTag != catalogVersionSentinel { + currentCatalogVersion, found := values.GetStringValue("options.openshift.catalogs.version") + if !found || currentCatalogVersion != "v5.0" { return nil } - return values.SetStringValue("options.openshift.catalogs.version", clusterTag) + v, err := versionutils.GetCurrentOCPMinorVersion(releaseVersion) + if err != nil { + return fmt.Errorf("error parsing release version for catalog tag: %w", err) + } + return values.SetStringValue("options.openshift.catalogs.version", fmt.Sprintf("v%d.%d", v.Major, v.Minor)) } func splitYAMLDocuments(content string) []string { diff --git a/pkg/controller/helm_test.go b/pkg/controller/helm_test.go index 349200f6..b5852db2 100644 --- a/pkg/controller/helm_test.go +++ b/pkg/controller/helm_test.go @@ -76,38 +76,44 @@ func TestRenderHelmTemplate(t *testing.T) { require.Equal(t, compareData, testData) } -func TestApplyCatalogImageTagOverride(t *testing.T) { +func TestCatalogImageTagOverride(t *testing.T) { const versionKey = "options.openshift.catalogs.version" tests := []struct { - name string - initialTag string // set at versionKey before the call; empty means key is absent - clusterTag string - expectedTag string // expected value at versionKey after the call; empty means key absent + name string + initialTag string // catalog version in Helm values; empty means key absent + releaseVersion string // RELEASE_VERSION env var value + expectedTag string // expected catalog version after renderHelmTemplate logic }{ { - name: "clusterTag empty - no override regardless of Helm value", - initialTag: catalogVersionSentinel, - clusterTag: "", - expectedTag: catalogVersionSentinel, + name: "4.23 release with v5.0 catalog - override to v4.23", + initialTag: "v5.0", + releaseVersion: "4.23.0", + expectedTag: "v4.23", + }, + { + name: "5.0 release with v5.0 catalog - no override", + initialTag: "v5.0", + releaseVersion: "5.0.0", + expectedTag: "v5.0", }, { - name: "sentinel present, cluster is 4.23 - override to v4.23", - initialTag: catalogVersionSentinel, - clusterTag: "v4.23", - expectedTag: "v4.23", + name: "5.1 release with v5.0 catalog - no override", + initialTag: "v5.0", + releaseVersion: "5.1.0", + expectedTag: "v5.0", }, { - name: "Helm pins v4.23 - not the sentinel, no override", - initialTag: "v4.23", - clusterTag: "v4.23", - expectedTag: "v4.23", + name: "5.1 release with v5.1 catalog - no override", + initialTag: "v5.1", + releaseVersion: "5.1.0", + expectedTag: "v5.1", }, { - name: "version key absent in Helm values - no override", - initialTag: "", - clusterTag: "v4.23", - expectedTag: "", + name: "4.22 release with v4.22 catalog - no override (not v5.0)", + initialTag: "v4.22", + releaseVersion: "4.22.0", + expectedTag: "v4.22", }, } @@ -118,7 +124,7 @@ func TestApplyCatalogImageTagOverride(t *testing.T) { require.NoError(t, hv.SetStringValue(versionKey, tt.initialTag)) } - require.NoError(t, applyCatalogImageTagOverride(hv, tt.clusterTag)) + require.NoError(t, applyCatalogImageTagOverride(hv, tt.releaseVersion)) got, _ := hv.GetStringValue(versionKey) require.Equal(t, tt.expectedTag, got)