OCPBUGS-105407: Remove VSphereMultiNetworks feature gate - #2975
Conversation
|
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: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (7)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe change removes the Merge Risk: ⚪ Minimal · up to The PR removes an already-GA feature gate and makes the existing network limit unconditional; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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 |
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
Code Review by Qodo
1. Networks MinItems not documented
|
PR Summary by QodoRemove VSphereMultiNetworks feature gate and un-gate vSphere networks maxItems=10
AI Description
Diagram
High-Level Assessment
Files changed (74)
|
0ddd0d6 to
881230b
Compare
|
/pipeline auto |
|
Pipeline controller notification The |
|
Scheduling tests matching the |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
2 similar comments
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/retest |
1 similar comment
|
/retest |
881230b to
f8813d1
Compare
|
Scheduling tests matching the |
|
/assign @everettraven |
|
@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. |
yuqi-zhang
left a comment
There was a problem hiding this comment.
Looking at the references to this FG, I think we're mostly good to go? I found one reference in https://github.com/openshift/release/blob/53450d3f29242e488f422a22bc4c64fb6e83aac9/ci-operator/config/openshift/release/openshift-release-main__nightly-5.1.yaml#L1088 that may be relevant given that it's for the release-5.1 branch. Not sure if we need to clean that up or not.
I'll clean up that 5.1 job. Not sure why that is still CNU.
If it aint broke, don't fix it :) If we want to adjust it, i'd rather a separate OCPBUG as it can take a long time for reviews even for a simple change like that. |
|
Seems fine, marking as shadow review complete |
| featureGates: | ||
| - VSphereMultiNetworks |
There was a problem hiding this comment.
You can actually drop just these two lines and leave the tests as they are otherwise.
That is probably good from a long term sustainability perspective
There was a problem hiding this comment.
I wasn't sure the pattern we like to use in this situation. Sounds like its better to keep the old file? I can make this update.
| - name: Should be able to create a minimal Infrastructure | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Infrastructure | ||
| spec: {} # No spec is required for a Infrastructure | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Infrastructure | ||
| spec: {} |
There was a problem hiding this comment.
We don't need this test if we do keep this file
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>
f8813d1 to
429abb9
Compare
|
[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 |
|
Scheduling tests matching the |
|
/pipeline required |
|
Scheduling tests matching the |
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.