fix(CStorVolumeAttachment): Remove finalizer on unsuccesful mounts an…

…d on driver restarts (#82) (#85)

Signed-off-by: Payes <[email protected]>

pkg/driver/node.go

@@ -126,12 +126,13 @@ func (ns *node) NodeStageVolume(
 	}
 	if isMountRequired {
 		vol.Spec.Volume.StagingTargetPath = stagingTargetPath
+		// This is placed to clean up stale iSCSI Sessions
 		vol.Finalizers = []string{utils.NodeIDENV}
 		vol.Spec.Volume.DevicePath = strings.Join([]string{
 			"/dev/disk/by-path/ip", vol.Spec.ISCSI.TargetPortal,
 			"iscsi", vol.Spec.ISCSI.Iqn, "lun", fmt.Sprint(0)}, "-",
 		)
-		err = utils.UpdateCStorVolumeAttachmentCR(vol)
+		vol, err = utils.UpdateCStorVolumeAttachmentCR(vol)
 		if err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
@@ -147,6 +148,12 @@ func (ns *node) NodeStageVolume(
 		// Login to the volume and attempt mount operation on the requested path
 		devicePath, err := ns.attachDisk(vol)
 		if err != nil {
+			vol.Finalizers = nil
+			// There might still be a case that the attach was successful,
+			// therefore not cleaning up the staging path from CR
+			if _, err1 := utils.UpdateCStorVolumeAttachmentCR(vol); err1 != nil {
+				return nil, status.Error(codes.Internal, err1.Error())
+			}
 			logrus.Errorf("NodeStageVolume: failed to attachDisk for volume %v, err: %v", volumeID, err)
 			return nil, status.Error(codes.Internal, err.Error())
 		}
@@ -163,6 +170,12 @@ func (ns *node) NodeStageVolume(
 
 		logrus.Info("NodeStageVolume: start format and mount operation")
 		if err := ns.formatAndMount(req, devicePath); err != nil {
+			vol.Finalizers = nil
+			// There might still be a case that the attach was successful,
+			// therefore not cleaning up the staging path from CR
+			if _, uerr := utils.UpdateCStorVolumeAttachmentCR(vol); uerr != nil {
+				logrus.Errorf("Failed to update CStorVolumeAttachment:%v", uerr.Error())
+			}
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 
@@ -236,7 +249,7 @@ func (ns *node) NodeUnstageVolume(
 
 	vol.Finalizers = nil
 	vol.Spec.Volume.StagingTargetPath = ""
-	if err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil {
+	if _, err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 	logrus.Infof("hostpath: volume %s path: %s has been unmounted.",
@@ -270,7 +283,7 @@ func (ns *node) NodePublishVolume(
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 	vol.Spec.Volume.TargetPath = req.GetTargetPath()
-	if err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil {
+	if _, err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
@@ -316,7 +329,7 @@ func (ns *node) NodeUnpublishVolume(
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 	vol.Spec.Volume.TargetPath = ""
-	if err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil {
+	if _, err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 

pkg/driver/service.go

@@ -87,6 +87,7 @@ func New(config *config.Config) *CSIDriver {
 		driver.cs = NewController(driver)
 
 	case "node":
+		utils.CleanupOnRestart()
 		// Start monitor goroutine to monitor the
 		// mounted paths. If a path goes down or
 		// becomes read only (in case of RW mount

pkg/utils/kubernetes.go

@@ -134,11 +134,10 @@ func CreateCStorVolumeAttachmentCR(csivol *apis.CStorVolumeAttachment, nodeID st
 }
 
 // UpdateCStorVolumeAttachmentCR updates CStorVolumeAttachment CR related to current nodeID
-func UpdateCStorVolumeAttachmentCR(csivol *apis.CStorVolumeAttachment) error {
+func UpdateCStorVolumeAttachmentCR(csivol *apis.CStorVolumeAttachment) (*apis.CStorVolumeAttachment, error) {
 
-	_, err := csivolume.NewKubeclient().
+	return csivolume.NewKubeclient().
 		WithNamespace(OpenEBSNamespace).Update(csivol)
-	return err
 }
 
 // TODO Explain when a create of csi volume happens & when it

pkg/utils/utils.go

@@ -13,6 +13,7 @@ import (
 	"github.com/kubernetes-csi/csi-lib-utils/protosanitizer"
 	apis "github.com/openebs/cstor-csi/pkg/apis/cstor/v1"
 	"github.com/openebs/cstor-csi/pkg/cstor/snapshot"
+	iscsiutils "github.com/openebs/cstor-csi/pkg/iscsi"
 	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc"
 	"k8s.io/utils/mount"
@@ -298,6 +299,46 @@ func MonitorMounts() {
 	}
 }
 
+// CleanupOnRestart unmounts and detaches the volumes having
+// DeletionTimestamp set and removes finalizers from the
+// corresponding CStorVolumeAttachment CRs
+func CleanupOnRestart() {
+	var (
+		err        error
+		csivolList *apis.CStorVolumeAttachmentList
+	)
+	// Get list of mounted paths present with the node
+	TransitionVolListLock.Lock()
+	defer TransitionVolListLock.Unlock()
+	if csivolList, err = GetVolListForNode(); err != nil {
+		return
+	}
+	for _, Vol := range csivolList.Items {
+		if Vol.DeletionTimestamp == nil {
+			continue
+		}
+		vol := Vol
+		TransitionVolList[vol.Spec.Volume.Name] = apis.CStorVolumeAttachmentStatusUnmountUnderProgress
+		// This is being run in a go routine so that if unmount and detach
+		// commands take time, the startup is not delayed
+		go func(vol *apis.CStorVolumeAttachment) {
+			if err := iscsiutils.UnmountAndDetachDisk(vol, vol.Spec.Volume.StagingTargetPath); err == nil {
+				vol.Finalizers = nil
+				if vol, err = UpdateCStorVolumeAttachmentCR(vol); err != nil {
+					logrus.Errorf(err.Error())
+				}
+			} else {
+				logrus.Errorf(err.Error())
+			}
+
+			TransitionVolListLock.Lock()
+			TransitionVolList[vol.Spec.Volume.Name] = apis.CStorVolumeAttachmentStatusUnmounted
+			delete(TransitionVolList, vol.Spec.Volume.Name)
+			TransitionVolListLock.Unlock()
+		}(&vol)
+	}
+}
+
 // WaitForVolumeReadyAndReachable waits until the volume is ready to accept IOs
 // and is reachable, this function will not come out until both the conditions
 // are met. This function stops the driver from overloading the OS with iSCSI