diff --git a/csi/node.go b/csi/node.go index 1c2b653b7..158ca10f3 100644 --- a/csi/node.go +++ b/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( diff --git a/csi/node_test.go b/csi/node_test.go index a6affe2f3..a9f0de57e 100644 --- a/csi/node_test.go +++ b/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)