add validation to check if stagingPath exists

It's CO responsibility to create the
stagingPath as per the CSI spec.

The CO SHALL ensure
// that the path is directory and that the process serving the
// request has `read` and `write` permission to that directory. The
// CO SHALL be responsible for creating the directory if it does not
// exist.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

pkg/cephfs/nodeserver.go

@@ -104,11 +104,6 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 		}
 	}
 
-	if err = util.CreateMountPoint(stagingTargetPath); err != nil {
-		klog.Errorf("failed to create staging mount point at %s for volume %s: %v", stagingTargetPath, volID, err)
-		return nil, status.Error(codes.Internal, err.Error())
-	}
-
 	idLk := nodeVolumeIDLocker.Lock(string(volID))
 	defer nodeVolumeIDLocker.Unlock(idLk, string(volID))
 
@@ -289,10 +284,6 @@ func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	if err = os.Remove(stagingTargetPath); err != nil {
-		return nil, status.Error(codes.Internal, err.Error())
-	}
-
 	klog.Infof("cephfs: successfully unmounted volume %s from %s", req.GetVolumeId(), stagingTargetPath)
 
 	return &csi.NodeUnstageVolumeResponse{}, nil

pkg/rbd/nodeserver.go

@@ -91,10 +91,10 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 	idLk := nodeVolumeIDLocker.Lock(volID)
 	defer nodeVolumeIDLocker.Unlock(idLk, volID)
 
-	// Check if that target path exists properly
+	var isNotMnt bool
-	isNotMnt, err := ns.createMountPath(stagingTargetPath, isBlock)
+	// check if stagingPath is already mounted
-	if err != nil {
+	isNotMnt, err = mount.IsNotMountPoint(ns.mounter, stagingTargetPath)
-		klog.Errorf("stat failed: %v", err)
+	if err != nil && !os.IsNotExist(err) {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
@@ -117,19 +117,27 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 	klog.V(4).Infof("rbd image: %s/%s was successfully mapped at %s\n", req.GetVolumeId(), volOptions.Pool, devicePath)
 
 	isMounted := false
+	isStagePathCreated := false
 	// if mounting to stagingpath fails unmap the rbd device. this wont leave any
 	// stale rbd device if unstage is not called
 	defer func() {
 		if err != nil {
-			ns.cleanupStagingPath(stagingTargetPath, devicePath, volID, isBlock, isMounted)
+			ns.cleanupStagingPath(stagingTargetPath, devicePath, volID, isStagePathCreated, isBlock, isMounted)
 		}
 	}()
+	err = ns.createStageMountPoint(stagingTargetPath, isBlock)
+	if err != nil {
+		return nil, status.Error(codes.Internal, err.Error())
+	}
+	isStagePathCreated = true
+
 	// nodeStage Path
 	err = ns.mountVolumeToStagePath(req, stagingTargetPath, devicePath)
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 	isMounted = true
+
 	err = os.Chmod(stagingTargetPath, 0777)
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
@@ -140,7 +148,7 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 	return &csi.NodeStageVolumeResponse{}, nil
 }
 
-func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID string, isBlock, isMounted bool) {
+func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID string, isStagePathCreated, isBlock, isMounted bool) {
 	var err error
 	if isMounted {
 		err = ns.mounter.Unmount(stagingTargetPath)
@@ -149,7 +157,7 @@ func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID st
 		}
 	}
 	// remove the block file created on staging path
-	if isBlock {
+	if isBlock && isStagePathCreated {
 		err = os.Remove(stagingTargetPath)
 		if err != nil {
 			klog.Errorf("failed to remove stagingtargetPath: %s with error: %v", stagingTargetPath, err)
@@ -161,6 +169,21 @@ func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID st
 	}
 }
 
+func (ns *NodeServer) createStageMountPoint(mountPath string, isBlock bool) error {
+	if isBlock {
+		pathFile, err := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750)
+		if err != nil {
+			klog.Errorf("failed to create mountPath:%s with error: %v", mountPath, err)
+			return status.Error(codes.Internal, err.Error())
+		}
+		if err = pathFile.Close(); err != nil {
+			klog.Errorf("failed to close mountPath:%s with error: %v", mountPath, err)
+			return status.Error(codes.Internal, err.Error())
+		}
+	}
+	return nil
+}
+
 // NodePublishVolume mounts the volume mounted to the device path to the target
 // path
 func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
@@ -180,7 +203,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	defer targetPathLocker.Unlock(idLk, targetPath)
 
 	// Check if that target path exists properly
-	notMnt, err := ns.createMountPath(targetPath, isBlock)
+	notMnt, err := ns.createTargetMountPath(targetPath, isBlock)
 	if err != nil {
 		return nil, err
 	}
@@ -280,7 +303,7 @@ func (ns *NodeServer) mountVolume(stagingPath string, req *csi.NodePublishVolume
 	return nil
 }
 
-func (ns *NodeServer) createMountPath(mountPath string, isBlock bool) (bool, error) {
+func (ns *NodeServer) createTargetMountPath(mountPath string, isBlock bool) (bool, error) {
 	// Check if that mount path exists properly
 	notMnt, err := mount.IsNotMountPoint(ns.mounter, mountPath)
 	if err != nil {
@@ -376,9 +399,11 @@ unmount:
 			stagingTargetPath = blockStagingPath
 			goto unmount
 		}
-		if err = os.RemoveAll(stagingTargetPath); err != nil {
+		if stagingTargetPath == blockStagingPath {
+			if err = os.Remove(stagingTargetPath); err != nil {
 				return nil, status.Error(codes.Internal, err.Error())
 			}
+		}
 		return &csi.NodeUnstageVolumeResponse{}, nil
 	}
 	// Unmount the volume
@@ -391,9 +416,11 @@ unmount:
 		return nil, err
 	}
 
-	if err = os.RemoveAll(stagingTargetPath); err != nil {
+	if stagingTargetPath == blockStagingPath {
+		if err = os.Remove(stagingTargetPath); err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
+	}
 
 	klog.Infof("rbd: successfully unmounted volume %s from %s", req.GetVolumeId(), stagingTargetPath)
 

pkg/util/util.go

@@ -125,6 +125,14 @@ func CreateMountPoint(mountPath string) error {
 	return os.MkdirAll(mountPath, 0750)
 }
 
+// checkDirExists checks directory  exists or not
+func checkDirExists(p string) bool {
+	if _, err := os.Stat(p); os.IsNotExist(err) {
+		return false
+	}
+	return true
+}
+
 // IsMountPoint checks if the given path is mountpoint or not
 func IsMountPoint(p string) (bool, error) {
 	dummyMount := mount.New("")

pkg/util/validate.go

@@ -24,6 +24,11 @@ func ValidateNodeStageVolumeRequest(req *csi.NodeStageVolumeRequest) error {
 		return status.Error(codes.InvalidArgument, "stage secrets cannot be nil or empty")
 	}
 
+	// validate stagingpath exists
+	ok := checkDirExists(req.GetStagingTargetPath())
+	if !ok {
+		return status.Error(codes.InvalidArgument, "staging path doesnot exists on node")
+	}
 	return nil
 }