Automated cherry pick of #1379: Remove CSI NodeUnpublish lstat

csi/node.go

@@ -171,50 +171,15 @@ func (s *OsdCsiServer) NodeUnpublishVolume(
 				req.GetVolumeId())
 		}
 	}
-	vol := vols[0]
 
-	if !vol.IsAttached() {
-		logrus.Infof("Volume %s was already unmounted", req.GetVolumeId())
-		return &csi.NodeUnpublishVolumeResponse{}, nil
-	}
-
-	// Get information about the target since the request does not
-	// tell us if it is for block or mount point.
-	// https://github.com/container-storage-interface/spec/issues/285
-	fileInfo, err := os.Lstat(req.GetTargetPath())
-	if err != nil && os.IsNotExist(err) {
-		// For idempotency, return that there is nothing to unmount
-		logrus.Infof("NodeUnpublishVolume on target path %s but it does "+
-			"not exist, returning there is nothing to do", req.GetTargetPath())
-		return &csi.NodeUnpublishVolumeResponse{}, nil
-	} else if err != nil {
-		return nil, status.Errorf(
-			codes.Internal,
-			"Unknown error while verifying target location %s: %s",
+	// Mount volume onto the path
+	if err = s.driver.Unmount(req.GetVolumeId(), req.GetTargetPath(), nil); err != nil {
+		logrus.Infof("Unable to unmount volume %s onto %s: %s",
+			req.GetVolumeId(),
 			req.GetTargetPath(),
 			err.Error())
 	}
 
-	// Check if it is block or not
-	if fileInfo.Mode()&os.ModeSymlink != 0 {
-		// If block, we just need to remove the link.
-		os.Remove(req.GetTargetPath())
-	} else {
-		if !fileInfo.IsDir() {
-			return nil, status.Errorf(
-				codes.NotFound,
-				"Target location %s is not a directory", req.GetTargetPath())
-		}
-
-		// Mount volume onto the path
-		if err = s.driver.Unmount(req.GetVolumeId(), req.GetTargetPath(), nil); err != nil {
-			logrus.Infof("Unable to unmount volume %s onto %s: %s",
-				req.GetVolumeId(),
-				req.GetTargetPath(),
-				err.Error())
-		}
-	}
-
 	if s.driver.Type() == api.DriverType_DRIVER_TYPE_BLOCK {
 		if err = s.driver.Detach(req.GetVolumeId(), nil); err != nil {
 			return nil, status.Errorf(

csi/node_test.go

@@ -495,53 +495,6 @@ func TestNodeUnpublishVolumeVolumeNotFound(t *testing.T) {
 	assert.Contains(t, serverError.Message(), "not found")
 }
 
-func TestNodeUnpublishVolumeInvalidTargetLocation(t *testing.T) {
-	// Create server and client connection
-	s := newTestServer(t)
-	defer s.Stop()
-
-	testargs := []struct {
-		expectedErrorContains string
-		targetPath            string
-	}{
-		{
-			expectedErrorContains: "not a directory",
-			targetPath:            "/etc/hosts",
-		},
-	}
-
-	c := csi.NewNodeClient(s.Conn())
-	name := "myvol"
-
-	req := &csi.NodeUnpublishVolumeRequest{
-		VolumeId: name,
-	}
-
-	for _, testarg := range testargs {
-		s.MockDriver().
-			EXPECT().
-			Inspect([]string{name}).
-			Return([]*api.Volume{
-				&api.Volume{
-					Id:            name,
-					AttachPath:    []string{testarg.targetPath},
-					AttachedOn:    "node1",
-					State:         api.VolumeState_VOLUME_STATE_ATTACHED,
-					AttachedState: api.AttachState_ATTACH_STATE_EXTERNAL,
-				},
-			}, nil).
-			Times(1)
-
-		req.TargetPath = testarg.targetPath
-		_, err := c.NodeUnpublishVolume(context.Background(), req)
-		assert.NotNil(t, err)
-		serverError, ok := status.FromError(err)
-		assert.True(t, ok)
-		assert.Equal(t, serverError.Code(), codes.NotFound)
-		assert.Contains(t, serverError.Message(), testarg.expectedErrorContains)
-	}
-}
-
 func TestNodeUnpublishVolumeFailedToUnmount(t *testing.T) {
 	// Create server and client connection
 	s := newTestServer(t)
@@ -719,49 +672,6 @@ func TestNodeUnpublishVolumeUnmount(t *testing.T) {
 	assert.NotNil(t, r)
 }
 
-func TestNodeUnpublishVolumeUnmountIdempotent(t *testing.T) {
-	// Create server and client connection
-	s := newTestServer(t)
-	defer s.Stop()
-
-	// Make a call
-	c := csi.NewNodeClient(s.Conn())
-
-	name := "myvolMounted"
-	size := uint64(10)
-	targetPath := "/mnt"
-	gomock.InOrder(
-		s.MockDriver().
-			EXPECT().
-			Inspect([]string{name}).
-			Return([]*api.Volume{
-				&api.Volume{
-					Id: name,
-					Locator: &api.VolumeLocator{
-						Name: name,
-					},
-					Spec: &api.VolumeSpec{
-						Size: size,
-					},
-					// Already unmounted and detached:
-					AttachPath: []string{},
-					AttachedOn: "",
-					State:      api.VolumeState_VOLUME_STATE_DETACHED,
-				},
-			}, nil).
-			Times(1),
-	)
-
-	req := &csi.NodeUnpublishVolumeRequest{
-		VolumeId:   name,
-		TargetPath: targetPath,
-	}
-
-	r, err := c.NodeUnpublishVolume(context.Background(), req)
-	assert.Nil(t, err)
-	assert.NotNil(t, r)
-}
-
 func TestNodeGetCapabilities(t *testing.T) {
 	// Create server and client connection
 	s := newTestServer(t)