diff --git a/driver/mounter.go b/driver/mounter.go index 6a7373419..a458c4cf3 100644 --- a/driver/mounter.go +++ b/driver/mounter.go @@ -57,7 +57,7 @@ type Mounter interface { // in a correct way (i.e: propagated). It returns true if it's mounted. An // error is returned in case of system errors or if it's mounted // incorrectly. - IsMounted(source, target string) (bool, error) + IsMounted(target string) (bool, error) } // TODO(arslan): this is Linux only for now. Refactor this into a package with @@ -215,11 +215,7 @@ func (m *mounter) IsFormatted(source string) (bool, error) { return true, nil } -func (m *mounter) IsMounted(source, target string) (bool, error) { - if source == "" { - return false, errors.New("source is not specified for checking the mount") - } - +func (m *mounter) IsMounted(target string) (bool, error) { if target == "" { return false, errors.New("target is not specified for checking the mount") } @@ -233,12 +229,12 @@ func (m *mounter) IsMounted(source, target string) (bool, error) { return false, err } - findmntArgs := []string{"-o", "TARGET,PROPAGATION,FSTYPE,OPTIONS", source, "-J"} + findmntArgs := []string{"-o", "TARGET,PROPAGATION,FSTYPE,OPTIONS", "-M", target, "-J"} m.log.WithFields(logrus.Fields{ "cmd": findmntCmd, "args": findmntArgs, - }).Info("checking if source is mounted") + }).Info("checking if target is mounted") out, err := exec.Command(findmntCmd, findmntArgs...).CombinedOutput() if err != nil { @@ -251,6 +247,11 @@ func (m *mounter) IsMounted(source, target string) (bool, error) { err, findmntCmd, string(out)) } + // no response means there is no mount + if string(out) == "" { + return false, nil + } + var resp *findmntResponse err = json.Unmarshal(out, &resp) if err != nil { @@ -261,7 +262,7 @@ func (m *mounter) IsMounted(source, target string) (bool, error) { for _, fs := range resp.FileSystems { // check if the mount is propagated correctly. It should be set to shared. if fs.Propagation != "shared" { - return true, fmt.Errorf("mount propagation for target %q is not enabled or the block device %q does not exist anymore", target, source) + return true, fmt.Errorf("mount propagation for target %q is not enabled", target) } // the mountpoint should match as well diff --git a/driver/node.go b/driver/node.go index ad5261b1d..09b580137 100644 --- a/driver/node.go +++ b/driver/node.go @@ -28,7 +28,6 @@ import ( "context" "net/http" "path/filepath" - "strings" csi "github.com/container-storage-interface/spec/lib/go/csi/v0" "github.com/sirupsen/logrus" @@ -104,7 +103,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe ll.Info("mounting the volume for staging") - mounted, err := d.mounter.IsMounted(source, target) + mounted, err := d.mounter.IsMounted(target) if err != nil { return nil, err } @@ -138,17 +137,7 @@ func (d *Driver) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolu }) ll.Info("node unstage volume called") - vol, resp, err := d.doClient.Storage.GetVolume(ctx, req.VolumeId) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - // we assume it's deleted already for idempotency - return &csi.NodeUnstageVolumeResponse{}, nil - } - return nil, err - } - - source := getDiskSource(vol.Name) - mounted, err := d.mounter.IsMounted(source, req.StagingTargetPath) + mounted, err := d.mounter.IsMounted(req.StagingTargetPath) if err != nil { return nil, err } @@ -186,16 +175,6 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu return nil, status.Error(codes.InvalidArgument, "NodePublishVolume Volume Capability must be provided") } - vol, resp, err := d.doClient.Storage.GetVolume(ctx, req.VolumeId) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - return nil, status.Errorf(codes.NotFound, "volume %q not found", req.VolumeId) - } - return nil, err - } - - diskSource := getDiskSource(vol.Name) - source := req.StagingTargetPath target := req.TargetPath @@ -223,9 +202,7 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu "method": "node_publish_volume", }) - // we can only check if target is mounted with the diskSource directly. - // The staging target path (which is a directory itself) won't work in this case - mounted, err := d.mounter.IsMounted(diskSource, target) + mounted, err := d.mounter.IsMounted(target) if err != nil { return nil, err } @@ -260,15 +237,21 @@ func (d *Driver) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublish }) ll.Info("node unpublish volume called") - ll.Info("unmounting the target path") - err := d.mounter.Unmount(req.TargetPath) - // Invalid argument is returned by the `mount` command in this container - // image when a path is already umounted. We only return an error for - // anything else - if err != nil && !strings.Contains(err.Error(), "Invalid argument") { + mounted, err := d.mounter.IsMounted(req.TargetPath) + if err != nil { return nil, err } + if mounted { + ll.Info("unmounting the target path") + err := d.mounter.Unmount(req.TargetPath) + if err != nil { + return nil, err + } + } else { + ll.Info("target path is already unmounted") + } + ll.Info("unmounting volume is finished") return &csi.NodeUnpublishVolumeResponse{}, nil }