From fc53d6436776d5de052075e98f44417f04ced7e7 Mon Sep 17 00:00:00 2001 From: moricho Date: Wed, 22 Apr 2020 15:04:18 +0900 Subject: refactor and add test for bindmount Signed-off-by: moricho --- runsc/boot/fs.go | 59 ++++++++++++--------------------- runsc/container/container_test.go | 22 ++++++++++++ runsc/container/multi_container_test.go | 2 +- runsc/specutils/specutils.go | 14 +++++++- 4 files changed, 57 insertions(+), 40 deletions(-) (limited to 'runsc') 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. -- cgit v1.2.3