Fix staging / unmounting volume operations on Windows

- `GetDeviceNameFromMount` now returns the disk number for a mount path on Windows
- Cleanup the stage path via Rmdir in `proxyMounter.Unmount()`
- Remove unnecessary call to `proxyMounter.WriteVolumeCache()` when unpublishing a volume
- Return a nil error in `GetDeviceNameFromMount` if the target path does not exist

Signed-off-by: Eddie Torres <[email protected]>

pkg/driver/mount_windows.go

@@ -56,7 +56,7 @@ func (m NodeMounter) GetDeviceNameFromMount(mountPath string) (string, int, erro
 		// internal Get-Item cmdlet didn't fail but no item/device was found at the
 		// path so we should return empty string and nil error just like the Linux
 		// implementation would.
-		pattern := `(Get-Item -Path \S+).Target, output: , error: <nil>`
+		pattern := `(Get-Item -Path \S+).Target, output: , error: <nil>|because it does not exist`
 		matched, matchErr := regexp.MatchString(pattern, err.Error())
 		if matched {
 			return "", 0, nil
@@ -128,8 +128,6 @@ func (m *NodeMounter) Unpublish(target string) error {
 	if !ok {
 		return fmt.Errorf("failed to cast mounter to csi proxy mounter")
 	}
-	// WriteVolumeCache before unmount
-	proxyMounter.WriteVolumeCache(target)
 	// Remove symlink
 	err := proxyMounter.Rmdir(target)
 	if err != nil {
@@ -150,11 +148,6 @@ func (m *NodeMounter) Unstage(target string) error {
 	if err != nil {
 		return err
 	}
-	// Cleanup stage path
-	err = proxyMounter.Rmdir(target)
-	if err != nil {
-		return err
-	}
 	return nil
 }
 

pkg/driver/node.go

@@ -220,13 +220,14 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 	// This operation (NodeStageVolume) MUST be idempotent.
 	// If the volume corresponding to the volume_id is already staged to the staging_target_path,
 	// and is identical to the specified volume_capability the Plugin MUST reply 0 OK.
+	klog.V(4).InfoS("NodeStageVolume: checking if volume is already staged", "device", device, "source", source, "target", target)
 	if device == source {
 		klog.V(4).InfoS("NodeStageVolume: volume already staged", "volumeID", volumeID)
 		return &csi.NodeStageVolumeResponse{}, nil
 	}
 
 	// FormatAndMount will format only if needed
-	klog.V(4).InfoS("NodeStageVolume: formatting and mounting with fstype", "source", source, "target", target, "fstype", fsType)
+	klog.V(4).InfoS("NodeStageVolume: formatting and mounting with fstype", "source", source, "volumeID", volumeID, "target", target, "fstype", fsType)
 	formatOptions := []string{}
 	if len(blockSize) > 0 {
 		if fsType == FSTypeXfs {
@@ -255,6 +256,7 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 			return nil, status.Errorf(codes.Internal, "Could not resize volume %q (%q):  %v", volumeID, source, err)
 		}
 	}
+	klog.V(4).InfoS("NodeStageVolume: successfully formatted and mounted volume", "source", source, "volumeID", volumeID, "target", target, "fstype", fsType)
 	return &csi.NodeStageVolumeResponse{}, nil
 }
 
@@ -283,7 +285,7 @@ func (d *nodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
 	// returns the device name, reference count, and error code
 	dev, refCount, err := d.mounter.GetDeviceNameFromMount(target)
 	if err != nil {
-		msg := fmt.Sprintf("failed to check if volume is mounted: %v", err)
+		msg := fmt.Sprintf("failed to check if target %q is a mount point: %v", target, err)
 		return nil, status.Error(codes.Internal, msg)
 	}
 
@@ -304,7 +306,7 @@ func (d *nodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not unmount target %q: %v", target, err)
 	}
-
+	klog.V(4).InfoS("NodeUnStageVolume: successfully unstaged volume", "volumeID", volumeID, "target", target)
 	return &csi.NodeUnstageVolumeResponse{}, nil
 }
 

pkg/mounter/safe_mounter_windows.go

@@ -97,37 +97,45 @@ func (mounter *CSIProxyMounter) Unmount(target string) error {
 		TargetPath: normalizeWindowsPath(target),
 	}
 	volumeIdResponse, err := mounter.VolumeClient.GetVolumeIDFromTargetPath(context.Background(), getVolumeIdRequest)
-	volumeId := volumeIdResponse.GetVolumeId()
+	if err != nil {
+		return err
+	}
 
 	// Call UnmountVolume CSI proxy function which flushes data cache to disk and removes the global staging path
 	unmountVolumeRequest := &volume.UnmountVolumeRequest{
-		VolumeId:   volumeId,
+		VolumeId:   volumeIdResponse.VolumeId,
 		TargetPath: normalizeWindowsPath(target),
 	}
 	_, err = mounter.VolumeClient.UnmountVolume(context.Background(), unmountVolumeRequest)
 	if err != nil {
 		return err
 	}
 
+	// Cleanup stage path
+	err = mounter.Rmdir(target)
+	if err != nil {
+		return err
+	}
+
 	// Get disk number
 	getDiskNumberRequest := &volume.GetDiskNumberFromVolumeIDRequest{
-		VolumeId: volumeId,
+		VolumeId: volumeIdResponse.VolumeId,
 	}
 	getDiskNumberResponse, err := mounter.VolumeClient.GetDiskNumberFromVolumeID(context.Background(), getDiskNumberRequest)
-	diskNumber := getDiskNumberResponse.GetDiskNumber()
 	if err != nil {
 		return err
 	}
 
 	// Offline the disk
 	setDiskStateRequest := &disk.SetDiskStateRequest{
-		DiskNumber: diskNumber,
+		DiskNumber: getDiskNumberResponse.DiskNumber,
 		IsOnline:   false,
 	}
 	_, err = mounter.DiskClient.SetDiskState(context.Background(), setDiskStateRequest)
 	if err != nil {
 		return err
 	}
+	klog.V(4).InfoS("Successfully unmounted volume", "diskNumber", getDiskNumberResponse.DiskNumber, "volumeId", volumeIdResponse.VolumeId, "target", target)
 	return nil
 }
 
@@ -208,15 +216,23 @@ func (mounter *CSIProxyMounter) DeviceOpened(pathname string) (bool, error) {
 	return false, fmt.Errorf("DeviceOpened not implemented for CSIProxyMounter")
 }
 
-// GetDeviceNameFromMount returns the volume ID for a mount path.
-func (mounter *CSIProxyMounter) GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error) {
+// GetDeviceNameFromMount returns the disk number for a mount path.
+func (mounter *CSIProxyMounter) GetDeviceNameFromMount(mountPath, _ string) (string, error) {
 	req := &volume.GetVolumeIDFromTargetPathRequest{TargetPath: normalizeWindowsPath(mountPath)}
 	resp, err := mounter.VolumeClient.GetVolumeIDFromTargetPath(context.Background(), req)
 	if err != nil {
 		return "", err
 	}
-
-	return resp.VolumeId, nil
+	// Get disk number
+	getDiskNumberRequest := &volume.GetDiskNumberFromVolumeIDRequest{
+		VolumeId: resp.VolumeId,
+	}
+	getDiskNumberResponse, err := mounter.VolumeClient.GetDiskNumberFromVolumeID(context.Background(), getDiskNumberRequest)
+	if err != nil {
+		return "", err
+	}
+	klog.V(4).InfoS("GetDeviceNameFromMount called", "diskNumber", getDiskNumberResponse.DiskNumber, "volumeID", resp.VolumeId, "mountPath", mountPath)
+	return fmt.Sprint(getDiskNumberResponse.DiskNumber), nil
 }
 
 func (mounter *CSIProxyMounter) MakeRShared(path string) error {