OCPBUGS-105407: Remove VSphereMultiNetworks feature gate - #2975
Conversation
The VSphereMultiNetworks feature gate has been GA for more than one release. Remove the gate and make maxItems=10 on the networks field unconditional. Tests from the feature-gated test files are merged into the ungated test suites. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@vr4manta: This pull request references Jira Issue OCPBUGS-105407, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
Hello @vr4manta! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughThe vSphere topology network limit is now unconditional. Configurations must contain one to ten network entries. Create and update tests cover these validation cases. The Possibly related PRs
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ 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.12.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
[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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@payload-command/render/legacyfeaturegates.go`:
- Line 23: Correct the maintainer reference in the comment above the legacy
feature-gate list from `@deads2kleg` to `@deads2k`, without changing the comment’s
other wording.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fdcf861c-4b6d-4c5a-83a3-7574c5936142
⛔ Files ignored due to path filters (48)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Hypershift-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-SelfManagedHA-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AWSDualStackInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AzureDualStackInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/BGPBasedVIPManagement.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/DualReplica.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/DyanmicServiceEndpointIBMCloud.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/GCPSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/MutableTopology.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/NutanixMultiSubnets.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/OnPremDNSRecords.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/VSphereHostVMGroupZonal.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/VSphereMultiNetworks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/VSphereMultiVCenterDay2.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Hypershift-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-SelfManagedHA-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSDualStackInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSEuropeanSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AzureDualStackInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/BGPBasedVIPManagement.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/DualReplica.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/DyanmicServiceEndpointIBMCloud.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/GCPSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/MutableTopology.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/OnPremDNSRecords.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/VSphereHostVMGroupZonal.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/VSphereMultiNetworks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/VSphereMultiVCenterDay2.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (30)
config/v1/tests/infrastructures.config.openshift.io/AAA_ungated.yamlconfig/v1/tests/infrastructures.config.openshift.io/VSphereMultiNetworks.yamlconfig/v1/types_infrastructure.gofeatures.mdfeatures/features.gofeatures/legacyfeaturegates.gomachineconfiguration/v1/tests/controllerconfigs.machineconfiguration.openshift.io/VSphereMultiNetworks.yamlpayload-command/render/legacyfeaturegates.gopayload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-Hypershift-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-Hypershift-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Hypershift-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Hypershift-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml
💤 Files with no reviewable changes (13)
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yaml
- features/legacyfeaturegates.go
- features.md
- config/v1/tests/infrastructures.config.openshift.io/VSphereMultiNetworks.yaml
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yaml
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
- payload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yaml
- features/features.go
- payload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yaml
- machineconfiguration/v1/tests/controllerconfigs.machineconfiguration.openshift.io/VSphereMultiNetworks.yaml
| // never add to this list, if you think you have an exception ask @deads2k | ||
| "ClusterAPIInstall", | ||
| // never add to this list, if you think you have an exception ask @deads2k | ||
| // never add to this list, if you think you have an exception ask @deads2kleg |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the maintainer reference typo.
The comment reads "ask @deads2kleg" instead of "ask @deads2k". Every other entry in this list uses "ask @deads2k". Correct the typo to keep the reference consistent and readable.
✏️ Proposed fix
- // never add to this list, if you think you have an exception ask `@deads2kleg`
+ // never add to this list, if you think you have an exception ask `@deads2k`
"ClusterAPIInstallIBMCloud",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@payload-command/render/legacyfeaturegates.go` at line 23, Correct the
maintainer reference in the comment above the legacy feature-gate list from
`@deads2kleg` to `@deads2k`, without changing the comment’s other wording.
Code Review by Qodo
1. Networks MinItems not documented
|
| // +kubebuilder:validation:MaxItems=10 | ||
| // +kubebuilder:validation:MinItems=1 |
There was a problem hiding this comment.
1. networks minitems not documented 📜 Skill insight ✧ Quality
The Networks field has +kubebuilder:validation:MinItems=1 but its comment only documents the maximum (10) and does not state the minimum item count. This violates the requirement that all kubebuilder constraint markers be documented in field comments.
Agent Prompt
## Issue description
`Networks` has `MinItems=1` and `MaxItems=10`, but the doc comment only mentions the max (“Up to 10...”) and omits the minimum item constraint.
## Issue Context
Compliance requires that every kubebuilder constraint marker (including `MinItems`/`MaxItems`) is described in the field’s human-readable comment.
## Fix Focus Areas
- config/v1/types_infrastructure.go[1517-1529]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // never add to this list, if you think you have an exception ask @deads2k | ||
| "ClusterAPIInstall", | ||
| // never add to this list, if you think you have an exception ask @deads2k | ||
| // never add to this list, if you think you have an exception ask @deads2kleg |
There was a problem hiding this comment.
2. Comment handle typo 🐞 Bug ⚙ Maintainability
payload-command/render/legacyfeaturegates.go introduces a typo in a maintainer/contact comment
("@deads2kleg"), which can misdirect future maintainers and is likely accidental.
Agent Prompt
### Issue description
A comment in `payload-command/render/legacyfeaturegates.go` was changed to reference `@deads2kleg`, which appears to be an accidental concatenation/typo.
### Issue Context
This PR removes `VSphereMultiNetworks` and touched nearby lines; the comment change is not intentional behavior change but degrades maintainability.
### Fix Focus Areas
- payload-command/render/legacyfeaturegates.go[21-24]
### Proposed fix
Change `@deads2kleg` back to `@deads2k` (or remove the handle entirely if that’s preferred).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
PR Summary by QodoRemove VSphereMultiNetworks feature gate and un-gate vSphere networks maxItems=10
AI Description
Diagram
High-Level Assessment
Files changed (74)
|
|
@vr4manta: 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. |
OCPBUGS-105407
Changes
Notes
The VSphereMultiNetworks feature gate has been GA for more than one release. Remove the gate and make maxItems=10 on the networks field unconditional. Tests from the feature-gated test files are merged into the ungated test suites.