[VMOwnedVolumes][Draft]Core Workflows — Attach, Snapshot, Detach, Snapshot Deletion, Revert Re-adoption

Signed-off-by: Deepak Kinni <deepak.kinni@broadcom.com>

Author: deepakkinni

pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1/cnsnodebatchvmattachment_types.go

@@ -61,6 +61,20 @@ const (
 	// ReasonDetachBlocked reflects that a ReconfigVM remove was blocked, typically
 	// because a vSphere snapshot still retains the disk.
 	ReasonDetachBlocked = "DetachBlocked"
+
+	// ConditionAttachMethod is the condition type set by CSI on a per-volume basis
+	// to communicate to vm-operator which attach mechanism was used.
+	// "Reconfig" means CSI unregistered the FCD and vm-operator should use
+	// ReconfigVM_Task with a file-backed disk. "CnsAttach" means CSI used the
+	// legacy CNS attach path.
+	ConditionAttachMethod = "AttachMethod"
+	// ReasonReconfig is the condition reason for ConditionAttachMethod when CSI
+	// has unregistered the FCD and placed diskPath + diskUUID on the volume
+	// status for vm-operator to use in ReconfigVM_Task.
+	ReasonReconfig = "Reconfig"
+	// ReasonCnsAttach is the condition reason for ConditionAttachMethod when CSI
+	// followed the legacy BatchAttachVolumes path (brownfield or FSS disabled).
+	ReasonCnsAttach = "CnsAttach"
 )
 
 // SharingMode is the sharing mode of the virtual disk.
@@ -152,10 +166,17 @@ type PersistentVolumeClaimStatus struct {
 	CnsVolumeID string `json:"cnsVolumeId,omitempty"`
 	// +optional
 
-	// DiskUUID is the ID obtained when volume is attached to a VM.
+	// DiskUUID is the stable identifier for the virtual disk
+	// (VirtualDisk.Backing.Uuid). Populated by CSI on greenfield attach so
+	// that vm-operator can verify the disk matches after ReconfigVM_Task.
 	DiskUUID string `json:"diskUUID,omitempty"`
 	// +optional
 
+	// DiskPath is the datastore path for the VMDK (populated by CSI on
+	// greenfield attach for vm-operator to use in ReconfigVM_Task).
+	DiskPath string `json:"diskPath,omitempty"`
+	// +optional
+
 	// Conditions describes any conditions associated with this volume.
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
 }

pkg/apis/cnsoperator/csivolumeinfo/csivolumeinfoservice.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	csivolumeinfov1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/csivolumeinfo/v1alpha1"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
@@ -86,6 +87,16 @@ type CsiVolumeInfoService interface {
 	// error when no matching CR is found.
 	GetCsiVolumeInfoByPVCName(ctx context.Context, namespace, pvcName string) (
 		*csivolumeinfov1alpha1.CsiVolumeInfo, error)
+
+	// AddCVIProtectionFinalizer adds the cvi-protection finalizer to the
+	// CsiVolumeInfo for the given volumeID in namespace. It is idempotent:
+	// if the finalizer is already present, the call is a no-op.
+	AddCVIProtectionFinalizer(ctx context.Context, namespace, volumeID string) error
+
+	// RemoveCVIProtectionFinalizer removes the cvi-protection finalizer from the
+	// CsiVolumeInfo for the given volumeID in namespace. It is idempotent:
+	// if the finalizer is absent, the call is a no-op.
+	RemoveCVIProtectionFinalizer(ctx context.Context, namespace, volumeID string) error
 }
 
 // csiVolumeInfoSvc is the concrete singleton implementing CsiVolumeInfoService.
@@ -337,6 +348,72 @@ func (s *csiVolumeInfoSvc) CsiVolumeInfoExists(
 	return cvi != nil, nil
 }
 
+// AddCVIProtectionFinalizer adds the cvi-protection finalizer to the CsiVolumeInfo
+// identified by volumeID / namespace. The operation is idempotent.
+func (s *csiVolumeInfoSvc) AddCVIProtectionFinalizer(
+	ctx context.Context, namespace, volumeID string) error {
+	log := logger.GetLogger(ctx)
+	name := GetCsiVolumeInfoCRName(volumeID)
+
+	cvi, err := s.GetCsiVolumeInfo(ctx, namespace, volumeID)
+	if err != nil {
+		return fmt.Errorf("AddCVIProtectionFinalizer: failed to fetch CsiVolumeInfo %s/%s: %w",
+			namespace, name, err)
+	}
+	if cvi == nil {
+		return fmt.Errorf("AddCVIProtectionFinalizer: CsiVolumeInfo %s/%s not found",
+			namespace, name)
+	}
+	if controllerutil.ContainsFinalizer(cvi, csivolumeinfov1alpha1.CVIProtectionFinalizer) {
+		log.Infof("AddCVIProtectionFinalizer: finalizer already present on CsiVolumeInfo %s/%s",
+			namespace, name)
+		return nil
+	}
+
+	patch := client.MergeFrom(cvi.DeepCopy())
+	controllerutil.AddFinalizer(cvi, csivolumeinfov1alpha1.CVIProtectionFinalizer)
+	if err := s.k8sClient.Patch(ctx, cvi, patch); err != nil {
+		return fmt.Errorf("AddCVIProtectionFinalizer: failed to patch CsiVolumeInfo %s/%s: %w",
+			namespace, name, err)
+	}
+	log.Infof("AddCVIProtectionFinalizer: added finalizer on CsiVolumeInfo %s/%s", namespace, name)
+	return nil
+}
+
+// RemoveCVIProtectionFinalizer removes the cvi-protection finalizer from the
+// CsiVolumeInfo identified by volumeID / namespace. The operation is idempotent.
+func (s *csiVolumeInfoSvc) RemoveCVIProtectionFinalizer(
+	ctx context.Context, namespace, volumeID string) error {
+	log := logger.GetLogger(ctx)
+	name := GetCsiVolumeInfoCRName(volumeID)
+
+	cvi, err := s.GetCsiVolumeInfo(ctx, namespace, volumeID)
+	if err != nil {
+		return fmt.Errorf("RemoveCVIProtectionFinalizer: failed to fetch CsiVolumeInfo %s/%s: %w",
+			namespace, name, err)
+	}
+	if cvi == nil {
+		log.Infof("RemoveCVIProtectionFinalizer: CsiVolumeInfo %s/%s not found; no-op",
+			namespace, name)
+		return nil
+	}
+	if !controllerutil.ContainsFinalizer(cvi, csivolumeinfov1alpha1.CVIProtectionFinalizer) {
+		log.Infof("RemoveCVIProtectionFinalizer: finalizer absent on CsiVolumeInfo %s/%s; no-op",
+			namespace, name)
+		return nil
+	}
+
+	patch := client.MergeFrom(cvi.DeepCopy())
+	controllerutil.RemoveFinalizer(cvi, csivolumeinfov1alpha1.CVIProtectionFinalizer)
+	if err := s.k8sClient.Patch(ctx, cvi, patch); err != nil {
+		return fmt.Errorf("RemoveCVIProtectionFinalizer: failed to patch CsiVolumeInfo %s/%s: %w",
+			namespace, name, err)
+	}
+	log.Infof("RemoveCVIProtectionFinalizer: removed finalizer from CsiVolumeInfo %s/%s",
+		namespace, name)
+	return nil
+}
+
 // GetCsiVolumeInfoByPVCName lists all CsiVolumeInfo CRs in the given namespace
 // and returns the one whose spec.pvcName matches pvcName. Returns nil and no
 // error when no matching CR is found. If multiple CRs match (data integrity

pkg/syncer/admissionhandler/validatepvc.go

@@ -133,6 +133,7 @@ func validatePVC(ctx context.Context, req *admissionv1.AdmissionRequest) *admiss
 
 			// Block deletion of snapshot-retained PVCs tracked by CsiVolumeInfo.
 			if req.Operation == admissionv1.Delete &&
+				commonco.ContainerOrchestratorUtility != nil &&
 				commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.VMOwnedVolumes) {
 				if cviDenied, cviMsg := checkCVISnapshotRetained(ctx, oldPVC.Namespace, oldPVC.Name); cviDenied {
 					return &admissionv1.AdmissionResponse{

pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go

@@ -55,6 +55,7 @@ import (
 	cbtconfigv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cbtconfig/v1alpha1"
 	cnsoperatorapis "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1"
+	csivolumeinfo "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/csivolumeinfo"
 	volumes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
@@ -165,21 +166,38 @@ func Add(mgr manager.Manager, clusterFlavor cnstypes.CnsClusterFlavor,
 	}
 
 	recorder := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: cnsoperatorapis.GroupName})
+
+	// Initialise the CsiVolumeInfo service when the VM-owned volumes FSS is
+	// enabled. The service is a package-level singleton; errors are non-fatal
+	// here — the reconciler will fall back to the legacy attach path.
+	var cviSvc csivolumeinfo.CsiVolumeInfoService
+	if commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.VMOwnedVolumes) {
+		var cviErr error
+		cviSvc, cviErr = csivolumeinfo.InitCsiVolumeInfoService(ctx)
+		if cviErr != nil {
+			log.Warnf("CsiVolumeInfo service initialisation failed; VMOwnedVolumes attach path will be skipped. Err: %v",
+				cviErr)
+			cviSvc = nil
+		}
+	}
+
 	return add(mgr, newReconciler(mgr, configInfo, volumeManager, vmOperatorClient, cnsOperatorClient,
-		recorder, cbtClient))
+		recorder, cbtClient, cviSvc))
 }
 
 func newReconciler(mgr manager.Manager, configInfo *config.ConfigurationInfo,
 	volumeManager volumes.Manager, vmOperatorClient client.Client,
 	cnsOperatorClient client.Client,
-	recorder record.EventRecorder, cbtClient client.Client) reconcile.Reconciler {
+	recorder record.EventRecorder, cbtClient client.Client,
+	cviSvc csivolumeinfo.CsiVolumeInfoService) reconcile.Reconciler {
 	return &Reconciler{client: mgr.GetClient(),
 		scheme:     mgr.GetScheme(),
 		configInfo: *configInfo, volumeManager: volumeManager,
 		vmOperatorClient:  vmOperatorClient,
 		cnsOperatorClient: cnsOperatorClient,
 		recorder:          recorder,
 		cbtClient:         cbtClient,
+		cviService:        cviSvc,
 		instanceLock:      sync.Map{}}
 }
 
@@ -222,6 +240,10 @@ type Reconciler struct {
 	// cbtClient is set when isCSIBackupAPIEnabled (CSI_Backup_API FSS on); used for
 	// namespace-scoped CBTConfig lookups before batch attach.
 	cbtClient client.Client
+	// cviService provides CRUD operations for CsiVolumeInfo CRs.
+	// It is set when the VMOwnedVolumes FSS is enabled and is used by the
+	// ownership-transfer attach and detach paths.
+	cviService csivolumeinfo.CsiVolumeInfoService
 	// instanceLock to ensure that for an instance we have only
 	// one reconciliation at a time.
 	instanceLock sync.Map
@@ -470,10 +492,52 @@ func (r *Reconciler) detachVolumes(ctx context.Context,
 
 	volumesThatFailedToDetach := make([]string, 0)
 
+	// When the VMOwnedVolumes FSS is enabled, resolve the VM name once and
+	// check whether the VM uses the ownership-transfer detach path.
+	var vmName string
+	var isVMOwnedVolumesVM bool
+	if commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.VMOwnedVolumes) &&
+		r.cviService != nil {
+		var vmNameErr error
+		vmName, vmNameErr = getVMNameByInstanceUUID(ctx, r.vmOperatorClient,
+			instance.Spec.InstanceUUID, instance.Namespace)
+		if vmNameErr != nil {
+			log.Warnf("detachVolumes: failed to resolve VM name for instanceUUID %q; "+
+				"using legacy detach path for all volumes. Err: %v",
+				instance.Spec.InstanceUUID, vmNameErr)
+		} else if vmName != "" {
+			var checkErr error
+			isVMOwnedVolumesVM, checkErr = IsVMOwnedVolumesVM(ctx, r.vmOperatorClient,
+				instance.Namespace, vmName)
+			if checkErr != nil {
+				log.Warnf("detachVolumes: IsVMOwnedVolumesVM check failed for %s/%s; "+
+					"falling back to legacy detach. Err: %v",
+					instance.Namespace, vmName, checkErr)
+				isVMOwnedVolumesVM = false
+			}
+		}
+	}
+
 	for pvc, volumeId := range volumesToDetach {
 		log.Infof("Detach call started for PVC %s with volumeID %s in namespace %s for instance %s",
 			pvc, volumeId, instance.Namespace, instance.Name)
 
+		// For VMs with the VMOwnedVolumes annotation all volumes use the
+		// ownership-transfer detach path — the legacy CNS DetachVolume is
+		// never called for these VMs.
+		if isVMOwnedVolumesVM {
+			if detachErr := reconcileVMOwnedVolumesDetach(ctx,
+				r.cviService, r.volumeManager, r.client,
+				vm, instance, pvc, volumeId, &r.configInfo); detachErr != nil {
+				log.Errorf("detachVolumes: VMOwnedVolumes detach failed for PVC %s: %v",
+					pvc, detachErr)
+				updateInstanceVolumeStatus(ctx, instance, "", pvc, "", "", detachErr,
+					v1alpha1.ConditionDetached, v1alpha1.ReasonDetachFailed)
+				volumesThatFailedToDetach = append(volumesThatFailedToDetach, pvc)
+			}
+			continue
+		}
+
 		// Call CNS DetachVolume
 		faulttype, detachErr := r.volumeManager.DetachVolume(ctx, vm, volumeId)
 		if detachErr != nil {
@@ -496,7 +560,7 @@ func (r *Reconciler) detachVolumes(ctx context.Context,
 			// Remove finalizer from the PVC as the detach was successful.
 			volumesThatFailedToDetach = removeFinalizerAndStatusEntry(ctx, r.client, k8sClient, r.cnsOperatorClient,
 				instance, pvc, volumesThatFailedToDetach)
-			// Attempt lazy CVI creation for brownfield volumes when preconditions are met.
+			// Attempt lazy CVI creation for volumes provisioned before the VMOwnedVolumes FSS was enabled.
 			go MaybeLazilyCreateCVI(ctx, vm, r.volumeManager,
 				instance.Namespace,
 				volumeId, pvc, "", "",
@@ -573,12 +637,62 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, k8sClient kubernete
 			instance.Namespace)
 	}
 
+	// When the VMOwnedVolumes FSS is enabled, check once whether this VM
+	// carries the annotation. If it does, all volumes use the
+	// ownership-transfer attach path and BatchAttachVolumes is not called.
+	if commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.VMOwnedVolumes) &&
+		r.cviService != nil {
+		vmName, vmNameErr := getVMNameByInstanceUUID(ctx, r.vmOperatorClient,
+			instance.Spec.InstanceUUID, instance.Namespace)
+		if vmNameErr != nil {
+			log.Warnf("processBatchAttach: failed to resolve VM name for instanceUUID %q; "+
+				"falling back to legacy attach path. Err: %v",
+				instance.Spec.InstanceUUID, vmNameErr)
+		} else if vmName != "" {
+			isVMOwned, checkErr := IsVMOwnedVolumesVM(ctx, r.vmOperatorClient,
+				instance.Namespace, vmName)
+			if checkErr != nil {
+				log.Warnf("processBatchAttach: IsVMOwnedVolumesVM check failed for %s/%s; "+
+					"falling back to legacy attach path. Err: %v",
+					instance.Namespace, vmName, checkErr)
+			} else if isVMOwned {
+				log.Infof("processBatchAttach: VM %s/%s uses ownership-transfer attach path",
+					instance.Namespace, vmName)
+				for pvcName, volumeID := range volumesToAttach {
+					// Derive volumeName for status updates from the spec.
+					var volumeName string
+					for _, v := range instance.Spec.Volumes {
+						if v.PersistentVolumeClaim.ClaimName == pvcName {
+							volumeName = v.Name
+							break
+						}
+					}
+					if attachErr := processVMOwnedVolumesAttach(ctx,
+						r.cviService, r.vmOperatorClient, r.volumeManager,
+						instance, vmName, instance.Spec.InstanceUUID,
+						pvcName, volumeID, volumeName); attachErr != nil {
+						log.Errorf("processBatchAttach: VMOwnedVolumes attach failed for PVC %s: %v",
+							pvcName, attachErr)
+						err = errors.Join(err, attachErr)
+					}
+				}
+				// Persist BA status changes and return — this path never
+				// falls through to BatchAttachVolumes.
+				if patchErr := patchBAStatus(ctx, r.client, instance); patchErr != nil {
+					log.Errorf("processBatchAttach: failed to patch BA status: %v", patchErr)
+					err = errors.Join(err, patchErr)
+				}
+				return err
+			}
+		}
+	}
+
 	// Construct batch attach request
-	pvcsInAttachList, volumeIdsInAttachList, batchAttachRequest, err := constructBatchAttachRequest(ctx,
+	pvcsInAttachList, volumeIdsInAttachList, batchAttachRequest, batchErr := constructBatchAttachRequest(ctx,
 		volumesToAttach, instance)
-	if err != nil {
-		log.Errorf("failed to construct batch attach request. Err: %s", err)
-		return err
+	if batchErr != nil {
+		log.Errorf("failed to construct batch attach request. Err: %s", batchErr)
+		return errors.Join(err, batchErr)
 	}
 
 	// Manage CBT for each volume before batch attach.
@@ -619,10 +733,11 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, k8sClient kubernete
 		if result.Error == nil {
 			reason = ""
 			// Add finalizer on PVC as attach was successful.
-			err = addPvcFinalizer(ctx, r.client, k8sClient, pvcName, instance.Namespace, instance.Spec.InstanceUUID)
-			if err != nil {
+			finalizerErr := addPvcFinalizer(ctx, r.client, k8sClient, pvcName,
+				instance.Namespace, instance.Spec.InstanceUUID)
+			if finalizerErr != nil {
 				log.Errorf("failed to add finalizer %s on PVC %s", cnsoperatortypes.CNSPvcFinalizer, pvcName)
-				result.Error = err
+				result.Error = finalizerErr
 				attachErr = errors.Join(attachErr,
 					fmt.Errorf("failure during attach of PVC %s", pvcName))
 			}
@@ -632,7 +747,7 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, k8sClient kubernete
 			v1alpha1.ConditionAttached, reason)
 
 	}
-	return attachErr
+	return errors.Join(err, attachErr)
 }
 
 // recordEvent records the event, sets the backOffDuration for the instance