poc change with DeleteVolume call per CNS team recom

Author: zhangheliu

pkg/providers/vsphere/virtualmachine/delete.go

@@ -1,5 +1,5 @@
 // © Broadcom. All Rights Reserved.
-// The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.
+// The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
 // SPDX-License-Identifier: Apache-2.0
 
 package virtualmachine
@@ -10,13 +10,17 @@ import (
 	"path"
 
 	"github.com/go-logr/logr"
+	"github.com/vmware/govmomi/cns"
+	cnstypes "github.com/vmware/govmomi/cns/types"
 	"github.com/vmware/govmomi/fault"
 	"github.com/vmware/govmomi/object"
 	"github.com/vmware/govmomi/vim25"
 	vimtypes "github.com/vmware/govmomi/vim25/types"
+	"github.com/vmware/govmomi/vslm"
 
 	pkgconst "github.com/vmware-tanzu/vm-operator/pkg/constants"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
+	pkglog "github.com/vmware-tanzu/vm-operator/pkg/log"
 	vmutil "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm"
 )
 
@@ -26,6 +30,7 @@ var VMDeletePropertiesSelector = []string{
 	"recentTask",
 	"config.extraConfig",
 	"config.files",
+	"config.hardware.device",
 	"summary.runtime.connectionState",
 }
 
@@ -94,6 +99,18 @@ func DeleteVirtualMachine(
 		return fmt.Errorf("destroy VM task failed: %w", err)
 	}
 
+	// After Destroy(), delete any FCD-backed disks left in the VM's directory
+	// (left behind because keepAfterDeleteVm=true) via the CNS DeleteVolume
+	// API. This MUST run after Destroy() so the disk is no longer attached
+	// and CNS permits the delete. Using the CNS API (not raw VSLM) keeps
+	// CNS's internal state consistent so the subsequent PVC cleanup path
+	// handles any duplicate delete gracefully with no VC UI error.
+	// See deleteFCDsInVMDir for full context.
+	deleteFCDsInVMDir(
+		vmCtx,
+		vcVM.Client(),
+		vmCtx.MoVM.Config)
+
 	// Best-effort cleanup of VM dir after Destroy();
 	// no error return since delete is still considered successful.
 	if dc == nil ||
@@ -112,3 +129,160 @@ func DeleteVirtualMachine(
 		namespacePath)
 	return nil
 }
+
+// deleteFCDsInVMDir finds all FCD-backed disks in the VM's own directory and
+// deletes them via the CNS DeleteVolume API (with deleteDisk=true) before the
+// VM is destroyed.
+//
+// The unmanaged volumes registration process (unmanagedvolumes_register)
+// registers a VM's ephemeral disks as CNS FCDs via CnsRegisterVolume, which
+// sets keepAfterDeleteVm=true. During VM deletion, vcVM.Destroy() skips such
+// disks because of this flag. CleanupVMDir() then removes the directory via
+// the raw FileManager API without updating the FCD catalog, leaving a stale
+// catalog entry pointing to a now-deleted file.
+//
+// The CNS DeleteVolume API (rather than the lower-level VSLM
+// DeleteVStorageObject) is used because CNS tracks the deletion in its own
+// internal state. When CNS's PVC cleanup path subsequently also tries to
+// delete the same volume, CNS recognises it is already gone and handles the
+// duplicate call gracefully — avoiding "not found" errors visible in the
+// vCenter task UI.
+//
+// Only disks whose backing file is in the VM's own directory are affected;
+// user-attached PVC disks in separate directories are not touched.
+//
+// Note: after CSI registers a VMDK as an FCD, the VM hardware config's
+// backingObjectId field is not updated. The VSLM catalog is queried directly
+// to find FCDs whose backing file path matches the VM's disk paths.
+func deleteFCDsInVMDir(
+	ctx context.Context,
+	vimClient *vim25.Client,
+	config *vimtypes.VirtualMachineConfigInfo) {
+
+	if config == nil {
+		return
+	}
+	if vimClient.ServiceContent.VStorageObjectManager == nil {
+		return
+	}
+
+	vmDir := path.Dir(config.Files.VmPathName)
+	logger := pkglog.FromContextOrDefault(ctx)
+
+	logger.Info(
+		"Scanning VM disks for FCD cleanup",
+		"vmDir", vmDir,
+		"numDevices", len(config.Hardware.Device))
+
+	// Collect the file paths of all disks in the VM's own directory, and
+	// capture one datastore MoRef to use for VSLM queries.
+	diskFilesInVMDir := map[string]struct{}{}
+	var datastoreMoRef *vimtypes.ManagedObjectReference
+	for _, dev := range config.Hardware.Device {
+		disk, ok := dev.(*vimtypes.VirtualDisk)
+		if !ok {
+			continue
+		}
+		bi := getVirtualDiskFileBackingInfo(disk)
+		if bi == nil || bi.FileName == "" || bi.Datastore == nil {
+			continue
+		}
+		logger.Info(
+			"Found disk in VM",
+			"fileName", bi.FileName,
+			"inVMDir", path.Dir(bi.FileName) == vmDir)
+		if path.Dir(bi.FileName) != vmDir {
+			continue
+		}
+		diskFilesInVMDir[bi.FileName] = struct{}{}
+		datastoreMoRef = bi.Datastore
+	}
+
+	if len(diskFilesInVMDir) == 0 || datastoreMoRef == nil {
+		logger.Info(
+			"No disks found in VM directory, skipping FCD cleanup",
+			"vmDir", vmDir)
+		return
+	}
+
+	// List all FCDs on the datastore and find those whose backing file
+	// matches a disk in the VM's directory.
+	mgr := vslm.NewObjectManager(vimClient)
+	ds := object.NewDatastore(vimClient, *datastoreMoRef)
+
+	ids, err := mgr.List(ctx, ds)
+	if err != nil {
+		logger.Error(err,
+			"Failed to list FCDs on datastore for cleanup")
+		return
+	}
+
+	// Build CNS client once; used to delete matched FCDs via CNS DeleteVolume
+	// so that CNS internal state is updated and its PVC cleanup path handles
+	// the subsequent duplicate delete gracefully (no VC UI errors).
+	cnsClient, err := cns.NewClient(ctx, vimClient)
+	if err != nil {
+		logger.Error(err, "Failed to create CNS client for FCD cleanup")
+		return
+	}
+
+	for _, id := range ids {
+		obj, err := mgr.Retrieve(ctx, ds, id.Id)
+		if err != nil {
+			logger.Info(
+				"Failed to retrieve FCD, skipping",
+				"fcdID", id.Id,
+				"err", err)
+			continue
+		}
+
+		// Extract the backing file path.
+		fb, ok := obj.Config.Backing.(
+			vimtypes.BaseBaseConfigInfoFileBackingInfo)
+		if !ok {
+			continue
+		}
+		filePath := fb.GetBaseConfigInfoFileBackingInfo().FilePath
+
+		if _, found := diskFilesInVMDir[filePath]; !found {
+			continue
+		}
+
+		logger.Info(
+			"Deleting FCD disk in VM directory via CNS",
+			"fcdID", id.Id,
+			"diskFile", filePath)
+
+		task, err := cnsClient.DeleteVolume(
+			ctx,
+			[]cnstypes.CnsVolumeId{{Id: id.Id}},
+			true)
+		if err != nil {
+			logger.Error(err,
+				"Failed to initiate CNS volume delete",
+				"fcdID", id.Id,
+				"diskFile", filePath)
+			continue
+		}
+		if _, err := task.WaitForResult(ctx); err != nil {
+			logger.Error(err,
+				"CNS volume delete task failed",
+				"fcdID", id.Id,
+				"diskFile", filePath)
+		}
+	}
+}
+
+// getVirtualDiskFileBackingInfo returns the VirtualDeviceFileBackingInfo for
+// a virtual disk if the disk uses file-based backing, otherwise nil.
+func getVirtualDiskFileBackingInfo(
+	disk *vimtypes.VirtualDisk) *vimtypes.VirtualDeviceFileBackingInfo {
+
+	type hasFileBackingInfo interface {
+		GetVirtualDeviceFileBackingInfo() *vimtypes.VirtualDeviceFileBackingInfo
+	}
+	if fb, ok := disk.Backing.(hasFileBackingInfo); ok {
+		return fb.GetVirtualDeviceFileBackingInfo()
+	}
+	return nil
+}