Consider volumes in status also while removing finalizers from PVC in batch attach.

[skogta]

What this PR does / why we need it:

It is possible that after VM is deleted and batchattach spec has a deletionTimestamp, someone removes one or more of the volumes from batchattach spec.

This means CSI will never be able to remove finalizer from such volumes as CSI no longer has track record of which volumes were removed from the spec and which were attached to the VM.

There are 2 cases which need to be handled:

1. VM itself is gone from the VC - in this case CSI should consider volumes present in batchattach status also while removing PVC finalizers.
2. VM is not deleted yet, only batchattach spec has a deletion timestamp. In this case, CSI should detach all volumes attached to the VM on the vCenter and not only the ones mentioned in batchattach spec.

Testing done:

Detached 1 volume from batchattach CR - detach was succesful.

Deleted the batchattach CR, volume was detached and CR was deleted:

{"level":"info","time":"2025-12-05T07:12:00.713965369Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go:354","msg":"Deletion timestamp observed on instance test/test-new-2. Detaching all volumes.","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:00.713990037Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go:450","msg":"Detach call started for PVC pv-no-encrypt with volumeID d3171b93-a492-49f8-b39b-ae489a99aac9 in namespace test for instance test-new-2","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.584059742Z","caller":"volume/manager.go:1313","msg":"DetachVolume: Volume detached successfully. volumeID: \"d3171b93-a492-49f8-b39b-ae489a99aac9\", vm: \"VirtualMachine:vm-1002 [VirtualCenterHost: lvn-dvm-10-161-106-130.dvm.lvn.broadcom.net, UUID: 8b4e6ad5-b2b1-47bc-ad7f-ba9b0403735f, Datacenter: Datacenter [Datacenter: Datacenter:datacenter-3, VirtualCenterHost: lvn-dvm-10-161-106-130.dvm.lvn.broadcom.net]]\", opId: \"fb37ea79\"","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.584100759Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:909","msg":"Acquired lock for PVC test/pv-no-encrypt","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.584129943Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:776","msg":"Removing annotation cns.vmware.com/usedby-vm-8b4e6ad5-b2b1-47bc-ad7f-ba9b0403735f from PVC pv-no-encrypt","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.584153639Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:798","msg":"Patching PVC pv-no-encrypt with updated annotation","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.643335362Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:940","msg":"Successfully updated annotations on PVC pv-no-encrypt for VM 8b4e6ad5-b2b1-47bc-ad7f-ba9b0403735f","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.654608067Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:832","msg":"PVC pv-no-encrypt does not contain any annotations with prefix cns.vmware.com/usedby-vm-","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.654683699Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:956","msg":"VM 8b4e6ad5-b2b1-47bc-ad7f-ba9b0403735f was the last attached VM for the PVC pv-no-encrypt. Finalizer cns.vmware.com/pvc-protection can be safely removed from the PVC","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.654697861Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:969","msg":"Removing cns.vmware.com/pvc-protection finalizer from PVC: pv-no-encrypt on namespace: test","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.713667823Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:982","msg":"Successfully removed finalizer cns.vmware.com/pvc-protection from PVC pv-no-encrypt","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.714254864Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:912","msg":"Released lock for PVC test/pv-no-encrypt","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.714713921Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go:498","msg":"Successfully detached volume pv-no-encrypt from VM 8b4e6ad5-b2b1-47bc-ad7f-ba9b0403735f","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.71486518Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go:474","msg":"Detach call ended for PVC pv-no-encrypt in namespace test for instance test-new-2","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}
{"level":"info","time":"2025-12-05T07:12:01.715030952Z","caller":"cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go:77","msg":"Removing \"cns.vmware.com\" finalizer from CnsNodeVMBatchAttachment instance with name: \"test-new-2\" on namespace: \"test\"","TraceId":"fe1e9e45-70d7-40d3-9e18-142d97dfce35"}

WCP precheckin: https://jenkins-vcf-csifvt.devops.broadcom.net/job/wcp-instapp-e2e-pre-checkin/738/
VKS precheckin: https://jenkins-vcf-csifvt.devops.broadcom.net/job/vks-instapp-e2e-pre-checkin/684/

[k8s-ci-robot]

Hi @skogta. Thanks for your PR.

I'm waiting for a github.com 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.

[deepakkinni]

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

[deepakkinni]

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

[vdkotkar]

/ok-to-test

[deepakkinni]

FAILED --- Jenkins Build #715

[deepakkinni]

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

[divyenpatel]

@skogta Can we reject spec change on cnsnodevmbatchattachment instance, if it has deletion timestamp.
removal of volumes from batchattach spec should not allowed once instance has deletion timestamp.

Can we have webhook check for this?

[skogta]

@skogta Can we reject spec change on cnsnodevmbatchattachment instance, if it has deletion timestamp. removal of volumes from batchattach spec should not allowed once instance has deletion timestamp.

Can we have webhook check for this?

There can be a case where someone detaches a volume and then immediately deletes the VM. So in such a case, both the reconciliations will be combined.

For this case also we should consider volumes in status while detaching.

[deepakkinni]

SUCCESS --- Jenkins Build #738

[deepakkinni]

SUCCESS --- Jenkins Build #684

[divyenpatel]

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyenpatel, skogta

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:

• OWNERS [divyenpatel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment