Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

Commit dcd9e43

Browse files
committed
Check tmpfs mounts before create anon volume
This makes sure that things like `--tmpfs` mounts over an anonymous volume don't create volumes uneccessarily. One method only checks mountpoints, the other checks both mountpoints and tmpfs... the usage of these should likely be consolidated. Ideally, processing for `--tmpfs` mounts would get merged in with the rest of the mount parsing. I opted not to do that for this change so the fix is minimal and can potentially be backported with fewer changes of breaking things. Merging the mount processing for tmpfs can be handled in a followup. Signed-off-by: Brian Goff <cpuguy83@gmail.com> (cherry picked from commit f464c31668db6fe781b3195f23dc786ee09e91c0) Signed-off-by: Brian Goff <cpuguy83@gmail.com> Upstream-commit: 1d8da80dbf8212f177a4cc4f44bf8dc43e810afc Component: engine
1 parent 92806c1 commit dcd9e43

3 files changed

Lines changed: 69 additions & 3 deletions

File tree

components/engine/daemon/create_unix.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
4646

4747
// Skip volumes for which we already have something mounted on that
4848
// destination because of a --volume-from.
49-
if container.IsDestinationMounted(destination) {
49+
if container.HasMountFor(destination) {
50+
logrus.WithField("container", container.ID).WithField("destination", spec).Debug("mountpoint already exists, skipping anonymous volume")
51+
// Not an error, this could easily have come from the image config.
5052
continue
5153
}
5254
path, err := container.GetResourcePath(destination)

components/engine/integration/container/create_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,3 +535,50 @@ func TestCreateWithInvalidHealthcheckParams(t *testing.T) {
535535
})
536536
}
537537
}
538+
539+
// Make sure that anonymous volumes can be overritten by tmpfs
540+
// https://github.com/moby/moby/issues/40446
541+
func TestCreateTmpfsOverrideAnonymousVolume(t *testing.T) {
542+
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "windows does not support tmpfs")
543+
defer setupTest(t)()
544+
client := testEnv.APIClient()
545+
ctx := context.Background()
546+
547+
id := ctr.Create(ctx, t, client,
548+
ctr.WithVolume("/foo"),
549+
ctr.WithTmpfs("/foo"),
550+
ctr.WithVolume("/bar"),
551+
ctr.WithTmpfs("/bar:size=999"),
552+
ctr.WithCmd("/bin/sh", "-c", "mount | grep '/foo' | grep tmpfs && mount | grep '/bar' | grep tmpfs"),
553+
)
554+
555+
defer func() {
556+
err := client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{Force: true})
557+
assert.NilError(t, err)
558+
}()
559+
560+
inspect, err := client.ContainerInspect(ctx, id)
561+
assert.NilError(t, err)
562+
// tmpfs do not currently get added to inspect.Mounts
563+
// Normally an anoynmous volume would, except now tmpfs should prevent that.
564+
assert.Assert(t, is.Len(inspect.Mounts, 0))
565+
566+
chWait, chErr := client.ContainerWait(ctx, id, container.WaitConditionNextExit)
567+
assert.NilError(t, client.ContainerStart(ctx, id, types.ContainerStartOptions{}))
568+
569+
timeout := time.NewTimer(30 * time.Second)
570+
defer timeout.Stop()
571+
572+
select {
573+
case <-timeout.C:
574+
t.Fatal("timeout waiting for container to exit")
575+
case status := <-chWait:
576+
var errMsg string
577+
if status.Error != nil {
578+
errMsg = status.Error.Message
579+
}
580+
assert.Equal(t, int(status.StatusCode), 0, errMsg)
581+
case err := <-chErr:
582+
assert.NilError(t, err)
583+
}
584+
}

components/engine/integration/internal/container/ops.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package container
22

33
import (
44
"fmt"
5+
"strings"
56

67
containertypes "github.com/docker/docker/api/types/container"
78
mounttypes "github.com/docker/docker/api/types/mount"
@@ -77,12 +78,12 @@ func WithMount(m mounttypes.Mount) func(*TestContainerConfig) {
7778
}
7879

7980
// WithVolume sets the volume of the container
80-
func WithVolume(name string) func(*TestContainerConfig) {
81+
func WithVolume(target string) func(*TestContainerConfig) {
8182
return func(c *TestContainerConfig) {
8283
if c.Config.Volumes == nil {
8384
c.Config.Volumes = map[string]struct{}{}
8485
}
85-
c.Config.Volumes[name] = struct{}{}
86+
c.Config.Volumes[target] = struct{}{}
8687
}
8788
}
8889

@@ -93,6 +94,22 @@ func WithBind(src, target string) func(*TestContainerConfig) {
9394
}
9495
}
9596

97+
// WithTmpfs sets a target path in the container to a tmpfs
98+
func WithTmpfs(target string) func(config *TestContainerConfig) {
99+
return func(c *TestContainerConfig) {
100+
if c.HostConfig.Tmpfs == nil {
101+
c.HostConfig.Tmpfs = make(map[string]string)
102+
}
103+
104+
spec := strings.SplitN(target, ":", 2)
105+
var opts string
106+
if len(spec) > 1 {
107+
opts = spec[1]
108+
}
109+
c.HostConfig.Tmpfs[spec[0]] = opts
110+
}
111+
}
112+
96113
// WithIPv4 sets the specified ip for the specified network of the container
97114
func WithIPv4(network, ip string) func(*TestContainerConfig) {
98115
return func(c *TestContainerConfig) {

0 commit comments

Comments
 (0)