Browse code
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>
Brian Goff authored on 2020/02/04 05:28:19Showing 3 changed files
| ... | ... |
@@ -46,7 +46,9 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con |
| 46 | 46 |
|
| 47 | 47 |
// Skip volumes for which we already have something mounted on that |
| 48 | 48 |
// 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. |
|
| 50 | 52 |
continue |
| 51 | 53 |
} |
| 52 | 54 |
path, err := container.GetResourcePath(destination) |
| ... | ... |
@@ -534,3 +534,50 @@ func TestCreateWithInvalidHealthcheckParams(t *testing.T) {
|
| 534 | 534 |
}) |
| 535 | 535 |
} |
| 536 | 536 |
} |
| 537 |
+ |
|
| 538 |
+// Make sure that anonymous volumes can be overritten by tmpfs |
|
| 539 |
+// https://github.com/moby/moby/issues/40446 |
|
| 540 |
+func TestCreateTmpfsOverrideAnonymousVolume(t *testing.T) {
|
|
| 541 |
+ skip.If(t, testEnv.DaemonInfo.OSType == "windows", "windows does not support tmpfs") |
|
| 542 |
+ defer setupTest(t)() |
|
| 543 |
+ client := testEnv.APIClient() |
|
| 544 |
+ ctx := context.Background() |
|
| 545 |
+ |
|
| 546 |
+ id := ctr.Create(ctx, t, client, |
|
| 547 |
+ ctr.WithVolume("/foo"),
|
|
| 548 |
+ ctr.WithTmpfs("/foo"),
|
|
| 549 |
+ ctr.WithVolume("/bar"),
|
|
| 550 |
+ ctr.WithTmpfs("/bar:size=999"),
|
|
| 551 |
+ ctr.WithCmd("/bin/sh", "-c", "mount | grep '/foo' | grep tmpfs && mount | grep '/bar' | grep tmpfs"),
|
|
| 552 |
+ ) |
|
| 553 |
+ |
|
| 554 |
+ defer func() {
|
|
| 555 |
+ err := client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{Force: true})
|
|
| 556 |
+ assert.NilError(t, err) |
|
| 557 |
+ }() |
|
| 558 |
+ |
|
| 559 |
+ inspect, err := client.ContainerInspect(ctx, id) |
|
| 560 |
+ assert.NilError(t, err) |
|
| 561 |
+ // tmpfs do not currently get added to inspect.Mounts |
|
| 562 |
+ // Normally an anoynmous volume would, except now tmpfs should prevent that. |
|
| 563 |
+ assert.Assert(t, is.Len(inspect.Mounts, 0)) |
|
| 564 |
+ |
|
| 565 |
+ chWait, chErr := client.ContainerWait(ctx, id, container.WaitConditionNextExit) |
|
| 566 |
+ assert.NilError(t, client.ContainerStart(ctx, id, types.ContainerStartOptions{}))
|
|
| 567 |
+ |
|
| 568 |
+ timeout := time.NewTimer(30 * time.Second) |
|
| 569 |
+ defer timeout.Stop() |
|
| 570 |
+ |
|
| 571 |
+ select {
|
|
| 572 |
+ case <-timeout.C: |
|
| 573 |
+ t.Fatal("timeout waiting for container to exit")
|
|
| 574 |
+ case status := <-chWait: |
|
| 575 |
+ var errMsg string |
|
| 576 |
+ if status.Error != nil {
|
|
| 577 |
+ errMsg = status.Error.Message |
|
| 578 |
+ } |
|
| 579 |
+ assert.Equal(t, int(status.StatusCode), 0, errMsg) |
|
| 580 |
+ case err := <-chErr: |
|
| 581 |
+ assert.NilError(t, err) |
|
| 582 |
+ } |
|
| 583 |
+} |
| ... | ... |
@@ -2,6 +2,7 @@ package container |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 | 4 |
"fmt" |
| 5 |
+ "strings" |
|
| 5 | 6 |
|
| 6 | 7 |
containertypes "github.com/docker/docker/api/types/container" |
| 7 | 8 |
mounttypes "github.com/docker/docker/api/types/mount" |
| ... | ... |
@@ -77,12 +78,12 @@ func WithMount(m mounttypes.Mount) func(*TestContainerConfig) {
|
| 77 | 77 |
} |
| 78 | 78 |
|
| 79 | 79 |
// WithVolume sets the volume of the container |
| 80 |
-func WithVolume(name string) func(*TestContainerConfig) {
|
|
| 80 |
+func WithVolume(target string) func(*TestContainerConfig) {
|
|
| 81 | 81 |
return func(c *TestContainerConfig) {
|
| 82 | 82 |
if c.Config.Volumes == nil {
|
| 83 | 83 |
c.Config.Volumes = map[string]struct{}{}
|
| 84 | 84 |
} |
| 85 |
- c.Config.Volumes[name] = struct{}{}
|
|
| 85 |
+ c.Config.Volumes[target] = struct{}{}
|
|
| 86 | 86 |
} |
| 87 | 87 |
} |
| 88 | 88 |
|
| ... | ... |
@@ -93,6 +94,22 @@ func WithBind(src, target string) func(*TestContainerConfig) {
|
| 93 | 93 |
} |
| 94 | 94 |
} |
| 95 | 95 |
|
| 96 |
+// WithTmpfs sets a target path in the container to a tmpfs |
|
| 97 |
+func WithTmpfs(target string) func(config *TestContainerConfig) {
|
|
| 98 |
+ return func(c *TestContainerConfig) {
|
|
| 99 |
+ if c.HostConfig.Tmpfs == nil {
|
|
| 100 |
+ c.HostConfig.Tmpfs = make(map[string]string) |
|
| 101 |
+ } |
|
| 102 |
+ |
|
| 103 |
+ spec := strings.SplitN(target, ":", 2) |
|
| 104 |
+ var opts string |
|
| 105 |
+ if len(spec) > 1 {
|
|
| 106 |
+ opts = spec[1] |
|
| 107 |
+ } |
|
| 108 |
+ c.HostConfig.Tmpfs[spec[0]] = opts |
|
| 109 |
+ } |
|
| 110 |
+} |
|
| 111 |
+ |
|
| 96 | 112 |
// WithIPv4 sets the specified ip for the specified network of the container |
| 97 | 113 |
func WithIPv4(network, ip string) func(*TestContainerConfig) {
|
| 98 | 114 |
return func(c *TestContainerConfig) {
|