CORS-4387: Promote AWS DualStack to Default#2797
CORS-4387: Promote AWS DualStack to Default#2797sadasu wants to merge 1 commit intoopenshift:masterfrom
Conversation
Also make it available in OKD.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Hello @sadasu! Some important instructions when contributing to openshift/api: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis pull request introduces AWS dual-stack IP family configuration support. Changes include adding an ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
openshift-ci
bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Review Summary by QodoPromote AWS DualStack to Default and enable in OKD
WalkthroughsDescription• Promote AWSDualStackInstall feature gate to default in OCP and OKD • Enable feature in both Default and OKD product scopes • Add ipFamily field to AWS platform status with validation • Remove cloudLoadBalancerConfig from test expectations • Fix indentation in FeatureGateConfidentialCluster definition Diagramflowchart LR
A["AWSDualStackInstall Feature Gate"] -->|"Enable in Default"| B["Default Product Scope"]
A -->|"Enable in OKD"| C["OKD Product Scope"]
B -->|"Add ipFamily field"| D["AWS Platform Status"]
C -->|"Add ipFamily field"| D
D -->|"Validation Rules"| E["Immutable, IPv4/DualStack options"]
File Changes1. features/features.go
|
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| ipFamily: | ||
| default: IPv4 | ||
| description: |- | ||
| ipFamily specifies the IP protocol family that should be used for AWS | ||
| network resources. This controls whether AWS resources are created with | ||
| IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary | ||
| protocol family. | ||
| enum: | ||
| - IPv4 | ||
| - DualStackIPv6Primary | ||
| - DualStackIPv4Primary | ||
| type: string | ||
| x-kubernetes-validations: | ||
| - message: ipFamily is immutable once set | ||
| rule: oldSelf == '' || self == oldSelf |
There was a problem hiding this comment.
1. ipfamily docs omit constraints 📘 Rule violation ⚙ Maintainability
The new ipFamily schema includes a default, an enum, and an immutability x-kubernetes-validations rule, but its description does not document the default/allowed values or the immutability constraint. This violates the requirement that validation marker semantics be documented in field comments/user-facing docs.
Agent Prompt
## Issue description
The `ipFamily` field’s schema adds `default`, `enum`, and an immutability `XValidation`, but the user-facing field documentation does not describe:
- what happens when the field is omitted (defaulting)
- the allowed enum values (and what each means)
- that the value is immutable once set
## Issue Context
This is surfaced in generated CRD/OpenAPI documentation, so missing details violate the requirement that validation marker semantics be documented.
## Fix Focus Areas
- config/v1/types_infrastructure.go[567-577]
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yaml[1188-1202]
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yaml[1188-1202]
- machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml[1480-1494]
- machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml[1480-1494]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
This sounds like a legitimate issue to me
There was a problem hiding this comment.
Right. I think this is just saying the comment should contain something like:
// Valid values are "IPv4", "DualStackIPv4Primary", and "DualStackIPv6Primary". When omitted, the default value is "IPv4".
It's also outside the scope of this PR, so I would consider it more of a nice to have
sadasu
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sadasu: This pull request references CORS-4387 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. 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. |
|
/jira refresh |
|
@sadasu: This pull request references CORS-4387 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. |
|
@sadasu: The following test 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. |
|
Checking the promotion verification results, it looks like:
I also noticed openshift/release#76769 - this seems like testing that we would want to make sure is in place and has captured sufficient runs prior to promoting? |
It's not in the plan to support this in hypershift, so no. But it's a good question: hypershift should be able to take advantage of this work. On the other hand I'm sure further work is required to enable it, so we will move forward with no hypershift support.
Exactly. This is the only testing that is actually exercising the AWS Dual Stack functionality, so the other test failures/results are mostly noise... We have been running these tests in various forms, analyzing the test failures and fix tech debt in the e2e tests for a while now. openshift/release#76769 should merge today and we can accumulate some results quickly with gangway. Overall the feature is stable, and well tested. |
5 participants
Also make it available in OKD.