status: sort isUpgrading messages to prevent ClusterOperator write loop

[sdodson]

Problem

During a cluster upgrade, oc get co -w shows the dns ClusterOperator being updated many times per second, even when the pod counts in the Progressing message haven't changed.

Root Cause

isUpgrading() iterates over a map[string]string to build upgrade status messages. Go map iteration order is randomized, so when multiple operands are upgrading simultaneously (e.g. coredns and kube-rbac-proxy), the returned messages slice has a non-deterministic order on each call.

This caused a self-sustaining write loop:

1. isUpgrading() produces messages in order [A, B]
2. operatorStatusesEqual() returns false (Message differs from stored)
3. ClusterOperator is written
4. The CO watch fires, triggering another reconcile
5. isUpgrading() produces messages in order [B, A]
6. operatorStatusesEqual() returns false again
7. ClusterOperator is written again → goto 4

Fix

Sort the messages slice before returning from isUpgrading(), making the output deterministic and breaking the loop.

Testing

Added a unit test that calls isUpgrading() 100 times with multiple upgrading components and asserts the messages are always returned in the same sorted order.

Summary by CodeRabbit

• Bug Fixes
  • Made upgrade status messages consistently ordered across reconciliations.
  • Improved the stability of upgrade progress updates so message order no longer varies.
• Tests
  • Added a regression test to verify upgrade status message ordering stays deterministic.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 418d04f3-e0f8-43ab-a0b3-787f38620ecd

📥 Commits

Reviewing files that changed from the base of the PR and between 809337e and b31c7bb.

📒 Files selected for processing (1)

• pkg/operator/controller/status/controller_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/operator/controller/status/controller_test.go

---

Walkthrough

isUpgrading now sorts collected messages before returning them, making upgrade status output deterministic. A new regression test calls isUpgrading repeatedly and checks that the message order stays stable.

Changes

Deterministic upgrade message ordering

Layer / File(s)	Summary
Sort upgrade messages pkg/operator/controller/status/controller.go, pkg/operator/controller/status/controller_test.go	sort.Strings is applied before returning upgrade messages, and TestIsUpgradingMessageOrder verifies stable lexicographic ordering across repeated calls.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: sorting upgrade status messages to stop a ClusterOperator write loop.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PASS: The added test is a static Go test name, and the changed file contains no Ginkgo titles or dynamic test names.
Test Structure And Quality	✅ Passed	The new test is a pure unit test: one behavior, no resources or waits, and clear failure messages; Ginkgo-style concerns don’t apply.
Microshift Test Compatibility	✅ Passed	The PR only adds a Go unit test for isUpgrading; no Ginkgo/Describe/It tests, MicroShift guards, or unsupported e2e API assumptions were introduced.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Only a standard Go unit test was added; no Ginkgo/e2e test or SNO-sensitive multi-node assumption is present.
Topology-Aware Scheduling Compatibility	✅ Passed	Touched status-only code just sorts upgrade messages; no scheduling constraints, topology labels, replicas, or affinity changes were added.
Ote Binary Stdout Contract	✅ Passed	No stdout writes were added in process-level code; the PR only sorts upgrade messages and adds a unit test. The existing fmt.Printf is in a normal helper, not startup/setup.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the change is a pure unit test in pkg/operator/controller/status with no IPv4 or external connectivity assumptions.
No-Weak-Crypto	✅ Passed	Touched files only add sorting and a regression test in status controller; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons found.
Container-Privileges	✅ Passed	PASS: The PR only changes status sorting and a unit test; no manifest edits add privileged, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation=true.
No-Sensitive-Data-In-Logs	✅ Passed	The patch only sorts upgrade status messages and adds a unit test; no new logging paths or sensitive literals appear in the touched files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign knobunc for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/operator/controller/status/controller_test.go`:
- Around line 833-845: The test around isUpgrading only compares each result to
the first observed slice, which doesn’t assert the intended sorting contract.
Update the assertion in controller_test.go to compare messages directly against
the explicit expected sorted slice returned by isUpgrading, using the
isUpgrading symbol and the current/old/new version inputs to keep the test
deterministic.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ef0d9424-261b-4195-a5c0-36e47b155d27

📥 Commits

Reviewing files that changed from the base of the PR and between 8395f90 and 809337e.

📒 Files selected for processing (2)

• pkg/operator/controller/status/controller.go
• pkg/operator/controller/status/controller_test.go