Skip to content

Commit

Permalink
mount: add BindOptions.NonRecursive (API v1.40)
Browse files Browse the repository at this point in the history
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 <suda.akihiro@lab.ntt.co.jp>
  • Loading branch information
AkihiroSuda committed Nov 6, 2018
1 parent 12bba16 commit 596cdff
Show file tree
Hide file tree
Showing 12 changed files with 159 additions and 34 deletions.
10 changes: 10 additions & 0 deletions api/server/router/container/container_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion api/types/mount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 6 additions & 5 deletions container/mounts_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
6 changes: 6 additions & 0 deletions daemon/cluster/convert/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion daemon/oci_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
14 changes: 11 additions & 3 deletions daemon/volumes_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -129,11 +133,15 @@ func (daemon *Daemon) mountVolumes(container *container.Container) error {
return err
}

opts := "rbind,ro"
bindMode := "rbind"
if m.NonRecursive {
bindMode = "bind"
}
writeMode := "ro"
if m.Writable {
opts = "rbind,rw"
writeMode = "rw"
}

opts := strings.Join([]string{bindMode, writeMode}, ",")
if err := mount.Mount(m.Source, dest, bindMountType, opts); err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions docs/api/version-history.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ keywords: "API, Docker, rcli, REST, documentation"
on the node.label. The format of the label filter is `node.label=<key>`/`node.label=<key>=<value>`
to return those with the specified labels, or `node.label!=<key>`/`node.label!=<key>=<value>`
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

Expand Down
108 changes: 84 additions & 24 deletions integration/container/mounts_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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},
},
},
},
Expand All @@ -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, "")
Expand Down Expand Up @@ -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))
}
}
8 changes: 8 additions & 0 deletions integration/internal/container/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 18 additions & 0 deletions integration/internal/container/states.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

"github.com/docker/docker/client"
"github.com/pkg/errors"
"gotest.tools/poll"
)

Expand Down Expand Up @@ -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)
}
}
3 changes: 3 additions & 0 deletions volume/mounts/linux_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")}
}
Expand Down

0 comments on commit 596cdff

Please sign in to comment.