CNTRLPLANE-2123: feat: update latest supported version from 4.21 to 4.22 - #7315
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jparrill: This pull request references CNTRLPLANE-2123 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 task to target either version "4.21." or "openshift-4.21.", but it targets "openshift-4.22" instead. DetailsIn response to this:
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. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBumped supported OCP version range: MinSupportedVersion 4.15.0 (from 4.14.0) and LatestSupportedVersion 4.22.0 (from 4.21.0); updated tests, fixtures, constants, and added a kube-version mapping for 4.21.0. No public API signatures changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
area/control-plane-operator
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
area/hypershift-operator
|
/hold Until branching happens this friday |
openshift-ci
Bot
added
do-not-merge/hold
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go(1 hunks)hypershift-operator/controllers/nodepool/nodepool_controller_test.go(1 hunks)support/supportedversion/version.go(2 hunks)support/supportedversion/version_test.go(4 hunks)test/e2e/util/version.go(2 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:
support/supportedversion/version_test.gosupport/supportedversion/version.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.gohypershift-operator/controllers/nodepool/nodepool_controller_test.gotest/e2e/util/version.go
🧬 Code graph analysis (1)
support/supportedversion/version_test.go (1)
support/supportedversion/version.go (2)
Supported(81-87)String(248-250)
🔇 Additional comments (4)
test/e2e/util/version.go (1)
18-18: 4.22 constant wiring for e2e looks consistentAdding
Version422and including it in the init-time semver checks is consistent with the new supported range and doesn’t introduce issues.Also applies to: 34-42
hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go (1)
1645-1655: Supported-versions fixture now matches new minimum (4.15)Dropping 4.14 from the test ConfigMap and keeping 4.20–4.15 aligns this fixture with the updated supported range and is consistent with the rest of the PR.
hypershift-operator/controllers/nodepool/nodepool_controller_test.go (1)
1824-1845: Updated n‑4 skew test matches ValidateVersionSkew semanticsFor a 4.20 HostedCluster, treating 4.17 as the minimum compatible NodePool minor and rejecting 4.16 with the updated error message is consistent with the n‑3 skew rule.
support/supportedversion/version_test.go (1)
25-28: Tests correctly track the new 4.15–4.22 support window and skew rules
TestSupportedVersionsandTestGetSupportedOCPVersionsnow expect 4.22..4.15, matchingLatestSupportedVersion/MinSupportedVersion.TestGetMinSupportedVersionkeeps IBM Cloud at 4.14.0 while bumping other platforms to 4.15.0, consistent with the IBM‑specific minimum.- New
ValidateVersionSkewcases around 4.18/4.15 and 4.21/4.18–4.17 validate the n‑3 rule and error messaging as implemented.These updates look consistent with the underlying version logic.
Also applies to: 61-63, 196-258, 268-269, 366-429
| LatestSupportedVersion = semver.MustParse("4.22.0") | ||
| MinSupportedVersion = semver.MustParse("4.15.0") | ||
| IBMCloudMinSupportedVersion = semver.MustParse("4.14.0") | ||
| ) |
There was a problem hiding this comment.
LatestSupportedVersion (4.22.0) has no corresponding kube mapping entry
LatestSupportedVersion has been bumped to 4.22.0, but ocpVersionToKubeVersion still only goes up to "4.21.0". As a result, GetKubeVersionForSupportedVersion(LatestSupportedVersion) (or any call with 4.22.0) will fail with unknown supported version.
If 4.22 is truly supported, the map should include a "4.22.0" entry with the correct Kubernetes semver; otherwise, consider keeping LatestSupportedVersion at the highest OCP version for which you have a kube mapping.
Also applies to: 42-51
🤖 Prompt for AI Agents
In support/supportedversion/version.go around lines 35-38 (and map at 42-51),
LatestSupportedVersion was bumped to 4.22.0 but ocpVersionToKubeVersion has no
"4.22.0" key so GetKubeVersionForSupportedVersion will error; either add a
"4.22.0" entry to ocpVersionToKubeVersion with the correct Kubernetes semver
(use semver.MustParse with the proper kube version) or revert
LatestSupportedVersion to the highest OCP version present in the map (e.g.,
4.21.0); update the map and LatestSupportedVersion consistently so every
supported OCP version has a corresponding kube mapping.
|
As already mentioned in PM, we may need another pair of eyes for this one but. to me... |
|
/lgtm |
devguyio
left a comment
devguyio
left a comment
There was a problem hiding this comment.
Thanks for doing this!
Left a few comments
| @@ -1651,7 +1651,7 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { | |||
| }, | |||
| }, | |||
| Data: map[string]string{ | |||
| "supported-versions": "{\"versions\":[\"4.20\",\"4.19\",\"4.18\",\"4.17\",\"4.16\",\"4.15\",\"4.14\"]}", | |||
| "supported-versions": "{\"versions\":[\"4.20\",\"4.19\",\"4.18\",\"4.17\",\"4.16\",\"4.15\"]}", | |||
There was a problem hiding this comment.
- why not add 4.21?
- if we're keeping IBM min supported version to 4.14, should we still remove 4.14 from here?
There was a problem hiding this comment.
Adding it or not it will not cause a difference, this is just a fixed test.
There was a problem hiding this comment.
I'm a little confused, so why are we removing the 4.14 if it won't make a difference?
| np.Spec.Release.Image = "quay.io/openshift-release-dev/ocp-release:4.16.0-x86_64" | ||
| return np | ||
| }(), | ||
| hostedCluster: func() *hyperv1.HostedCluster { | ||
| hc := baseHostedCluster.DeepCopy() | ||
| hc.Status.Version.History[0].Version = "4.18.0" | ||
| hc.Status.Version.History[0].Version = "4.20.0" | ||
| return hc | ||
| }(), | ||
| releaseProvider: &fakereleaseprovider.FakeReleaseProvider{ | ||
| Version: "4.14.0", | ||
| Version: "4.16.0", | ||
| }, | ||
| expectedCondition: &hyperv1.NodePoolCondition{ | ||
| Type: hyperv1.NodePoolSupportedVersionSkewConditionType, | ||
| Status: corev1.ConditionFalse, | ||
| Reason: hyperv1.NodePoolUnsupportedSkewReason, | ||
| Message: "NodePool minor version 4.14 is less than 4.15, which is the minimum NodePool version compatible with the 4.18 HostedCluster", | ||
| Message: "NodePool minor version 4.16 is less than 4.17, which is the minimum NodePool version compatible with the 4.20 HostedCluster", |
There was a problem hiding this comment.
shouldn't this be nodePool set to 4.18 and hostedcluster set to 4.22 ? or 4.21 if it must be an existing release? I didn't look deeply but the name of the test is giving me that indication
| @@ -24,7 +24,7 @@ import ( | |||
|
|
|||
| func TestSupportedVersions(t *testing.T) { | |||
| g := NewGomegaWithT(t) | |||
| g.Expect(Supported()).To(Equal([]string{"4.21", "4.20", "4.19", "4.18", "4.17", "4.16", "4.15", "4.14"})) | |||
| g.Expect(Supported()).To(Equal([]string{"4.22", "4.21", "4.20", "4.19", "4.18", "4.17", "4.16", "4.15"})) | |||
There was a problem hiding this comment.
same comment about keeping IBM version at 4.14, should we keep 4.14 here still?
There was a problem hiding this comment.
humm I would say no, talking yesterday with Cesar he mentioned that we should report 4.15 as minimum version supported for all the customers but keep 4.14 for IBM, in order to let the take the initiative to put a PR. w/e the case I will talk in the IBM sync meeting this Thursday to make sure they know we are following this path or if they agree remove the 4.14 completely.
| Version421 = semver.MustParse("4.21.0") | ||
| Version420 = semver.MustParse("4.20.0") | ||
| Version419 = semver.MustParse("4.19.0") | ||
| Version418 = semver.MustParse("4.18.0") | ||
| Version417 = semver.MustParse("4.17.0") | ||
| Version416 = semver.MustParse("4.16.0") | ||
| Version415 = semver.MustParse("4.15.0") | ||
| Version414 = semver.MustParse("4.14.0") |
There was a problem hiding this comment.
should we leave it if we're keeping IBM at 4.14?
There was a problem hiding this comment.
I don't think so, we don't have any reference to this part of the code and I don't think we will add it tbh.
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
dc88f08 to
97a7864
Compare
| LatestSupportedVersion = semver.MustParse("4.21.0") | ||
| MinSupportedVersion = semver.MustParse("4.14.0") | ||
| LatestSupportedVersion = semver.MustParse("4.22.0") | ||
| MinSupportedVersion = semver.MustParse("4.15.0") |
There was a problem hiding this comment.
This might break IBM clusters if they're using the supported versions CM, no?
There was a problem hiding this comment.
It should not, the function GetMinSupportedVersion contains a switch which select per platform, in this case we return another minimum for IBM which in this case is IBMCloudMinSupportedVersion = semver.MustParse("4.14.0").
switch hc.Spec.Platform.Type {
case hyperv1.IBMCloudPlatform:
return IBMCloudMinSupportedVersion
default:
return defaultMinVersion
}
So I would say that would not happen.
|
@jparrill: This pull request references CNTRLPLANE-2123 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 task to target either version "4.21." or "openshift-4.21.", but it targets "openshift-4.22" instead. DetailsIn response to this:
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. |
|
/lgtm |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "4.19.0": semver.MustParse("1.32.0"), | ||
| "4.20.0": semver.MustParse("1.33.0"), | ||
| "4.21.0": semver.MustParse("1.34.0"), | ||
| } |
There was a problem hiding this comment.
Bug: Missing Kubernetes version mapping for OCP 4.22
The LatestSupportedVersion is updated to 4.22.0, but the ocpVersionToKubeVersion map only contains entries up to 4.21.0. The mapping for 4.22.0 to its corresponding Kubernetes version is missing. While current usages only query MinSupportedVersion, this inconsistency means GetKubeVersionForSupportedVersion(LatestSupportedVersion) would fail with "unknown supported version" error. The PR description mentions adding the 4.21 → 1.34 mapping, but the 4.22 entry was not added.
Additional Locations (1)
| }, | ||
| Data: map[string]string{ | ||
| "supported-versions": "{\"versions\":[\"4.20\",\"4.19\",\"4.18\",\"4.17\",\"4.16\",\"4.15\",\"4.14\"]}", | ||
| "supported-versions": "{\"versions\":[\"4.21\",\"4.20\",\"4.19\",\"4.18\",\"4.17\",\"4.16\",\"4.15\",\"4.14\"]}", |
There was a problem hiding this comment.
Bug: Test ConfigMap version list is out of sync
The test ConfigMap's supported-versions data contains versions 4.21 through 4.14, but the updated Supported() function returns versions 4.22 through 4.15. The test data is missing 4.22 and incorrectly includes 4.14, which is now below MinSupportedVersion. This inconsistency between test fixtures and actual supported version logic may cause tests to pass with incorrect assumptions about version support.
Additional Locations (1)
|
/retest |
|
Something wrong with loki /override "Red Hat Konflux / hypershift-operator-enterprise-contract / hypershift-operator-main" |
|
@sjenning: Overrode contexts on behalf of sjenning: Red Hat Konflux / hypershift-operator-enterprise-contract / hypershift-operator-main, Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main DetailsIn response to this:
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. |
|
/hold We need to remove support for 4.14 as a separate step. There are CI complications doing it all at once. See comment here |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
|
/retest |
97a7864 to
eb22fe2
Compare
Update supported version range to include OpenShift 4.22 and adjust minimum version: - Increase LatestSupportedVersion from 4.21.0 to 4.22.0 - Add Kubernetes 1.34.0 mapping for OCP 4.21.0 - Update test expectations to reflect new supported version range (4.14-4.22) - Add Version422 constant and remove Version414 from e2e utilities - Fix NodePool version skew test to use compatible versions with new minimum 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
eb22fe2 to
c2f8609
Compare
|
@jparrill: This pull request references CNTRLPLANE-2123 which is a valid jira issue. DetailsIn response to this:
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. |
|
@jparrill: This pull request references CNTRLPLANE-2123 which is a valid jira issue. DetailsIn response to this:
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. |
|
/lgtm |
|
/verified later @sjenning |
openshift-ci-robot
added
verified-later
verified
|
@sjenning: This PR has been marked to be verified later by DetailsIn response to this:
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. |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/retest-required |
|
@jparrill: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
7 participants
Summary
Changes
Fixes
Test plan
hypershift versioncommand shows 4.22.0 as latest supported🤖 Generated with Claude Code