From 639ead416ebffc74f1481aa23d4201b8e9c8e542 Mon Sep 17 00:00:00 2001 From: Pawan Date: Mon, 8 Jun 2020 11:33:27 +0530 Subject: [PATCH] feat(mount): moving to legacy mount We can not mount the datasets to more than one path via zfs mount command, shifting to the legacy way of handling ZFS volumes where we can mount/umount the datasets via legacy mount and umount commands. This will also add a building block for SINGLE-NODE-MULTI-WRITER Capability. Signed-off-by: Pawan --- changelogs/unreleased/151-pawanpraka1 | 1 + pkg/zfs/mount.go | 84 ++++++++++++++++++--------- pkg/zfs/zfs_util.go | 35 ++++++----- 3 files changed, 74 insertions(+), 46 deletions(-) create mode 100644 changelogs/unreleased/151-pawanpraka1 diff --git a/changelogs/unreleased/151-pawanpraka1 b/changelogs/unreleased/151-pawanpraka1 new file mode 100644 index 0000000..c90dc08 --- /dev/null +++ b/changelogs/unreleased/151-pawanpraka1 @@ -0,0 +1 @@ +moving to legacy mount diff --git a/pkg/zfs/mount.go b/pkg/zfs/mount.go index eab8609..5dd2e71 100644 --- a/pkg/zfs/mount.go +++ b/pkg/zfs/mount.go @@ -3,6 +3,7 @@ package zfs import ( "fmt" "os" + "os/exec" "github.com/Sirupsen/logrus" apis "github.com/openebs/zfs-localpv/pkg/apis/openebs.io/zfs/v1" @@ -60,29 +61,28 @@ func UmountVolume(vol *apis.ZFSVolume, targetPath string, return nil } - if vol.Spec.VolumeType == VOLTYPE_DATASET { - if err = UmountZFSDataset(vol); err != nil { - logrus.Errorf( - "zfspv failed to umount dataset: path %s Error: %v", - targetPath, err, - ) - return err - } - } else { - if err = mounter.Unmount(targetPath); err != nil { - logrus.Errorf( - "zfspv failed to unmount zvol: path %s Error: %v", - targetPath, err, - ) - return err - } + if err = mounter.Unmount(targetPath); err != nil { + logrus.Errorf( + "zfs: failed to unmount %s: path %s err: %v", + vol.Name, targetPath, err, + ) + return err + } + + if err = SetDatasetLegacyMount(vol); err != nil { + // ignoring the failure as the volume has already + // been umounted, now the new pod can mount it + logrus.Warningf( + "zfs: failed to set legacy mountpoint: %s err: %v", + vol.Name, err, + ) } if err := os.Remove(targetPath); err != nil { - logrus.Errorf("zfspv: failed to remove mount path Error: %v", err) + logrus.Errorf("zfspv: failed to remove mount path vol %s err : %v", vol.Name, err) } - logrus.Infof("umount done path %v", targetPath) + logrus.Infof("umount done %s path %v", vol.Name, targetPath) return nil } @@ -132,19 +132,19 @@ func IsMountPath(path string) bool { func verifyMountRequest(vol *apis.ZFSVolume, mountpath string) error { if len(mountpath) == 0 { - return status.Error(codes.InvalidArgument, "mount path missing in request") + return status.Error(codes.InvalidArgument, "verifyMount: mount path missing in request") } if len(vol.Spec.OwnerNodeID) > 0 && vol.Spec.OwnerNodeID != NodeID { - return status.Error(codes.Internal, "volume is owned by different node") + return status.Error(codes.Internal, "verifyMount: volume is owned by different node") } devicePath, err := GetVolumeDevPath(vol) if err != nil { logrus.Errorf("can not get device for volume:%s dev %s err: %v", vol.Name, devicePath, err.Error()) - return err + return status.Errorf(codes.Internal, "verifyMount: GetVolumePath failed %s", err.Error()) } /* @@ -158,13 +158,13 @@ func verifyMountRequest(vol *apis.ZFSVolume, mountpath string) error { if err != nil { logrus.Errorf("can not get mounts for volume:%s dev %s err: %v", vol.Name, devicePath, err.Error()) - return err + return status.Errorf(codes.Internal, "verifyMount: Getmounts failed %s", err.Error()) } else if len(currentMounts) >= 1 { logrus.Errorf( "can not mount, volume:%s already mounted dev %s mounts: %v", vol.Name, devicePath, currentMounts, ) - return status.Error(codes.Internal, "device already mounted") + return status.Errorf(codes.Internal, "verifyMount: device already mounted at %s", currentMounts) } return nil } @@ -174,7 +174,7 @@ func MountZvol(vol *apis.ZFSVolume, mount *apis.MountInfo) error { volume := vol.Spec.PoolName + "/" + vol.Name err := verifyMountRequest(vol, mount.MountPath) if err != nil { - return status.Error(codes.Internal, "zvol can not be mounted") + return err } devicePath := ZFS_DEVPATH + volume @@ -194,15 +194,43 @@ func MountDataset(vol *apis.ZFSVolume, mount *apis.MountInfo) error { volume := vol.Spec.PoolName + "/" + vol.Name err := verifyMountRequest(vol, mount.MountPath) if err != nil { - return status.Error(codes.Internal, "invalid mount request") + return err } - err = MountZFSDataset(vol, mount.MountPath) + val, err := GetVolumeProperty(vol, "mountpoint") if err != nil { - return status.Errorf(codes.Internal, "zfs: mount failed err : %v", err.Error()) + return err } - logrus.Infof("dataset %v mounted %v", volume, mount.MountPath) + if val == "legacy" { + var MountVolArg []string + var mntopt string + + for _, option := range mount.MountOptions { + mntopt += option + "," + } + + MountVolArg = append(MountVolArg, "-o", mntopt, "-t", "zfs", volume, mount.MountPath) + cmd := exec.Command("mount", 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 status.Errorf(codes.Internal, "dataset: mount failed err : %s", string(out)) + } + logrus.Infof("dataset : legacy mounted %s => %s", volume, mount.MountPath) + } else { + /* + * We might have created volumes and then upgraded the node agent before + * getting the mount request for that volume. In this case volume will + * not be created with mountpoint as legacy. Handling the mount in old way. + */ + err = MountZFSDataset(vol, mount.MountPath) + if err != nil { + return status.Errorf(codes.Internal, "zfs: mount failed err : %s", err.Error()) + } + logrus.Infof("dataset : mounted %s => %s", volume, mount.MountPath) + } return nil } diff --git a/pkg/zfs/zfs_util.go b/pkg/zfs/zfs_util.go index 9e2bf4a..d16638d 100644 --- a/pkg/zfs/zfs_util.go +++ b/pkg/zfs/zfs_util.go @@ -140,7 +140,7 @@ func buildCloneCreateArgs(vol *apis.ZFSVolume) []string { reservationProperty := "reservation=" + vol.Spec.Capacity ZFSVolArg = append(ZFSVolArg, "-o", reservationProperty) } - ZFSVolArg = append(ZFSVolArg, "-o", "mountpoint=none") + ZFSVolArg = append(ZFSVolArg, "-o", "mountpoint=legacy") } if len(vol.Spec.Dedup) != 0 { @@ -235,7 +235,7 @@ func buildDatasetCreateArgs(vol *apis.ZFSVolume) []string { } // set the mount path to none, by default zfs mounts it to the default dataset path - ZFSVolArg = append(ZFSVolArg, "-o", "mountpoint=none", volume) + ZFSVolArg = append(ZFSVolArg, "-o", "mountpoint=legacy", volume) return ZFSVolArg } @@ -421,25 +421,24 @@ func MountZFSDataset(vol *apis.ZFSVolume, mountpath string) error { return nil } -// UmountZFSDataset umounts the dataset -func UmountZFSDataset(vol *apis.ZFSVolume) error { - volume := vol.Spec.PoolName + "/" + vol.Name - var MountVolArg []string - MountVolArg = append(MountVolArg, "umount", volume) - cmd := exec.Command(ZFSVolCmd, MountVolArg...) - out, err := cmd.CombinedOutput() +// SetDatasetLegacyMount sets the dataset mountpoint to legacy if not set +func SetDatasetLegacyMount(vol *apis.ZFSVolume) error { + if vol.Spec.VolumeType != VOLTYPE_DATASET { + return nil + } + + prop, err := GetVolumeProperty(vol, "mountpoint") if err != nil { - logrus.Errorf("zfs: could not umount the dataset %v cmd %v error: %s", - volume, MountVolArg, string(out)) return err } - // ignoring the failure of setting the mountpoint to none - // as the dataset has already been umounted, now the new pod - // can mount it and it will change that to desired mountpath - // and try to mount it if not mounted - SetDatasetMountProp(volume, "none") - return nil + if prop != "legacy" { + // set the mountpoint to legacy + volume := vol.Spec.PoolName + "/" + vol.Name + err = SetDatasetMountProp(volume, "legacy") + } + + return err } // GetVolumeProperty gets zfs properties for the volume @@ -454,7 +453,7 @@ func GetVolumeProperty(vol *apis.ZFSVolume, prop string) (string, error) { if err != nil { logrus.Errorf("zfs: could not get %s on dataset %v cmd %v error: %s", prop, volume, ZFSVolArg, string(out)) - return "", fmt.Errorf("get %s failed, %s", prop, string(out)) + return "", fmt.Errorf("zfs get %s failed, %s", prop, string(out)) } val := out[:len(out)-1] return string(val), nil