refactor and add test for bindmount

Signed-off-by: moricho <ikeda.morito@gmail.com>

4 files changed, 57 insertions, 40 deletions

diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go
index 78d6a0c14..4875452e2 100644
--- a/runsc/boot/fs.go
+++ b/runsc/boot/fs.go
@@ -63,13 +63,8 @@ const (
 	nonefs   = "none"
 )
 
-var (
-	// tmpfs has some extra supported options that we must pass through.
-	tmpfsAllowedOptions = []string{"mode", "uid", "gid"}
-
-	// filesystems supported on gVisor.
-	supportedFilesystems = []string{bind, devpts, devtmpfs, proc, sysfs, tmpfs}
-)
+// tmpfs has some extra supported options that we must pass through.
+var tmpfsAllowedOptions = []string{"mode", "uid", "gid"}
 
 func addOverlay(ctx context.Context, conf *Config, lower *fs.Inode, name string, lowerFlags fs.MountSourceFlags) (*fs.Inode, error) {
 	// Upper layer uses the same flags as lower, but it must be read-write.
@@ -225,7 +220,8 @@ func mountFlags(opts []string) fs.MountSourceFlags {
 		case "noexec":
 			mf.NoExec = true
 		case "bind", "rbind":
-			mf.Bind = true
+			// When options include either "bind" or "rbind",
+			// it's converted to a 9P mount.
 		default:
 			log.Warningf("ignoring unknown mount option %q", o)
 		}
@@ -237,10 +233,6 @@ func isSupportedMountFlag(fstype, opt string) bool {
 	switch opt {
 	case "rw", "ro", "noatime", "noexec":
 		return true
-	case "bind", "rbind":
-		if fstype == nonefs || !specutils.ContainsStr(supportedFilesystems, fstype) {
-			return true
-		}
 	}
 	if fstype == tmpfs {
 		ok, err := parseMountOption(opt, tmpfsAllowedOptions...)
@@ -767,17 +759,6 @@ func (c *containerMounter) createRootMount(ctx context.Context, conf *Config) (*
 	return rootInode, nil
 }
 
-// getBindMountNameAndOptions retrieves the fsName, opts, and useOverlay values
-// used for bind mounts.
-func (c *containerMounter) getBindMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool) {
-	fd := c.fds.remove()
-	fsName := "9p"
-	opts := p9MountOptions(fd, c.getMountAccessType(m))
-	// If configured, add overlay to all writable mounts.
-	useOverlay := conf.Overlay && !mountFlags(m.Options).ReadOnly
-	return fsName, opts, useOverlay
-}
-
 // getMountNameAndOptions retrieves the fsName, opts, and useOverlay values
 // used for mounts.
 func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool, error) {
@@ -787,17 +768,20 @@ func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) (
 		useOverlay bool
 	)
 
+	for _, opt := range m.Options {
+		// When options include either "bind" or "rbind", this behaves as
+		// bind mount even if the mount type is equal to a filesystem supported
+		// on runsc.
+		if opt == "bind" || opt == "rbind" {
+			m.Type = bind
+			break
+		}
+	}
+
 	switch m.Type {
 	case devpts, devtmpfs, proc, sysfs:
 		fsName = m.Type
 	case nonefs:
-		for _, opt := range m.Options {
-			if opt == "bind" || opt == "rbind" {
-				fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m)
-				return fsName, opts, useOverlay, nil
-			}
-		}
-
 		fsName = sysfs
 	case tmpfs:
 		fsName = m.Type
@@ -807,16 +791,15 @@ func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) (
 		if err != nil {
 			return "", nil, false, err
 		}
+
 	case bind:
-		fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m)
-	default:
-		for _, opt := range m.Options {
-			if opt == "bind" || opt == "rbind" {
-				fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m)
-				return fsName, opts, useOverlay, nil
-			}
-		}
+		fd := c.fds.remove()
+		fsName = "9p"
+		opts = p9MountOptions(fd, c.getMountAccessType(m))
+		// If configured, add overlay to all writable mounts.
+		useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly
 
+	default:
 		log.Warningf("ignoring unknown filesystem type %q", m.Type)
 	}
 	return fsName, opts, useOverlay, nil
diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go
index 3ff89f38c..c963c2153 100644
--- a/runsc/container/container_test.go
+++ b/runsc/container/container_test.go
@@ -1523,6 +1523,28 @@ func TestReadonlyMount(t *testing.T) {
 	}
 }
 
+func TestBindMountByOption(t *testing.T) {
+	for _, conf := range configs(t, overlay) {
+		t.Logf("Running test with conf: %+v", conf)
+
+		dir, err := ioutil.TempDir(testutil.TmpDir(), "bind-mount")
+		spec := testutil.NewSpecWithArgs("/bin/touch", path.Join(dir, "file"))
+		if err != nil {
+			t.Fatalf("ioutil.TempDir() failed: %v", err)
+		}
+		spec.Mounts = append(spec.Mounts, specs.Mount{
+			Destination: dir,
+			Source:      dir,
+			Type:        "none",
+			Options:     []string{"rw", "bind"},
+		})
+
+		if err := run(spec, conf); err != nil {
+			t.Fatalf("error running sandbox: %v", err)
+		}
+	}
+}
+
 // TestAbbreviatedIDs checks that runsc supports using abbreviated container
 // IDs in place of full IDs.
 func TestAbbreviatedIDs(t *testing.T) {
diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go
index e3704b453..f6861b1dd 100644
--- a/runsc/container/multi_container_test.go
+++ b/runsc/container/multi_container_test.go
@@ -1394,7 +1394,7 @@ func TestMultiContainerSharedMountUnsupportedOptions(t *testing.T) {
 		Destination: "/mydir/test",
 		Source:      "/some/dir",
 		Type:        "tmpfs",
-		Options:     []string{"rw", "rbind", "relatime"},
+		Options:     []string{"rw", "relatime"},
 	}
 	podSpec[0].Mounts = append(podSpec[0].Mounts, mnt0)
 
diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go
index 837d5e238..202518b58 100644
--- a/runsc/specutils/specutils.go
+++ b/runsc/specutils/specutils.go
@@ -311,7 +311,19 @@ func capsFromNames(names []string, skipSet map[linux.Capability]struct{}) (auth.
 
 // Is9PMount returns true if the given mount can be mounted as an external gofer.
 func Is9PMount(m specs.Mount) bool {
-	return m.Type == "bind" && m.Source != "" && IsSupportedDevMount(m)
+	var isBind bool
+	switch m.Type {
+	case "bind":
+		isBind = true
+	default:
+		for _, opt := range m.Options {
+			if opt == "bind" || opt == "rbind" {
+				isBind = true
+				break
+			}
+		}
+	}
+	return isBind && m.Source != "" && IsSupportedDevMount(m)
 }
 
 // IsSupportedDevMount returns true if the mount is a supported /dev mount.