Skip to content

Conversation

@divyenpatel
Copy link
Member

@divyenpatel divyenpatel commented Dec 12, 2025

What this PR does / why we need it:

When the disk-backing annotation is not present on a PVC, dynamically query the backing type from vSphere VirtualDiskManager API and cache it on the PVC annotation for future attach operations.

Changes:

  • Add queryBackingTypeFromVirtualDiskInfo() to query disk type via CNS QueryVolume API and VirtualDiskManager.QueryVirtualDiskInfo()
  • Add convertDiskTypeToBackingType() to map vSphere disk types (thin, preallocated, eagerZeroedThick, etc.) to backing types
  • Add patchPVCBackingTypeAnnotation() to persist queried backing type on PVC annotation for caching
  • Update constructBatchAttachRequest() to query and cache backing type when annotation is missing
  • Add unit tests for convertDiskTypeToBackingType and patchPVCBackingTypeAnnotation functions

Testing done:
Verified Creating Dynamic PVC and attaching it to VM. When PVC does not have cns.vmware.com.protected/disk-backing annotation set, cnsnodevmbatchattachment reconciler is setting this annotation after querying backing type.

{"level":"info","time":"2025-12-13T16:13:42.151459229Z","caller":"volume/manager.go:2473","msg":"QueryVolumeAsync successfully returned CnsQueryResult, opId: "56aa5c18"","TraceId":"b5fab59c-178c-4c7c-90aa-7ce25b1b5921"}
{"level":"info","time":"2025-12-13T16:13:42.203838795Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:568","msg":"Successfully retrieved BackingType FlatVer2BackingInfo for PVC vm-dj5y-pvc-0 from VirtualDiskManager","TraceId":"b5fab59c-178c-4c7c-90aa-7ce25b1b5921"}
{"level":"info","time":"2025-12-13T16:13:42.204150985Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:802","msg":"Setting BackingType annotation cns.vmware.com.protected/disk-backing=FlatVer2BackingInfo on PVC vm-dj5y-pvc-0","TraceId":"b5fab59c-178c-4c7c-90aa-7ce25b1b5921"}
{"level":"info","time":"2025-12-13T16:13:42.204341388Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:819","msg":"Patching PVC vm-dj5y-pvc-0 with BackingType annotation","TraceId":"b5fab59c-178c-4c7c-90aa-7ce25b1b5921"}
{"level":"info","time":"2025-12-13T16:13:42.224495615Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:833","msg":"Successfully patched PVC: vm-dj5y-pvc-0 with BackingType annotation map[cns.vmware.com.protected/disk-backing:FlatVer2BackingInfo csi.vsphere.volume-accessible-topology:[{"topology.kubernetes.io/zone":"domain-c36"}] csi.vsphere.volume-requested-topology:[{"topology.kubernetes.io/zone":"domain-c36"}] pv.kubernetes.io/bind-completed:yes pv.kubernetes.io/bound-by-controller:yes volume.beta.kubernetes.io/storage-provisioner:csi.vsphere.vmware.com volume.kubernetes.io/storage-provisioner:csi.vsphere.vmware.com]","TraceId":"b5fab59c-178c-4c7c-90aa-7ce25b1b5921"}
{"level":"info","time":"2025-12-13T16:13:42.224673309Z","caller":"volume/manager.go:3571","msg":"Starting batch attach for VM 694a5650-07df-49be-a8e2-30180e4f2a1a","TraceId":"b5fab59c-178c-4c7c-90aa-7ce25b1b5921"}

Special notes for your reviewer:

Release note:

Add dynamic BackingType detection via VirtualDiskManager

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyenpatel

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

The pull request process is described here

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 12, 2025
@divyenpatel divyenpatel force-pushed the query-backing-disk-type-for-batch-attach branch from c87ef5d to 261e4f1 Compare December 12, 2025 01:54
@divyenpatel divyenpatel force-pushed the query-backing-disk-type-for-batch-attach branch from 261e4f1 to aaa0d8b Compare December 12, 2025 06:12
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2025
@divyenpatel divyenpatel force-pushed the query-backing-disk-type-for-batch-attach branch from aaa0d8b to 22fd7a7 Compare December 12, 2025 23:41
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2025
@divyenpatel divyenpatel force-pushed the query-backing-disk-type-for-batch-attach branch 2 times, most recently from 44961e6 to 75a45a0 Compare December 13, 2025 16:27
@divyenpatel divyenpatel changed the title [WIP] Add dynamic BackingType detection via VirtualDiskManager Add dynamic BackingType detection via VirtualDiskManager Dec 13, 2025
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 13, 2025
@divyenpatel divyenpatel force-pushed the query-backing-disk-type-for-batch-attach branch from 75a45a0 to a92fd95 Compare December 13, 2025 16:45
When the disk-backing annotation is not present on a PVC, dynamically
query the backing type from vSphere VirtualDiskManager API and cache
it on the PVC annotation for future attach operations.

Changes:
- Add queryBackingTypeFromVirtualDiskInfo() to query disk type via
  CNS QueryVolume API and VirtualDiskManager.QueryVirtualDiskInfo()
- Add convertDiskTypeToBackingType() to map vSphere disk types
  (thin, preallocated, eagerZeroedThick, etc.) to backing types
- Add patchPVCBackingTypeAnnotation() to persist queried backing type
  on PVC annotation for caching
- Update constructBatchAttachRequest() to query and cache backing type
  when annotation is missing
- Add unit tests for convertDiskTypeToBackingType and
  patchPVCBackingTypeAnnotation functions
@divyenpatel divyenpatel force-pushed the query-backing-disk-type-for-batch-attach branch from a92fd95 to 0ebdcf1 Compare December 13, 2025 17:23
divyenpatel added a commit to divyenpatel/vsphere-csi-driver that referenced this pull request Dec 14, 2025
This commit adds handling for CnsNotRegisteredFault in various CNS volume
operations for the WORKLOAD cluster flavor. When a volume operation fails
with CnsNotRegisteredFault, the driver now attempts to re-register the
volume with CNS and retries the operation.

Changes include:
- Add clusterId and clusterDistribution parameters to GetManager() for
  volume re-registration
- Add reRegisterVolume() function to re-register unregistered volumes
- Add IsCnsNotRegisteredFault() helper to detect the fault type
- Add IsCnsVolumeAlreadyExistsFault() helper for idempotent re-registration
- Handle CnsNotRegisteredFault in:
  - AttachVolume
  - DetachVolume
  - DeleteVolume (with improved idempotency)
  - UpdateVolumeMetadata
  - UpdateVolumeCrypto
  - ExpandVolume (both regular and with improved idempotency)
  - CreateSnapshot (both regular and with improved idempotency)
  - DeleteSnapshot
- Add TODO for batch attach reconcile to handle unregistered volumes
  when QueryBackingTypeFromVirtualDiskInfo is used (PR kubernetes-sigs#3799)

The re-registration is only attempted once per operation to prevent
infinite loops. If re-registration fails or the retry fails, the
original error is returned.
divyenpatel added a commit to divyenpatel/vsphere-csi-driver that referenced this pull request Dec 14, 2025
This commit adds handling for CnsNotRegisteredFault in various CNS volume
operations for the WORKLOAD cluster flavor. When a volume operation fails
with CnsNotRegisteredFault, the driver now attempts to re-register the
volume with CNS and retries the operation.

Changes include:
- Add clusterId and clusterDistribution parameters to GetManager() for
  volume re-registration
- Add reRegisterVolume() function to re-register unregistered volumes
- Add IsCnsNotRegisteredFault() helper to detect the fault type
- Add IsCnsVolumeAlreadyExistsFault() helper for idempotent re-registration
- Handle CnsNotRegisteredFault in:
  - AttachVolume
  - DetachVolume
  - DeleteVolume (with improved idempotency)
  - UpdateVolumeMetadata
  - UpdateVolumeCrypto
  - ExpandVolume (with improved idempotency)
  - CreateSnapshot (with improved idempotency and with transaction)
  - DeleteSnapshot
- Add TODO for batch attach reconcile to handle unregistered volumes
  when QueryBackingTypeFromVirtualDiskInfo is used (PR kubernetes-sigs#3799)

The re-registration is only attempted once per operation to prevent
infinite loops. If re-registration fails or the retry fails, the
original error is returned.
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants