OTA-1786: pkg/payload/precondition/clusterversion/gianthop: Allow updates to 5.0

[wking]

Not sure yet if OCP 5.0 will be a thing, but to help folks feeling out that space just in case, relax the current giant-hop guard in 4.22 to allow updates to 5.0. I'm special-casing 5.0 (e.g. no allowing 4 -> 6 or 4 -> 5.1) to make the smallest possible change at this early-testing stage.

[openshift-ci-robot]

@wking: This pull request references OTA-1786 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:

Not sure yet if OCP 5.0 will be a thing, but to help folks feeling out that space just in case, relax the current giant-hop guard in 4.22 to allow updates to 5.0. I'm special-casing 5.0 (e.g. no allowing 4 -> 6 or 4 -> 5.1) to make the smallest possible change at this early-testing stage.

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

Adds a temporary special-case in GiantHop.Run to allow direct updates from current major 4 to target 5.0 only; rejects updates to 5.1+ with a MajorVersionUpdate error. Adds two tests validating success for 5.0 and rejection for 5.1.

Changes

Cohort / File(s)	Summary
Version 4→5.0 special-case pkg/payload/precondition/clusterversion/gianthop.go	Adds conditional logic in GiantHop.Run to permit a direct jump from current major 4 to target 5.0 only; preserves existing MajorVersionUpdate error path for other major jumps (including 5.1+).
Tests for 5.0 and 5.1 behavior pkg/payload/precondition/clusterversion/gianthop_test.go	Adds two test cases: "major version is 5.0" (DesiredUpdate 5.0.0, current 4.0.0 — expects no error) and "major version is 5.1" (DesiredUpdate 5.1.0, current 4.0.0 — expects MajorVersionUpdate error with message about only supporting within-current-major or to 5.0).

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

---

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

[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 7cebe65 and a8354d1.

📒 Files selected for processing (2)

• pkg/payload/precondition/clusterversion/gianthop.go (1 hunks)
• pkg/payload/precondition/clusterversion/gianthop_test.go (1 hunks)

🧰 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/gianthop_test.go
• pkg/payload/precondition/clusterversion/gianthop.go

🧬 Code graph analysis (2)

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

pkg/payload/payload.go (1)

• Update (104-117)

pkg/version/version.go (1)

• Version (16-16)

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

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

• Error (14-20)

🔇 Additional comments (1)

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

67-98: LGTM!

The new test cases appropriately cover both the allowed transition (4.x → 5.0) and the blocked transition (4.x → 5.1+). The expected error message aligns with the implementation intent.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

68-78: Implementation correctly handles the 4→5.0 special case.

The logic properly allows updates from any 4.x to 5.0 while rejecting 5.1+. The nested conditionals are clear, and the error messages provide appropriate context. The comment acknowledging this as temporary technical debt is good practice.

Optional: Consider adding a test with realistic version numbers

While the current tests use simplified versions like 4.0.0, you could add a test case with a more realistic scenario like 4.22.0 → 5.0.0 to better reflect production usage. This would help verify the logic works with actual deployment versions.

Example test case to add in gianthop_test.go:

{
    name: "realistic 4.22 to 5.0",
    clusterVersion: configv1.ClusterVersion{
        Spec: configv1.ClusterVersionSpec{
            DesiredUpdate: &configv1.Update{
                Version: "5.0.0",
            },
        },
        Status: configv1.ClusterVersionStatus{
            Desired: configv1.Release{
                Version: "4.22.0",
            },
        },
    },
    expected: "",
},

📜 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 a8354d1 and 3297a67.

📒 Files selected for processing (2)

• pkg/payload/precondition/clusterversion/gianthop.go (1 hunks)
• pkg/payload/precondition/clusterversion/gianthop_test.go (1 hunks)

🧰 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/gianthop.go
• pkg/payload/precondition/clusterversion/gianthop_test.go

🧬 Code graph analysis (2)

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

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

• Error (14-20)

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

pkg/payload/payload.go (1)

• Update (104-117)

pkg/version/version.go (1)

• Version (16-16)

🔇 Additional comments (1)

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

67-98: Test coverage appropriately validates the new behavior.

The two test cases correctly verify that 4.0.0 → 5.0.0 succeeds while 4.0.0 → 5.1.0 fails with the expected error message. The test expectations align with the implementation logic.

[DavidHurta]

/cc

[DavidHurta]

/hold

One comment. Feel free to address it or unhold the PR.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

83-98: Test case correctly validates the 4.x -> 5.1+ rejection path.

The test appropriately verifies that updates from version 4.0.0 to 5.1.0 are blocked with the expected error message mentioning "or to 5.0".

Optional: Consider more descriptive test names.

For improved clarity, consider renaming:

• "major version is 5.0" → "allow major version hop from 4.x to 5.0"
• "major version is 5.1" → "block major version hop from 4.x to 5.1 and above"

🔎 Suggested test name improvements

-		name: "major version is 5.0",
+		name: "allow major version hop from 4.x to 5.0",

-		name: "major version is 5.1",
+		name: "block major version hop from 4.x to 5.1 and above",

📜 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 3297a67 and d544606.

📒 Files selected for processing (2)

• pkg/payload/precondition/clusterversion/gianthop.go
• pkg/payload/precondition/clusterversion/gianthop_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/gianthop.go
• pkg/payload/precondition/clusterversion/gianthop_test.go

🧬 Code graph analysis (1)

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

pkg/payload/payload.go (1)

• Update (104-117)

pkg/version/version.go (1)

• Version (16-16)

🔇 Additional comments (2)

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

69-78: LGTM! The special-case logic is correct and well-documented.

The implementation correctly allows updates from any 4.x version to 5.0.0 only, while blocking 5.1+ with an appropriate error message. The TODO comment clearly marks this as temporary code for early testing purposes, aligning with the PR objectives.

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

67-82: Test case correctly validates the 4.x -> 5.0 success path.

The test appropriately verifies that updates from version 4.0.0 to 5.0.0 are allowed (no error expected).

[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

[wking]

/verified by unit

which will have included:

go test -v ./pkg/payload/precondition/clusterversion          
...
--- PASS: TestGiantHopRun (0.00s)
    ...
    --- PASS: TestGiantHopRun/major_version (0.00s)
    --- PASS: TestGiantHopRun/major_version_is_5.0 (0.00s)
    --- PASS: TestGiantHopRun/major_version_is_5.1 (0.00s)
...

[openshift-ci-robot]

@wking: This PR has been marked as verified by unit.

Details

In response to this:

/verified by unit

which will have included:

go test -v ./pkg/payload/precondition/clusterversion          
...
--- PASS: TestGiantHopRun (0.00s)
   ...
   --- PASS: TestGiantHopRun/major_version (0.00s)
   --- PASS: TestGiantHopRun/major_version_is_5.0 (0.00s)
   --- PASS: TestGiantHopRun/major_version_is_5.1 (0.00s)
...

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.

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 76ea813 and 2 for PR HEAD d544606 in total

[wking]

OVNKubernetesResourceRetryFailure and the 2m Pod-setup delay between the 17:56:10 PodScheduled and the 17:58:12 PodReadyToStartContainers that broke Image extract should extract content from an image's expectations are both unrelated to my change.

/override ci/prow/e2e-agnostic-ovn

[openshift-ci]

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-ovn

Details

In response to this:

OVNKubernetesResourceRetryFailure and the 2m Pod-setup delay between the 17:56:10 PodScheduled and the 17:58:12 PodReadyToStartContainers that broke Image extract should extract content from an image's expectations are both unrelated to my change.

/override ci/prow/e2e-agnostic-ovn

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.

[wking]

The deprovision timeout is unrelated to my change:

/override ci/prow/e2e-aws-ovn-techpreview

[openshift-ci]

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-aws-ovn-techpreview

Details

In response to this:

The deprovision timeout is unrelated to my change:

/override ci/prow/e2e-aws-ovn-techpreview

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.

[wking]

error: failed to push image ...: unknown version reference "machine-os" also unrelated to my change; see openshift/release#72920 also running away, and coreos/rhel-coreos-config#134 trying to fix the issue. But we don't need to wait on the fix here.

/override ci/prow/okd-scos-images

[openshift-ci]

@wking: Overrode contexts on behalf of wking: ci/prow/okd-scos-images

Details

In response to this:

error: failed to push image ...: unknown version reference "machine-os" also unrelated to my change; see openshift/release#72920 also running away, and coreos/rhel-coreos-config#134 trying to fix the issue. But we don't need to wait on the fix here.

/override ci/prow/okd-scos-images

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.

[openshift-ci]

@wking: all tests passed!

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.