Initial impl of delete workflow

Author: kolluria

pkg/syncer/cnsoperator/controller/cnsunregistervolume/controller.go

@@ -195,9 +195,15 @@ func (r *Reconciler) Reconcile(ctx context.Context,
 	}()
 
 	if instance.DeletionTimestamp != nil {
-		// TODO: Need to handle deletion if the instance is not fully processed.
-		log.Info("instance is being deleted")
-		err = removeFinalizer(ctx, r.client, instance)
+		log.Info("instance is marked for deletion")
+		err = r.reconcileDelete(ctx, *instance, request)
+		if err != nil {
+			log.Error("failed to reconcile with error ", err)
+			return reconcile.Result{}, err
+		}
+
+		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)
 			return reconcile.Result{}, err
@@ -220,81 +226,150 @@ func (r *Reconciler) Reconcile(ctx context.Context,
 		return reconcile.Result{}, err
 	}
 
-	// Initialize backOffDuration for the instance, if required.
-	duration := getBackoffDuration(ctx, request.NamespacedName)
-	params, err := getValidatedParams(ctx, *instance)
+	err = r.reconcile(ctx, instance, request)
 	if err != nil {
-		log.Error("failed to get input parameters with error ", err)
+		log.Error("failed to reconcile with error ", err)
 		setInstanceError(ctx, r, instance, err.Error())
-		return reconcile.Result{RequeueAfter: duration}, nil
+		return reconcile.Result{RequeueAfter: getBackoffDuration(ctx, request.NamespacedName)}, nil
 	}
 
-	k8sClient, err := newK8sClient(ctx)
+	msg := "successfully unregistered the volume"
+	err = setInstanceSuccess(ctx, r, instance, msg)
 	if err != nil {
-		log.Error("failed to init K8s client for volume unregistration with error ", err)
-		setInstanceError(ctx, r, instance, "failed to init K8s client for volume unregistration")
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Warn("failed to set instance to success with error ", err)
+		setInstanceError(ctx, r, instance, "failed to set instance to success")
+		return reconcile.Result{RequeueAfter: getBackoffDuration(ctx, request.NamespacedName)}, nil
+	}
+
+	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.Error("invalid input parameters", err)
+		return err
 	}
 
-	usageInfo, err := getVolumeUsageInfo(ctx, k8sClient, params.pvcName, params.namespace,
+	usageInfo, err := getVolumeUsageInfo(ctx, params.pvcName, params.namespace,
 		instance.Spec.ForceUnregister)
 	if err != nil {
-		setInstanceError(ctx, r, instance, err.Error())
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Error("failed to get volume usage info with error ", err)
+		return err
 	}
 
 	if usageInfo.isInUse {
 		msg := fmt.Sprintf("volume %s cannot be unregistered because %s", params.volumeID, usageInfo)
 		log.Error(msg)
-		setInstanceError(ctx, r, instance, msg)
-		return reconcile.Result{RequeueAfter: duration}, nil
+		return errors.New(msg)
+	}
+
+	err = unregisterVolume(ctx, *r, request, *params)
+	if err != nil {
+		log.Error("failed to unregister volume with error ", err)
+		return err
+	}
+
+	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 {
+		// If input parameters are invalid, the instance can be deleted
+		// since the volume cannot be unregistered.
+		log.Info("invalid input parameters")
+		return nil
+	}
+
+	usageInfo, err := getVolumeUsageInfo(ctx, params.pvcName, params.namespace,
+		instance.Spec.ForceUnregister)
+	if err != nil {
+		log.Error("failed to get volume usage info with error ", err)
+		return err
+	}
+
+	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 = unregisterVolume(ctx, *r, request, *params)
+	if err != nil {
+		log.Error("failed to unregister volume with error ", err)
+		return err
+	}
+
+	log.Info("successfully unregistered volume")
+	return nil
+}
+
+func unregisterVolume(ctx context.Context, r Reconciler,
+	request reconcile.Request, params params) error {
+	log := logger.GetLogger(ctx).With("name", request.NamespacedName)
+
+	k8sClient, err := newK8sClient(ctx)
+	if err != nil {
+		log.Error("failed to init K8s client for volume unregistration with error ", err)
+		return err
 	}
 
 	err = protectPVC(ctx, k8sClient, params.pvcName, params.namespace,
 		cnsoptypes.CNSUnregisterProtectionFinalizer)
 	if err != nil {
-		setInstanceError(ctx, r, instance, "failed to add finalizer on PVC")
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Error("failed to add finalizer on PVC with error ", err)
+		return errors.New("failed to protect PVC")
 	}
 
 	err = deletePVC(ctx, k8sClient, params.pvcName, params.namespace)
 	if err != nil {
-		setInstanceError(ctx, r, instance, "failed to delete PVC")
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Error("failed to delete PVC with error ", err)
+		return errors.New("failed to delete PVC")
 	}
 
 	err = deletePV(ctx, k8sClient, params.pvName)
 	if err != nil {
-		setInstanceError(ctx, r, instance, "failed to delete PV")
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Error("failed to delete PV with error ", err)
+		return errors.New("failed to delete PV")
 	}
 
-	unregDisk := !instance.Spec.RetainFCD
+	unregDisk := !params.retainFCD // If retainFCD is false, unregister the FCD too.
 	err = r.volumeManager.UnregisterVolume(ctx, params.volumeID, unregDisk)
 	if err != nil {
-		setInstanceError(ctx, r, instance, "failed to unregister volume")
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Error("failed to unregister volume with error ", err)
+		return errors.New("failed to unregister volume")
 	}
 
 	err = removeFinalizerFromPVC(ctx, k8sClient, params.pvcName, params.namespace,
 		cnsoptypes.CNSUnregisterProtectionFinalizer)
 	if err != nil {
-		setInstanceError(ctx, r, instance, "failed to remove finalizer from PVC")
-		return reconcile.Result{RequeueAfter: duration}, nil
+		log.Error("failed to remove finalizer from PVC with error ", err)
+		return errors.New("failed to remove finalizer from PVC")
 	}
 
 	log.Infof("successfully unregistered CNS volume %s", params.volumeID)
-	msg := "successfully unregistered the volume"
-	err = setInstanceSuccess(ctx, r, instance, msg)
-	if err != nil {
-		log.Warn("failed to set instance to success with error ", err)
-		setInstanceError(ctx, r, instance, "failed to set instance to success")
-		return reconcile.Result{RequeueAfter: duration}, nil
-	}
-
-	deleteBackoffEntry(ctx, request.NamespacedName)
-	log.Info(msg)
-	return reconcile.Result{}, nil
+	return nil
 }
 
 // setInstanceError sets error and records an event on the CnsUnregisterVolume
@@ -359,6 +434,11 @@ type params struct {
 	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) {
@@ -398,6 +478,7 @@ func _getValidatedParams(ctx context.Context, instance v1a1.CnsUnregisterVolume)
 		log.Info("no PV found for the Volume ID ", p.volumeID)
 	}
 
+	log.Debug("validated input parameters: ", p)
 	return &p, nil
 }
 

pkg/syncer/cnsoperator/controller/cnsunregistervolume/util.go

@@ -70,17 +70,23 @@ 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
 	}
 
 	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, volumeUsageInfo.isInUse, err = getPodsForPVC(ctx, pvcName, pvcNamespace, k8sClient)
 	if err != nil {
 		return nil, err

pkg/syncer/cnsoperator/types/types.go

@@ -41,7 +41,7 @@ const (
 	CNSUnregisterVolumeFinalizer = "cns.vmware.com/unregister-volume"
 
 	// CNSUnregisterProtectionFinalizer is the finalizer on Supervisor PVC
-	// to avoid deletion of PVCs which are in the process of UnregisterVolume operation.
+	// 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