Skip to content

OCPBUGS-74510: Removed VSphereMultiDisk feature gate - #1532

Open
vr4manta wants to merge 1 commit into
openshift:mainfrom
vr4manta:OCPBUGS-74510
Open

OCPBUGS-74510: Removed VSphereMultiDisk feature gate #1532
vr4manta wants to merge 1 commit into
openshift:mainfrom
vr4manta:OCPBUGS-74510

Conversation

@vr4manta

@vr4manta vr4manta commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

OCPBUGS-74510

Changes

  • Removed usages of VSphereMultiDisk feature gate
  • Updated tests to assume GA of feature gate

Notes

Removed VSphereMultiDisk feature gate now that it has been GA for several releases

Summary by CodeRabbit

  • New Features

    • vSphere machines now support multiple data disks by default, without requiring a separate feature toggle.
    • Data disk validation is consistently applied, including size, naming, and provisioning constraints.
  • Bug Fixes

    • Removed the previous configuration restriction that blocked additional disks when the feature toggle was disabled.
    • Updated validation and end-to-end coverage to reflect the default multi-disk behavior.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@vr4manta: This pull request references Jira Issue OCPBUGS-74510, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

OCPBUGS-74510

Changes

  • Removed usages of VSphereMultiDisk feature gate
  • Updated tests to assume GA of feature gate

Notes

Removed VSphereMultiDisk feature gate now that it has been GA for several releases

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.

@openshift-ci-robot openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 48638a20-2f47-4b16-bc70-4b99f16daaa5

📥 Commits

Reviewing files that changed from the base of the PR and between d3f61d1 and 220e176.

📒 Files selected for processing (1)
  • pkg/webhooks/machine_webhook_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/webhooks/machine_webhook_test.go

Walkthrough

The VSphereMultiDisk feature gate was removed from registration, defaults, reconciliation, and webhook validation. vSphere data disks are now processed and validated without the gate. Unit and end-to-end tests were updated.

Changes

VSphereMultiDisk gate removal

Layer / File(s) Summary
Remove gate configuration
cmd/machineset/main.go, cmd/vsphere/main.go, pkg/operator/..., pkg/util/testing/testing.go
Command, operator, and test configuration no longer registers or enables VSphereMultiDisk.
Enable unconditional disk processing
pkg/controller/vsphere/reconciler.go, pkg/webhooks/machine_webhook.go
Reconciliation and webhook validation no longer require VSphereMultiDisk for vSphere data disks.
Update disk validation coverage
pkg/webhooks/machine_webhook_test.go, test/e2e/vsphere/machines.go
Tests no longer configure or require the feature gate. Existing disk validation cases remain covered.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: theobarberbany, racheljpg

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning test/e2e/vsphere/machines.go creates Machine and MachineSet resources but has no DeferCleanup/AfterEach, and many Expect(err).NotTo(HaveOccurred()) assertions lack messages. Register deferred cleanup after each resource is created, wait for deletion, and add operation-specific messages to every cluster-operation assertion.
Single Node Openshift (Sno) Test Compatibility ⚠️ Warning The changed vSphere Describe has no SNO guard, while its tests create/delete machines and scale a MachineSet, which assumes node-scaling support. Because the test is [Serial], run /payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-aws-ovn-single-node-serial; add a SNO skip label or runtime guard if unsupported.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing the VSphereMultiDisk feature gate.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The PR changes only a static Ginkgo Describe label; It and Entry titles are static and contain no generated names, timestamps, IPs, namespaces, or other run-dependent values.
Microshift Test Compatibility ✅ Passed The PR adds no new e2e test logic; it only removes a Describe label. Existing vSphere tests retain [apigroup:machine.openshift.io] tags, which protect them from MicroShift.
Topology-Aware Scheduling Compatibility ✅ Passed The diff only removes the VSphereMultiDisk gate and updates validation/tests; it adds no affinity, topology spread, node selectors, tolerations, PDBs, or workload replica constraints.
Ote Binary Stdout Contract ✅ Passed The commit does not alter OTE setup; main calls logs.InitLogs, klog defaults to stderr, and OTE config sets GinkgoWriter to os.Stderr. fmt.Printf sites run from It paths.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The only e2e change removes the VSphereMultiDisk label; no new Ginkgo test, IPv4 literal, IPv4-only parsing, public URL, or external connectivity was added.
No-Weak-Crypto ✅ Passed The PR changes only feature-gate and disk-validation logic; added-line and Go crypto scans found no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons.
Container-Privileges ✅ Passed The patch changes only feature-gate and vSphere validation code; it adds or modifies no container or Kubernetes manifest privilege settings.
No-Sensitive-Data-In-Logs ✅ Passed The patch adds no logging calls or logged values. It only removes the VSphereMultiDisk initialization log; existing data-disk logs are unchanged and contain disk fields, not credentials or tokens.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Comment @coderabbitai help to get the list of available commands.

…eral releases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vr4manta

Copy link
Copy Markdown
Contributor Author

/assign @RadekManak

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@vr4manta: This PR was included in a payload test run from openshift/api#2973
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/2c2845e0-970a-11f1-8e70-9635b1627004-0

@damdo damdo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

/pipeline auto

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damdo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2026
@damdo damdo added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-operator
/test e2e-aws-ovn
/test e2e-aws-ovn-upgrade
/test e2e-metal-ipi
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-virtualmedia

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@vr4manta: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-metal-ipi-virtualmedia 220e176 link true /test e2e-metal-ipi-virtualmedia

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants