From 6897095e56a5b856863dcf6aae2d5ddbc66a3564 Mon Sep 17 00:00:00 2001 From: Deyuan Deng Date: Thu, 2 Apr 2015 21:08:04 -0400 Subject: [PATCH] Change mount.Interface.Mount to exec('mount'), instead of syscall --- pkg/util/mount/fake.go | 4 +- pkg/util/mount/mount.go | 11 ++-- pkg/util/mount/mount_linux.go | 91 ++++++++++++++++++++++++----- pkg/util/mount/mount_unsupported.go | 7 +-- pkg/volume/aws_ebs/aws_ebs.go | 15 +++-- pkg/volume/aws_ebs/aws_util.go | 23 ++++---- pkg/volume/aws_ebs/aws_util_test.go | 2 +- pkg/volume/empty_dir/empty_dir.go | 11 ++-- pkg/volume/gce_pd/gce_pd.go | 15 +++-- pkg/volume/gce_pd/gce_pd_test.go | 2 +- pkg/volume/gce_pd/gce_util.go | 22 +++---- pkg/volume/gce_pd/gce_util_test.go | 2 +- pkg/volume/glusterfs/glusterfs.go | 39 +++++-------- pkg/volume/iscsi/disk_manager.go | 10 ++-- pkg/volume/iscsi/iscsi.go | 22 ++++--- pkg/volume/iscsi/iscsi_test.go | 2 +- pkg/volume/iscsi/iscsi_util.go | 6 +- pkg/volume/nfs/nfs.go | 20 ++++--- pkg/volume/nfs/nfs_mount.go | 72 ----------------------- pkg/volume/nfs/nfs_test.go | 52 ++++------------- 20 files changed, 189 insertions(+), 239 deletions(-) delete mode 100644 pkg/volume/nfs/nfs_mount.go diff --git a/pkg/util/mount/fake.go b/pkg/util/mount/fake.go index a744c40d291da..c848d81ddaa62 100644 --- a/pkg/util/mount/fake.go +++ b/pkg/util/mount/fake.go @@ -38,13 +38,13 @@ func (f *FakeMounter) ResetLog() { f.Log = []FakeAction{} } -func (f *FakeMounter) Mount(source string, target string, fstype string, flags uintptr, data string) error { +func (f *FakeMounter) Mount(source string, target string, fstype string, options []string) error { f.MountPoints = append(f.MountPoints, MountPoint{Device: source, Path: target, Type: fstype}) f.Log = append(f.Log, FakeAction{Action: FakeActionMount, Target: target, Source: source, FSType: fstype}) return nil } -func (f *FakeMounter) Unmount(target string, flags int) error { +func (f *FakeMounter) Unmount(target string) error { newMountpoints := []MountPoint{} for _, mp := range f.MountPoints { if mp.Path != target { diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index 3bc8ea49abc61..6364e823d1bf4 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -18,14 +18,11 @@ limitations under the License. // an alternate platform, we will need to abstract further. package mount -// Each supported platform must define the following flags: -// - FlagBind: specifies a bind mount -// - FlagReadOnly: the mount will be read-only type Interface interface { - // Mount wraps syscall.Mount(). - Mount(source string, target string, fstype string, flags uintptr, data string) error - // Umount wraps syscall.Mount(). - Unmount(target string, flags int) error + // Mount mounts source to target as fstype with given options. + Mount(source string, target string, fstype string, options []string) error + // Unmount unmounts given target. + Unmount(target string) error // List returns a list of all mounted filesystems. This can be large. // On some platforms, reading mounts is not guaranteed consistent (i.e. // it could change between chunked reads). This is guaranteed to be diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index e2d292af3647e..239719df9f728 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -24,6 +24,7 @@ import ( "hash/adler32" "io" "os" + "os/exec" "strconv" "strings" "syscall" @@ -31,28 +32,91 @@ import ( "github.com/golang/glog" ) -const FlagBind = syscall.MS_BIND -const FlagReadOnly = syscall.MS_RDONLY +const ( + // How many times to retry for a consistent read of /proc/mounts. + maxListTries = 3 + // Number of fields per line in "/proc/mounts", as per the fstab man page. + expectedNumFieldsPerLine = 6 +) // Mounter implements mount.Interface for linux platform. type Mounter struct{} -// Mount wraps syscall.Mount() -func (mounter *Mounter) Mount(source string, target string, fstype string, flags uintptr, data string) error { - glog.V(5).Infof("Mounting %s %s %s %d %s", source, target, fstype, flags, data) - return syscall.Mount(source, target, fstype, flags, data) +// Mount mounts source to target as fstype with given options. 'source' and 'fstype' must +// be an emtpy string in case it's not required, e.g. for remount, or for auto filesystem +// type, where kernel handles fs type for you. The mount 'options' is a list of options, +// currently come from mount(8), e.g. "ro", "remount", "bind", etc. If no more option is +// required, call Mount with an empty string list or nil. +func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { + // The remount options to use in case of bind mount, due to the fact that bind mount doesn't + // respect mount options. The list equals: + // options - 'bind' + 'remount' (no duplicate) + bindRemountOpts := []string{"remount"} + bind := false + + if len(options) != 0 { + for _, option := range options { + switch option { + case "bind": + bind = true + break + case "remount": + break + default: + bindRemountOpts = append(bindRemountOpts, option) + } + } + } + + if bind { + err := doMount(source, target, fstype, []string{"bind"}) + if err != nil { + return err + } + return doMount(source, target, fstype, bindRemountOpts) + } else { + return doMount(source, target, fstype, options) + } } -// Unmount wraps syscall.Unmount() -func (mounter *Mounter) Unmount(target string, flags int) error { - return syscall.Unmount(target, flags) +func doMount(source string, target string, fstype string, options []string) error { + glog.V(5).Infof("Mounting %s %s %s %v", source, target, fstype, options) + // Build mount command as follows: + // mount [-t $fstype] [-o $options] [$source] $target + mountArgs := []string{} + if len(fstype) > 0 { + mountArgs = append(mountArgs, "-t", fstype) + } + if len(options) > 0 { + mountArgs = append(mountArgs, "-o", strings.Join(options, ",")) + } + if len(source) > 0 { + mountArgs = append(mountArgs, source) + } + mountArgs = append(mountArgs, target) + command := exec.Command("mount", mountArgs...) + output, err := command.CombinedOutput() + if err != nil { + glog.Errorf("Mount failed: %v\nMounting arguments: %s %s %s %v\nOutput: %s\n", + err, source, target, fstype, options, string(output)) + } + return err } -// How many times to retry for a consistent read of /proc/mounts. -const maxListTries = 3 +// Unmount unmounts target with given options. +func (mounter *Mounter) Unmount(target string) error { + glog.V(5).Infof("Unmounting %s %v") + command := exec.Command("umount", target) + output, err := command.CombinedOutput() + if err != nil { + glog.Errorf("Unmount failed: %v\nUnmounting arguments: %s\nOutput: %s\n", err, target, string(output)) + return err + } + return nil +} // List returns a list of all mounted filesystems. -func (*Mounter) List() ([]MountPoint, error) { +func (mounter *Mounter) List() ([]MountPoint, error) { hash1, err := readProcMounts(nil) if err != nil { return nil, err @@ -89,9 +153,6 @@ func (mounter *Mounter) IsMountPoint(file string) (bool, error) { return stat.Sys().(*syscall.Stat_t).Dev != rootStat.Sys().(*syscall.Stat_t).Dev, nil } -// As per the fstab man page. -const expectedNumFieldsPerLine = 6 - // readProcMounts reads /proc/mounts and produces a hash of the contents. If the out // argument is not nil, this fills it with MountPoint structs. func readProcMounts(out *[]MountPoint) (uint32, error) { diff --git a/pkg/util/mount/mount_unsupported.go b/pkg/util/mount/mount_unsupported.go index c8c7645ff0e4d..e7d3ac71027ac 100644 --- a/pkg/util/mount/mount_unsupported.go +++ b/pkg/util/mount/mount_unsupported.go @@ -18,16 +18,13 @@ limitations under the License. package mount -const FlagBind = 0 -const FlagReadOnly = 0 - type Mounter struct{} -func (mounter *Mounter) Mount(source string, target string, fstype string, flags uintptr, data string) error { +func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { return nil } -func (mounter *Mounter) Unmount(target string, flags int) error { +func (mounter *Mounter) Unmount(target string) error { return nil } diff --git a/pkg/volume/aws_ebs/aws_ebs.go b/pkg/volume/aws_ebs/aws_ebs.go index 43aea43553994..38ce0cfcf6a3a 100644 --- a/pkg/volume/aws_ebs/aws_ebs.go +++ b/pkg/volume/aws_ebs/aws_ebs.go @@ -192,11 +192,6 @@ func (pd *awsElasticBlockStore) SetUpAt(dir string) error { return err } - flags := uintptr(0) - if pd.readOnly { - flags = mount.FlagReadOnly - } - if err := os.MkdirAll(dir, 0750); err != nil { // TODO: we should really eject the attach/detach out into its own control loop. detachDiskLogError(pd) @@ -204,7 +199,11 @@ func (pd *awsElasticBlockStore) SetUpAt(dir string) error { } // Perform a bind mount to the full path to allow duplicate mounts of the same PD. - err = pd.mounter.Mount(globalPDPath, dir, "", mount.FlagBind|flags, "") + options := []string{"bind"} + if pd.readOnly { + options = append(options, "ro") + } + err = pd.mounter.Mount(globalPDPath, dir, "", options) if err != nil { mountpoint, mntErr := pd.mounter.IsMountPoint(dir) if mntErr != nil { @@ -212,7 +211,7 @@ func (pd *awsElasticBlockStore) SetUpAt(dir string) error { return err } if mountpoint { - if mntErr = pd.mounter.Unmount(dir, 0); mntErr != nil { + if mntErr = pd.mounter.Unmount(dir); mntErr != nil { glog.Errorf("Failed to unmount: %v", mntErr) return err } @@ -291,7 +290,7 @@ func (pd *awsElasticBlockStore) TearDownAt(dir string) error { return err } // Unmount the bind-mount inside this pod - if err := pd.mounter.Unmount(dir, 0); err != nil { + if err := pd.mounter.Unmount(dir); err != nil { glog.V(2).Info("Error unmounting dir ", dir, ": ", err) return err } diff --git a/pkg/volume/aws_ebs/aws_util.go b/pkg/volume/aws_ebs/aws_util.go index 22d8a3fdf308b..d3b8d42455621 100644 --- a/pkg/volume/aws_ebs/aws_util.go +++ b/pkg/volume/aws_ebs/aws_util.go @@ -36,10 +36,6 @@ func (util *AWSDiskUtil) AttachAndMountDisk(pd *awsElasticBlockStore, globalPDPa if err != nil { return err } - flags := uintptr(0) - if pd.readOnly { - flags = mount.FlagReadOnly - } devicePath, err := volumes.AttachDisk("", pd.volumeID, pd.readOnly) if err != nil { return err @@ -76,8 +72,12 @@ func (util *AWSDiskUtil) AttachAndMountDisk(pd *awsElasticBlockStore, globalPDPa return err } } + options := []string{} + if pd.readOnly { + options = append(options, "ro") + } if !mountpoint { - err = pd.diskMounter.Mount(devicePath, globalPDPath, pd.fsType, flags, "") + err = pd.diskMounter.Mount(devicePath, globalPDPath, pd.fsType, options) if err != nil { os.Remove(globalPDPath) return err @@ -90,7 +90,7 @@ func (util *AWSDiskUtil) AttachAndMountDisk(pd *awsElasticBlockStore, globalPDPa func (util *AWSDiskUtil) DetachDisk(pd *awsElasticBlockStore) error { // Unmount the global PD mount, which should be the only one. globalPDPath := makeGlobalPDPath(pd.plugin.host, pd.volumeID) - if err := pd.mounter.Unmount(globalPDPath, 0); err != nil { + if err := pd.mounter.Unmount(globalPDPath); err != nil { glog.V(2).Info("Error unmount dir ", globalPDPath, ": ", err) return err } @@ -121,18 +121,21 @@ type awsSafeFormatAndMount struct { } // uses /usr/share/google/safe_format_and_mount to optionally mount, and format a disk -func (mounter *awsSafeFormatAndMount) Mount(source string, target string, fstype string, flags uintptr, data string) error { +func (mounter *awsSafeFormatAndMount) Mount(source string, target string, fstype string, options []string) error { + // Don't attempt to format if mounting as readonly. Go straight to mounting. // Don't attempt to format if mounting as readonly. Go straight to mounting. - if (flags & mount.FlagReadOnly) != 0 { - return mounter.Interface.Mount(source, target, fstype, flags, data) + for _, option := range options { + if option == "ro" { + return mounter.Interface.Mount(source, target, fstype, options) + } } args := []string{} // ext4 is the default for safe_format_and_mount if len(fstype) > 0 && fstype != "ext4" { args = append(args, "-m", fmt.Sprintf("mkfs.%s", fstype)) } + args = append(args, options...) args = append(args, source, target) - // TODO: Accept other options here? glog.V(5).Infof("exec-ing: /usr/share/google/safe_format_and_mount %v", args) cmd := mounter.runner.Command("/usr/share/google/safe_format_and_mount", args...) dataOut, err := cmd.CombinedOutput() diff --git a/pkg/volume/aws_ebs/aws_util_test.go b/pkg/volume/aws_ebs/aws_util_test.go index 82dfd19b9bea6..567db16b4ebef 100644 --- a/pkg/volume/aws_ebs/aws_util_test.go +++ b/pkg/volume/aws_ebs/aws_util_test.go @@ -64,7 +64,7 @@ func TestSafeFormatAndMount(t *testing.T) { runner: &fake, } - err := mounter.Mount("/dev/foo", "/mnt/bar", test.fstype, 0, "") + err := mounter.Mount("/dev/foo", "/mnt/bar", test.fstype, nil) if test.err == nil && err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/pkg/volume/empty_dir/empty_dir.go b/pkg/volume/empty_dir/empty_dir.go index e28f6872a6de5..43b3cbef72041 100644 --- a/pkg/volume/empty_dir/empty_dir.go +++ b/pkg/volume/empty_dir/empty_dir.go @@ -197,6 +197,7 @@ func (ed *emptyDir) setupTmpfs(dir string) error { if isMnt && medium == mediumMemory { return nil } + // By default a tmpfs mount will receive a different SELinux context // from that of the Kubelet root directory which is not readable from // the SELinux context of a docker container. @@ -206,15 +207,15 @@ func (ed *emptyDir) setupTmpfs(dir string) error { // the container. opts := ed.getTmpfsMountOptions() glog.V(3).Infof("pod %v: mounting tmpfs for volume %v with opts %v", ed.podUID, ed.volName, opts) - return ed.mounter.Mount("tmpfs", dir, "tmpfs", 0, opts) + return ed.mounter.Mount("tmpfs", dir, "tmpfs", opts) } -func (ed *emptyDir) getTmpfsMountOptions() string { +func (ed *emptyDir) getTmpfsMountOptions() []string { if ed.rootContext == "" { - return "" + return []string{""} } - return fmt.Sprintf("rootcontext=\"%v\"", ed.rootContext) + return []string{fmt.Sprintf("rootcontext=\"%v\"", ed.rootContext)} } func (ed *emptyDir) GetPath() string { @@ -261,7 +262,7 @@ func (ed *emptyDir) teardownTmpfs(dir string) error { if ed.mounter == nil { return fmt.Errorf("memory storage requested, but mounter is nil") } - if err := ed.mounter.Unmount(dir, 0); err != nil { + if err := ed.mounter.Unmount(dir); err != nil { return err } if err := os.RemoveAll(dir); err != nil { diff --git a/pkg/volume/gce_pd/gce_pd.go b/pkg/volume/gce_pd/gce_pd.go index 163f450d8760b..3f05ec874e4e0 100644 --- a/pkg/volume/gce_pd/gce_pd.go +++ b/pkg/volume/gce_pd/gce_pd.go @@ -201,11 +201,6 @@ func (pd *gcePersistentDisk) SetUpAt(dir string) error { return err } - flags := uintptr(0) - if pd.readOnly { - flags = mount.FlagReadOnly - } - if err := os.MkdirAll(dir, 0750); err != nil { // TODO: we should really eject the attach/detach out into its own control loop. detachDiskLogError(pd) @@ -213,7 +208,11 @@ func (pd *gcePersistentDisk) SetUpAt(dir string) error { } // Perform a bind mount to the full path to allow duplicate mounts of the same PD. - err = pd.mounter.Mount(globalPDPath, dir, "", mount.FlagBind|flags, "") + options := []string{"bind"} + if pd.readOnly { + options = append(options, "ro") + } + err = pd.mounter.Mount(globalPDPath, dir, "", options) if err != nil { mountpoint, mntErr := pd.mounter.IsMountPoint(dir) if mntErr != nil { @@ -221,7 +220,7 @@ func (pd *gcePersistentDisk) SetUpAt(dir string) error { return err } if mountpoint { - if mntErr = pd.mounter.Unmount(dir, 0); mntErr != nil { + if mntErr = pd.mounter.Unmount(dir); mntErr != nil { glog.Errorf("Failed to unmount: %v", mntErr) return err } @@ -279,7 +278,7 @@ func (pd *gcePersistentDisk) TearDownAt(dir string) error { return err } // Unmount the bind-mount inside this pod - if err := pd.mounter.Unmount(dir, 0); err != nil { + if err := pd.mounter.Unmount(dir); err != nil { return err } // If len(refs) is 1, then all bind mounts have been removed, and the diff --git a/pkg/volume/gce_pd/gce_pd_test.go b/pkg/volume/gce_pd/gce_pd_test.go index a23f5950f57cd..c3911e1e0cd4a 100644 --- a/pkg/volume/gce_pd/gce_pd_test.go +++ b/pkg/volume/gce_pd/gce_pd_test.go @@ -80,7 +80,7 @@ func (fake *fakePDManager) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPat fake.attachCalled = true // Simulate the global mount so that the fakeMounter returns the // expected number of mounts for the attached disk. - pd.mounter.Mount(globalPath, globalPath, pd.fsType, 0, "") + pd.mounter.Mount(globalPath, globalPath, pd.fsType, nil) return nil } diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go index e37d75496a88b..6d96e3dce3f9d 100644 --- a/pkg/volume/gce_pd/gce_util.go +++ b/pkg/volume/gce_pd/gce_util.go @@ -39,10 +39,6 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath if err != nil { return err } - flags := uintptr(0) - if pd.readOnly { - flags = mount.FlagReadOnly - } if err := gce.(*gce_cloud.GCECloud).AttachDisk(pd.pdName, pd.readOnly); err != nil { return err } @@ -79,8 +75,12 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath return err } } + options := []string{} + if pd.readOnly { + options = append(options, "ro") + } if !mountpoint { - err = pd.diskMounter.Mount(devicePath, globalPDPath, pd.fsType, flags, "") + err = pd.diskMounter.Mount(devicePath, globalPDPath, pd.fsType, options) if err != nil { os.Remove(globalPDPath) return err @@ -93,7 +93,7 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath func (util *GCEDiskUtil) DetachDisk(pd *gcePersistentDisk) error { // Unmount the global PD mount, which should be the only one. globalPDPath := makeGlobalPDName(pd.plugin.host, pd.pdName) - if err := pd.mounter.Unmount(globalPDPath, 0); err != nil { + if err := pd.mounter.Unmount(globalPDPath); err != nil { return err } if err := os.Remove(globalPDPath); err != nil { @@ -120,18 +120,20 @@ type gceSafeFormatAndMount struct { } // uses /usr/share/google/safe_format_and_mount to optionally mount, and format a disk -func (mounter *gceSafeFormatAndMount) Mount(source string, target string, fstype string, flags uintptr, data string) error { +func (mounter *gceSafeFormatAndMount) Mount(source string, target string, fstype string, options []string) error { // Don't attempt to format if mounting as readonly. Go straight to mounting. - if (flags & mount.FlagReadOnly) != 0 { - return mounter.Interface.Mount(source, target, fstype, flags, data) + for _, option := range options { + if option == "ro" { + return mounter.Interface.Mount(source, target, fstype, options) + } } args := []string{} // ext4 is the default for safe_format_and_mount if len(fstype) > 0 && fstype != "ext4" { args = append(args, "-m", fmt.Sprintf("mkfs.%s", fstype)) } + args = append(args, options...) args = append(args, source, target) - // TODO: Accept other options here? glog.V(5).Infof("exec-ing: /usr/share/google/safe_format_and_mount %v", args) cmd := mounter.runner.Command("/usr/share/google/safe_format_and_mount", args...) dataOut, err := cmd.CombinedOutput() diff --git a/pkg/volume/gce_pd/gce_util_test.go b/pkg/volume/gce_pd/gce_util_test.go index 88d668054540d..5c202e2429fb1 100644 --- a/pkg/volume/gce_pd/gce_util_test.go +++ b/pkg/volume/gce_pd/gce_util_test.go @@ -64,7 +64,7 @@ func TestSafeFormatAndMount(t *testing.T) { runner: &fake, } - err := mounter.Mount("/dev/foo", "/mnt/bar", test.fstype, 0, "") + err := mounter.Mount("/dev/foo", "/mnt/bar", test.fstype, nil) if test.err == nil && err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index f59431c456758..8fab0b388aa16 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -19,7 +19,6 @@ package glusterfs import ( "math/rand" "os" - "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/types" @@ -129,16 +128,15 @@ func (glusterfsVolume *glusterfs) SetUpAt(dir string) error { if mountpoint { return nil } - path := glusterfsVolume.path + os.MkdirAll(dir, 0750) - err = glusterfsVolume.execMount(glusterfsVolume.hosts, path, dir, glusterfsVolume.readonly) + err = glusterfsVolume.setUpAtInternal(dir) if err == nil { return nil } - // cleanup upon failure + // Cleanup upon failure. glusterfsVolume.cleanup(dir) - // return error return err } @@ -165,7 +163,7 @@ func (glusterfsVolume *glusterfs) cleanup(dir string) error { return os.RemoveAll(dir) } - if err := glusterfsVolume.mounter.Unmount(dir, 0); err != nil { + if err := glusterfsVolume.mounter.Unmount(dir); err != nil { glog.Errorf("Glusterfs: Unmounting failed: %v", err) return err } @@ -183,30 +181,21 @@ func (glusterfsVolume *glusterfs) cleanup(dir string) error { return nil } -func (glusterfsVolume *glusterfs) execMount(hosts *api.Endpoints, path string, mountpoint string, readonly bool) error { +func (glusterfsVolume *glusterfs) setUpAtInternal(dir string) error { var errs error - var command exec.Cmd - var mountArgs []string - var opt []string - - // build option array - if readonly == true { - opt = []string{"-o", "ro"} - } else { - opt = []string{"-o", "rw"} + + options := []string{} + if glusterfsVolume.readonly { + options = append(options, "ro") } - l := len(hosts.Subsets) - // avoid mount storm, pick a host randomly + l := len(glusterfsVolume.hosts.Subsets) + // Avoid mount storm, pick a host randomly. start := rand.Int() % l - // iterate all hosts until mount succeeds. + // Iterate all hosts until mount succeeds. for i := start; i < start+l; i++ { - arg := []string{"-t", "glusterfs", hosts.Subsets[i%l].Addresses[0].IP + ":" + path, mountpoint} - mountArgs = append(arg, opt...) - glog.V(1).Infof("Glusterfs: mount cmd: mount %v", strings.Join(mountArgs, " ")) - command = glusterfsVolume.exe.Command("mount", mountArgs...) - - _, errs = command.CombinedOutput() + hostIP := glusterfsVolume.hosts.Subsets[i%l].Addresses[0].IP + errs = glusterfsVolume.mounter.Mount(hostIP+":"+glusterfsVolume.path, dir, "glusterfs", options) if errs == nil { return nil } diff --git a/pkg/volume/iscsi/disk_manager.go b/pkg/volume/iscsi/disk_manager.go index 55c1236d6ce26..a0e1490756f40 100644 --- a/pkg/volume/iscsi/disk_manager.go +++ b/pkg/volume/iscsi/disk_manager.go @@ -55,11 +55,11 @@ func diskSetUp(manager diskManager, disk iscsiDisk, volPath string, mounter moun return err } // Perform a bind mount to the full path to allow duplicate mounts of the same disk. - flags := uintptr(0) + options := []string{"bind"} if disk.readOnly { - flags = mount.FlagReadOnly + options = append(options, "ro") } - err = mounter.Mount(globalPDPath, volPath, "", mount.FlagBind|flags, "") + err = mounter.Mount(globalPDPath, volPath, "", options) if err != nil { glog.Errorf("failed to bind mount:%s", globalPDPath) return err @@ -83,8 +83,8 @@ func diskTearDown(manager diskManager, disk iscsiDisk, volPath string, mounter m glog.Errorf("failed to get reference count %s", volPath) return err } - if err := mounter.Unmount(volPath, 0); err != nil { - glog.Errorf("failed to umount %s", volPath) + if err := mounter.Unmount(volPath); err != nil { + glog.Errorf("failed to unmount %s", volPath) return err } // If len(refs) is 1, then all bind mounts have been removed, and the diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 80cf64044c2f9..b77aee65f7518 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -110,6 +110,11 @@ func (plugin *ISCSIPlugin) newCleanerInternal(volName string, podUID types.UID, }, nil } +func (plugin *ISCSIPlugin) execCommand(command string, args []string) ([]byte, error) { + cmd := plugin.exe.Command(command, args...) + return cmd.CombinedOutput() +} + type iscsiDisk struct { volName string podUID types.UID @@ -142,15 +147,13 @@ func (iscsi *iscsiDisk) SetUpAt(dir string) error { return err } globalPDPath := iscsi.manager.MakeGlobalPDName(*iscsi) - // make mountpoint rw/ro work as expected - //FIXME revisit pkg/util/mount and ensure rw/ro is implemented as expected - mode := "rw" + var options []string if iscsi.readOnly { - mode = "ro" + options = []string{"remount", "ro"} + } else { + options = []string{"remount", "rw"} } - iscsi.plugin.execCommand("mount", []string{"-o", "remount," + mode, globalPDPath, dir}) - - return nil + return iscsi.mounter.Mount(globalPDPath, dir, "", options) } // Unmounts the bind mount, and detaches the disk only if the disk @@ -162,8 +165,3 @@ func (iscsi *iscsiDisk) TearDown() error { func (iscsi *iscsiDisk) TearDownAt(dir string) error { return diskTearDown(iscsi.manager, *iscsi, dir, iscsi.mounter) } - -func (plugin *ISCSIPlugin) execCommand(command string, args []string) ([]byte, error) { - cmd := plugin.exe.Command(command, args...) - return cmd.CombinedOutput() -} diff --git a/pkg/volume/iscsi/iscsi_test.go b/pkg/volume/iscsi/iscsi_test.go index bc1396de94530..eed6a99930fa0 100644 --- a/pkg/volume/iscsi/iscsi_test.go +++ b/pkg/volume/iscsi/iscsi_test.go @@ -55,7 +55,7 @@ func (fake *fakeDiskManager) AttachDisk(disk iscsiDisk) error { } // Simulate the global mount so that the fakeMounter returns the // expected number of mounts for the attached disk. - disk.mounter.Mount(globalPath, globalPath, disk.fsType, 0, "") + disk.mounter.Mount(globalPath, globalPath, disk.fsType, nil) fake.attachCalled = true return nil diff --git a/pkg/volume/iscsi/iscsi_util.go b/pkg/volume/iscsi/iscsi_util.go index 8903ff93bf534..477ce56b024e1 100644 --- a/pkg/volume/iscsi/iscsi_util.go +++ b/pkg/volume/iscsi/iscsi_util.go @@ -112,7 +112,7 @@ func (util *ISCSIUtil) AttachDisk(iscsi iscsiDisk) error { return err } - err = iscsi.mounter.Mount(devicePath, globalPDPath, iscsi.fsType, uintptr(0), "") + err = iscsi.mounter.Mount(devicePath, globalPDPath, iscsi.fsType, nil) if err != nil { glog.Errorf("iscsi: failed to mount iscsi volume %s [%s] to %s, error %v", devicePath, iscsi.fsType, globalPDPath, err) } @@ -126,8 +126,8 @@ func (util *ISCSIUtil) DetachDisk(iscsi iscsiDisk, mntPath string) error { glog.Errorf("iscsi detach disk: failed to get device from mnt: %s\nError: %v", mntPath, err) return err } - if err = iscsi.mounter.Unmount(mntPath, 0); err != nil { - glog.Errorf("iscsi detach disk: failed to umount: %s\nError: %v", mntPath, err) + if err = iscsi.mounter.Unmount(mntPath); err != nil { + glog.Errorf("iscsi detach disk: failed to unmount: %s\nError: %v", mntPath, err) return err } cnt-- diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index 9d09b33edda15..edffd8dd6f4bf 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -17,23 +17,25 @@ limitations under the License. package nfs import ( + "fmt" "os" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/types" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/mount" "github.com/GoogleCloudPlatform/kubernetes/pkg/volume" "github.com/golang/glog" ) // This is the primary entrypoint for volume plugins. func ProbeVolumePlugins() []volume.VolumePlugin { - return []volume.VolumePlugin{&nfsPlugin{nil, newNFSMounter()}} + return []volume.VolumePlugin{&nfsPlugin{nil, mount.New()}} } type nfsPlugin struct { host volume.VolumeHost - mounter nfsMountInterface + mounter mount.Interface } var _ volume.VolumePlugin = &nfsPlugin{} @@ -66,7 +68,7 @@ func (plugin *nfsPlugin) NewBuilder(spec *volume.Spec, podRef *api.ObjectReferen return plugin.newBuilderInternal(spec, podRef, plugin.mounter) } -func (plugin *nfsPlugin) newBuilderInternal(spec *volume.Spec, podRef *api.ObjectReference, mounter nfsMountInterface) (volume.Builder, error) { +func (plugin *nfsPlugin) newBuilderInternal(spec *volume.Spec, podRef *api.ObjectReference, mounter mount.Interface) (volume.Builder, error) { return &nfs{ volName: spec.Name, server: spec.VolumeSource.NFS.Server, @@ -82,7 +84,7 @@ func (plugin *nfsPlugin) NewCleaner(volName string, podUID types.UID) (volume.Cl return plugin.newCleanerInternal(volName, podUID, plugin.mounter) } -func (plugin *nfsPlugin) newCleanerInternal(volName string, podUID types.UID, mounter nfsMountInterface) (volume.Cleaner, error) { +func (plugin *nfsPlugin) newCleanerInternal(volName string, podUID types.UID, mounter mount.Interface) (volume.Cleaner, error) { return &nfs{ volName: volName, server: "", @@ -101,7 +103,7 @@ type nfs struct { server string exportPath string readOnly bool - mounter nfsMountInterface + mounter mount.Interface plugin *nfsPlugin } @@ -119,9 +121,13 @@ func (nfsVolume *nfs) SetUpAt(dir string) error { if mountpoint { return nil } - exportDir := nfsVolume.exportPath os.MkdirAll(dir, 0750) - err = nfsVolume.mounter.Mount(nfsVolume.server, exportDir, dir, nfsVolume.readOnly) + source := fmt.Sprintf("%s:%s", nfsVolume.server, nfsVolume.exportPath) + options := []string{} + if nfsVolume.readOnly { + options = append(options, "ro") + } + err = nfsVolume.mounter.Mount(source, dir, "nfs", options) if err != nil { mountpoint, mntErr := nfsVolume.mounter.IsMountPoint(dir) if mntErr != nil { diff --git a/pkg/volume/nfs/nfs_mount.go b/pkg/volume/nfs/nfs_mount.go deleted file mode 100644 index 9c3c2b4050d8b..0000000000000 --- a/pkg/volume/nfs/nfs_mount.go +++ /dev/null @@ -1,72 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package nfs - -import ( - "os/exec" - - "github.com/GoogleCloudPlatform/kubernetes/pkg/util/mount" - "github.com/golang/glog" -) - -type nfsMountInterface interface { - // Mount takes an NFS host ip or hostname, a source directory (the exported directory), a target directory where the source directory will be mounted, and a boolean readOnly - Mount(server string, source string, target string, readOnly bool) error - - // Umount wraps syscall.Mount(). - Unmount(target string) error - - List() ([]mount.MountPoint, error) - - IsMountPoint(dir string) (bool, error) -} - -// newNFSMounter returns an nfsMountInterface for the current system. -func newNFSMounter() nfsMountInterface { - return &nfsMounter{} -} - -type nfsMounter struct{} - -func (mounter *nfsMounter) Mount(server string, exportDir string, mountDir string, readOnly bool) error { - mountOptions := "rw" - if readOnly { - mountOptions = "ro" - } - mountArgs := []string{"-t", "nfs", server + ":" + exportDir, mountDir, "-o", mountOptions} - command := exec.Command("mount", mountArgs...) - output, errs := command.CombinedOutput() - if errs != nil { - glog.Errorf("NFS mounting failed: %v\n\tMount args are: %v\n\texportDir is: %v\n\tmountDir is: %v\n\tserver is: %v\n\tmount output is: %v", errs, mountArgs, exportDir, mountDir, server, string(output)) - return errs - } - return nil -} - -func (mounter *nfsMounter) Unmount(target string) error { - unmounter := mount.New() - return unmounter.Unmount(target, 0) -} - -func (mounter *nfsMounter) List() ([]mount.MountPoint, error) { - return nil, nil -} - -func (mounter *nfsMounter) IsMountPoint(dir string) (bool, error) { - isMounter := mount.New() - return isMounter.IsMountPoint(dir) -} diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go index a67538fb0665e..28428939669f7 100644 --- a/pkg/volume/nfs/nfs_test.go +++ b/pkg/volume/nfs/nfs_test.go @@ -17,7 +17,6 @@ limitations under the License. package nfs import ( - "fmt" "os" "testing" @@ -67,35 +66,6 @@ func contains(modes []api.AccessModeType, mode api.AccessModeType) bool { return false } -type fakeNFSMounter struct { - FakeMounter mount.FakeMounter -} - -func (fake *fakeNFSMounter) Mount(server string, source string, target string, readOnly bool) error { - flags := 0 - if readOnly { - flags |= mount.FlagReadOnly - } - fake.FakeMounter.MountPoints = append(fake.FakeMounter.MountPoints, mount.MountPoint{Device: server, Path: target, Type: "nfs", Opts: nil, Freq: 0, Pass: 0}) - return fake.FakeMounter.Mount(fmt.Sprintf("%s:%s", server, source), target, "nfs", 0, "") -} - -func (fake *fakeNFSMounter) Unmount(target string) error { - fake.FakeMounter.MountPoints = []mount.MountPoint{} - return fake.FakeMounter.Unmount(target, 0) -} - -func (fake *fakeNFSMounter) List() ([]mount.MountPoint, error) { - list, _ := fake.FakeMounter.List() - return list, nil -} - -func (fake *fakeNFSMounter) IsMountPoint(dir string) (bool, error) { - list, _ := fake.FakeMounter.List() - isMount := len(list) > 0 - return isMount, nil -} - func TestPlugin(t *testing.T) { plugMgr := volume.VolumePluginMgr{} plugMgr.InitPlugins(ProbeVolumePlugins(), volume.NewFakeVolumeHost("/tmp/fake", nil, nil)) @@ -107,7 +77,7 @@ func TestPlugin(t *testing.T) { Name: "vol1", VolumeSource: api.VolumeSource{NFS: &api.NFSVolumeSource{"localhost", "/tmp", false}}, } - fake := &fakeNFSMounter{} + fake := &mount.FakeMounter{} builder, err := plug.(*nfsPlugin).newBuilderInternal(volume.NewSpecFromVolume(spec), &api.ObjectReference{UID: types.UID("poduid")}, fake) volumePath := builder.GetPath() if err != nil { @@ -133,14 +103,14 @@ func TestPlugin(t *testing.T) { if builder.(*nfs).readOnly { t.Errorf("The volume source should not be read-only and it is.") } - if len(fake.FakeMounter.Log) != 1 { - t.Errorf("Mount was not called exactly one time. It was called %d times.", len(fake.FakeMounter.Log)) + if len(fake.Log) != 1 { + t.Errorf("Mount was not called exactly one time. It was called %d times.", len(fake.Log)) } else { - if fake.FakeMounter.Log[0].Action != mount.FakeActionMount { - t.Errorf("Unexpected mounter action: %#v", fake.FakeMounter.Log[0]) + if fake.Log[0].Action != mount.FakeActionMount { + t.Errorf("Unexpected mounter action: %#v", fake.Log[0]) } } - fake.FakeMounter.ResetLog() + fake.ResetLog() cleaner, err := plug.(*nfsPlugin).newCleanerInternal("vol1", types.UID("poduid"), fake) if err != nil { @@ -157,13 +127,13 @@ func TestPlugin(t *testing.T) { } else if !os.IsNotExist(err) { t.Errorf("SetUp() failed: %v", err) } - if len(fake.FakeMounter.Log) != 1 { - t.Errorf("Unmount was not called exactly one time. It was called %d times.", len(fake.FakeMounter.Log)) + if len(fake.Log) != 1 { + t.Errorf("Unmount was not called exactly one time. It was called %d times.", len(fake.Log)) } else { - if fake.FakeMounter.Log[0].Action != mount.FakeActionUnmount { - t.Errorf("Unexpected mounter action: %#v", fake.FakeMounter.Log[0]) + if fake.Log[0].Action != mount.FakeActionUnmount { + t.Errorf("Unexpected mounter action: %#v", fake.Log[0]) } } - fake.FakeMounter.ResetLog() + fake.ResetLog() }