From 28da214aef98cac91dd56806b76cf750cb55f130 Mon Sep 17 00:00:00 2001 From: Satyanarayana Kolluri Date: Thu, 5 Jun 2025 17:13:50 +0530 Subject: [PATCH] Enhancement - Introduce `pvcName` input param in CNSUnregisterVolume spec Adds validation rules using x-kubernetes-validations to ensure that only one of the volumeID or pvcName is specified to avoid ambiguity. Updates the reconciler to use the informer cache to find out the PV and VolumeID when the PVC Name is supplied. Optimises the reconciler logic to ignore all such events that do not increment the generation. Optimises the usage calculation logic by failing fast when usages are detected. Updates the reconciler to add a finalizer to the CR before reconciling to have control over deletion process Updates the reconciler to process the delete events and remove the finalizer for graceful deletion. Updates the reconciler to protect the PVC using a finalizer to gracefully reconciler in case of retries Removes the code that retains the persistent volume as it's no longer required Updates the reconciler to remove the finalizer on the PVC once unregistration is successful for graceful deletion of the PVC. Adds/updates unit tests wherever necessary and applicable. --- .../v1alpha1/cnsunregistervolume_types.go | 5 +- .../config/cnsunregistervolume_crd.yaml | 118 ++- pkg/common/cns-lib/volume/manager.go | 5 + pkg/common/unittestcommon/utils.go | 2 +- pkg/csi/service/common/commonco/coagnostic.go | 5 +- .../k8sorchestrator/k8sorchestrator.go | 4 +- pkg/kubernetes/kubernetes.go | 131 ++- .../cnscsi_admissionhandler_test.go | 2 +- .../cnsnodevmbatchattachment_helper.go | 18 +- .../cnsregistervolume_controller_test.go | 2 +- .../cnsunregistervolume/controller.go | 373 +++++-- .../cnsunregistervolume/controller_test.go | 961 ++++++++++++------ .../controller/cnsunregistervolume/util.go | 76 +- pkg/syncer/cnsoperator/types/types.go | 8 + 14 files changed, 1190 insertions(+), 520 deletions(-) diff --git a/pkg/apis/cnsoperator/cnsunregistervolume/v1alpha1/cnsunregistervolume_types.go b/pkg/apis/cnsoperator/cnsunregistervolume/v1alpha1/cnsunregistervolume_types.go index fc6fdb7852..69aa1f34ca 100644 --- a/pkg/apis/cnsoperator/cnsunregistervolume/v1alpha1/cnsunregistervolume_types.go +++ b/pkg/apis/cnsoperator/cnsunregistervolume/v1alpha1/cnsunregistervolume_types.go @@ -24,7 +24,10 @@ import ( // +k8s:openapi-gen=true type CnsUnregisterVolumeSpec struct { // VolumeID indicates the volume handle of CNS volume to be unregistered - VolumeID string `json:"volumeID"` + VolumeID string `json:"volumeID,omitempty"` + + // PVCName indicates the name of the PVC to be unregistered. + PVCName string `json:"pvcName,omitempty"` // RetainFCD indicates if the volume should be retained as an FCD. // If set to false or not specified, the volume will be retained as a VMDK. diff --git a/pkg/apis/cnsoperator/config/cnsunregistervolume_crd.yaml b/pkg/apis/cnsoperator/config/cnsunregistervolume_crd.yaml index 3097fb42a6..8015f09fff 100644 --- a/pkg/apis/cnsoperator/config/cnsunregistervolume_crd.yaml +++ b/pkg/apis/cnsoperator/config/cnsunregistervolume_crd.yaml @@ -1,4 +1,3 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition @@ -16,66 +15,73 @@ spec: singular: cnsunregistervolume scope: Namespaced versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - description: CnsUnregisterVolume is the Schema for the cnsunregistervolumes API - properties: - apiVersion: - description: 'APIVersion defines the versioned schema of this representation + - name: v1alpha1 + schema: + openAPIV3Schema: + description: CnsUnregisterVolume is the Schema for the cnsunregistervolumes API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' - type: string - kind: - description: 'Kind is a string value representing the REST resource this + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - metadata: - type: object - spec: - description: CnsUnregisterVolumeSpec defines the desired state of CnsUnregisterVolume - properties: - volumeID: - description: VolumeID indicates the volume handle of CNS volume to be unregistered. - type: string - pattern: '^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$' - retainFCD: - description: RetainFCD indicates if the volume should be retained as an FCD. - If set to false or not specified, the volume will be retained as a VMDK. - type: boolean - forceUnregister: - description: ForceUnregister indicates if the volume should be forcefully unregistered. - If set to true, the volume will be unregistered even if it is still in use by any VM. - This should be used with caution as it may lead to data loss. - type: boolean - required: - - volumeID - type: object - status: - description: CnsUnregisterVolumeStatus defines the observed state of CnsUnregisterVolume - properties: - error: - description: The last error encountered during export operation, if - any. This field must only be set by the entity completing the export - operation, i.e. the CNS Operator. - type: string - unregistered: - description: Indicates the volume is successfully unregistered. - This field must only be set by the entity completing the unregister - operation, i.e. the CNS Operator. - type: boolean - required: - - unregistered - type: object - type: object - served: true - storage: true - subresources: - status: {} + type: string + metadata: + type: object + spec: + description: CnsUnregisterVolumeSpec defines the desired state of CnsUnregisterVolume + properties: + volumeID: + description: VolumeID indicates the volume handle of CNS volume to be unregistered. + type: string + pattern: '^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$' + pvcName: + description: PVCName indicates the name of the PVC to be unregistered. + type: string + pattern: '^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$' + retainFCD: + description: RetainFCD indicates if the volume should be retained as an FCD. + If set to false or not specified, the volume will be retained as a VMDK. + type: boolean + forceUnregister: + description: ForceUnregister indicates if the volume should be forcefully unregistered. + If set to true, the volume will be unregistered even if it is still in use by any VM. + This should be used with caution as it may lead to data loss. + type: boolean + type: object + x-kubernetes-validations: + - rule: "has(self.volumeID) || has(self.pvcName)" + message: "Either 'volumeID' or 'pvcName' must be specified, but not both" + - rule: "!(has(self.volumeID) && has(self.pvcName))" + message: "Cannot specify both 'volumeID' and 'pvcName' at the same time. Please specify only one of them" + status: + description: CnsUnregisterVolumeStatus defines the observed state of CnsUnregisterVolume + properties: + error: + description: The last error encountered during export operation, if + any. This field must only be set by the entity completing the export + operation, i.e. the CNS Operator. + type: string + unregistered: + description: Indicates the volume is successfully unregistered. + This field must only be set by the entity completing the unregister + operation, i.e. the CNS Operator. + type: boolean + required: + - unregistered + type: object + type: object + served: true + storage: true + subresources: + status: { } status: acceptedNames: kind: "" plural: "" - conditions: [] - storedVersions: [] \ No newline at end of file + conditions: [ ] + storedVersions: [ ] diff --git a/pkg/common/cns-lib/volume/manager.go b/pkg/common/cns-lib/volume/manager.go index ab6706cfcd..306ec7c06b 100644 --- a/pkg/common/cns-lib/volume/manager.go +++ b/pkg/common/cns-lib/volume/manager.go @@ -3766,6 +3766,11 @@ func (m *defaultManager) UnregisterVolume(ctx context.Context, volumeID string, func (m *defaultManager) unregisterVolume(ctx context.Context, volumeID string, unregisterDisk bool) error { log := logger.GetLogger(ctx) + if volumeID == "" { + log.Debugf("UnregisterVolume: volumeID is empty, nothing to unregister") + return nil + } + if m.virtualCenter == nil { return errors.New("invalid manager instance") } diff --git a/pkg/common/unittestcommon/utils.go b/pkg/common/unittestcommon/utils.go index 174c1dc945..a616a0fd01 100644 --- a/pkg/common/unittestcommon/utils.go +++ b/pkg/common/unittestcommon/utils.go @@ -431,7 +431,7 @@ func (c *FakeK8SOrchestrator) GetPVCNameFromCSIVolumeID(volumeID string) (string } // GetVolumeIDFromPVCName simlates an invalid case when pvcName contains "invalid". -func (c *FakeK8SOrchestrator) GetVolumeIDFromPVCName(pvcName string) (string, bool) { +func (c *FakeK8SOrchestrator) GetVolumeIDFromPVCName(namespace string, pvcName string) (string, bool) { if strings.Contains(pvcName, "invalid") { // Simulate a case where the volumeID is invalid and does not correspond to any PVC. return "", false diff --git a/pkg/csi/service/common/commonco/coagnostic.go b/pkg/csi/service/common/commonco/coagnostic.go index fd84a15c87..7faebc764b 100644 --- a/pkg/csi/service/common/commonco/coagnostic.go +++ b/pkg/csi/service/common/commonco/coagnostic.go @@ -98,9 +98,8 @@ type COCommonInterface interface { GetPVNameFromCSIVolumeID(volumeID string) (string, bool) // GetPVCNameFromCSIVolumeID returns `pvc name` and `pvc namespace` for the given volumeID using volumeIDToPvcMap. GetPVCNameFromCSIVolumeID(volumeID string) (string, string, bool) - // GetVolumeIDFromPVCName returns volumeID the given pvcName using pvcToVolumeIDMap. - // PVC name is its namespaced name. - GetVolumeIDFromPVCName(pvcName string) (string, bool) + // GetVolumeIDFromPVCName returns volumeID for the given pvc name and namespace. + GetVolumeIDFromPVCName(namespace string, pvcName string) (string, bool) // InitializeCSINodes creates CSINode instances for each K8s node with the appropriate topology keys. InitializeCSINodes(ctx context.Context) error // StartZonesInformer starts a dynamic informer which listens on Zones CR in diff --git a/pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go b/pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go index 04e706cc46..af7782caf6 100644 --- a/pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go +++ b/pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go @@ -2016,8 +2016,8 @@ func (c *K8sOrchestrator) GetPVCNameFromCSIVolumeID(volumeID string) ( // GetVolumeIDFromPVCName returns volumeID the given pvcName using pvcToVolumeIDMap. // PVC name is its namespaced name. -func (c *K8sOrchestrator) GetVolumeIDFromPVCName(pvcName string) (string, bool) { - return c.pvcToVolumeIDMap.get(pvcName) +func (c *K8sOrchestrator) GetVolumeIDFromPVCName(namespace string, pvcName string) (string, bool) { + return c.pvcToVolumeIDMap.get(fmt.Sprintf("%s/%s", namespace, pvcName)) } // IsLinkedCloneRequest checks if the pvc is a linked clone request diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index c07f9ac13a..f57d0205e6 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -713,85 +713,52 @@ func PatchFinalizers(ctx context.Context, c client.Client, obj client.Object, fi return c.Patch(ctx, obj, patch) } -// RetainPersistentVolume updates the PersistentVolume's ReclaimPolicy to Retain. -// This is useful to preserve the PersistentVolume even if the associated PersistentVolumeClaim is deleted. -func RetainPersistentVolume(ctx context.Context, k8sClient clientset.Interface, pvName string) error { - log := logger.GetLogger(ctx) - - if pvName == "" { - log.Debugf("PersistentVolume name is empty. Exiting...") - return nil - } - - log.Debugf("Retaining PersistentVolume %q", pvName) - pv, err := k8sClient.CoreV1().PersistentVolumes().Get(ctx, pvName, metav1.GetOptions{}) - if err != nil { - if apierrors.IsNotFound(err) { - log.Debugf("PersistentVolume %q not found. Exiting...", pvName) - return nil - } - - return logger.LogNewErrorf(log, "Failed to get PersistentVolume %q. Error: %s", pvName, err.Error()) - } - - pv.Spec.PersistentVolumeReclaimPolicy = v1.PersistentVolumeReclaimRetain - _, err = k8sClient.CoreV1().PersistentVolumes().Update(ctx, pv, metav1.UpdateOptions{}) - if err != nil { - return logger.LogNewErrorf(log, "Failed to update PersistentVolume %q to retain policy. Error: %s", - pvName, err.Error()) - } - - log.Debugf("Successfully retained PersistentVolume %q", pvName) - return nil -} - // DeletePersistentVolumeClaim deletes the PersistentVolumeClaim with the given name and namespace. func DeletePersistentVolumeClaim(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace string) error { - log := logger.GetLogger(ctx) + log := logger.GetLogger(ctx).With("name", fmt.Sprintf("%s/%s", pvcNamespace, pvcName)) if pvcName == "" { - log.Debugf("PVC name is empty. Exiting...") + log.Debug("PVC name is empty. Exiting...") return nil } - log.Debugf("Deleting PersistentVolumeClaim %q in namespace %q", pvcName, pvcNamespace) + log.Debug("Deleting PVC") err := k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Delete(ctx, pvcName, metav1.DeleteOptions{}) if err != nil { if apierrors.IsNotFound(err) { - log.Debugf("PersistentVolumeClaim %q in namespace %q not found. Exiting...", pvcName, pvcNamespace) + log.Debug("PVC not found. Exiting...") return nil } - return logger.LogNewErrorf(log, "Failed to delete PersistentVolumeClaim %q in namespace %q. Error: %s", - pvcName, pvcNamespace, err.Error()) + return logger.LogNewErrorf(log, "Failed to delete PVC. Error: %s", err.Error()) } - log.Debugf("Successfully deleted PersistentVolumeClaim %q in namespace %q", pvcName, pvcNamespace) + log.Debug("Successfully deleted PVC") return nil } // DeletePersistentVolume deletes the PersistentVolume with the given name. func DeletePersistentVolume(ctx context.Context, k8sClient clientset.Interface, pvName string) error { - log := logger.GetLogger(ctx) + log := logger.GetLogger(ctx).With("name", pvName) if pvName == "" { - log.Debugf("PersistentVolume name is empty. Exiting...") + log.Debug("PersistentVolume name is empty. Exiting...") return nil } - log.Debugf("Deleting PersistentVolume %q", pvName) + log.Debug("Deleting PV") err := k8sClient.CoreV1().PersistentVolumes().Delete(ctx, pvName, metav1.DeleteOptions{}) if err != nil { if apierrors.IsNotFound(err) { - log.Debugf("PersistentVolume %q not found. Exiting...", pvName) + log.Debug("PV not found. Exiting...") return nil } - return logger.LogNewErrorf(log, "Failed to delete PersistentVolume %q. Error: %s", pvName, err.Error()) + return logger.LogNewErrorf(log, "Failed to delete PV. Error: %s", err.Error()) } - log.Debugf("Successfully deleted PersistentVolume %q", pvName) + log.Debug("Successfully deleted PV") return nil } @@ -811,6 +778,80 @@ func UpdateStatus(ctx context.Context, c client.Client, obj client.Object) error return nil } +// AddFinalizerOnPVC adds the specified finalizer to the PersistentVolumeClaim (PVC) +// if it is not already present. +func AddFinalizerOnPVC(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, + finalizer string) error { + log := logger.GetLogger(ctx).With("name", fmt.Sprintf("%s/%s", pvcNamespace, pvcName)) + + if pvcName == "" { + log.Debug("PVC name is empty. Exiting...") + return nil + } + + pvc, err := k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Get(ctx, pvcName, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + log.Debug("PVC not found. Exiting...", pvcName, pvcNamespace) + return nil + } + + return logger.LogNewErrorf(log, "Failed to get PVC. Error: %s", err.Error()) + } + + // If the finalizer is already present, no action is needed + if !controllerutil.AddFinalizer(pvc, finalizer) { + log.Debugf("Finalizer %s already present on PVC. No action needed.", finalizer) + return nil + } + + // Update the PVC with the new finalizer + _, err = k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(ctx, pvc, metav1.UpdateOptions{}) + if err != nil { + return logger.LogNewErrorf(log, "Failed to add finalizer %s to PVC. Error: %s", finalizer, err.Error()) + } + + log.Debugf("Successfully added finalizer %s to PVC", finalizer) + return nil +} + +// RemoveFinalizerFromPVC removes the specified finalizer from the PersistentVolumeClaim (PVC) +// if it is present. +func RemoveFinalizerFromPVC(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, + finalizer string) error { + log := logger.GetLogger(ctx).With("name", fmt.Sprintf("%s/%s", pvcNamespace, pvcName)) + + if pvcName == "" { + log.Debug("PVC name is empty. Exiting...") + return nil + } + + pvc, err := k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Get(ctx, pvcName, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + log.Debug("PVC not found. Exiting...", pvcName, pvcNamespace) + return nil + } + + return logger.LogNewErrorf(log, "Failed to get PVC. Error: %s", err.Error()) + } + + // If the finalizer is not present, no action is needed + if !controllerutil.RemoveFinalizer(pvc, finalizer) { + log.Debugf("Finalizer %s not present on PVC. No action needed.", finalizer) + return nil + } + + // Update the PVC to remove the finalizer + _, err = k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(ctx, pvc, metav1.UpdateOptions{}) + if err != nil { + return logger.LogNewErrorf(log, "Failed to remove finalizer %s from PVC. Error: %s", finalizer, err.Error()) + } + + log.Debugf("Successfully removed finalizer %s from PVC", finalizer) + return nil +} + // AddFinalizer adds the specified finalizer to the given Kubernetes object if it is not already present. // It updates the object in the Kubernetes cluster to persist the change. func AddFinalizer(ctx context.Context, c client.Client, obj client.Object, finalizer string) error { diff --git a/pkg/syncer/admissionhandler/cnscsi_admissionhandler_test.go b/pkg/syncer/admissionhandler/cnscsi_admissionhandler_test.go index b056114293..f9049a97a7 100644 --- a/pkg/syncer/admissionhandler/cnscsi_admissionhandler_test.go +++ b/pkg/syncer/admissionhandler/cnscsi_admissionhandler_test.go @@ -188,7 +188,7 @@ func (m *MockCOCommonInterface) GetPVCNameFromCSIVolumeID(volumeID string) (stri return args.String(0), args.String(1), args.Bool(2) } -func (m *MockCOCommonInterface) GetVolumeIDFromPVCName(pvcName string) (string, bool) { +func (m *MockCOCommonInterface) GetVolumeIDFromPVCName(namespace, pvcName string) (string, bool) { args := m.Called(pvcName) return args.String(0), args.Bool(1) } diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go index d6695c9b59..c5f55f325a 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go @@ -33,7 +33,7 @@ import ( vimtypes "github.com/vmware/govmomi/vim25/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - v1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1" + "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1" volumes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume" cnsvsphere "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere" "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config" @@ -71,11 +71,6 @@ func removeFinalizerFromCRDInstance(ctx context.Context, return k8s.PatchFinalizers(ctx, c, instance, finalizersOnInstance) } -// getNamespacedPvcName take namespace and pvcName sends back namespace + "/" + pvcName. -func getNamespacedPvcName(namespace string, pvcName string) string { - return namespace + "/" + pvcName -} - // getVolumesToDetachFromInstance finds out which are the volumes to detach by finding out which are // the volumes present in attachedFCDs but not in spec of the instance. func getVolumesToDetachFromInstance(ctx context.Context, @@ -310,8 +305,8 @@ func getVolumeNameVolumeIdMapsInSpec(ctx context.Context, volumeIdsInSpec = make(map[string]string) volumeNamesInSpec = make(map[string]string) for _, volume := range instance.Spec.Volumes { - namespacedPvcName := getNamespacedPvcName(instance.Namespace, volume.PersistentVolumeClaim.ClaimName) - volumeId, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName(namespacedPvcName) + volumeId, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName( + instance.Namespace, volume.PersistentVolumeClaim.ClaimName) if !ok { msg := fmt.Sprintf("failed to find volumeID for PVC %s", volume.PersistentVolumeClaim.ClaimName) log.Errorf(msg) @@ -328,8 +323,8 @@ func getVolumeNameVolumeIdMapsInSpec(ctx context.Context, func getPvcsInSpec(instance *v1alpha1.CnsNodeVmBatchAttachment) (map[string]string, error) { pvcsInSpec := make(map[string]string) for _, volume := range instance.Spec.Volumes { - namespacedPvcName := getNamespacedPvcName(instance.Namespace, volume.PersistentVolumeClaim.ClaimName) - volumeId, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName(namespacedPvcName) + volumeId, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName( + instance.Namespace, volume.PersistentVolumeClaim.ClaimName) if !ok { return pvcsInSpec, fmt.Errorf("failed to find volumeID for PVC %s", volume.PersistentVolumeClaim.ClaimName) } @@ -398,8 +393,7 @@ func constructBatchAttachRequest(ctx context.Context, volumeName := volume.Name // Find volumeID for PVC. - namespacedPvcName := getNamespacedPvcName(instance.Namespace, pvcName) - attachVolumeId, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName(namespacedPvcName) + attachVolumeId, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName(instance.Namespace, pvcName) if !ok { err := fmt.Errorf("failed to find volumeID for PVC %s", pvcName) log.Error(err) diff --git a/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go b/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go index 519dd5142b..14e395bd51 100644 --- a/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go +++ b/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go @@ -397,7 +397,7 @@ func (m *mockCOCommon) GetPvcObjectByName(ctx context.Context, pvcName string, return nil, nil } -func (m *mockCOCommon) GetVolumeIDFromPVCName(pvcName string) (string, bool) { +func (m *mockCOCommon) GetVolumeIDFromPVCName(namespace string, pvcName string) (string, bool) { return "vol-1", true } diff --git a/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller.go b/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller.go index 3927978537..03e7fcfc56 100644 --- a/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller.go +++ b/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller.go @@ -18,6 +18,7 @@ package cnsunregistervolume import ( "context" + "errors" "fmt" "sync" "time" @@ -33,6 +34,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -45,7 +47,7 @@ import ( "sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger" k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes" "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer" - cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types" + cnsoptypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types" "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/util" ) @@ -107,8 +109,13 @@ func Add(mgr manager.Manager, clusterFlavor cnstypes.CnsClusterFlavor, // newReconciler returns a new reconcile.Reconciler. func newReconciler(mgr manager.Manager, configInfo *commonconfig.ConfigurationInfo, volumeManager volumes.Manager, recorder record.EventRecorder) reconcile.Reconciler { - return &Reconciler{client: mgr.GetClient(), scheme: mgr.GetScheme(), - configInfo: configInfo, volumeManager: volumeManager, recorder: recorder} + return &Reconciler{ + client: mgr.GetClient(), + scheme: mgr.GetScheme(), + configInfo: configInfo, + volumeManager: volumeManager, + recorder: recorder, + } } // add adds a new Controller to mgr with r as the reconcile.Reconciler. @@ -121,9 +128,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { err := ctrl.NewControllerManagedBy(mgr).Named("cnsunregistervolume-controller"). For(&v1a1.CnsUnregisterVolume{}). WithEventFilter(predicate.GenerationChangedPredicate{}). - WithOptions(controller.Options{ - MaxConcurrentReconciles: maxWorkerThreads}, - ). + WithOptions(controller.Options{MaxConcurrentReconciles: maxWorkerThreads}). Complete(r) if err != nil { log.Errorf("Failed to build application controller. Err: %v", err) @@ -138,7 +143,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { // reconcile.Reconciler. var _ reconcile.Reconciler = &Reconciler{} -// Reconciler reconciles a CnsUÌ„nregisterVolume object. +// Reconciler reconciles a CnsUnregisterVolume object. type Reconciler struct { // This client, initialized using mgr.Client() above, is a split client // that reads objects from the cache and writes to the apiserver. @@ -150,10 +155,11 @@ type Reconciler struct { } var ( - newK8sClient = k8s.NewClient - retainPV = k8s.RetainPersistentVolume - deletePVC = k8s.DeletePersistentVolumeClaim - deletePV = k8s.DeletePersistentVolume + newK8sClient = k8s.NewClient + protectPVC = k8s.AddFinalizerOnPVC + deletePVC = k8s.DeletePersistentVolumeClaim + deletePV = k8s.DeletePersistentVolume + removeFinalizerFromPVC = k8s.RemoveFinalizerFromPVC ) // Reconcile reads that state of the cluster for a Reconciler object @@ -166,124 +172,217 @@ var ( func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { ctx = logger.NewContextWithLogger(ctx) - log := logger.GetLogger(ctx) + log := logger.GetLogger(ctx).With("name", request.NamespacedName) // Fetch the CnsUnregisterVolume instance. instance := &v1a1.CnsUnregisterVolume{} err := r.client.Get(ctx, request.NamespacedName, instance) if err != nil { if apierrors.IsNotFound(err) { - log.Infof("CnsUnregisterVolume resource not found. Ignoring since object must be deleted.") + log.Info("instance not found. Ignoring since it must be deleted.") return reconcile.Result{}, nil } - log.Errorf("Error reading the CnsUnregisterVolume with name: %q on namespace: %q. Err: %+v", - request.Name, request.Namespace, err) - // Error reading the object - return with err. + log.Error("Error reading the instance. ", err) return reconcile.Result{}, err } - // Initialize backOffDuration for the instance, if required. - backOffDurationMapMutex.Lock() - var timeout time.Duration - if _, exists := backOffDuration[request.NamespacedName]; !exists { - backOffDuration[request.NamespacedName] = time.Second + log.Info("reconciling instance") + defer func() { + log.Info("finished reconciling instance") + }() + + backoff := getBackoffDuration(ctx, request.NamespacedName) + log.Info("backoff duration is ", backoff) + + // Handle deletion of the instance. + // The finalizer will be removed only if the volume is unregistered + // or if the input parameters are invalid or if the volume is in use. + if instance.DeletionTimestamp != nil { + log.Info("instance is marked for deletion") + err = r.reconcileDelete(ctx, *instance, request) + if err != nil { + log.Error("failed to reconcile with error ", err) + setInstanceError(ctx, r, instance, err.Error()) + return reconcile.Result{RequeueAfter: backoff}, nil + } + + log.Info("removing finalizer and allowing deletion of the instance") + err := removeFinalizer(ctx, r.client, instance) + if err != nil { + log.Error("failed to remove finalizer from the instance with error ", err) + setInstanceError(ctx, r, instance, "failed to remove finalizer from the instance") + return reconcile.Result{RequeueAfter: backoff}, nil + } + + deleteBackoffEntry(ctx, request.NamespacedName) + return reconcile.Result{}, nil } - timeout = backOffDuration[request.NamespacedName] - backOffDurationMapMutex.Unlock() // If the volume is already unregistered, remove the instance from the queue. if instance.Status.Unregistered { - backOffDurationMapMutex.Lock() - delete(backOffDuration, request.NamespacedName) - backOffDurationMapMutex.Unlock() + log.Debug("instance is already unregistered") + deleteBackoffEntry(ctx, request.NamespacedName) return reconcile.Result{}, nil } - log.Infof("Reconciling CnsUnregisterVolume instance %q from namespace %q. timeout %q seconds", - instance.Name, request.Namespace, timeout) - pvName, err := getPVName(ctx, instance.Spec.VolumeID) + err = r.reconcile(ctx, *instance, request) if err != nil { + log.Error("failed to reconcile with error ", err) setInstanceError(ctx, r, instance, err.Error()) - return reconcile.Result{RequeueAfter: timeout}, nil + return reconcile.Result{RequeueAfter: backoff}, nil } - pvcName, pvcNamespace, err := getPVCName(ctx, instance.Spec.VolumeID) + msg := "successfully unregistered the volume" + err = setInstanceSuccess(ctx, r, instance, msg) if err != nil { - setInstanceError(ctx, r, instance, err.Error()) - return reconcile.Result{RequeueAfter: timeout}, nil + log.Warn("failed to update status to success with error ", err) + setInstanceError(ctx, r, instance, "failed to update status to success") + return reconcile.Result{RequeueAfter: backoff}, nil } - k8sClient, err := newK8sClient(ctx) + deleteBackoffEntry(ctx, request.NamespacedName) + log.Info(msg) + return reconcile.Result{}, nil +} + +func (r *Reconciler) reconcile(ctx context.Context, + instance v1a1.CnsUnregisterVolume, request reconcile.Request) error { + log := logger.GetLogger(ctx).With("name", request.NamespacedName) + + params, err := getValidatedParams(ctx, instance) if err != nil { - log.Warn("Failed to init K8S client for volume unregistration") - setInstanceError(ctx, r, instance, "Failed to init K8S client for volume unregistration") - return reconcile.Result{RequeueAfter: timeout}, nil + log.Error("invalid input parameters ", err) + return err } - usageInfo, err := getVolumeUsageInfo(ctx, k8sClient, pvcName, pvcNamespace, - instance.Spec.ForceUnregister) + // Only protect the instance if input parameters are valid. + // This ensures faster deletion of instances with invalid parameters. + err = protectInstance(ctx, r.client, &instance) if err != nil { - log.Warn(err) - setInstanceError(ctx, r, instance, err.Error()) - return reconcile.Result{RequeueAfter: timeout}, nil + log.Error("failed to protect instance with error ", err) + return err + } + + usageInfo, err := getVolumeUsageInfo(ctx, params.pvcName, params.namespace, params.force) + if err != nil { + log.Error("failed to get volume usage info with error ", err) + return err } if usageInfo.isInUse { - msg := fmt.Sprintf("Volume %q cannot be unregistered because %s", instance.Spec.VolumeID, usageInfo) - log.Warn(msg) - setInstanceError(ctx, r, instance, msg) - return reconcile.Result{RequeueAfter: timeout}, nil + msg := fmt.Sprintf("volume %s cannot be unregistered because %s", params.volumeID, usageInfo) + log.Error(msg) + return errors.New(msg) } - err = retainPV(ctx, k8sClient, pvName) + err = unregisterVolume(ctx, r.volumeManager, request, *params) if err != nil { - log.Warn(err) - setInstanceError(ctx, r, instance, err.Error()) - return reconcile.Result{RequeueAfter: timeout}, nil + log.Error("failed to unregister volume with error ", err) + return err } - err = deletePVC(ctx, k8sClient, pvcName, pvcNamespace) + log.Info("successfully unregistered volume") + return nil +} + +// reconcileDelete handles deletion of a CnsUnregisterVolume instance. +// The reconciler tries to keep the system consistent by carefully deciding +// when to continue with volume unregistration. +// The finalizer on the instance will only be removed if the result will keep the system in a consistent state. +func (r *Reconciler) reconcileDelete(ctx context.Context, + instance v1a1.CnsUnregisterVolume, request reconcile.Request) error { + log := logger.GetLogger(ctx).With("name", request.NamespacedName) + + if instance.Status.Unregistered { + // If the volume is already unregistered, the instance can be deleted. + log.Info("volume is already unregistered") + return nil + } + + params, err := getValidatedParams(ctx, instance) if err != nil { - log.Warn(err) - setInstanceError(ctx, r, instance, err.Error()) - return reconcile.Result{RequeueAfter: timeout}, nil + // If input parameters are invalid, the instance can be deleted + // since the volume cannot be unregistered. + log.Info("invalid input parameters ", instance.Spec) + return nil } - err = deletePV(ctx, k8sClient, pvName) + usageInfo, err := getVolumeUsageInfo(ctx, params.pvcName, params.namespace, params.force) if err != nil { - log.Warn(err) - setInstanceError(ctx, r, instance, err.Error()) - return reconcile.Result{RequeueAfter: timeout}, nil + log.Error("failed to get volume usage info with error ", err) + return err } - unregDisk := false - if !instance.Spec.RetainFCD { - unregDisk = true + if usageInfo.isInUse { + // If the volume is in use, the instance can be deleted + // since the volume cannot be unregistered. + log.Info(usageInfo) + return nil } - err = r.volumeManager.UnregisterVolume(ctx, instance.Spec.VolumeID, unregDisk) + + // Try to unregister the volume. This ensures that the system remains + // consistent and the volume is not left in an unusable state. + // If unregistration fails, the instance will be re-queued for + // reconciliation. + err = unregisterVolume(ctx, r.volumeManager, request, *params) if err != nil { - msg := fmt.Sprintf("Failed to unregister volume %q", instance.Spec.VolumeID) - log.Warnf(msg+".Error: %s", err.Error()) - setInstanceError(ctx, r, instance, msg) - return reconcile.Result{RequeueAfter: timeout}, nil + log.Error("failed to unregister volume with error ", err) + return err } - log.Infof("Unregistered CNS volume %q", instance.Spec.VolumeID) - msg := "Successfully unregistered the volume" - err = setInstanceSuccess(ctx, r, instance, msg) + log.Info("successfully unregistered volume") + return nil +} + +var unregisterVolume = _unregisterVolume + +func _unregisterVolume(ctx context.Context, volMgr volumes.Manager, + request reconcile.Request, params params) error { + log := logger.GetLogger(ctx).With("name", request.NamespacedName) + + k8sClient, err := newK8sClient(ctx) if err != nil { - msg := fmt.Sprintf("Failed to update CnsUnregisterVolume instance with error: %s", err) - log.Warn(msg) - setInstanceError(ctx, r, instance, msg) - return reconcile.Result{RequeueAfter: timeout}, nil + log.Error("failed to init K8s client for volume unregistration with error ", err) + return errors.New("failed to init K8s client for volume unregistration") } - backOffDurationMapMutex.Lock() - delete(backOffDuration, request.NamespacedName) - backOffDurationMapMutex.Unlock() - log.Info(msg) - return reconcile.Result{}, nil + err = protectPVC(ctx, k8sClient, params.pvcName, params.namespace, + cnsoptypes.CNSUnregisterProtectionFinalizer) + if err != nil { + log.Error("failed to protect associated PVC with error ", err) + return fmt.Errorf("failed to protect associated PVC %s/%s", params.namespace, params.pvcName) + } + + err = deletePVC(ctx, k8sClient, params.pvcName, params.namespace) + if err != nil { + log.Error("failed to delete associated PVC with error ", err) + return fmt.Errorf("failed to delete associated PVC %s/%s", params.namespace, params.pvcName) + } + + err = deletePV(ctx, k8sClient, params.pvName) + if err != nil { + log.Error("failed to delete associated PV with error ", err) + return fmt.Errorf("failed to delete associated PV %s", params.pvName) + } + + unregDisk := !params.retainFCD // If retainFCD is false, unregister the FCD too. + err = volMgr.UnregisterVolume(ctx, params.volumeID, unregDisk) + if err != nil { + log.Error("failed to unregister associated volume with error ", err) + return fmt.Errorf("failed to unregister associated volume %s", params.volumeID) + } + + err = removeFinalizerFromPVC(ctx, k8sClient, params.pvcName, params.namespace, + cnsoptypes.CNSUnregisterProtectionFinalizer) + if err != nil { + log.Error("failed to remove finalizer from associated PVC with error ", err) + return fmt.Errorf("failed to remove finalizer from associated PVC %s/%s", + params.namespace, params.pvcName) + } + + log.Debug("successfully unregistered CNS volume ", params.volumeID) + return nil } // setInstanceError sets error and records an event on the CnsUnregisterVolume @@ -325,16 +424,120 @@ func recordEvent(ctx context.Context, r *Reconciler, switch eventtype { case v1.EventTypeWarning: // Double backOff duration. - backOffDurationMapMutex.Lock() - backOffDuration[namespacedName] = min(backOffDuration[namespacedName]*2, - cnsoperatortypes.MaxBackOffDurationForReconciler) + doubleBackoffDuration(ctx, namespacedName) r.recorder.Event(instance, v1.EventTypeWarning, "CnsUnregisterVolumeFailed", msg) - backOffDurationMapMutex.Unlock() case v1.EventTypeNormal: // Reset backOff duration to one second. - backOffDurationMapMutex.Lock() - backOffDuration[namespacedName] = time.Second + updateBackoffEntry(ctx, namespacedName, time.Second) r.recorder.Event(instance, v1.EventTypeNormal, "CnsUnregisterVolumeSucceeded", msg) - backOffDurationMapMutex.Unlock() } } + +type params struct { + retainFCD bool + force bool + namespace string + volumeID string + pvcName string + pvName string +} + +func (p *params) String() string { + return fmt.Sprintf("retainFCD: %t, force: %t, namespace: %s, volumeID: %s, pvcName: %s, pvName: %s", + p.retainFCD, p.force, p.namespace, p.volumeID, p.pvcName, p.pvName) +} + +var getValidatedParams = _getValidatedParams + +func _getValidatedParams(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + log := logger.GetLogger(ctx).With("name", instance.Namespace+"/"+instance.Name) + var err error + p := params{ + retainFCD: instance.Spec.RetainFCD, + force: instance.Spec.ForceUnregister, + namespace: instance.Namespace, + } + + if instance.Spec.VolumeID == "" && instance.Spec.PVCName == "" { + return nil, errors.New("either VolumeID or PVCName must be specified") + } + + if instance.Spec.VolumeID != "" && instance.Spec.PVCName != "" { + return nil, errors.New("both VolumeID and PVCName cannot be specified") + } + + if instance.Spec.VolumeID != "" { + p.volumeID = instance.Spec.VolumeID + + p.pvcName, _, err = getPVCName(ctx, instance.Spec.VolumeID) + if err != nil { + log.Info("no PVC found for the Volume ID ", instance.Spec.VolumeID) + } + } else { + p.pvcName = instance.Spec.PVCName + p.volumeID, err = getVolumeID(ctx, p.pvcName, p.namespace) + if err != nil { + log.Info("no Volume found for the PVC ", p.pvcName) + } + } + + p.pvName, err = getPVName(ctx, p.volumeID) + if err != nil { + log.Info("no PV found for the Volume ID ", p.volumeID) + } + + log.Debug("validated input parameters: ", p) + return &p, nil +} + +func getBackoffDuration(ctx context.Context, name types.NamespacedName) time.Duration { + backOffDurationMapMutex.Lock() + defer backOffDurationMapMutex.Unlock() + if _, exists := backOffDuration[name]; !exists { + backOffDuration[name] = time.Second + } + + return backOffDuration[name] +} + +func doubleBackoffDuration(ctx context.Context, name types.NamespacedName) { + d := getBackoffDuration(ctx, name) + d = min(d*2, cnsoptypes.MaxBackOffDurationForReconciler) + updateBackoffEntry(ctx, name, d) +} + +func updateBackoffEntry(ctx context.Context, name types.NamespacedName, duration time.Duration) { + backOffDurationMapMutex.Lock() + defer backOffDurationMapMutex.Unlock() + backOffDuration[name] = duration +} + +func deleteBackoffEntry(ctx context.Context, name types.NamespacedName) { + backOffDurationMapMutex.Lock() + defer backOffDurationMapMutex.Unlock() + delete(backOffDuration, name) +} + +func protectInstance(ctx context.Context, c client.Client, obj *v1a1.CnsUnregisterVolume) error { + log := logger.GetLogger(ctx).With("name", obj.Namespace+"/"+obj.Name) + + if !controllerutil.AddFinalizer(obj, cnsoptypes.CNSUnregisterVolumeFinalizer) { + log.Debugf("finalizer %s already exists on instance", cnsoptypes.CNSUnregisterVolumeFinalizer) + return nil + } + + log.Infof("adding finalizer %s to instance", cnsoptypes.CNSUnregisterVolumeFinalizer) + return c.Update(ctx, obj) +} + +func removeFinalizer(ctx context.Context, c client.Client, obj *v1a1.CnsUnregisterVolume) error { + log := logger.GetLogger(ctx).With("name", obj.Namespace+"/"+obj.Name) + + if controllerutil.RemoveFinalizer(obj, cnsoptypes.CNSUnregisterVolumeFinalizer) { + log.Infof("removing finalizer %s from instance", cnsoptypes.CNSUnregisterVolumeFinalizer) + return c.Update(ctx, obj) + } + + log.Debugf("finalizer %s does not exist on instance", cnsoptypes.CNSUnregisterVolumeFinalizer) + return nil +} diff --git a/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller_test.go b/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller_test.go index 3fd5b7e774..1b4190af90 100644 --- a/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller_test.go +++ b/pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller_test.go @@ -19,6 +19,7 @@ package cnsunregistervolume import ( "context" "errors" + "fmt" "testing" "time" @@ -29,8 +30,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes" - fakeclientset "k8s.io/client-go/kubernetes/fake" + clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -39,404 +39,777 @@ import ( apis "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator" v1a1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsunregistervolume/v1alpha1" "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume" - "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/unittestcommon" - "sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco" + cnsoptypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types" ) func TestReconciler_Reconcile(t *testing.T) { - newK8sClientOriginal := newK8sClient - getPVNameOriginal := getPVName - getPVCNameOriginal := getPVCName + // function overrides + getValidatedParamsOriginal := getValidatedParams getVolumeUsageInfoOriginal := getVolumeUsageInfo - retainPVOriginal := retainPV - deletePVCOriginal := deletePVC - deletePVOriginal := deletePV + unregisterVolumeOriginal := unregisterVolume + backOffDurationOriginal := backOffDuration defer func() { - newK8sClient = newK8sClientOriginal - getPVName = getPVNameOriginal - getPVCName = getPVCNameOriginal + getValidatedParams = getValidatedParamsOriginal getVolumeUsageInfo = getVolumeUsageInfoOriginal - retainPV = retainPVOriginal - deletePVC = deletePVCOriginal - deletePV = deletePVOriginal + unregisterVolume = unregisterVolumeOriginal + backOffDuration = backOffDurationOriginal }() + setup := func(t *testing.T, runtimeObjs []client.Object, interceptorFuncs interceptor.Funcs, + volumeManager volume.Manager) *Reconciler { + t.Helper() + c := newClient(t, runtimeObjs, interceptorFuncs) + backOffDuration = make(map[types.NamespacedName]time.Duration) + return &Reconciler{ + client: c, + recorder: record.NewFakeRecorder(10), + volumeManager: volumeManager, + } + } + assertUtil := func(t *testing.T, r *Reconciler, req reconcile.Request, res reconcile.Result, err error, + expRequeue bool, expRequeueAfter time.Duration, expBackoff bool, expErr, expEvent, expStatus string) { + t.Helper() + + if expErr == "" { + assert.Nil(t, err, "Expected no error") + } else { + assert.NotNil(t, err, "Expected an error") + } + + if expRequeue { + assert.Equal(t, expRequeueAfter, res.RequeueAfter, "Expected requeue after duration") + } else { + assert.True(t, res.IsZero(), "Expected no requeue") + } + + if expBackoff { + assert.Equal(t, expRequeueAfter*2, backOffDuration[req.NamespacedName], "Expected backoff duration to be doubled") + } else { + assert.NotContains(t, backOffDuration, req.NamespacedName, "Expected no backoff duration") + } + + if expEvent != "" { + if r.recorder == nil { + t.Fatal("Expected recorder to be initialized") + } + + event, ok := <-r.recorder.(*record.FakeRecorder).Events + if !ok { + t.Fatal("Expected an event but none found") + } + + assert.Contains(t, event, expEvent) + } + + if expStatus != "" { + var updatedInstance v1a1.CnsUnregisterVolume + err := r.client.Get(context.Background(), req.NamespacedName, &updatedInstance) + if err != nil { + t.Fatalf("Failed to get updated instance: %v", err) + } + + assert.Equal(t, expStatus, updatedInstance.Status.Error, "Expected status error to match") + } + } request := reconcile.Request{ NamespacedName: types.NamespacedName{ Name: "mock-instance", Namespace: "mock-namespace", }, } + t.Run("WhenGettingInstanceFails", func(tt *testing.T) { tt.Run("WhenNotFound", func(tt *testing.T) { - cb := fake.NewClientBuilder().WithInterceptorFuncs( - interceptor.Funcs{ - Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, - obj client.Object, opts ...client.GetOption) error { - return apierrors.NewNotFound( - schema.GroupResource{ - Group: "cns.vmware.com", - Resource: "cnsunregistervolume", - }, key.Name) - }, + // Setup + reconciler := setup(tt, nil, interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, + obj client.Object, opts ...client.GetOption) error { + return apierrors.NewNotFound( + schema.GroupResource{ + Group: "cns.vmware.com", + Resource: "cnsunregistervolume", + }, key.Name) }, - ) - reconciler := &Reconciler{ - client: cb.Build(), - } - ctx := context.Background() - res, err := reconciler.Reconcile(ctx, request) - assert.Nil(tt, err) - assert.True(tt, res.IsZero(), "Expected no requeue") + }, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") }) tt.Run("WhenOtherError", func(tt *testing.T) { - cb := fake.NewClientBuilder().WithInterceptorFuncs( - interceptor.Funcs{ - Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, - obj client.Object, opts ...client.GetOption) error { - return apierrors.NewInternalError(errors.New("other error")) - }, + // Setup + reconciler := setup(tt, nil, interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, + obj client.Object, opts ...client.GetOption) error { + return apierrors.NewInternalError(errors.New("other error")) + }, + }, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, time.Second, false, "other error", "", "") + }) + }) + + t.Run("WhenInstanceIsBeingDeleted", func(tt *testing.T) { + tt.Run("WhenVolumeIsAlreadyUnregistered", func(tt *testing.T) { + // Setup + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, true, true) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") + }) + tt.Run("WhenParamsAreInvalid", func(tt *testing.T) { + // Setup + errMsg := "invalid parameters" + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, true) + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return nil, errors.New(errMsg) + } + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") + }) + tt.Run("WhenGettingVolumeUsageInfoFails", func(tt *testing.T) { + // Setup + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return nil, errors.New("internal error") + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, true) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, + time.Second, true, "", "", "") + }) + tt.Run("WhenVolumeIsInUse", func(tt *testing.T) { + // Setup + usageInfo := &volumeUsageInfo{ + pods: []string{"pod"}, + isInUse: true, + } + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return usageInfo, nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, true) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") + }) + tt.Run("WhenUnregisteringVolumeFails", func(tt *testing.T) { + // Setup + errMsg := "internal server error" + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return &volumeUsageInfo{}, nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, true) + unregisterVolume = func(ctx context.Context, volMgr volume.Manager, request reconcile.Request, params params) error { + return errors.New(errMsg) + } + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, "", errMsg, errMsg) + }) + tt.Run("WhenRemovingFinalizerFails", func(tt *testing.T) { + // Setup + errMsg := "internal server error" + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return &volumeUsageInfo{}, nil + } + unregisterVolume = func(ctx context.Context, volMgr volume.Manager, request reconcile.Request, params params) error { + return nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, true) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{ + Update: func(ctx context.Context, client client.WithWatch, + obj client.Object, opts ...client.UpdateOption) error { + return apierrors.NewInternalError(errors.New(errMsg)) }, - ) - reconciler := &Reconciler{ - client: cb.Build(), - } - ctx := context.Background() - res, err := reconciler.Reconcile(ctx, request) - assert.NotNil(tt, err) - assert.True(tt, res.IsZero(), "Expected no requeue") + }, nil) + expStatus := "failed to remove finalizer from the instance" + expErrMsg := expStatus + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, + "", expErrMsg, expStatus) + }) + tt.Run("WhenSuccessful", func(tt *testing.T) { + // Setup + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return &volumeUsageInfo{}, nil + } + unregisterVolume = func(ctx context.Context, volMgr volume.Manager, request reconcile.Request, params params) error { + return nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, false, false, false, true) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") }) }) t.Run("WhenInstanceIsUnregistered", func(tt *testing.T) { // Setup - cb := fake.NewClientBuilder() - registerSchemes(tt, cb) - ctx := context.Background() - backOffDuration = make(map[types.NamespacedName]time.Duration) - instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", - false, false, true, "") - registerRuntimeObjects(tt, cb, instance) - reconciler := &Reconciler{ - client: cb.Build(), - } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + nil, false, false, true, false) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) // Execute - res, err := reconciler.Reconcile(ctx, request) + res, err := reconciler.Reconcile(context.Background(), request) // Assert - assert.Nil(tt, err, "Expected no error") - assert.True(tt, res.IsZero(), "Expected no requeue") - assert.NotContains(tt, backOffDuration, request, "Expected no backoff duration for unregistered instance") + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") }) - cb := fake.NewClientBuilder() - registerSchemes(t, cb) - ctx := context.Background() - instance := newInstance(t, "mock-instance", "mock-namespace", "mock-volume-id", - false, false, false, "") - registerRuntimeObjects(t, cb, instance) + t.Run("WhenNormalReconcile", func(tt *testing.T) { + tt.Run("WhenParamsAreInvalid", func(tt *testing.T) { + // Setup + errMsg := "invalid parameters" + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, false) + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return nil, errors.New(errMsg) + } + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) - t.Run("WhenGettingPVNameFails", func(tt *testing.T) { - // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), - } - getPVName = func(ctx context.Context, volumeID string) (string, error) { - return "", errors.New("failed to get PV name") - } + // Execute + res, err := reconciler.Reconcile(context.Background(), request) - // Execute - res, err := reconciler.Reconcile(ctx, request) + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, "", errMsg, errMsg) + }) + tt.Run("WhenProtectingInstanceFails", func(tt *testing.T) { + // Setup + errMsg := "failed to add finalizer" + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + nil, true, false, false, false) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{ + Update: func(ctx context.Context, client client.WithWatch, + obj client.Object, opts ...client.UpdateOption) error { + return errors.New(errMsg) + }, + }, nil) - // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], "Expected backoff duration to be 2 seconds") + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, "", errMsg, errMsg) + }) + tt.Run("WhenGettingVolumeUsageInfoFails", func(tt *testing.T) { + // Setup + errMsg := "failed to get volume usage info" + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, false) + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return nil, errors.New(errMsg) + } + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, "", errMsg, errMsg) + }) + tt.Run("WhenVolumeIsInUse", func(tt *testing.T) { + // Setup + usageInfo := &volumeUsageInfo{ + pods: []string{"pod"}, + isInUse: true, + } + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{volumeID: "mock-volume-id"}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return usageInfo, nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, false) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + expErr := fmt.Sprintf("volume %s cannot be unregistered because %s", instance.Spec.VolumeID, usageInfo) + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, "", expErr, expErr) + }) + tt.Run("WhenUnregisteringVolumeFails", func(tt *testing.T) { + // Setup + errMsg := "internal server error" + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return &volumeUsageInfo{}, nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, false) + unregisterVolume = func(ctx context.Context, volMgr volume.Manager, request reconcile.Request, params params) error { + return errors.New(errMsg) + } + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, "", errMsg, errMsg) + }) + tt.Run("WhenUpdatingStatusFails", func(tt *testing.T) { + // Setup + errMsg := "internal server error" + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return &volumeUsageInfo{}, nil + } + unregisterVolume = func(ctx context.Context, volMgr volume.Manager, request reconcile.Request, params params) error { + return nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, false) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{ + SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, + obj client.Object, opts ...client.SubResourceUpdateOption) error { + return apierrors.NewInternalError(errors.New(errMsg)) + }, + }, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, true, time.Second, true, + "", "", "") + }) + tt.Run("WhenSuccessful", func(tt *testing.T) { + // Setup + getValidatedParams = func(ctx context.Context, instance v1a1.CnsUnregisterVolume) (*params, error) { + return ¶ms{}, nil + } + getVolumeUsageInfo = func(ctx context.Context, + pvcName, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { + return &volumeUsageInfo{}, nil + } + unregisterVolume = func(ctx context.Context, volMgr volume.Manager, request reconcile.Request, params params) error { + return nil + } + instance := newInstance(tt, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + []string{cnsoptypes.CNSUnregisterVolumeFinalizer}, true, false, false, false) + reconciler := setup(tt, []client.Object{instance}, interceptor.Funcs{}, nil) + + // Execute + res, err := reconciler.Reconcile(context.Background(), request) + + // Assert + assertUtil(tt, reconciler, request, res, err, false, 0, false, "", "", "") + }) }) +} - getPVName = func(ctx context.Context, volumeID string) (string, error) { - return "mock-pv-name", nil - } +func TestUnregisterVolume(t *testing.T) { + // function overrides + newK8sClientOriginal := newK8sClient + protectPVCOriginal := protectPVC + deletePVCOriginal := deletePVC + deletePVOriginal := deletePV + removeFinalizerFromPVCOriginal := removeFinalizerFromPVC + defer func() { + newK8sClient = newK8sClientOriginal + protectPVC = protectPVCOriginal + deletePVC = deletePVCOriginal + deletePV = deletePVOriginal + removeFinalizerFromPVC = removeFinalizerFromPVCOriginal + }() - t.Run("WhenGettingPVCNameFails", func(tt *testing.T) { + t.Run("WhenGettingK8sClientFails", func(tt *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), - } - getPVCName = func(ctx context.Context, volumeID string) (string, string, error) { - return "", "", errors.New("failed to get PVC name") + errMsg := "internal server error" + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return nil, errors.New(errMsg) } + expErr := "failed to init K8s client for volume unregistration" // Execute - res, err := reconciler.Reconcile(ctx, request) + err := unregisterVolume(context.Background(), nil, reconcile.Request{}, params{}) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], "Expected backoff duration to be 2 seconds") - + assert.Equal(tt, expErr, err.Error(), "Expected error to match") }) - - getPVCName = func(ctx context.Context, volumeID string) (string, string, error) { - return "mock-pvc-name", "mock-pvc-namespace", nil - } - - t.Run("WhenCreatingK8sClientFails", func(tt *testing.T) { + t.Run("WhenProtectingPVCFails", func(tt *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - newK8sClient = func(ctx context.Context) (kubernetes.Interface, error) { - return nil, errors.New("failed to create k8s client") + errMsg := "internal server error" + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return &clientset.Clientset{}, nil } - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), + protectPVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, finalizer string) error { + return errors.New(errMsg) } + params := params{} + expErr := fmt.Sprintf("failed to protect associated PVC %s/%s", params.namespace, params.pvcName) // Execute - res, err := reconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: "mock-instance", - Namespace: "mock-namespace", - }, - }) + err := unregisterVolume(context.Background(), nil, reconcile.Request{}, params) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[types.NamespacedName{ - Name: "mock-instance", - Namespace: "mock-namespace", - }], "Expected backoff duration to be 2 seconds") + assert.Equal(tt, expErr, err.Error(), "Expected error to match") }) - - newK8sClient = func(ctx context.Context) (kubernetes.Interface, error) { - return fakeclientset.NewClientset(), nil - } - - t.Run("WhenGettingVolumeUsageInfoFails", func(tt *testing.T) { + t.Run("WhenDeletingPVCFails", func(tt *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - getVolumeUsageInfo = func(ctx context.Context, k8sClient kubernetes.Interface, pvcName string, - pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { - return nil, errors.New("failed to get volume usage info") + errMsg := "internal server error" + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return &clientset.Clientset{}, nil + } + protectPVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, finalizer string) error { + return nil } - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), + deletePVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace string) error { + return errors.New(errMsg) } + params := params{} + expErr := fmt.Sprintf("failed to delete associated PVC %s/%s", params.namespace, params.pvcName) // Execute - res, err := reconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: "mock-instance", - Namespace: "mock-namespace", - }, - }) + err := unregisterVolume(context.Background(), nil, reconcile.Request{}, params) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[types.NamespacedName{ - Name: "mock-instance", - Namespace: "mock-namespace", - }], "Expected backoff duration to be 2 seconds") + assert.Equal(tt, expErr, err.Error(), "Expected error to match") }) - - t.Run("WhenVolumeIsInUse", func(tt *testing.T) { + t.Run("WhenDeletingPVFails", func(tt *testing.T) { // Setup - getVolumeUsageInfo = func(ctx context.Context, k8sClient kubernetes.Interface, pvcName string, - pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { - return &volumeUsageInfo{ - isInUse: true, - pods: []string{"pod1", "pod2"}, - }, nil + errMsg := "internal server error" + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return &clientset.Clientset{}, nil } - newK8sClient = func(ctx context.Context) (kubernetes.Interface, error) { - return fakeclientset.NewClientset(), nil + protectPVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, finalizer string) error { + return nil } - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), + deletePVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace string) error { + return nil } - ctx := context.Background() - backOffDuration = make(map[types.NamespacedName]time.Duration) - commonco.ContainerOrchestratorUtility = &unittestcommon.FakeK8SOrchestrator{} + deletePV = func(ctx context.Context, k8sClient clientset.Interface, pvName string) error { + return errors.New(errMsg) + } + params := params{pvName: "mock-pv"} + expErr := fmt.Sprintf("failed to delete associated PV %s", params.pvName) // Execute - res, err := reconciler.Reconcile(ctx, request) + err := unregisterVolume(context.Background(), nil, reconcile.Request{}, params) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], - "Expected backoff duration to be 2 seconds") + assert.Equal(tt, expErr, err.Error(), "Expected error to match") }) - - getVolumeUsageInfo = func(ctx context.Context, k8sClient kubernetes.Interface, pvcName string, - pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { - return &volumeUsageInfo{ - isInUse: false, - }, nil - } - - t.Run("WhenRetainingPVFails", func(tt *testing.T) { + t.Run("WhenUnregisteringVolumeFromCNSFails", func(tt *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - retainPV = func(ctx context.Context, k8sClient kubernetes.Interface, pvName string) error { - return errors.New("failed to retain PV") + errMsg := "internal server error" + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return &clientset.Clientset{}, nil + } + protectPVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, finalizer string) error { + return nil } - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), + deletePVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace string) error { + return nil } + deletePV = func(ctx context.Context, k8sClient clientset.Interface, pvName string) error { + return nil + } + mockMgr := volume.NewMockManager(true, errors.New(errMsg)) + params := params{volumeID: "mock-volume-id"} + expErr := fmt.Sprintf("failed to unregister associated volume %s", params.volumeID) // Execute - res, err := reconciler.Reconcile(ctx, request) + err := unregisterVolume(context.Background(), mockMgr, reconcile.Request{}, params) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], - "Expected backoff duration to be 2 seconds") + assert.Equal(tt, expErr, err.Error(), "Expected error to match") }) - - retainPV = func(ctx context.Context, k8sClient kubernetes.Interface, pvName string) error { - return nil - } - - t.Run("WhenDeletingPVCFails", func(tt *testing.T) { + t.Run("WhenRemovingFinalizerFromPVCFails", func(tt *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - deletePVC = func(ctx context.Context, k8sClient kubernetes.Interface, pvcName string, pvcNamespace string) error { - return errors.New("failed to delete PVC") + errMsg := "internal server error" + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return &clientset.Clientset{}, nil + } + protectPVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, finalizer string) error { + return nil } - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), + deletePVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace string) error { + return nil } + deletePV = func(ctx context.Context, k8sClient clientset.Interface, pvName string) error { + return nil + } + removeFinalizerFromPVC = func(ctx context.Context, k8sClient clientset.Interface, + pvcName, pvcNamespace, finalizer string) error { + return errors.New(errMsg) + } + mockMgr := volume.NewMockManager(true, nil) + + params := params{pvcName: "mock-pvc", namespace: "mock-namespace"} + expErr := fmt.Sprintf("failed to remove finalizer from associated PVC %s/%s", params.namespace, params.pvcName) // Execute - res, err := reconciler.Reconcile(ctx, request) + err := unregisterVolume(context.Background(), mockMgr, reconcile.Request{}, params) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], - "Expected backoff duration to be 2 seconds") + assert.Equal(tt, expErr, err.Error(), "Expected error to match") }) - - deletePVC = func(ctx context.Context, k8sClient kubernetes.Interface, pvcName string, pvcNamespace string) error { - return nil - } - - t.Run("WhenDeletingPVFails", func(tt *testing.T) { + t.Run("WhenSuccessful", func(tt *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - deletePV = func(ctx context.Context, k8sClient kubernetes.Interface, pvName string) error { - return errors.New("failed to delete PV") + newK8sClient = func(ctx context.Context) (clientset.Interface, error) { + return &clientset.Clientset{}, nil + } + protectPVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace, finalizer string) error { + return nil + } + deletePVC = func(ctx context.Context, k8sClient clientset.Interface, pvcName, pvcNamespace string) error { + return nil } - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), + deletePV = func(ctx context.Context, k8sClient clientset.Interface, pvName string) error { + return nil } + removeFinalizerFromPVC = func(ctx context.Context, k8sClient clientset.Interface, + pvcName, pvcNamespace, finalizer string) error { + return nil + } + mockMgr := volume.NewMockManager(true, nil) // Execute - res, err := reconciler.Reconcile(ctx, request) + err := unregisterVolume(context.Background(), mockMgr, reconcile.Request{}, params{}) // Assert assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], - "Expected backoff duration to be 2 seconds") }) +} - deletePV = func(ctx context.Context, k8sClient kubernetes.Interface, pvName string) error { - return nil - } +func TestGetValidatedParams(t *testing.T) { + // function overrides + getPVCNameOriginal := getPVCName + getPVNameOriginal := getPVName + getVolumeIDOriginal := getVolumeID + defer func() { + getPVCName = getPVCNameOriginal + getPVName = getPVNameOriginal + getVolumeID = getVolumeIDOriginal + }() - t.Run("WhenUnregisteringVolumeFails", func(tt *testing.T) { + t.Run("WhenVolumeIDAndPVCNameAreEmpty", func(t *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - mockVolManager := volume.NewMockManager(true, errors.New("failed to unregister volume")) - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), - volumeManager: mockVolManager, - } + instance := newInstance(t, "mock-instance", "mock-namespace", "", "", "", + nil, false, false, false, false) // Execute - res, err := reconciler.Reconcile(ctx, request) + params, err := getValidatedParams(context.Background(), *instance) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], - "Expected backoff duration to be 2 seconds") + assert.Nil(t, params, "Expected params to be nil") + assert.NotNil(t, err, "Expected an error") }) - mockVolManager := volume.NewMockManager(false, nil) - - t.Run("WhenUpdatingStatusFails", func(tt *testing.T) { + t.Run("WhenVolumeIDAndPVCNameAreBothSet", func(t *testing.T) { // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - cb.WithInterceptorFuncs( - interceptor.Funcs{ - SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, - obj client.Object, opts ...client.SubResourceUpdateOption) error { - return apierrors.NewInternalError(errors.New("failed to update status")) - }, - }, - ) - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), - volumeManager: mockVolManager, - } + instance := newInstance(t, "mock-instance", "mock-namespace", "mock-volume-id", "", "mock-pvc-name", + nil, false, false, false, false) // Execute - res, err := reconciler.Reconcile(ctx, request) + params, err := getValidatedParams(context.Background(), *instance) // Assert - assert.Nil(tt, err, "Expected no error") - assert.Equal(tt, res.RequeueAfter, time.Second, "Expected requeue after 1 second") - assert.Equal(tt, 2*time.Second, backOffDuration[request.NamespacedName], - "Expected backoff duration to be 2 seconds") + assert.Nil(t, params, "Expected params to be nil") + assert.NotNil(t, err, "Expected an error") }) - cb.WithInterceptorFuncs( - interceptor.Funcs{ - SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, - obj client.Object, opts ...client.SubResourceUpdateOption) error { - return nil // Simulate successful update - }, - }, - ) + t.Run("WhenVolumeIDIsSet", func(t *testing.T) { + t.Run("WhenPVCNotFound", func(t *testing.T) { + // Setup + getPVCName = func(ctx context.Context, volumeID string) (string, string, error) { + return "", "", errors.New("PVC not found") + } + getPVName = func(ctx context.Context, volumeID string) (string, error) { + return "mock-pv", nil + } + instance := newInstance(t, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + nil, false, false, false, false) + expParams := params{ + retainFCD: false, + force: false, + namespace: "mock-namespace", + volumeID: "mock-volume-id", + pvcName: "", + pvName: "mock-pv", + } - t.Run("WhenReconcileSucceeds", func(tt *testing.T) { - // Setup - backOffDuration = make(map[types.NamespacedName]time.Duration) - reconciler := &Reconciler{ - client: cb.Build(), - recorder: record.NewFakeRecorder(10), - volumeManager: mockVolManager, - } + // Execute + params, err := getValidatedParams(context.Background(), *instance) - // Execute - res, err := reconciler.Reconcile(ctx, request) + // Assert + assert.Nil(t, err, "Expected no error") + assert.Equal(t, expParams, *params, "Expected params to match") + }) + t.Run("WhenPVNotFound", func(t *testing.T) { + // Setup + cb := fake.NewClientBuilder() + registerSchemes(t, cb) + getPVCName = func(ctx context.Context, volumeID string) (string, string, error) { + return "mock-pvc", "mock-namespace", nil + } + getPVName = func(ctx context.Context, volumeID string) (string, error) { + return "", errors.New("PV not found") + } + instance := newInstance(t, "mock-instance", "mock-namespace", "mock-volume-id", "", "", + nil, false, false, false, false) + expParams := params{ + retainFCD: false, + force: false, + namespace: "mock-namespace", + volumeID: "mock-volume-id", + pvcName: "mock-pvc", + pvName: "", + } - // Assert - assert.Nil(tt, err, "Expected no error") - assert.True(tt, res.IsZero(), "Expected no requeue") - assert.NotContains(tt, backOffDuration, request.NamespacedName, - "Expected no backoff duration for successful reconciliation") + // Execute + params, err := getValidatedParams(context.Background(), *instance) + + // Assert + assert.Nil(t, err, "Expected no error") + assert.Equal(t, expParams, *params, "Expected params to match") + }) + }) + + t.Run("WhenPVCNameIsSet", func(t *testing.T) { + t.Run("WhenVolumeIDNotFound", func(t *testing.T) { + // Setup + getVolumeID = func(ctx context.Context, pvcName, namespace string) (string, error) { + return "", errors.New("VolumeID not found") + } + getPVName = func(ctx context.Context, volumeID string) (string, error) { + return "mock-pv", nil + } + instance := newInstance(t, "mock-instance", "mock-namespace", "", "", "mock-pvc-name", + nil, false, false, false, false) + expParams := params{ + retainFCD: false, + force: false, + namespace: "mock-namespace", + volumeID: "", + pvcName: "mock-pvc-name", + pvName: "mock-pv", + } + + // Execute + params, err := getValidatedParams(context.Background(), *instance) + + // Assert + assert.Nil(t, err, "Expected no error") + assert.Equal(t, expParams, *params, "Expected params to match") + }) + t.Run("WhenPVNotFound", func(t *testing.T) { + // Setup + getVolumeID = func(ctx context.Context, pvcName, namespace string) (string, error) { + return "mock-volume-id", nil + } + getPVName = func(ctx context.Context, volumeID string) (string, error) { + return "", errors.New("PV not found") + } + instance := newInstance(t, "mock-instance", "mock-namespace", "", "", "mock-pvc-name", + nil, false, false, false, false) + expParams := params{ + retainFCD: false, + force: false, + namespace: "mock-namespace", + volumeID: "mock-volume-id", + pvcName: "mock-pvc-name", + pvName: "", + } + + // Execute + params, err := getValidatedParams(context.Background(), *instance) + + // Assert + assert.Nil(t, err, "Expected no error") + assert.Equal(t, expParams, *params, "Expected params to match") + }) }) } +func newClient(t *testing.T, runtimeObjs []client.Object, interceptorFuncs interceptor.Funcs) client.Client { + t.Helper() + cb := fake.NewClientBuilder() + registerSchemes(t, cb) + registerRuntimeObjects(t, cb, runtimeObjs...) + cb.WithInterceptorFuncs(interceptorFuncs) + return cb.Build() +} + func registerSchemes(t *testing.T, cb *fake.ClientBuilder) { t.Helper() scheme := runtime.NewScheme() @@ -458,22 +831,30 @@ func registerRuntimeObjects(t *testing.T, cb *fake.ClientBuilder, objs ...client cb.WithStatusSubresource(objs...) } -func newInstance(t *testing.T, name, namespace, volumeID string, - retainFCD, forceUnregister, unregistered bool, err string) *v1a1.CnsUnregisterVolume { +func newInstance(t *testing.T, name, namespace, volumeID, errMsg, pvcName string, finalizers []string, + retainFCD, forceUnregister, unregistered, withDeletionTS bool) *v1a1.CnsUnregisterVolume { t.Helper() - return &v1a1.CnsUnregisterVolume{ + instance := &v1a1.CnsUnregisterVolume{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, + Name: name, + Namespace: namespace, + Finalizers: finalizers, }, Spec: v1a1.CnsUnregisterVolumeSpec{ VolumeID: volumeID, + PVCName: pvcName, RetainFCD: retainFCD, ForceUnregister: forceUnregister, }, Status: v1a1.CnsUnregisterVolumeStatus{ Unregistered: unregistered, - Error: err, + Error: errMsg, }, } + + if withDeletionTS { + instance.DeletionTimestamp = &metav1.Time{Time: time.Now()} + } + + return instance } diff --git a/pkg/syncer/cnsoperator/controller/cnsunregistervolume/util.go b/pkg/syncer/cnsoperator/controller/cnsunregistervolume/util.go index 4bd0bc1207..572c975bbe 100644 --- a/pkg/syncer/cnsoperator/controller/cnsunregistervolume/util.go +++ b/pkg/syncer/cnsoperator/controller/cnsunregistervolume/util.go @@ -68,45 +68,57 @@ func (v volumeUsageInfo) String() string { var getVolumeUsageInfo = _getVolumeUsageInfo // getVolumeUsageInfo checks if the PVC is in use by any resources in the specified namespace. +// For the sake of efficiency, the function returns as soon as it finds that the volume is in use by any resource. // If ignoreVMUsage is set to true, the function skips checking if the volume is in use by any virtual machines. -func _getVolumeUsageInfo(ctx context.Context, k8sClient clientset.Interface, pvcName string, pvcNamespace string, +func _getVolumeUsageInfo(ctx context.Context, pvcName string, pvcNamespace string, ignoreVMUsage bool) (*volumeUsageInfo, error) { log := logger.GetLogger(ctx) var volumeUsageInfo volumeUsageInfo if pvcName == "" { - log.Debugf("PVC name is empty. Nothing to do.") + log.Debug("PVC name is empty. Nothing to do.") return &volumeUsageInfo, nil } - pods, isInUse, err := getPodsForPVC(ctx, pvcName, pvcNamespace, k8sClient) + var err error + k8sClient, err := k8s.NewClient(ctx) if err != nil { + log.Errorf("failed to create k8s client. Error: %v", err) return nil, err } - volumeUsageInfo.pods = pods - volumeUsageInfo.isInUse = volumeUsageInfo.isInUse || isInUse + volumeUsageInfo.pods, volumeUsageInfo.isInUse, err = getPodsForPVC(ctx, pvcName, pvcNamespace, k8sClient) + if err != nil { + return nil, err + } + + if volumeUsageInfo.isInUse { + return &volumeUsageInfo, nil + } cfg, err := k8s.GetKubeConfig(ctx) if err != nil { return nil, err } - guestClusters, isInUse, err := getGuestClustersForPVC(ctx, pvcName, pvcNamespace, *cfg) + volumeUsageInfo.guestClusters, volumeUsageInfo.isInUse, err = getGuestClustersForPVC( + ctx, pvcName, pvcNamespace, *cfg) if err != nil { return nil, err } - volumeUsageInfo.guestClusters = guestClusters - volumeUsageInfo.isInUse = volumeUsageInfo.isInUse || isInUse + if volumeUsageInfo.isInUse { + return &volumeUsageInfo, nil + } - snapshots, isInUse, err := getSnapshotsForPVC(ctx, pvcName, pvcNamespace, *cfg) + volumeUsageInfo.snapshots, volumeUsageInfo.isInUse, err = getSnapshotsForPVC(ctx, pvcName, pvcNamespace, *cfg) if err != nil { return nil, err } - volumeUsageInfo.snapshots = snapshots - volumeUsageInfo.isInUse = volumeUsageInfo.isInUse || isInUse + if volumeUsageInfo.isInUse { + return &volumeUsageInfo, nil + } if ignoreVMUsage { log.Debugf("Skipping check for virtual machines using PVC %q in namespace %q as ignoreVMUsage is set to true", @@ -114,13 +126,11 @@ func _getVolumeUsageInfo(ctx context.Context, k8sClient clientset.Interface, pvc return &volumeUsageInfo, nil } - vms, isInUse, err := getVMsForPVC(ctx, pvcName, pvcNamespace, *cfg) + volumeUsageInfo.virtualMachines, volumeUsageInfo.isInUse, err = getVMsForPVC(ctx, pvcName, pvcNamespace, *cfg) if err != nil { return nil, err } - volumeUsageInfo.virtualMachines = vms - volumeUsageInfo.isInUse = volumeUsageInfo.isInUse || isInUse return &volumeUsageInfo, nil } @@ -162,6 +172,25 @@ func _getPVCName(ctx context.Context, volumeID string) (string, string, error) { return pvc, ns, nil } +var getVolumeID = _getVolumeID + +func _getVolumeID(ctx context.Context, pvcName, namespace string) (string, error) { + log := logger.GetLogger(ctx) + if commonco.ContainerOrchestratorUtility == nil { + err := errors.New("ContainerOrchestratorUtility is not initialized") + log.Warn(err) + return "", err + } + + volID, ok := commonco.ContainerOrchestratorUtility.GetVolumeIDFromPVCName(namespace, pvcName) + if !ok { + log.Infof("no volumeID found for PVC %q", pvcName) + } else { + log.Infof("volumeID %q found for PVC %q", volID, pvcName) + } + return volID, nil +} + // getPodsForPVC returns a list of pods that are using the specified PVC. func getPodsForPVC(ctx context.Context, pvcName string, pvcNamespace string, k8sClient clientset.Interface) ([]string, bool, error) { @@ -169,8 +198,9 @@ func getPodsForPVC(ctx context.Context, pvcName string, pvcNamespace string, // TODO: check if we can use informer cache list, err := k8sClient.CoreV1().Pods(pvcNamespace).List(ctx, metav1.ListOptions{}) if err != nil { - return nil, false, logger.LogNewErrorf(log, "Failed to list pods in namespace %q for PVC %q. Error: %q", + log.Warnf("Failed to list pods in namespace %q for PVC %q. Error: %q", pvcNamespace, pvcName, err.Error()) + return nil, false, errors.New("failed to list pods") } var pods []string @@ -197,17 +227,17 @@ func getSnapshotsForPVC(ctx context.Context, pvcName string, pvcNamespace string log := logger.GetLogger(ctx) c, err := snapshotclient.NewForConfig(&cfg) if err != nil { - return nil, false, logger.LogNewErrorf(log, - "Failed to initialize snapshot client for PVC %q in namespace %q. Error: %q", + log.Warnf("Failed to create snapshot client for PVC %q in namespace %q. Error: %q", pvcName, pvcNamespace, err.Error()) + return nil, false, errors.New("failed to create snapshot client") } // TODO: check if we can use informer cache list, err := c.SnapshotV1().VolumeSnapshots(pvcNamespace).List(ctx, metav1.ListOptions{}) if err != nil { - return nil, false, logger.LogNewErrorf(log, - "Failed to list VolumeSnapshots in namespace %q for PVC %q. Error: %q", + log.Warnf("Failed to list VolumeSnapshots in namespace %q for PVC %q. Error: %q", pvcNamespace, pvcName, err.Error()) + return nil, false, errors.New("failed to list VolumeSnapshots") } var snapshots []string @@ -243,9 +273,9 @@ func getGuestClustersForPVC(ctx context.Context, pvcName, pvcNamespace string, return nil, false, nil } - return nil, false, logger.LogNewErrorf(log, - "Failed to get CnsVolumeMetadata %q in namespace %q. Error: %q", + log.Warnf("Failed to get CnsVolumeMetadata %q in namespace %q. Error: %q", pvcName, pvcNamespace, err.Error()) + return nil, false, errors.New("failed to get CnsVolumeMetadata") } var gcs []string @@ -264,13 +294,13 @@ func getVMsForPVC(ctx context.Context, pvcName string, pvcNamespace string, cfg rest.Config) ([]string, bool, error) { c, err := k8s.NewClientForGroup(ctx, &cfg, vmoperatortypes.GroupName) if err != nil { - return nil, false, err + return nil, false, errors.New("failed to create client for virtual machine group") } // TODO: check if we can use informer cache list, err := utils.ListVirtualMachines(ctx, c, pvcNamespace) if err != nil { - return nil, false, err + return nil, false, errors.New("failed to list virtual machines") } var vms []string diff --git a/pkg/syncer/cnsoperator/types/types.go b/pkg/syncer/cnsoperator/types/types.go index 85eb98998c..8f160822f7 100644 --- a/pkg/syncer/cnsoperator/types/types.go +++ b/pkg/syncer/cnsoperator/types/types.go @@ -36,6 +36,14 @@ const ( // This finalizer is added to avoid deletion of such VolumeSnapshots directly from Supervisor. CNSSnapshotFinalizer = "cns.vmware.com/volumesnapshot-protection" + // CNSUnregisterVolumeFinalizer is the finalizer added to CNSUnregisterVolume CRs + // to handle deletion gracefully. + CNSUnregisterVolumeFinalizer = "cns.vmware.com/unregister-volume" + + // CNSUnregisterProtectionFinalizer is the finalizer on Supervisor PVC + // to avoid deletion of PVCs which are in the process of being unregistered. + CNSUnregisterProtectionFinalizer = "cns.vmware.com/unregister-protection" + // VSphereCSIDriverName is the vsphere CSI driver name VSphereCSIDriverName = "csi.vsphere.vmware.com"