OCPBUGS-135357: Drop hard coded upgradeable 4.23 or 5.0

staging/operator-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go

@@ -195,7 +195,6 @@ var _ = Describe("ClusterOperator controller", func() {
 			}).Should(Succeed())
 		}()
 
-		parsedVersion := semver.MustParse(clusterVersion)
 		Eventually(func() ([]configv1.ClusterOperatorStatusCondition, error) {
 			err := k8sClient.Get(ctx, clusterOperatorName, co)
 			return co.Status.Conditions, err
@@ -207,7 +206,7 @@ var _ = Describe("ClusterOperator controller", func() {
 				{
 					namespace:           ns.GetName(),
 					name:                incompatible.GetName(),
-					maxOpenShiftVersion: fmt.Sprintf("%d.%d", parsedVersion.Major, parsedVersion.Minor),
+					maxOpenShiftVersion: clusterVersion,
 				},
 			}.String(),
 			LastTransitionTime: fixedNow(),
@@ -266,7 +265,6 @@ var _ = Describe("ClusterOperator controller", func() {
 			return k8sClient.Update(ctx, incompatible)
 		}).Should(Succeed())
 
-		_, parseErr := semver.ParseTolerant("garbage")
 		Eventually(func() ([]configv1.ClusterOperatorStatusCondition, error) {
 			err := k8sClient.Get(ctx, clusterOperatorName, co)
 			return co.Status.Conditions, err
@@ -276,9 +274,9 @@ var _ = Describe("ClusterOperator controller", func() {
 			Reason: IncompatibleOperatorsInstalled,
 			Message: skews{
 				{
-					namespace: ns.GetName(),
-					name:      incompatible.GetName(),
-					err:       fmt.Errorf(`failed to parse "garbage" as semver: %w`, parseErr),
+					namespace:           ns.GetName(),
+					name:                incompatible.GetName(),
+					maxOpenShiftVersion: "garbage",
 				},
 			}.String(),
 			LastTransitionTime: fixedNow(),

staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"sort"
 	"strings"
 	"sync"
 
@@ -74,39 +75,97 @@ func versionsMatch(a []configv1.OperandVersion, b []configv1.OperandVersion) boo
 	return len(counts) < 1
 }
 
-type skews []skew
+type skews []*skew
 
 func (s skews) String() string {
-	msg := make([]string, len(s))
-	i, j := 0, len(s)-1
-	for _, sk := range s {
-		m := sk.String()
-		// Partial order: error skews first
-		if sk.err != nil {
-			msg[i] = m
-			i++
-			continue
+	// Sorting depends on a populated .err, to sort invalid skews
+	// first.  incompatibleOperators sets .err through one call path,
+	// but check again here, to ensure reliability, regardless of call-chain.
+	var minVersion *semver.Version
+	for i := range s {
+		s[i].process()
+		if s[i].err == nil && (minVersion == nil || s[i].normalizedMaxOpenShiftVersion.LT(*minVersion)) {
+			minVersion = s[i].normalizedMaxOpenShiftVersion
 		}
-		msg[j] = m
-		j--
 	}
 
-	// TODO Revisit this message after 4.23 release once release schedule is determined for future possible minor versions.
-	return fmt.Sprintf("ClusterServiceVersions blocking minor version upgrades to 4.23 or major version upgrades to 5.0:\n%s", strings.Join(msg, "\n"))
+	// Sort so the message does not depend on the (unstable) order in which the
+	// CSVs were listed: error skews first, then by namespace/name. A different
+	// order would be a different Message, and so a condition transition.
+	sorted := make(skews, len(s))
+	copy(sorted, s)
+	sort.SliceStable(sorted, func(i, j int) bool {
+		a, b := sorted[i], sorted[j]
+		if (a.err != nil) != (b.err != nil) {
+			return a.err != nil
+		}
+		if a.namespace != b.namespace {
+			return a.namespace < b.namespace
+		}
+		return a.name < b.name
+	})
+
+	msg := make([]string, len(sorted))
+	for i, sk := range sorted {
+		msg[i] = sk.String()
+	}
+
+	var next string
+	if minVersion == nil {
+		next = "next major or minor version"
+	} else {
+		n := nextY(*minVersion)
+		next = fmt.Sprintf("%d.%d", n.Major, n.Minor)
+	}
+	return fmt.Sprintf("ClusterServiceVersions blocking minor version upgrades to %s or higher:\n%s", next, strings.Join(msg, "\n"))
 }
 
 type skew struct {
 	namespace           string
 	name                string
 	maxOpenShiftVersion string
-	err                 error
+
+	// the following properties are populated by skew.process()
+	err                           error
+	normalizedMaxOpenShiftVersion *semver.Version
+}
+
+func (s *skew) process() {
+	if s == nil || s.normalizedMaxOpenShiftVersion != nil || s.err != nil {
+		return
+	}
+
+	if s.maxOpenShiftVersion == "" {
+		// Handle "" separately, so parse doesn't treat it as a zero
+		s.err = fmt.Errorf(`%s value cannot be "" (an empty string)`, MaxOpenShiftVersionProperty)
+		s.normalizedMaxOpenShiftVersion = &semver.Version{}
+		return
+	}
+
+	version, err := semver.ParseTolerant(s.maxOpenShiftVersion)
+	s.normalizedMaxOpenShiftVersion = &version
+	if err != nil {
+		s.err = fmt.Errorf(`failed to parse %s=%q as semver: %w`, MaxOpenShiftVersionProperty, s.maxOpenShiftVersion, err)
+		return
+	}
+
+	truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor}
+	if !s.normalizedMaxOpenShiftVersion.EQ(truncatedVersion) {
+		s.err = fmt.Errorf("%s property must specify only <major>.<minor> version, got invalid value %q", MaxOpenShiftVersionProperty, s.normalizedMaxOpenShiftVersion)
+		return
+	}
+
+	// Normalize the operator's maxOpenShiftVersion: 4.23 and 5.0 are equivalent.
+	version = normalizeOCPVersion(*s.normalizedMaxOpenShiftVersion)
+	s.normalizedMaxOpenShiftVersion = &version
 }
 
-func (s skew) String() string {
+func (s *skew) String() string {
+	s.process()
 	if s.err != nil {
 		return fmt.Sprintf("- %s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err)
 	}
-	return fmt.Sprintf("- maximum supported OCP version for %s/%s is %s", s.namespace, s.name, s.maxOpenShiftVersion)
+	return fmt.Sprintf("- maximum supported OCP version for %s/%s is %d.%d", s.namespace, s.name, s.normalizedMaxOpenShiftVersion.Major, s.normalizedMaxOpenShiftVersion.Minor)
 }
 
 type transientError struct {
@@ -146,29 +205,18 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
 			continue
 		}
 
-		s := skew{
-			name:      csv.GetName(),
-			namespace: csv.GetNamespace(),
-		}
-		max, err := maxOpenShiftVersion(&csv)
-		if err != nil {
-			s.err = err
-			incompatible = append(incompatible, s)
+		s := maxOpenShiftVersion(&csv)
+		if s == nil {
 			continue
-		}
-
-		if max == nil {
+		} else if s.err != nil {
+			incompatible = append(incompatible, s)
 			continue
 		}
 
-		// Normalize the operator's maxOpenShiftVersion: 4.23 and 5.0 are equivalent.
-		normalizedMax := normalizeOCPVersion(*max)
-		if normalizedMax.GTE(nextMinor) {
+		if s.normalizedMaxOpenShiftVersion.GTE(nextMinor) {
 			continue
 		}
 
-		s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor)
-
 		incompatible = append(incompatible, s)
 	}
 
@@ -246,16 +294,22 @@ const (
 	MaxOpenShiftVersionProperty = "olm.maxOpenShiftVersion"
 )
 
-func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver.Version, error) {
+func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) *skew {
+	s := &skew{
+		name:      csv.GetName(),
+		namespace: csv.GetNamespace(),
+	}
+
 	// Extract the property from the CSV's annotations if possible
 	annotation, ok := csv.GetAnnotations()[projection.PropertiesAnnotationKey]
 	if !ok {
-		return nil, nil
+		return nil
 	}
 
 	properties, err := projection.PropertyListFromPropertiesAnnotation(annotation)
 	if err != nil {
-		return nil, err
+		s.err = err
+		return s
 	}
 
 	var max *string
@@ -265,33 +319,23 @@ func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver.
 		}
 
 		if max != nil {
-			return nil, fmt.Errorf(`defining more than one "%s" property is not allowed`, MaxOpenShiftVersionProperty)
+			s.err = fmt.Errorf(`defining more than one %q property is not allowed`, MaxOpenShiftVersionProperty)
+			return s
 		}
 
 		max = &property.Value
 	}
 
 	if max == nil {
-		return nil, nil
+		return nil
 	}
 
 	// Account for any additional quoting
 	value := strings.Trim(*max, "\"")
-	if value == "" {
-		// Handle "" separately, so parse doesn't treat it as a zero
-		return nil, fmt.Errorf(`value cannot be "" (an empty string)`)
-	}
 
-	version, err := semver.ParseTolerant(value)
-	if err != nil {
-		return nil, fmt.Errorf(`failed to parse "%s" as semver: %w`, value, err)
-	}
-
-	truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor}
-	if !version.EQ(truncatedVersion) {
-		return nil, fmt.Errorf("property %s must specify only <major>.<minor> version, got invalid value %s", MaxOpenShiftVersionProperty, version)
-	}
-	return &truncatedVersion, nil
+	s.maxOpenShiftVersion = value
+	s.process()
+	return s
 }
 
 func notCopiedSelector() (labels.Selector, error) {