✨ Populate diskMode, provisioning mode and Scrubing of disks in VM status #1337
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
lubronzhan
reviewed
Nov 25, 2025
|
|
||
| // Update disk attachment properties for ALL volumes (classic and managed). | ||
| // This is done in a single pass at the end to ensure consistency. | ||
| // For managed volumes, the volume controllers populate status from CnsNodeVmAttachment, |
Contributor
There was a problem hiding this comment.
Better to copy over the ProvisioningMode in volumeBatch controller as well
https://github.com/vmware-tanzu/vm-operator/blob/main/controllers/virtualmachine/volumebatch/volumebatch_controller.go#L650
https://github.com/vmware-tanzu/vm-operator/blob/main/controllers/virtualmachine/volumebatch/volumebatch_controller.go#L1020
Otherwise it will keep getting removing and adding
Contributor
There was a problem hiding this comment.
This got me thinking, we are also patching status.volume with detaching suffix here, then we should also add these for detaching disks in volumeController. You don't have to include this as part of this PR
aruneshpa
force-pushed
the
features/rac-and-wsfc/add-disk-mode-instatus
branch
from
2809588 to
5998c87
Compare
This change modifies VM status to include additional properties of a
virtual disk such as:
- Disk mode: this includes persistent, independent_persistent,
independent_nonpersistent, nopersistent, undoable and append
- Provisioning mode: Thin vs thick provisioned
- Scrubing: Lazy vs eager zero-ing
Diskmode is already available in the Status but not being populated.
This change not sets that property. A new property called
provisioningMode is introduced to handle zero-ing of disks.
Testing Done:
- Tested in a real env. Here's the status:
```
root@42177b6e043ae6c535c54e0d5c038e1f [ ~ ]# k get vm -n prod-vmsvc-reco-vds-1309 parunesh-vm -o=jsonpath={.status.volumes} | jq
[
{
"attached": true,
"diskMode": "Persistent",
"diskUUID": "6000C292-d460-6d94-62cf-2c4c9e7c7bfd",
"limit": "10Mi",
"name": "parunesh-vol",
"provisioningMode": "Thin",
"requested": "10Mi",
"sharingMode": "None",
"type": "Managed",
"used": "939"
},
{
"attached": true,
"diskMode": "Persistent",
"diskUUID": "6000C293-fd2c-c9ba-14b7-ffc2f5ea2ea7",
"limit": "10Gi",
"name": "disk-ee05ebc8",
"provisioningMode": "Thin",
"requested": "10Gi",
"sharingMode": "None",
"type": "Classic",
"used": "2948596348"
}
]
```
aruneshpa
force-pushed
the
features/rac-and-wsfc/add-disk-mode-instatus
branch
from
5998c87 to
6c6ad79
Compare
Minimum allowed line rate is |
bryanv
reviewed
Dec 3, 2025
| } | ||
|
|
||
| // Populate disk attachment properties from vSphere disk info. | ||
| if di.DiskMode != "" { |
Contributor
There was a problem hiding this comment.
If these are "", should the status field be cleared?
Collaborator
Author
There was a problem hiding this comment.
Didn't bother about that because a diskMode going from not-empty to empty is not a valid scenario unless the disk's backing is changed to the two specific backing that doesn't have disk mode:
- VirtualDiskRawDiskVer2BackingInfo
- VirtualDiskPartitionedRawDiskVer2BackingInfo
Which are very rare.
https://github.com/vmware-tanzu/vm-operator/blob/main/pkg/util/devices.go#L288-L335.
Also, for regular disks, diskMode is immutable property, so there's no case of a disk's mode changing once it is non-empty.
LMK if this makes sense.
| }) | ||
|
|
||
| // Update disk attachment properties for ALL volumes (classic and managed). | ||
| // This is done in a single pass at the end to ensure consistency. |
Contributor
There was a problem hiding this comment.
What do you mean by consistency here? And why not in the earlier loop?
Collaborator
Author
There was a problem hiding this comment.
Earlier loop has separate handling for classic / unmanaged disks and PVCs. So, if I handle it there, I would need to duplicate it in both, if and else blocks.
Consistency may have been wrong choice of a word.
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?
This change modifies VM status to include additional properties of a virtual disk such as:
Diskmode is already available in the Status but not being populated. This change not sets that property. A new property called provisioningMode is introduced to handle zero-ing of disks.
Please add a release note if necessary: