diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index d085574741..b0805dd35c 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -312,3 +312,75 @@ func TestVolumesMountedAsReadonly(t *testing.T) { logDone("run - volumes as readonly mount") } + +func TestVolumesFromInReadonlyMode(t *testing.T) { + cmd := exec.Command(dockerBinary, "run", "--name", "parent", "-v", "/test", "busybox", "true") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + cmd = exec.Command(dockerBinary, "run", "--volumes-from", "parent:ro", "busybox", "touch", "/test/file") + if code, err := runCommand(cmd); err == nil || code == 0 { + t.Fatalf("run should fail because volume is ro: exit code %d", code) + } + + deleteAllContainers() + + logDone("run - volumes from as readonly mount") +} + +// Regression test for #1201 +func TestVolumesFromInReadWriteMode(t *testing.T) { + cmd := exec.Command(dockerBinary, "run", "--name", "parent", "-v", "/test", "busybox", "true") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + cmd = exec.Command(dockerBinary, "run", "--volumes-from", "parent", "busybox", "touch", "/test/file") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + deleteAllContainers() + + logDone("run - volumes from as read write mount") +} + +// Test for #1351 +func TestApplyVolumesFromBeforeVolumes(t *testing.T) { + cmd := exec.Command(dockerBinary, "run", "--name", "parent", "-v", "/test", "busybox", "touch", "/test/foo") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + cmd = exec.Command(dockerBinary, "run", "--volumes-from", "parent", "-v", "/test", "busybox", "cat", "/test/foo") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + deleteAllContainers() + + logDone("run - volumes from mounted first") +} + +func TestMultipleVolumesFrom(t *testing.T) { + cmd := exec.Command(dockerBinary, "run", "--name", "parent1", "-v", "/test", "busybox", "touch", "/test/foo") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + cmd = exec.Command(dockerBinary, "run", "--name", "parent2", "-v", "/other", "busybox", "touch", "/other/bar") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + cmd = exec.Command(dockerBinary, "run", "--volumes-from", "parent1", "--volumes-from", "parent2", + "busybox", "sh", "-c", "cat /test/foo && cat /other/bar") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + deleteAllContainers() + + logDone("run - multiple volumes from") +} diff --git a/integration/container_test.go b/integration/container_test.go index d089e7dc45..43f51c1e5f 100644 --- a/integration/container_test.go +++ b/integration/container_test.go @@ -1273,123 +1273,6 @@ func TestBindMounts(t *testing.T) { } } -// Test that -volumes-from supports both read-only mounts -func TestFromVolumesInReadonlyMode(t *testing.T) { - runtime := mkRuntime(t) - defer nuke(runtime) - container, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"/bin/echo", "-n", "foobar"}, - Volumes: map[string]struct{}{"/test": {}}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container) - _, err = container.Output() - if err != nil { - t.Fatal(err) - } - if !container.VolumesRW["/test"] { - t.Fail() - } - - container2, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"/bin/echo", "-n", "foobar"}, - VolumesFrom: container.ID + ":ro", - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container2) - - _, err = container2.Output() - if err != nil { - t.Fatal(err) - } - - if container.Volumes["/test"] != container2.Volumes["/test"] { - t.Logf("container volumes do not match: %s | %s ", - container.Volumes["/test"], - container2.Volumes["/test"]) - t.Fail() - } - - _, exists := container2.VolumesRW["/test"] - if !exists { - t.Logf("container2 is missing '/test' volume: %s", container2.VolumesRW) - t.Fail() - } - - if container2.VolumesRW["/test"] != false { - t.Log("'/test' volume mounted in read-write mode, expected read-only") - t.Fail() - } -} - -// Test that VolumesRW values are copied to the new container. Regression test for #1201 -func TestVolumesFromReadonlyMount(t *testing.T) { - runtime := mkRuntime(t) - defer nuke(runtime) - container, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"/bin/echo", "-n", "foobar"}, - Volumes: map[string]struct{}{"/test": {}}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container) - _, err = container.Output() - if err != nil { - t.Fatal(err) - } - if !container.VolumesRW["/test"] { - t.Fail() - } - - container2, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"/bin/echo", "-n", "foobar"}, - VolumesFrom: container.ID, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container2) - - _, err = container2.Output() - if err != nil { - t.Fatal(err) - } - - if container.Volumes["/test"] != container2.Volumes["/test"] { - t.Fail() - } - - actual, exists := container2.VolumesRW["/test"] - if !exists { - t.Fail() - } - - if container.VolumesRW["/test"] != actual { - t.Fail() - } -} - // Test that restarting a container with a volume does not create a new volume on restart. Regression test for #819. func TestRestartWithVolumes(t *testing.T) { runtime := mkRuntime(t) @@ -1434,73 +1317,6 @@ func TestRestartWithVolumes(t *testing.T) { } } -// Test for #1351 -func TestVolumesFromWithVolumes(t *testing.T) { - runtime := mkRuntime(t) - defer nuke(runtime) - - container, _, err := runtime.Create(&runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"sh", "-c", "echo -n bar > /test/foo"}, - Volumes: map[string]struct{}{"/test": {}}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container) - - for key := range container.Config.Volumes { - if key != "/test" { - t.Fail() - } - } - - _, err = container.Output() - if err != nil { - t.Fatal(err) - } - - expected := container.Volumes["/test"] - if expected == "" { - t.Fail() - } - - container2, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"cat", "/test/foo"}, - VolumesFrom: container.ID, - Volumes: map[string]struct{}{"/test": {}}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container2) - - output, err := container2.Output() - if err != nil { - t.Fatal(err) - } - - if string(output) != "bar" { - t.Fail() - } - - if container.Volumes["/test"] != container2.Volumes["/test"] { - t.Fail() - } - - // Ensure it restarts successfully - _, err = container2.Output() - if err != nil { - t.Fatal(err) - } -} - func TestContainerNetwork(t *testing.T) { runtime := mkRuntime(t) defer nuke(runtime) @@ -1636,81 +1452,3 @@ func TestUnprivilegedCannotMount(t *testing.T) { t.Fatal("Could mount into secure container") } } - -func TestMultipleVolumesFrom(t *testing.T) { - runtime := mkRuntime(t) - defer nuke(runtime) - - container, _, err := runtime.Create(&runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"sh", "-c", "echo -n bar > /test/foo"}, - Volumes: map[string]struct{}{"/test": {}}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container) - - for key := range container.Config.Volumes { - if key != "/test" { - t.Fail() - } - } - - _, err = container.Output() - if err != nil { - t.Fatal(err) - } - - expected := container.Volumes["/test"] - if expected == "" { - t.Fail() - } - - container2, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"sh", "-c", "echo -n bar > /other/foo"}, - Volumes: map[string]struct{}{"/other": {}}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container2) - - for key := range container2.Config.Volumes { - if key != "/other" { - t.FailNow() - } - } - if _, err := container2.Output(); err != nil { - t.Fatal(err) - } - - container3, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"/bin/echo", "-n", "foobar"}, - VolumesFrom: strings.Join([]string{container.ID, container2.ID}, ","), - }, "") - - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container3) - - if _, err := container3.Output(); err != nil { - t.Fatal(err) - } - - if container3.Volumes["/test"] != container.Volumes["/test"] { - t.Fail() - } - if container3.Volumes["/other"] != container2.Volumes["/other"] { - t.Fail() - } -} diff --git a/runconfig/compare.go b/runconfig/compare.go index 122687f669..5c1bf46575 100644 --- a/runconfig/compare.go +++ b/runconfig/compare.go @@ -14,8 +14,7 @@ func Compare(a, b *Config) bool { a.MemorySwap != b.MemorySwap || a.CpuShares != b.CpuShares || a.OpenStdin != b.OpenStdin || - a.Tty != b.Tty || - a.VolumesFrom != b.VolumesFrom { + a.Tty != b.Tty { return false } if len(a.Cmd) != len(b.Cmd) || diff --git a/runconfig/config.go b/runconfig/config.go index 9db545976b..33a7882b6f 100644 --- a/runconfig/config.go +++ b/runconfig/config.go @@ -27,7 +27,6 @@ type Config struct { Cmd []string Image string // Name of the image as it was passed by the operator (eg. could be symbolic) Volumes map[string]struct{} - VolumesFrom string WorkingDir string Entrypoint []string NetworkDisabled bool @@ -49,7 +48,6 @@ func ContainerConfigFromJob(job *engine.Job) *Config { OpenStdin: job.GetenvBool("OpenStdin"), StdinOnce: job.GetenvBool("StdinOnce"), Image: job.Getenv("Image"), - VolumesFrom: job.Getenv("VolumesFrom"), WorkingDir: job.Getenv("WorkingDir"), NetworkDisabled: job.GetenvBool("NetworkDisabled"), } diff --git a/runconfig/config_test.go b/runconfig/config_test.go index d5ab75b3b9..f71528ff8e 100644 --- a/runconfig/config_test.go +++ b/runconfig/config_test.go @@ -163,37 +163,25 @@ func TestCompare(t *testing.T) { volumes1 := make(map[string]struct{}) volumes1["/test1"] = struct{}{} config1 := Config{ - PortSpecs: []string{"1111:1111", "2222:2222"}, - Env: []string{"VAR1=1", "VAR2=2"}, - VolumesFrom: "11111111", - Volumes: volumes1, + PortSpecs: []string{"1111:1111", "2222:2222"}, + Env: []string{"VAR1=1", "VAR2=2"}, + Volumes: volumes1, } config3 := Config{ - PortSpecs: []string{"0000:0000", "2222:2222"}, - Env: []string{"VAR1=1", "VAR2=2"}, - VolumesFrom: "11111111", - Volumes: volumes1, - } - config4 := Config{ - PortSpecs: []string{"0000:0000", "2222:2222"}, - Env: []string{"VAR1=1", "VAR2=2"}, - VolumesFrom: "22222222", - Volumes: volumes1, + PortSpecs: []string{"0000:0000", "2222:2222"}, + Env: []string{"VAR1=1", "VAR2=2"}, + Volumes: volumes1, } volumes2 := make(map[string]struct{}) volumes2["/test2"] = struct{}{} config5 := Config{ - PortSpecs: []string{"0000:0000", "2222:2222"}, - Env: []string{"VAR1=1", "VAR2=2"}, - VolumesFrom: "11111111", - Volumes: volumes2, + PortSpecs: []string{"0000:0000", "2222:2222"}, + Env: []string{"VAR1=1", "VAR2=2"}, + Volumes: volumes2, } if Compare(&config1, &config3) { t.Fatalf("Compare should return false, PortSpecs are different") } - if Compare(&config1, &config4) { - t.Fatalf("Compare should return false, VolumesFrom are different") - } if Compare(&config1, &config5) { t.Fatalf("Compare should return false, Volumes are different") } @@ -207,10 +195,9 @@ func TestMerge(t *testing.T) { volumesImage["/test1"] = struct{}{} volumesImage["/test2"] = struct{}{} configImage := &Config{ - PortSpecs: []string{"1111:1111", "2222:2222"}, - Env: []string{"VAR1=1", "VAR2=2"}, - VolumesFrom: "1111", - Volumes: volumesImage, + PortSpecs: []string{"1111:1111", "2222:2222"}, + Env: []string{"VAR1=1", "VAR2=2"}, + Volumes: volumesImage, } volumesUser := make(map[string]struct{}) @@ -251,10 +238,6 @@ func TestMerge(t *testing.T) { } } - if configUser.VolumesFrom != "1111" { - t.Fatalf("Expected VolumesFrom to be 1111, found %s", configUser.VolumesFrom) - } - ports, _, err := nat.ParsePortSpecs([]string{"0000"}) if err != nil { t.Error(err) diff --git a/runconfig/hostconfig.go b/runconfig/hostconfig.go index 5c5e291bad..127c06d9cb 100644 --- a/runconfig/hostconfig.go +++ b/runconfig/hostconfig.go @@ -16,6 +16,7 @@ type HostConfig struct { PublishAllPorts bool Dns []string DnsSearch []string + VolumesFrom string } func ContainerHostConfigFromJob(job *engine.Job) *HostConfig { @@ -23,6 +24,7 @@ func ContainerHostConfigFromJob(job *engine.Job) *HostConfig { ContainerIDFile: job.Getenv("ContainerIDFile"), Privileged: job.GetenvBool("Privileged"), PublishAllPorts: job.GetenvBool("PublishAllPorts"), + VolumesFrom: job.Getenv("VolumesFrom"), } job.GetenvJson("LxcConf", &hostConfig.LxcConf) job.GetenvJson("PortBindings", &hostConfig.PortBindings) diff --git a/runconfig/merge.go b/runconfig/merge.go index 7a089855b2..1240dbcacd 100644 --- a/runconfig/merge.go +++ b/runconfig/merge.go @@ -100,9 +100,6 @@ func Merge(userConf, imageConf *Config) error { if userConf.WorkingDir == "" { userConf.WorkingDir = imageConf.WorkingDir } - if userConf.VolumesFrom == "" { - userConf.VolumesFrom = imageConf.VolumesFrom - } if userConf.Volumes == nil || len(userConf.Volumes) == 0 { userConf.Volumes = imageConf.Volumes } else { diff --git a/runconfig/parse.go b/runconfig/parse.go index 58d6c9ebb9..b76f59a360 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -215,7 +215,6 @@ func parseRun(cmd *flag.FlagSet, args []string, sysInfo *sysinfo.SysInfo) (*Conf Cmd: runCmd, Image: image, Volumes: flVolumes.GetMap(), - VolumesFrom: strings.Join(flVolumesFrom.GetAll(), ","), Entrypoint: entrypoint, WorkingDir: *flWorkingDir, } @@ -230,6 +229,7 @@ func parseRun(cmd *flag.FlagSet, args []string, sysInfo *sysinfo.SysInfo) (*Conf PublishAllPorts: *flPublishAll, Dns: flDns.GetAll(), DnsSearch: flDnsSearch.GetAll(), + VolumesFrom: strings.Join(flVolumesFrom.GetAll(), ","), } if sysInfo != nil && flMemory > 0 && !sysInfo.SwapLimit { diff --git a/runtime/volumes.go b/runtime/volumes.go index 40db177174..e74442e1b5 100644 --- a/runtime/volumes.go +++ b/runtime/volumes.go @@ -59,8 +59,9 @@ func setupMountsForContainer(container *Container, envPath string) error { } func applyVolumesFrom(container *Container) error { - if container.Config.VolumesFrom != "" { - for _, containerSpec := range strings.Split(container.Config.VolumesFrom, ",") { + volumesFrom := container.hostConfig.VolumesFrom + if volumesFrom != "" { + for _, containerSpec := range strings.Split(volumesFrom, ",") { var ( mountRW = true specParts = strings.SplitN(containerSpec, ":", 2) @@ -68,7 +69,7 @@ func applyVolumesFrom(container *Container) error { switch len(specParts) { case 0: - return fmt.Errorf("Malformed volumes-from specification: %s", container.Config.VolumesFrom) + return fmt.Errorf("Malformed volumes-from specification: %s", volumesFrom) case 2: switch specParts[1] { case "ro":