Skip to content

Commit

Permalink
Remove CSI NodeUnpublish lstat
Browse files Browse the repository at this point in the history
Signed-off-by: Grant Griffiths <grant@portworx.com>
  • Loading branch information
ggriffiths committed Jan 24, 2020
1 parent 4a9094c commit 6e79e9c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 129 deletions.
43 changes: 4 additions & 39 deletions csi/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
90 changes: 0 additions & 90 deletions csi/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 6e79e9c

Please sign in to comment.