From 596cdffb9fdea5323ccd7196f196544912b59c1f Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Wed, 10 Oct 2018 19:20:13 +0900 Subject: [PATCH] mount: add BindOptions.NonRecursive (API v1.40) This allows non-recursive bind-mount, i.e. mount(2) with "bind" rather than "rbind". Swarm-mode will be supported in a separate PR because of mutual vendoring. Signed-off-by: Akihiro Suda --- .../router/container/container_routes.go | 10 ++ api/swagger.yaml | 4 + api/types/mount/mount.go | 3 +- container/mounts_unix.go | 11 +- daemon/cluster/convert/container.go | 6 + daemon/oci_linux.go | 6 +- daemon/volumes_unix.go | 16 ++- docs/api/version-history.md | 2 + integration/container/mounts_linux_test.go | 108 ++++++++++++++---- integration/internal/container/ops.go | 8 ++ integration/internal/container/states.go | 18 +++ volume/mounts/linux_parser.go | 3 + 12 files changed, 160 insertions(+), 35 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 9282cea09c..c1f32724ee 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -465,6 +465,16 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo hostConfig.AutoRemove = false } + // When using API 1.39 and under, BindOptions.NonRecursive should be ignored because it + // was added in API 1.40. + if hostConfig != nil && versions.LessThan(version, "1.40") { + for _, m := range hostConfig.Mounts { + if bo := m.BindOptions; bo != nil { + bo.NonRecursive = false + } + } + } + ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{ Name: name, Config: config, diff --git a/api/swagger.yaml b/api/swagger.yaml index 3bf0d8f0b1..c2057b25d3 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -265,6 +265,10 @@ definitions: - "rshared" - "slave" - "rslave" + NonRecursive: + description: "Disable recursive bind mount." + type: "boolean" + default: false VolumeOptions: description: "Optional configuration for the `volume` type." type: "object" diff --git a/api/types/mount/mount.go b/api/types/mount/mount.go index 3fef974df8..ab4446b38f 100644 --- a/api/types/mount/mount.go +++ b/api/types/mount/mount.go @@ -79,7 +79,8 @@ const ( // BindOptions defines options specific to mounts of type "bind". type BindOptions struct { - Propagation Propagation `json:",omitempty"` + Propagation Propagation `json:",omitempty"` + NonRecursive bool `json:",omitempty"` } // VolumeOptions represents the options for a mount of type volume. diff --git a/container/mounts_unix.go b/container/mounts_unix.go index 62f4441dce..2c1160464b 100644 --- a/container/mounts_unix.go +++ b/container/mounts_unix.go @@ -4,9 +4,10 @@ package container // import "github.com/docker/docker/container" // Mount contains information for a mount operation. type Mount struct { - Source string `json:"source"` - Destination string `json:"destination"` - Writable bool `json:"writable"` - Data string `json:"data"` - Propagation string `json:"mountpropagation"` + Source string `json:"source"` + Destination string `json:"destination"` + Writable bool `json:"writable"` + Data string `json:"data"` + Propagation string `json:"mountpropagation"` + NonRecursive bool `json:"nonrecursive"` } diff --git a/daemon/cluster/convert/container.go b/daemon/cluster/convert/container.go index effd362a12..37f562ad26 100644 --- a/daemon/cluster/convert/container.go +++ b/daemon/cluster/convert/container.go @@ -321,6 +321,12 @@ func containerToGRPC(c *types.ContainerSpec) (*swarmapi.ContainerSpec, error) { } else if string(m.BindOptions.Propagation) != "" { return nil, fmt.Errorf("invalid MountPropagation: %q", m.BindOptions.Propagation) } + + if m.BindOptions.NonRecursive { + // TODO(AkihiroSuda): NonRecursive is unsupported for Swarm-mode now because of mutual vendoring + // across moby and swarmkit. Will be available soon after the moby PR gets merged. + return nil, fmt.Errorf("invalid NonRecursive: %q", m.BindOptions.Propagation) + } } if m.VolumeOptions != nil { diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index dfe02796a5..83d9b03a7f 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -565,7 +565,11 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c } } - opts := []string{"rbind"} + bindMode := "rbind" + if m.NonRecursive { + bindMode = "bind" + } + opts := []string{bindMode} if !m.Writable { opts = append(opts, "ro") } diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index 5ddb926de7..5b47c46616 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" + mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/container" "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/mount" @@ -58,6 +59,9 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er Writable: m.RW, Propagation: string(m.Propagation), } + if m.Spec.Type == mounttypes.TypeBind && m.Spec.BindOptions != nil { + mnt.NonRecursive = m.Spec.BindOptions.NonRecursive + } if m.Volume != nil { attributes := map[string]string{ "driver": m.Volume.DriverName(), @@ -129,11 +133,15 @@ func (daemon *Daemon) mountVolumes(container *container.Container) error { return err } - opts := "rbind,ro" - if m.Writable { - opts = "rbind,rw" + bindMode := "rbind" + if m.NonRecursive { + bindMode = "bind" } - + writeMode := "ro" + if m.Writable { + writeMode = "rw" + } + opts := strings.Join([]string{bindMode, writeMode}, ",") if err := mount.Mount(m.Source, dest, bindMountType, opts); err != nil { return err } diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 83ba5af87a..611ac38fae 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -27,6 +27,8 @@ keywords: "API, Docker, rcli, REST, documentation" on the node.label. The format of the label filter is `node.label=`/`node.label==` to return those with the specified labels, or `node.label!=`/`node.label!==` to return those without the specified labels. +* `POST /containers/create`, `GET /containers/{id}/json`, and `GET /containers/json` now supports + `BindOptions.NonRecursive`. ## V1.39 API changes diff --git a/integration/container/mounts_linux_test.go b/integration/container/mounts_linux_test.go index 8e7e663fd4..2a25b018fa 100644 --- a/integration/container/mounts_linux_test.go +++ b/integration/container/mounts_linux_test.go @@ -5,17 +5,22 @@ import ( "fmt" "path/filepath" "testing" + "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/mount" + containertypes "github.com/docker/docker/api/types/container" + mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/versions" "github.com/docker/docker/client" + "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/internal/test/request" + "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/system" "gotest.tools/assert" is "gotest.tools/assert/cmp" "gotest.tools/fs" + "gotest.tools/poll" "gotest.tools/skip" ) @@ -32,11 +37,11 @@ func TestContainerNetworkMountsNoChown(t *testing.T) { tmpNWFileMount := tmpDir.Join("nwfile") - config := container.Config{ + config := containertypes.Config{ Image: "busybox", } - hostConfig := container.HostConfig{ - Mounts: []mount.Mount{ + hostConfig := containertypes.HostConfig{ + Mounts: []mounttypes.Mount{ { Type: "bind", Source: tmpNWFileMount, @@ -93,39 +98,39 @@ func TestMountDaemonRoot(t *testing.T) { for _, test := range []struct { desc string - propagation mount.Propagation - expected mount.Propagation + propagation mounttypes.Propagation + expected mounttypes.Propagation }{ { desc: "default", propagation: "", - expected: mount.PropagationRSlave, + expected: mounttypes.PropagationRSlave, }, { desc: "private", - propagation: mount.PropagationPrivate, + propagation: mounttypes.PropagationPrivate, }, { desc: "rprivate", - propagation: mount.PropagationRPrivate, + propagation: mounttypes.PropagationRPrivate, }, { desc: "slave", - propagation: mount.PropagationSlave, + propagation: mounttypes.PropagationSlave, }, { desc: "rslave", - propagation: mount.PropagationRSlave, - expected: mount.PropagationRSlave, + propagation: mounttypes.PropagationRSlave, + expected: mounttypes.PropagationRSlave, }, { desc: "shared", - propagation: mount.PropagationShared, + propagation: mounttypes.PropagationShared, }, { desc: "rshared", - propagation: mount.PropagationRShared, - expected: mount.PropagationRShared, + propagation: mounttypes.PropagationRShared, + expected: mounttypes.PropagationRShared, }, } { t.Run(test.desc, func(t *testing.T) { @@ -139,26 +144,26 @@ func TestMountDaemonRoot(t *testing.T) { bindSpecRoot := info.DockerRootDir + ":" + "/foo" + propagationSpec bindSpecSub := filepath.Join(info.DockerRootDir, "containers") + ":/foo" + propagationSpec - for name, hc := range map[string]*container.HostConfig{ + for name, hc := range map[string]*containertypes.HostConfig{ "bind root": {Binds: []string{bindSpecRoot}}, "bind subpath": {Binds: []string{bindSpecSub}}, "mount root": { - Mounts: []mount.Mount{ + Mounts: []mounttypes.Mount{ { - Type: mount.TypeBind, + Type: mounttypes.TypeBind, Source: info.DockerRootDir, Target: "/foo", - BindOptions: &mount.BindOptions{Propagation: test.propagation}, + BindOptions: &mounttypes.BindOptions{Propagation: test.propagation}, }, }, }, "mount subpath": { - Mounts: []mount.Mount{ + Mounts: []mounttypes.Mount{ { - Type: mount.TypeBind, + Type: mounttypes.TypeBind, Source: filepath.Join(info.DockerRootDir, "containers"), Target: "/foo", - BindOptions: &mount.BindOptions{Propagation: test.propagation}, + BindOptions: &mounttypes.BindOptions{Propagation: test.propagation}, }, }, }, @@ -167,7 +172,7 @@ func TestMountDaemonRoot(t *testing.T) { hc := hc t.Parallel() - c, err := client.ContainerCreate(ctx, &container.Config{ + c, err := client.ContainerCreate(ctx, &containertypes.Config{ Image: "busybox", Cmd: []string{"true"}, }, hc, nil, "") @@ -206,3 +211,58 @@ func TestMountDaemonRoot(t *testing.T) { }) } } + +func TestContainerBindMountNonRecursive(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux" || testEnv.IsRemoteDaemon()) + skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "BindOptions.NonRecursive requires API v1.40") + + defer setupTest(t)() + + tmpDir1 := fs.NewDir(t, "tmpdir1", fs.WithMode(0755), + fs.WithDir("mnt", fs.WithMode(0755))) + defer tmpDir1.Remove() + tmpDir1Mnt := filepath.Join(tmpDir1.Path(), "mnt") + tmpDir2 := fs.NewDir(t, "tmpdir2", fs.WithMode(0755), + fs.WithFile("file", "should not be visible when NonRecursive", fs.WithMode(0644))) + defer tmpDir2.Remove() + + err := mount.Mount(tmpDir2.Path(), tmpDir1Mnt, "none", "bind,ro") + if err != nil { + t.Fatal(err) + } + defer func() { + if err := mount.Unmount(tmpDir1Mnt); err != nil { + t.Fatal(err) + } + }() + + // implicit is recursive (NonRecursive: false) + implicit := mounttypes.Mount{ + Type: "bind", + Source: tmpDir1.Path(), + Target: "/foo", + ReadOnly: true, + } + recursive := implicit + recursive.BindOptions = &mounttypes.BindOptions{ + NonRecursive: false, + } + recursiveVerifier := []string{"test", "-f", "/foo/mnt/file"} + nonRecursive := implicit + nonRecursive.BindOptions = &mounttypes.BindOptions{ + NonRecursive: true, + } + nonRecursiveVerifier := []string{"test", "!", "-f", "/foo/mnt/file"} + + ctx := context.Background() + client := request.NewAPIClient(t) + containers := []string{ + container.Run(t, ctx, client, container.WithMount(implicit), container.WithCmd(recursiveVerifier...)), + container.Run(t, ctx, client, container.WithMount(recursive), container.WithCmd(recursiveVerifier...)), + container.Run(t, ctx, client, container.WithMount(nonRecursive), container.WithCmd(nonRecursiveVerifier...)), + } + + for _, c := range containers { + poll.WaitOn(t, container.IsSuccessful(ctx, client, c), poll.WithDelay(100*time.Millisecond)) + } +} diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index df5598b62f..3825981b67 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -4,6 +4,7 @@ import ( "fmt" containertypes "github.com/docker/docker/api/types/container" + mounttypes "github.com/docker/docker/api/types/mount" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/strslice" "github.com/docker/go-connections/nat" @@ -68,6 +69,13 @@ func WithWorkingDir(dir string) func(*TestContainerConfig) { } } +// WithMount adds an mount +func WithMount(m mounttypes.Mount) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.Mounts = append(c.HostConfig.Mounts, m) + } +} + // WithVolume sets the volume of the container func WithVolume(name string) func(*TestContainerConfig) { return func(c *TestContainerConfig) { diff --git a/integration/internal/container/states.go b/integration/internal/container/states.go index 088407deb8..2397159188 100644 --- a/integration/internal/container/states.go +++ b/integration/internal/container/states.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/docker/docker/client" + "github.com/pkg/errors" "gotest.tools/poll" ) @@ -39,3 +40,20 @@ func IsInState(ctx context.Context, client client.APIClient, containerID string, return poll.Continue("waiting for container to be one of (%s), currently %s", strings.Join(state, ", "), inspect.State.Status) } } + +// IsSuccessful verifies state.Status == "exited" && state.ExitCode == 0 +func IsSuccessful(ctx context.Context, client client.APIClient, containerID string) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + inspect, err := client.ContainerInspect(ctx, containerID) + if err != nil { + return poll.Error(err) + } + if inspect.State.Status == "exited" { + if inspect.State.ExitCode == 0 { + return poll.Success() + } + return poll.Error(errors.Errorf("expected exit code 0, got %d", inspect.State.ExitCode)) + } + return poll.Continue("waiting for container to be \"exited\", currently %s", inspect.State.Status) + } +} diff --git a/volume/mounts/linux_parser.go b/volume/mounts/linux_parser.go index 8e436aec0e..035a24a8d9 100644 --- a/volume/mounts/linux_parser.go +++ b/volume/mounts/linux_parser.go @@ -97,6 +97,9 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour return &errMountConfig{mnt, fmt.Errorf("must not set ReadOnly mode when using anonymous volumes")} } case mount.TypeTmpfs: + if mnt.BindOptions != nil { + return &errMountConfig{mnt, errExtraField("BindOptions")} + } if len(mnt.Source) != 0 { return &errMountConfig{mnt, errExtraField("Source")} }