Skip to content

Conversation

skogta
Copy link
Contributor

@skogta skogta commented Jul 11, 2025

What this PR does / why we need it:
Creating a new CRD called CnsVolumeAttachment to keep track of VMs that are attached to a given PVC.
This CRD is not user facing and is only meant for book keeping.

This CRD is required because with RWX volumes, a volume can be attached to multiple VMs. It is not safe to delete such a volumes unless all VMs attached to it have been detached.

CSI can rely on this CRD to find out if a given volumes is currently attached or not.

There are 3 functions for this CRD - to add a VM to the instance, to remove a VM from the instance and to find if the CR exists or not.

Testing done:

apiVersion: v1
items:
- apiVersion: cns.vmware.com/v1alpha1
  kind: CnsVolumeAttachment
  metadata:
    creationTimestamp: "2025-07-31T13:33:17Z"
    finalizers:
    - cns.vmware.com
    generation: 1
    name: rwm-pvc-3
    namespace: test
    resourceVersion: "384214"
    uid: ed6c967e-859b-4c26-9f56-0981c29a67c6
  spec:
    attachedVms:
    - 2bcce803-cea3-44ac-968d-e49f2de90379
kind: List
metadata:
  resourceVersion: ""

PVC has finalizer

Name:          pvc-1
Namespace:     test
StorageClass:  vsan-default-storage-policy
Status:        Bound
Volume:        pvc-08dbd91e-63eb-406f-867e-c6c6dfd3fc5e
Labels:        <none>
Annotations:   csi.vsphere.volume-accessible-topology: [{"topology.kubernetes.io/zone":"az1"}]
               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
               volumehealth.storage.kubernetes.io/health: accessible
               volumehealth.storage.kubernetes.io/health-timestamp: Mon Aug  4 14:26:46 UTC 2025
Finalizers:    [kubernetes.io/pvc-protection cns.vmware.com/pvc-protection]
Capacity:      100Mi
Access Modes:  RWO
VolumeMode:    Filesystem
Used By:       <none>
Events:
  Type    Reason                 Age                    From                                                                                          Message
  ----    ------                 ----                   ----                                                                                          -------
  Normal  Provisioning           2m44s                  csi.vsphere.vmware.com_4200d5d06c7f5c505e20ac022d1225d2_d3cc96bb-a2c5-4996-8bcd-41a6b7ce5584  External provisioner is provisioning volume for claim "test/pvc-1"
  Normal  ExternalProvisioning   2m40s (x4 over 2m44s)  persistentvolume-controller                                                                   Waiting for a volume to be created either by the external provisioner 'csi.vsphere.vmware.com' or manually by the system administrator. If volume creation is delayed, please verify that the provisioner is running and correctly registered.
  Normal  ProvisioningSucceeded  2m40s                  csi.vsphere.vmware.com_4200d5d06c7f5c505e20ac022d1225d2_d3cc96bb-a2c5-4996-8bcd-41a6b7ce5584  Successfully provisioned volume pvc-08dbd91e-63eb-406f-867e-c6c6dfd3fc5e

Logs

{"level":"info","time":"2025-07-31T13:33:47.260032259Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:196","msg":"Removing VmInstanceUUID 2bcce803-cea3-44ac-968d-e49f2de90379 from cnsVolumeAttachment test/rwm-pvc-3","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.260069943Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:205","msg":"Acquired lock for cnsVolumeAttachment instance test/rwm-pvc-3","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.277450849Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:231","msg":"Verifying if VM UUID test/rwm-pvc-3 exists in list of already attached VMs. Current list: [2bcce803-cea3-44ac-968d-e49f2de90379]","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.27752388Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:237","msg":"Removing VmUUID 2bcce803-cea3-44ac-968d-e49f2de90379 from Attached VMs list","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.277559598Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:241","msg":"attached VMs after this []","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.277587788Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:243","msg":"Deleting cnsVolumeAttachment instance test/rwm-pvc-3 from API server","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.277621165Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:341","msg":"Removing \"cns.vmware.com\" finalizer from CnsNodeVmBatchAttachment instance with name: \"rwm-pvc-3\" on namespace: \"test\"","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.456826302Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:265","msg":"Successfully deleted cnsVolumeAttachment instance test/rwm-pvc-3","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}
{"level":"info","time":"2025-07-31T13:33:47.456928646Z","caller":"cnsvolumeattachment/cnsvolumeattachment.go:208","msg":"Released lock for instance test/rwm-pvc-3","TraceId":"7dc4f216-ba2f-41f0-8eee-27ff02041af0"}

Deleted PVC and it got stuck in terminating state.
Delete batch attachment CR and observed the PVC also got deleted.

Added unit tests.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 11, 2025
@skogta skogta marked this pull request as draft July 11, 2025 09:19
@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 11, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @skogta. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 11, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 67de099 to 081f3de Compare July 11, 2025 10:03
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 11, 2025
@skogta skogta changed the title Add CnsSharedDiskVolumeClient CRD Add CnsSharedDiskVolumeInfo CRD Jul 11, 2025
@skogta skogta marked this pull request as ready for review July 11, 2025 10:22
@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 Jul 11, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 081f3de to df571ee Compare July 11, 2025 10:23
@akankshapanse
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 11, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from df571ee to 8c3c35d Compare July 13, 2025 09:20
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: skogta
Once this PR has been reviewed and has the lgtm label, please assign deepakkinni for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 13, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 8c3c35d to 3363b6b Compare July 13, 2025 12:51
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 3363b6b to 99c6a59 Compare July 21, 2025 08:51
@skogta skogta changed the title Add CnsSharedDiskVolumeInfo CRD Add CnsVolumeAttachment CRD Jul 21, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 99c6a59 to 968218a Compare July 29, 2025 08:09
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 31, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 1d03b27 to 7128196 Compare July 31, 2025 09:29
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 31, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 7128196 to 27cea3b Compare July 31, 2025 10:24
@skogta skogta marked this pull request as draft July 31, 2025 13:02
@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 Jul 31, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 27cea3b to 647c7eb Compare August 1, 2025 07:40
@skogta skogta marked this pull request as ready for review August 1, 2025 07:48
@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 Aug 1, 2025
@k8s-ci-robot k8s-ci-robot requested a review from chethanv28 August 1, 2025 07:48
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch 2 times, most recently from 09eed33 to 92d0712 Compare August 1, 2025 09:03
@skogta skogta changed the title Add CnsVolumeAttachment CRD Add CnsVolumeAttachment CRD to track VMs attached to a PVC Aug 1, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch 3 times, most recently from a33befa to bbb626c Compare August 4, 2025 14:55
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from bbb626c to dcb49e9 Compare August 4, 2025 14:56
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2025
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from dcb49e9 to 37c8635 Compare August 4, 2025 14:58
@skogta skogta force-pushed the topic/skogta/rwxVolumeClient branch from 37c8635 to 5142b69 Compare August 4, 2025 15:17
@skogta
Copy link
Contributor Author

skogta commented Aug 4, 2025

ok-to-test-wcp

@deepakkinni
Copy link
Collaborator

Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #32

@deepakkinni
Copy link
Collaborator

SUCCESS --- Jenkins Build #32

@ravikumarkp7
Copy link

ok-to-test-wcp

@deepakkinni
Copy link
Collaborator

Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #38

@deepakkinni
Copy link
Collaborator

FAILED --- Jenkins Build #38

@deepakkinni
Copy link
Collaborator

Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #39

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 22, 2025
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants