OTA-1787: pkg: Update Upgradeable condition to explicitly mention major version updates

[wking]

The Upgradeable condition already blocks both minor and major version updates when set to False, but all the error messages and comments only mentioned minor versions or minor level updates. This was misleading, especially in the context of enabling updates to version 5.0.

Changes:

• Update error messages in pkg/cvo/upgradeable.go to say minor or major versions
• Update comments in pkg/payload/precondition/clusterversion/upgradeable.go to clarify that major version updates are also blocked
• Update all test expectations to match the new messages
• Standardize terminology from minor level to minor version for consistency

🤖 Generated with Claude Code

My prompts were:

Change the Upgradeable ClusterVersion condition handling so that it applies to major version updates as well as the current minor version updates.

and then:

commit these changes, and set yourself as the commit author

[openshift-ci-robot]

@wking: This pull request references OTA-1787 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

The Upgradeable condition already blocks both minor and major version updates when set to False, but all the error messages and comments only mentioned minor versions or minor level updates. This was misleading, especially in the context of enabling updates to version 5.0.

Changes:

• Update error messages in pkg/cvo/upgradeable.go to say minor or major versions
• Update comments in pkg/payload/precondition/clusterversion/upgradeable.go to clarify that major version updates are also blocked
• Update all test expectations to match the new messages
• Standardize terminology from minor level to minor version for consistency

🤖 Generated with Claude Code

My prompts were:

Change the Upgradeable ClusterVersion condition handling so that it applies to major version updates as well as the current minor version updates.

and then:

commit these changes, and set yourself as the commit author

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Walkthrough

Updated wording across tests, checks, comments, and alerts to reference upgrades "between minor or major versions". Replaced minor-only detection with a major/minor-aware function that returns the completed version plus an upgrade type; no exported API signatures changed.

Changes

Cohort / File(s)	Summary
CVO tests pkg/cvo/cvo_test.go	Adjusted expected status/error message strings to mention "between minor or major versions".
Payload precondition tests pkg/payload/precondition/clusterversion/upgradeable_test.go	Updated phrasing ("minor version") and added a test case validating major-version-in-progress behavior (expects MajorVersionClusterUpdateInProgress and corresponding message/warning).
Upgradeable checks & messages pkg/cvo/upgradeable.go, pkg/internal/constants.go, install/0000_90_cluster-version-operator_02_servicemonitor.yaml	Reworded user-facing messages, comments, and alert summary to reference "minor or major versions" instead of only "minor versions".
Payload precondition logic pkg/payload/precondition/clusterversion/upgradeable.go	Replaced minorUpdateFrom with majorOrMinorUpdateFrom returning (completedVersion, "Major"

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 6c5311e and 07c1bb8.

📒 Files selected for processing (2)

• pkg/payload/precondition/clusterversion/upgradeable.go
• pkg/payload/precondition/clusterversion/upgradeable_test.go

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• pkg/payload/precondition/clusterversion/upgradeable.go
• pkg/payload/precondition/clusterversion/upgradeable_test.go

🧬 Code graph analysis (2)

pkg/payload/precondition/clusterversion/upgradeable.go (4)

pkg/cincinnati/cincinnati.go (1)

• Error (56-65)

pkg/payload/precondition/precondition.go (1)

• Error (14-20)

pkg/version/version.go (1)

• Version (16-16)

lib/resourcemerge/os.go (1)

• FindOperatorStatusCondition (45-53)

pkg/payload/precondition/clusterversion/upgradeable_test.go (1)

pkg/payload/precondition/precondition.go (1)

• Error (14-20)

🔇 Additional comments (6)

pkg/payload/precondition/clusterversion/upgradeable.go (4)

6-6: LGTM!

The strings import is appropriately added for strings.ToLower usage on line 116.

---

101-102: LGTM!

Comments now correctly clarify that both minor and major version updates are blocked when Upgradeable==False.

---

112-120: LGTM!

The call site correctly uses the two-value return from majorOrMinorUpdateFrom and constructs appropriate error reasons (MajorVersionClusterUpdateInProgress or MinorVersionClusterUpdateInProgress) with descriptive messages.

---

134-157: LGTM!

The refactored majorOrMinorUpdateFrom function correctly:

• Returns both the completed version and upgrade type ("Major", "Minor", or "")
• Checks major version transition first (line 149), then minor (line 152)
• Returns empty strings for non-applicable cases

The previous typo (completedVersoin) flagged in past reviews has been fixed.

pkg/payload/precondition/clusterversion/upgradeable_test.go (2)

139-142: LGTM!

Message text updated from "minor level update" to "minor version update" for consistency with the terminology standardization across the codebase.

---

143-155: LGTM!

New test case appropriately covers the major version upgrade scenario where:

• Completed version 4.1.1 → current version 5.0.1 represents a major version transition
• Retargeting to 5.0.3 during this in-progress upgrade correctly expects MajorVersionClusterUpdateInProgress
• The NonBlockingWarning: true aligns with the implementation behavior

---

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

[DavidHurta]

/cc

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between d01b53a and 1bc9513.

📒 Files selected for processing (4)

• install/0000_90_cluster-version-operator_02_servicemonitor.yaml
• pkg/internal/constants.go
• pkg/payload/precondition/clusterversion/upgradeable.go
• pkg/payload/precondition/clusterversion/upgradeable_test.go

✅ Files skipped from review due to trivial changes (1)

• pkg/internal/constants.go

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• pkg/payload/precondition/clusterversion/upgradeable.go
• install/0000_90_cluster-version-operator_02_servicemonitor.yaml
• pkg/payload/precondition/clusterversion/upgradeable_test.go

🧬 Code graph analysis (2)

pkg/payload/precondition/clusterversion/upgradeable.go (4)

pkg/cincinnati/cincinnati.go (1)

• Error (56-65)

pkg/payload/precondition/precondition.go (1)

• Error (14-20)

pkg/version/version.go (1)

• Version (16-16)

lib/resourcemerge/os.go (1)

• FindOperatorStatusCondition (45-53)

pkg/payload/precondition/clusterversion/upgradeable_test.go (1)

pkg/payload/precondition/precondition.go (1)

• Error (14-20)

🔇 Additional comments (6)

install/0000_90_cluster-version-operator_02_servicemonitor.yaml (1)

90-90: LGTM! Alert summary accurately reflects blocking behavior.

The updated alert summary now correctly indicates that the condition blocks both minor and major version updates, aligning with the broader PR changes.

pkg/payload/precondition/clusterversion/upgradeable.go (3)

6-6: LGTM! Import added for strings.ToLower usage.

The strings import is correctly added to support the lowercase conversion in the error message formatting.

---

101-102: LGTM! Comments accurately reflect blocking behavior.

The updated comments now correctly state that Upgradeable==False blocks both minor and major version updates, addressing previous reviewer feedback.

---

112-120: LGTM! Dynamic error handling for major/minor upgrades.

The logic correctly identifies whether a major or minor version upgrade is in progress and generates appropriate error reasons (e.g., MajorVersionClusterUpdateInProgress or MinorVersionClusterUpdateInProgress) and messages. The patchOnly check ensures this only applies to patch-level retargets during a major/minor transition.

pkg/payload/precondition/clusterversion/upgradeable_test.go (2)

139-139: LGTM! Terminology standardized to "minor version".

The change from "minor level update" to "minor version update" improves consistency and precision in the messaging.

---

143-155: LGTM! Comprehensive test coverage for major version upgrades.

The new test case correctly validates the major version upgrade scenario, mirroring the structure of the existing minor version test and ensuring the MajorVersionClusterUpdateInProgress reason is properly generated.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DavidHurta, wking

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [DavidHurta,wking]

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

[hongkailiu]

/retest-required

[openshift-ci]

@wking: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-images	07c1bb8	link	true	/test okd-scos-images

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.