feat: set node pool min version in the frontend to 4.20.8 for all users - #6659
Conversation
Disables support for node pool install and upgrades using ocp versions under 4.20.8 for all users. 4.20.8 was selected for the min version as versions below this were missing some features, mostly related to allowing dynamic retrieval of azure marketplace images for node pools. Moving away from these versions will allow us to remove any hardcoded marketplace images in the future. JIRA: https://redhat.atlassian.net/browse/ARO-29063
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JameelB The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR raises the minimum allowed OpenShift version for node pool create/upgrade to 4.20.8 for all users by enforcing the check unconditionally (i.e., no longer allowing lower versions even with the experimental release features flag). It also removes/updates integration and unit tests that previously validated node pool creation below that minimum.
Changes:
- Enforce node pool
properties.version.id >= 4.20.8regardless of feature flags. - Remove integration test artifacts that created a 4.20.0 node pool under experimental features.
- Update comprehensive node pool validation tests to reflect the new minimum-version behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/experimental-features/06-setClusterServiceID-node-pool-420/00-key.json | Removes an experimental integration-test step tied to a 4.20.0 node pool scenario. |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/experimental-features/05-httpCreate-node-pool-420/nodepool-4.20.0.json | Removes an integration-test payload that creates a node pool at version 4.20.0 (now below minimum). |
| test-integration/frontend/artifacts/FrontendCRUD/NodePool/experimental-features/05-httpCreate-node-pool-420/00-key.json | Removes the integration-test key file for the removed 4.20.0 node pool create step. |
| internal/validation/validate_nodepools.go | Moves/enforces the minimum node pool version check so it applies to all users (including experimental-flag users). |
| internal/validation/validate_nodepools_comprehensive_test.go | Updates node pool validation test cases to align with the new minimum version behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Skip version format validation if version hasn't changed | ||
| if oldObj == nil || newObj.ID != oldObj.ID { | ||
| errs = append(errs, OpenShiftWithOptionalPrerelease(ctx, op, fldPath.Child("id"), &newObj.ID, safe.Field(oldObj, toNodePoolVersionProfileID))...) | ||
| } | ||
|
|
||
| //ChannelGroup string `json:"channelGroup,omitempty"` | ||
| errs = append(errs, validate.RequiredValue(ctx, op, fldPath.Child("channelGroup"), &newObj.ChannelGroup, safe.Field(oldObj, toNodePoolVersionProfileChannelGroup))...) | ||
|
|
||
| if !op.HasOption(metadataapi.FeatureExperimentalReleaseFeatures) { | ||
| errs = append(errs, validate.Enum(ctx, op, fldPath.Child("channelGroup"), &newObj.ChannelGroup, safe.Field(oldObj, toNodePoolVersionProfileChannelGroup), metadataapi.AllowedChannelGroups, nil)...) | ||
| // without feature flag, only allow version 4.20.8 and above | ||
| errs = append(errs, VersionMustBeAtLeast(ctx, op, fldPath.Child("id"), &newObj.ID, safe.Field(oldObj, toNodePoolVersionProfileID), "4.20.8")...) | ||
| } else { | ||
| errs = append(errs, validate.Enum(ctx, op, fldPath.Child("channelGroup"), &newObj.ChannelGroup, safe.Field(oldObj, toNodePoolVersionProfileChannelGroup), metadataapi.AllowedChannelGroupsWithExperimentalFlag, nil)...) | ||
| } | ||
|
|
||
| errs = append(errs, VersionMustBeAtLeast(ctx, op, fldPath.Child("id"), &newObj.ID, safe.Field(oldObj, toNodePoolVersionProfileID), "4.20.8")...) | ||
|
|
| opOptions: testNodePoolFeatureOptions(metadataapi.FeatureExperimentalReleaseFeatures), | ||
| expectErrors: []utils.ExpectedError{ | ||
| {Message: "Invalid character(s) found in patch number", FieldPath: "properties.version.id"}, | ||
| {Message: "Invalid character(s) found in patch number", FieldPath: "properties.version.id"}, | ||
| }, |
|
Jameel Briones (@JameelB): The following tests failed, say
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. |
|
The copilot feedback looks good. Address those points and then I think this is good. |
JIRA: https://redhat.atlassian.net/browse/ARO-29063
What
Disables support for node pool install and upgrades using ocp versions under 4.20.8 for all users.
This follows on from #6609
Why
4.20.8 was selected for the min version as versions below this were missing some features, mostly related to allowing dynamic retrieval of azure marketplace images for node pools. Moving away from these versions will allow us to remove any hardcoded marketplace images in the future.
Testing
Special notes for your reviewer
N/A
PR Checklist
Screenshots included (if graph/UI/metrics changes)Draft PR used for WIP (if applicable)If E2E tests are included:
E2E tests follow Principles of Good E2E Test Case DesignIf new E2E use case is covered (via a new test or new check/verifier),demonstrate that the test is able to detect a defect/error and fail with
proper error message and logs which communicates nature of the problem.