Move volumesfrom to hostconfig

This also migrates the volumes from integration tests into the new cli
integration test framework.
Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

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")
+}

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()
-	}
-}

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) ||

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"),
 	}

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)

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)

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 {

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 {

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":