From 52765afcbc4896ff00173ad46a85b8ecef3e14cd Mon Sep 17 00:00:00 2001 From: Harald Klein Date: Wed, 28 Oct 2026 14:43:36 +0100 Subject: [PATCH] Sort incompatible CSVs in the Upgradeable condition message incompatibleOperators lists the CSVs from the informer cache. The cache returns them in map order, which changes between reconciles. skews.String() kept that order, so the same set of blocking CSVs produced different messages. conditionsEqual compares the message, so each reorder counted as a condition transition: the controller rewrote the ClusterOperator status and moved lastTransitionTime although nothing had changed. With two blocking CSVs on OCP 4.22 the message alternated between the two orders every one to two minutes. skews.String() now sorts a copy of the skews before it builds the message: skews with a property error first, as before, then by namespace and name. The same set of CSVs gives the same message in any input order, so the condition only transitions when the set changes. The regression test feeds three skews in three orders and expects one message. It fails on the previous implementation, which filled the non-error entries from the end of the slice. Fixes OCPBUGS-129618 Signed-off-by: Harald Klein Co-Authored-By: Claude --- pkg/controller/operators/openshift/helpers.go | 30 ++++++++++++------- .../operators/openshift/helpers_test.go | 17 +++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/pkg/controller/operators/openshift/helpers.go b/pkg/controller/operators/openshift/helpers.go index 2e85462db2..7b7cb6cae1 100644 --- a/pkg/controller/operators/openshift/helpers.go +++ b/pkg/controller/operators/openshift/helpers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "sort" "strings" "sync" @@ -77,18 +78,25 @@ func versionsMatch(a []configv1.OperandVersion, b []configv1.OperandVersion) boo 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 + // 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 } - msg[j] = m - j-- + return a.name < b.name + }) + + msg := make([]string, len(sorted)) + for i, sk := range sorted { + msg[i] = sk.String() } // TODO Revisit this message after 4.23 release once release schedule is determined for future possible minor versions. diff --git a/pkg/controller/operators/openshift/helpers_test.go b/pkg/controller/operators/openshift/helpers_test.go index 43d8c5e63f..ee81c2122c 100644 --- a/pkg/controller/operators/openshift/helpers_test.go +++ b/pkg/controller/operators/openshift/helpers_test.go @@ -815,3 +815,20 @@ func TestNormalizeOCPVersion(t *testing.T) { }) } } + +func TestSkewsStringIsDeterministic(t *testing.T) { + // The informer cache lists CSVs in map order. The condition message must not + // depend on that order, or every reorder counts as a condition transition. + a := skew{namespace: "openshift-logging", name: "cluster-logging.v6.6.1", maxOpenShiftVersion: "4.23"} + b := skew{namespace: "openshift-cluster-csi-drivers", name: "secrets-store-csi-driver-operator.v4.22.0", maxOpenShiftVersion: "5.0"} + invalid := skew{namespace: "openshift-logging", name: "broken.v1.0.0", err: fmt.Errorf("invalid")} + + expected := "ClusterServiceVersions blocking minor version upgrades to 4.23 or major version upgrades to 5.0:\n" + + invalid.String() + "\n" + // error skews first + b.String() + "\n" + // then by namespace/name + a.String() + + require.Equal(t, expected, skews{a, b, invalid}.String()) + require.Equal(t, expected, skews{invalid, b, a}.String()) + require.Equal(t, expected, skews{b, invalid, a}.String()) +}