✨ validate and merge controller configurations #1315
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zhangheliu
force-pushed
the
zhangheliu/feature/shared-disk-controller-conflicts
branch
from
dde958d to
e57b1c3
Compare
lubronzhan
requested review from
akutz,
aruneshpa and
bryanv
and removed request for
akutz and
aruneshpa
lubronzhan
approved these changes
Nov 25, 2025
Contributor
lubronzhan
left a comment
lubronzhan
left a comment
There was a problem hiding this comment.
Took sometime to understand CopyStorageControllersAndDisksWithOverride.
Have you verified in an real env?
pkg/providers/vsphere/upgrade/virtualmachine/vm_schema_upgrade_test.go
Outdated
Show resolved
Hide resolved
zhangheliu
force-pushed
the
zhangheliu/feature/shared-disk-controller-conflicts
branch
2 times, most recently
from
53a15af to
cf073ef
Compare
zhangheliu
commented
Dec 3, 2025
Collaborator
|
I don't think we should discard those disks. I think we should try and pair them with other controllers that can satisfy those disks. |
zhangheliu
changed the title
zhangheliu
force-pushed
the
zhangheliu/feature/shared-disk-controller-conflicts
branch
2 times, most recently
from
919a9fc to
e05198b
Compare
zhangheliu
changed the title
zhangheliu
force-pushed
the
zhangheliu/feature/shared-disk-controller-conflicts
branch
2 times, most recently
from
b18f467 to
2b98f2f
Compare
zhangheliu
changed the title
zhangheliu
force-pushed
the
zhangheliu/feature/shared-disk-controller-conflicts
branch
3 times, most recently
from
5512dcd to
182e223
Compare
Add validation and merging logic for storage controller configurations from VM Class, VM Image (OVF), and user-specified hardware spec. The webhook validates conflicts during VM creation, while the provider merges controllers during VM provisioning. Controller conflicts are detected when controllers at the same bus number have incompatible properties (e.g., different SCSI SharingMode or controller subtypes). This prevents runtime errors by catching incompatible controller configurations early and ensures proper controller merging during VM creation.
zhangheliu
force-pushed
the
zhangheliu/feature/shared-disk-controller-conflicts
branch
from
182e223 to
9a5692c
Compare
zhangheliu
changed the title
Minimum allowed line rate is |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do, and why is it needed?
Add validation and merging logic for storage controller
configurations from VM Class, VM Image (OVF), and user-specified
hardware spec. The webhook validates conflicts during VM creation,
while the provider merges controllers during VM provisioning.
Controller conflicts are detected when controllers at the same bus
number have incompatible properties (e.g., different SCSI
SharingMode or controller subtypes).
This prevents runtime errors by catching incompatible controller
configurations early and ensures proper controller merging during
VM creation.
Which issue(s) is/are addressed by this PR? (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes # N/A
Are there any special notes for your reviewer:
N/A
Please add a release note if necessary: