Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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."
Comment thread
AkihiroSuda marked this conversation as resolved.
Outdated
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"
Comment thread
AkihiroSuda marked this conversation as resolved.
Outdated
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())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably get rid of the remote daemon check if we do all the mounting in a container with shared propagation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you be more specific? Do you mean DIND?

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