Check if target path is mounted before unmounting

Signed-off-by: Simão Reis <[email protected]>

pkg/driver/node.go

@@ -149,8 +149,29 @@ func (d *Driver) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolu
 		return nil, status.Error(codes.InvalidArgument, "Staging target not provided")
 	}
 
+	// Check if target directory is a mount point. GetDeviceNameFromMount
+	// given a mnt point, finds the device from /proc/mounts
+	// returns the device name, reference count, and error code
+	dev, refCount, err := mount.GetDeviceNameFromMount(d.mounter, target)
+	if err != nil {
+		msg := fmt.Sprintf("failed to check if volume is mounted: %v", err)
+		return nil, status.Error(codes.Internal, msg)
+	}
+
+	// From the spec: If the volume corresponding to the volume_id
+	// is not staged to the staging_target_path, the Plugin MUST
+	// reply 0 OK.
+	if refCount == 0 {
+		klog.V(5).Infof("NodeUnstageVolume: %s target not mounted", target)
+		return &csi.NodeUnstageVolumeResponse{}, nil
+	}
+
+	if refCount > 1 {
+		klog.Warningf("NodeUnstageVolume: found %d references to device %s mounted at target path %s", refCount, dev, target)
+	}
+
 	klog.V(5).Infof("NodeUnstageVolume: unmounting %s", target)
-	err := d.mounter.Interface.Unmount(target)
+	err = d.mounter.Interface.Unmount(target)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not unmount target %q: %v", target, err)
 	}

pkg/driver/node_test.go

@@ -202,11 +202,12 @@ func TestNodeStageVolume(t *testing.T) {
 				t.Fatalf("Expected error %v, got no error", tc.expErrCode)
 			}
 
-			if len(tc.expActions) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
+			// if fake mounter did anything we should
+			// check if it was expected
+			if len(fakeMounter.Log) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
 				t.Fatalf("Expected actions {%+v}, got {%+v}", tc.expActions, fakeMounter.Log)
 			}
-
-			if len(tc.expMountPoints) > 0 && !reflect.DeepEqual(fakeMounter.MountPoints, tc.expMountPoints) {
+			if len(fakeMounter.MountPoints) > 0 && !reflect.DeepEqual(fakeMounter.MountPoints, tc.expMountPoints) {
 				t.Fatalf("Expected mount points {%+v}, got {%+v}", tc.expMountPoints, fakeMounter.MountPoints)
 			}
 		})
@@ -238,6 +239,14 @@ func TestNodeUnstageVolume(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "success no device mounted at target",
+			req: &csi.NodeUnstageVolumeRequest{
+				StagingTargetPath: "/test/path",
+				VolumeId:          "vol-test",
+			},
+			expActions: []mount.FakeAction{},
+		},
 		{
 			name: "fail no VolumeId",
 			req: &csi.NodeUnstageVolumeRequest{
@@ -274,8 +283,9 @@ func TestNodeUnstageVolume(t *testing.T) {
 			} else if tc.expErrCode != codes.OK {
 				t.Fatalf("Expected error %v, got no error", tc.expErrCode)
 			}
-
-			if len(tc.expActions) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
+			// if fake mounter did anything we should
+			// check if it was expected
+			if len(fakeMounter.Log) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
 				t.Fatalf("Expected actions {%+v}, got {%+v}", tc.expActions, fakeMounter.Log)
 			}
 		})
@@ -468,11 +478,12 @@ func TestNodePublishVolume(t *testing.T) {
 				t.Fatalf("Expected error %v and got no error", tc.expErrCode)
 			}
 
-			if len(tc.expActions) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
+			// if fake mounter did anything we should
+			// check if it was expected
+			if len(fakeMounter.Log) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
 				t.Fatalf("Expected actions {%+v}, got {%+v}", tc.expActions, fakeMounter.Log)
 			}
-
-			if len(tc.expMountPoints) > 0 && !reflect.DeepEqual(fakeMounter.MountPoints, tc.expMountPoints) {
+			if len(fakeMounter.MountPoints) > 0 && !reflect.DeepEqual(fakeMounter.MountPoints, tc.expMountPoints) {
 				t.Fatalf("Expected mount points {%+v}, got {%+v}", tc.expMountPoints, fakeMounter.MountPoints)
 			}
 		})
@@ -544,7 +555,9 @@ func TestNodeUnpublishVolume(t *testing.T) {
 				t.Fatalf("Expected error %v, got no error", tc.expErrCode)
 			}
 
-			if len(tc.expActions) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
+			// if fake mounter did anything we should
+			// check if it was expected
+			if len(fakeMounter.Log) > 0 && !reflect.DeepEqual(fakeMounter.Log, tc.expActions) {
 				t.Fatalf("Expected actions {%+v}, got {%+v}", tc.expActions, fakeMounter.Log)
 			}
 		})