OCPBUGS-74510: vsphere: remove VSphereMultiDisk feature gate references - #10753
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@vr4manta: This pull request references Jira Issue OCPBUGS-74510, 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. |
|
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 (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe vSphere disk setup path no longer depends on the multi-disk feature gate. Feature-gate validation no longer checks control-plane or compute data disks. Valid disk data still produces disk setup ignition. ChangesvSphere disk processing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
@vr4manta: This pull request references Jira Issue OCPBUGS-74510, which is valid. 3 validation(s) were run on this bug
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@pkg/asset/machines/util.go`:
- Around line 56-64: Add unit tests for NodeDiskSetup with
FeatureGateVSphereMultiDisk disabled, covering both a valid vsphere.DiskInfo
input and an unsupported dataDisk type. Assert that the valid input returns disk
setup ignition without error, while the unsupported type returns an error.
🪄 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: 91ad8051-7c58-4ae1-b3a0-b63885f676e5
📒 Files selected for processing (2)
pkg/asset/machines/util.gopkg/types/vsphere/validation/featuregates.go
💤 Files with no reviewable changes (1)
- pkg/types/vsphere/validation/featuregates.go
| if vsphereDataDisk, ok := dataDisk.(vsphere.DiskInfo); ok { | ||
| device := fmt.Sprintf(VsphereScsiByPath, vsphereDataDisk.Index+1) | ||
| diskSetupIgn, err := machineconfig.ForDiskSetup(role, device, label, path, diskSetup.Type) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "failed to create ignition to setup disks for master machines") | ||
| } | ||
| return nil, errors.Errorf("unsupported vsphere data disk type") | ||
| return diskSetupIgn, nil | ||
| } | ||
| return nil, errors.Errorf("unsupported vsphere data disk type") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add unit coverage for unconditional vSphere disk setup.
Add or update tests for NodeDiskSetup with FeatureGateVSphereMultiDisk disabled. Cover a valid vsphere.DiskInfo value and an unsupported dataDisk type. Assert that the valid case returns disk setup ignition and the unsupported case returns an error.
As per coding guidelines, all Go code under pkg/ must have unit tests.
🤖 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 `@pkg/asset/machines/util.go` around lines 56 - 64, Add unit tests for
NodeDiskSetup with FeatureGateVSphereMultiDisk disabled, covering both a valid
vsphere.DiskInfo input and an unsupported dataDisk type. Assert that the valid
input returns disk setup ignition without error, while the unsupported type
returns an error.
Source: Coding guidelines
The VSphereMultiDisk feature gate has been GA for several releases. Remove the gating checks so vSphere multi-disk functionality runs unconditionally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
eec51ce to
82c4096
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/lgtm |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcpowermac The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@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. |
|
/verified by @vr4manta |
openshift-ci-robot
added
the
verified
|
@vr4manta: This PR has been marked as verified by 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. |
4745066
into
openshift:main
|
@vr4manta: Jira Issue OCPBUGS-74510: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged:
All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-74510 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload. 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. |
3 participants
OCPBUGS-74510
Changes
Notes
The VSphereMultiDisk feature gate has been GA for several releases. Remove the gating checks so vSphere multi-disk functionality runs unconditionally.
Summary by CodeRabbit