diff --git a/pkg/driver/agent.go b/pkg/driver/agent.go index 156584ad8..24d52b84e 100644 --- a/pkg/driver/agent.go +++ b/pkg/driver/agent.go @@ -133,10 +133,8 @@ func (ns *node) NodeUnpublishVolume( ) (*csi.NodeUnpublishVolumeResponse, error) { var ( - err error - vol *apis.ZFSVolume - devpath string - currentMounts []string + err error + vol *apis.ZFSVolume ) if err = ns.validateNodeUnpublishReq(req); err != nil { @@ -147,37 +145,17 @@ func (ns *node) NodeUnpublishVolume( volumeID := req.GetVolumeId() if vol, err = zfs.GetZFSVolume(volumeID); err != nil { - return nil, err - } - - if devpath, err = zfs.GetVolumeDevPath(vol); err != nil { - goto NodeUnpublishResponse - } - - currentMounts, err = zfs.GetMounts(devpath) - if err != nil { - return nil, err - } else if len(currentMounts) == 0 { - return nil, status.Error(codes.Internal, "umount request for not mounted volume") - } else if len(currentMounts) == 1 { - if currentMounts[0] != targetPath { - return nil, status.Error(codes.Internal, "device not mounted at right path") - } - } else { - logrus.Errorf( - "can not unmount, more than one mounts for volume:%s path %s mounts: %v", - volumeID, targetPath, currentMounts, - ) - return nil, status.Error(codes.Internal, "device not mounted at rightpath") + return nil, status.Errorf(codes.Internal, + "not able to get the ZFSVolume %s err : %s", + volumeID, err.Error()) } - if err = zfs.UmountVolume(vol, req.GetTargetPath()); err != nil { - goto NodeUnpublishResponse - } + err = zfs.UmountVolume(vol, targetPath) -NodeUnpublishResponse: if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Errorf(codes.Internal, + "unable to umount the volume %s err : %s", + volumeID, err.Error()) } logrus.Infof("hostpath: volume %s path: %s has been unmounted.", volumeID, targetPath) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index f759a827b..6815be519 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -119,8 +119,8 @@ func CreateZFSVolume(req *csi.CreateVolumeRequest) (string, error) { err = zfs.ProvisionVolume(volObj) if err != nil { - return "", status.Error(codes.Internal, - "not able to provision the volume") + return "", status.Errorf(codes.Internal, + "not able to provision the volume %s", err.Error()) } return selected, nil @@ -168,8 +168,8 @@ func CreateZFSClone(req *csi.CreateVolumeRequest, snapshot string) (string, erro err = zfs.ProvisionVolume(volObj) if err != nil { - return "", status.Error(codes.Internal, - "not able to provision the volume") + return "", status.Errorf(codes.Internal, + "not able to provision the clone volume %s", err.Error()) } return selected, nil diff --git a/pkg/zfs/mount.go b/pkg/zfs/mount.go index 814f78d52..464b50bf3 100644 --- a/pkg/zfs/mount.go +++ b/pkg/zfs/mount.go @@ -32,7 +32,7 @@ func UmountVolume(vol *apis.ZFSVolume, targetPath string, ) error { mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: mount.NewOsExec()} - _, _, err := mount.GetDeviceNameFromMount(mounter, targetPath) + dev, ref, err := mount.GetDeviceNameFromMount(mounter, targetPath) if err != nil { logrus.Errorf( "zfspv umount volume: failed to get device from mnt: %s\nError: %v", @@ -41,6 +41,15 @@ func UmountVolume(vol *apis.ZFSVolume, targetPath string, return err } + // device has already been un-mounted, return successful + if len(dev) == 0 || ref == 0 { + logrus.Warningf( + "Warning: Unmount skipped because volume %s not mounted: %v", + vol.Name, targetPath, + ) + return nil + } + if pathExists, pathErr := mount.PathExists(targetPath); pathErr != nil { return fmt.Errorf("Error checking if path exists: %v", pathErr) } else if !pathExists { diff --git a/pkg/zfs/zfs_util.go b/pkg/zfs/zfs_util.go index 13741c00a..a00d6689e 100644 --- a/pkg/zfs/zfs_util.go +++ b/pkg/zfs/zfs_util.go @@ -387,7 +387,36 @@ func SetDatasetMountProp(volume string, mountpath string) error { func MountZFSDataset(vol *apis.ZFSVolume, mountpath string) error { volume := vol.Spec.PoolName + "/" + vol.Name - return SetDatasetMountProp(volume, mountpath) + // set the mountpoint to the path where this volume should be mounted + err := SetDatasetMountProp(volume, mountpath) + if err != nil { + return err + } + + /* + * see if we should attempt to mount the dataset. + * Setting the mountpoint is sufficient to mount the zfs dataset, + * but if dataset has been unmounted, then setting the mountpoint + * is not sufficient, we have to mount the dataset explicitly + */ + mounted, err := GetVolumeProperty(vol, "mounted") + if err != nil { + return err + } + + if mounted == "no" { + var MountVolArg []string + MountVolArg = append(MountVolArg, "mount", volume) + cmd := exec.Command(ZFSVolCmd, MountVolArg...) + out, err := cmd.CombinedOutput() + if err != nil { + logrus.Errorf("zfs: could not mount the dataset %v cmd %v error: %s", + volume, MountVolArg, string(out)) + return err + } + } + + return nil } // UmountZFSDataset umounts the dataset