diff --git a/cmd/cluster-olm-operator/main.go b/cmd/cluster-olm-operator/main.go index ce3bf140..7d7ee35d 100644 --- a/cmd/cluster-olm-operator/main.go +++ b/cmd/cluster-olm-operator/main.go @@ -230,6 +230,13 @@ 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) @@ -252,8 +259,9 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error Scope: meta.RESTScopeRoot, }, }, - FeatureGate: *fg, - Infrastructure: infra, + FeatureGate: *fg, + Infrastructure: infra, + ClusterCatalogImageTag: catalogImageTag, } staticResourceControllers, deploymentControllers, clusterCatalogControllers, relatedObjects, err := cb.BuildControllers("catalogd", "operator-controller") @@ -288,12 +296,6 @@ 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, @@ -338,7 +340,7 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error ) versionGetter := status.NewVersionGetter() - versionGetter.SetVersion("operator", status.VersionForOperatorFromEnv()) + versionGetter.SetVersion("operator", operatorImageVersion) // 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 ec78a924..e04d4e1b 100644 --- a/internal/versionutils/version_utils.go +++ b/internal/versionutils/version_utils.go @@ -61,6 +61,12 @@ 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 b64141cb..e3a5b615 100644 --- a/internal/versionutils/version_utils_test.go +++ b/internal/versionutils/version_utils_test.go @@ -95,6 +95,47 @@ 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: "4.23", + version: semver.Version{Major: 4, Minor: 23, Patch: 0}, + want: "v4.23", + }, + { + name: "5.0", + version: semver.Version{Major: 5, Minor: 0, Patch: 0}, + want: "v5.0", + }, + { + name: "5.1", + version: semver.Version{Major: 5, Minor: 1, Patch: 0}, + want: "v5.1", + }, + { + 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) + }) + } +} + func TestIsOperatorMaxOCPVersionCompatibleWithCluster(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/builder.go b/pkg/controller/builder.go index 2a229229..6252a4fb 100644 --- a/pkg/controller/builder.go +++ b/pkg/controller/builder.go @@ -40,12 +40,13 @@ import ( ) type Builder struct { - Assets string - Clients *clients.Clients - ControllerContext *controllercmd.ControllerContext - KnownRESTMappings map[schema.GroupVersionKind]*meta.RESTMapping - FeatureGate configv1.FeatureGate - Infrastructure *configv1.Infrastructure + Assets string + Clients *clients.Clients + ControllerContext *controllercmd.ControllerContext + KnownRESTMappings map[schema.GroupVersionKind]*meta.RESTMapping + FeatureGate configv1.FeatureGate + Infrastructure *configv1.Infrastructure + ClusterCatalogImageTag string } 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 b26e9f6e..ab991c34 100644 --- a/pkg/controller/helm.go +++ b/pkg/controller/helm.go @@ -80,6 +80,14 @@ 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) } + // OPRUN-4599: OCP 4.23 and 5.0 are co-released from the same branch, so + // the openshift.yaml values file pins options.openshift.catalogs.version to + // "v5.0". A cluster running 4.23 must use v4.23 catalog images instead. + // TODO: Remove this block once 4.23/5.0 are no longer co-released and the + // openshift.yaml values file no longer pins "v5.0". + if err := applyCatalogImageTagOverride(values, b.ClusterCatalogImageTag); err != nil { + return fmt.Errorf("error setting catalog image tag: %w", err) + } // On HighlyAvailable topologies scale to 2 replicas and enable the PDB so that rolling // updates never leave zero running pods. On SingleReplica (SNO) / External topologies @@ -222,6 +230,21 @@ type DocumentInfo struct { Order int } +// applyCatalogImageTagOverride overrides options.openshift.catalogs.version in the +// Helm values when the values file pins "v5.0" but the cluster runs a different +// version (e.g. OCP 4.23, which is co-released with 5.0 but needs v4.23 catalog +// images). It is a no-op when clusterTag is empty or the pinned tag is not "v5.0". +func applyCatalogImageTagOverride(values *helmvalues.HelmValues, clusterTag string) error { + if clusterTag == "" { + return nil + } + currentTag, found := values.GetStringValue("options.openshift.catalogs.version") + if !found || currentTag != "v5.0" { + return nil + } + return values.SetStringValue("options.openshift.catalogs.version", clusterTag) +} + func splitYAMLDocuments(content string) []string { // Split by document separators but preserve the original text lines := strings.Split(content, "\n") diff --git a/pkg/controller/helm_test.go b/pkg/controller/helm_test.go index 0c926b3f..7950ab50 100644 --- a/pkg/controller/helm_test.go +++ b/pkg/controller/helm_test.go @@ -14,6 +14,7 @@ import ( internalfeatures "github.com/openshift/cluster-olm-operator/internal/featuregates" "github.com/openshift/cluster-olm-operator/pkg/clients" + "github.com/openshift/cluster-olm-operator/pkg/helmvalues" ) func TestRenderHelmTemplate(t *testing.T) { @@ -75,6 +76,74 @@ func TestRenderHelmTemplate(t *testing.T) { require.Equal(t, compareData, testData) } +func TestApplyCatalogImageTagOverride(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: "clusterTag empty - no override regardless of Helm value", + initialTag: "v5.0", + clusterTag: "", + expectedTag: "v5.0", + }, + { + name: "Helm pins v5.0, cluster is 4.23 - override to v4.23", + initialTag: "v5.0", + clusterTag: "v4.23", + expectedTag: "v4.23", + }, + { + name: "Helm pins v5.0, cluster is also 5.0 - no-op", + initialTag: "v5.0", + clusterTag: "v5.0", + expectedTag: "v5.0", + }, + { + name: "Helm pins v5.0, cluster is 5.1 - override to v5.1", + initialTag: "v5.0", + clusterTag: "v5.1", + expectedTag: "v5.1", + }, + { + name: "Helm pins v4.23, cluster is 4.23 - guard fires only on v5.0, no override", + initialTag: "v4.23", + clusterTag: "v4.23", + expectedTag: "v4.23", + }, + { + name: "Helm pins v4.22, cluster tag provided - guard fires only on v5.0, no override", + initialTag: "v4.22", + clusterTag: "v4.22", + expectedTag: "v4.22", + }, + { + name: "version key absent in Helm values - no override", + initialTag: "", + clusterTag: "v4.23", + expectedTag: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := helmvalues.NewHelmValues() + if tt.initialTag != "" { + require.NoError(t, hv.SetStringValue(versionKey, tt.initialTag)) + } + + require.NoError(t, applyCatalogImageTagOverride(hv, tt.clusterTag)) + + got, _ := hv.GetStringValue(versionKey) + require.Equal(t, tt.expectedTag, got) + }) + } +} + func TestSplitYAMLDocuments(t *testing.T) { tests := []struct { name string diff --git a/pkg/helmvalues/helmvalues.go b/pkg/helmvalues/helmvalues.go index e11a216d..2ff5d4b5 100644 --- a/pkg/helmvalues/helmvalues.go +++ b/pkg/helmvalues/helmvalues.go @@ -73,6 +73,15 @@ func (v *HelmValues) HasEnabledFeatureGates() (bool, error) { return false, nil } +func (v *HelmValues) GetStringValue(location string) (string, bool) { + ss := strings.Split(location, ".") + val, found, err := unstructured.NestedString(v.values, ss...) + if err != nil || !found { + return "", false + } + return val, true +} + func (v *HelmValues) SetStringValue(location string, newValue string) error { if location == "" { return errors.New("location string has no locations") diff --git a/pkg/helmvalues/helmvalues_test.go b/pkg/helmvalues/helmvalues_test.go index 4cd41016..eb95ea9a 100644 --- a/pkg/helmvalues/helmvalues_test.go +++ b/pkg/helmvalues/helmvalues_test.go @@ -257,6 +257,69 @@ func TestHasEnabledFeatureGates(t *testing.T) { } } +func TestGetStringValue(t *testing.T) { + tests := []struct { + name string + values map[string]interface{} + location string + wantVal string + wantFound bool + }{ + { + name: "missing key", + values: map[string]interface{}{}, + location: "options.openshift.catalogs.version", + wantFound: false, + }, + { + name: "simple key", + values: map[string]interface{}{"key": "value"}, + location: "key", + wantVal: "value", + wantFound: true, + }, + { + name: "nested key", + values: map[string]interface{}{ + "options": map[string]interface{}{ + "openshift": map[string]interface{}{ + "catalogs": map[string]interface{}{ + "version": "v5.0", + }, + }, + }, + }, + location: "options.openshift.catalogs.version", + wantVal: "v5.0", + wantFound: true, + }, + { + name: "non-string value returns not found", + values: map[string]interface{}{ + "options": map[string]interface{}{ + "replicas": int64(2), + }, + }, + location: "options.replicas", + wantFound: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := NewHelmValues() + hv.values = tt.values + got, found := hv.GetStringValue(tt.location) + if got != tt.wantVal { + t.Errorf("GetStringValue() val = %q, want %q", got, tt.wantVal) + } + if found != tt.wantFound { + t.Errorf("GetStringValue() found = %v, want %v", found, tt.wantFound) + } + }) + } +} + func TestSetStringValue(t *testing.T) { tests := []struct { name string