Change mount.Interface.Mount to exec('mount'), instead of syscall

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, options []string) error {
 	newMountpoints := []MountPoint{}
 	for _, mp := range f.MountPoints {
 		if mp.Path != target {

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 target with given options.
+	Unmount(target string, options []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

pkg/util/mount/mount_linux.go

@@ -24,35 +24,105 @@ import (
 	"hash/adler32"
 	"io"
 	"os"
+	"os/exec"
 	"strconv"
 	"strings"
 	"syscall"
 
 	"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), 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. It equals:
+	//   options - 'bind' + 'remount' (no duplicate)
+	bindRemountOpts := []string{"remount"}
+	bind := false
+
+	if options != nil {
+		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 options != nil && 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, options []string) error {
+	glog.V(5).Infof("Unmounting %s %v", target, options)
+	unmountArgs := []string{}
+	if options != nil {
+		// If options is an empty list, this is a no-op.
+		unmountArgs = append(unmountArgs, options...)
+	}
+	unmountArgs = append(unmountArgs, target)
+	command := exec.Command("umount", unmountArgs...)
+	output, err := command.CombinedOutput()
+	if err != nil {
+		glog.Errorf("Unmount failed: %v\nUnmounting arguments: %s %v\nOutput: %s\n",
+			err, target, options, 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 +159,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) {

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, options []string) error {
 	return nil
 }
 

pkg/volume/aws_ebs/aws_ebs.go

@@ -192,27 +192,26 @@ 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)
 		return err
 	}
 
 	// 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 {
 			glog.Errorf("isMountpoint check failed: %v", mntErr)
 			return err
 		}
 		if mountpoint {
-			if mntErr = pd.mounter.Unmount(dir, 0); mntErr != nil {
+			if mntErr = pd.mounter.Unmount(dir, nil); 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, nil); err != nil {
 		glog.V(2).Info("Error unmounting dir ", dir, ": ", err)
 		return err
 	}

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, nil); 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()

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)
 		}

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, nil); err != nil {
 		return err
 	}
 	if err := os.RemoveAll(dir); err != nil {

pkg/volume/gce_pd/gce_pd.go

@@ -201,27 +201,26 @@ 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)
 		return err
 	}
 
 	// 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 {
 			glog.Errorf("isMountpoint check failed: %v", mntErr)
 			return err
 		}
 		if mountpoint {
-			if mntErr = pd.mounter.Unmount(dir, 0); mntErr != nil {
+			if mntErr = pd.mounter.Unmount(dir, nil); 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, nil); err != nil {
 		return err
 	}
 	// If len(refs) is 1, then all bind mounts have been removed, and the

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
 }