diff --git a/daemon/volumes.go b/daemon/volumes.go index 46ae5588af..ad2dd3a6ad 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -24,6 +24,7 @@ type Mount struct { volume *volumes.Volume Writable bool copyData bool + from *Container } func (mnt *Mount) Export(resource string) (io.ReadCloser, error) { @@ -42,9 +43,6 @@ func (container *Container) prepareVolumes() error { if container.Volumes == nil || len(container.Volumes) == 0 { container.Volumes = make(map[string]string) container.VolumesRW = make(map[string]bool) - if err := container.applyVolumesFrom(); err != nil { - return err - } } return container.createVolumes() @@ -73,13 +71,27 @@ func (container *Container) createVolumes() error { } } - return nil + // On every start, this will apply any new `VolumesFrom` entries passed in via HostConfig, which may override volumes set in `create` + return container.applyVolumesFrom() } func (m *Mount) initialize() error { // No need to initialize anything since it's already been initialized - if _, exists := m.container.Volumes[m.MountToPath]; exists { - return nil + if hostPath, exists := m.container.Volumes[m.MountToPath]; exists { + // If this is a bind-mount/volumes-from, maybe it was passed in at start instead of create + // We need to make sure bind-mounts/volumes-from passed on start can override existing ones. + if !m.volume.IsBindMount && m.from == nil { + return nil + } + if m.volume.Path == hostPath { + return nil + } + + // Make sure we remove these old volumes we don't actually want now. + // Ignore any errors here since this is just cleanup, maybe someone volumes-from'd this volume + v := m.container.daemon.volumes.Get(hostPath) + v.RemoveContainer(m.container.ID) + m.container.daemon.volumes.Delete(v.Path) } // This is the full path to container fs + mntToPath @@ -217,6 +229,7 @@ func (container *Container) applyVolumesFrom() error { for _, mounts := range mountGroups { for _, mnt := range mounts { + mnt.from = mnt.container mnt.container = container if err := mnt.initialize(); err != nil { return err diff --git a/docs/sources/reference/api/docker_remote_api.md b/docs/sources/reference/api/docker_remote_api.md index 530b15d41a..03613d9381 100644 --- a/docs/sources/reference/api/docker_remote_api.md +++ b/docs/sources/reference/api/docker_remote_api.md @@ -61,12 +61,6 @@ You can set the new container's MAC address explicitly. **New!** Volumes are now initialized when the container is created. -`POST /containers/(id)/start` - -**New!** -Passing the container's `HostConfig` on start is now deprecated. You should -set this when creating the container. - `POST /containers/(id)/copy` **New!** diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index f02f619c44..8b0b8fd696 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -4,7 +4,10 @@ import ( "bytes" "encoding/json" "io" + "io/ioutil" + "os" "os/exec" + "strings" "testing" "github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar" @@ -120,3 +123,131 @@ func TestContainerApiGetChanges(t *testing.T) { logDone("container REST API - check GET containers/changes") } + +func TestContainerApiStartVolumeBinds(t *testing.T) { + defer deleteAllContainers() + name := "testing" + config := map[string]interface{}{ + "Image": "busybox", + "Volumes": map[string]struct{}{"/tmp": {}}, + } + + if _, err := sockRequest("POST", "/containers/create?name="+name, config); err != nil && !strings.Contains(err.Error(), "201 Created") { + t.Fatal(err) + } + + bindPath, err := ioutil.TempDir(os.TempDir(), "test") + if err != nil { + t.Fatal(err) + } + + config = map[string]interface{}{ + "Binds": []string{bindPath + ":/tmp"}, + } + if _, err := sockRequest("POST", "/containers/"+name+"/start", config); err != nil && !strings.Contains(err.Error(), "204 No Content") { + t.Fatal(err) + } + + pth, err := inspectFieldMap(name, "Volumes", "/tmp") + if err != nil { + t.Fatal(err) + } + + if pth != bindPath { + t.Fatalf("expected volume host path to be %s, got %s", bindPath, pth) + } + + logDone("container REST API - check volume binds on start") +} + +func TestContainerApiStartVolumesFrom(t *testing.T) { + defer deleteAllContainers() + volName := "voltst" + volPath := "/tmp" + + if out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "run", "-d", "--name", volName, "-v", volPath, "busybox")); err != nil { + t.Fatal(out, err) + } + + name := "testing" + config := map[string]interface{}{ + "Image": "busybox", + "Volumes": map[string]struct{}{volPath: {}}, + } + + if _, err := sockRequest("POST", "/containers/create?name="+name, config); err != nil && !strings.Contains(err.Error(), "201 Created") { + t.Fatal(err) + } + + config = map[string]interface{}{ + "VolumesFrom": []string{volName}, + } + if _, err := sockRequest("POST", "/containers/"+name+"/start", config); err != nil && !strings.Contains(err.Error(), "204 No Content") { + t.Fatal(err) + } + + pth, err := inspectFieldMap(name, "Volumes", volPath) + if err != nil { + t.Fatal(err) + } + pth2, err := inspectFieldMap(volName, "Volumes", volPath) + if err != nil { + t.Fatal(err) + } + + if pth != pth2 { + t.Fatalf("expected volume host path to be %s, got %s", pth, pth2) + } + + logDone("container REST API - check VolumesFrom on start") +} + +// Ensure that volumes-from has priority over binds/anything else +// This is pretty much the same as TestRunApplyVolumesFromBeforeVolumes, except with passing the VolumesFrom and the bind on start +func TestVolumesFromHasPriority(t *testing.T) { + defer deleteAllContainers() + volName := "voltst" + volPath := "/tmp" + + if out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "run", "-d", "--name", volName, "-v", volPath, "busybox")); err != nil { + t.Fatal(out, err) + } + + name := "testing" + config := map[string]interface{}{ + "Image": "busybox", + "Volumes": map[string]struct{}{volPath: {}}, + } + + if _, err := sockRequest("POST", "/containers/create?name="+name, config); err != nil && !strings.Contains(err.Error(), "201 Created") { + t.Fatal(err) + } + + bindPath, err := ioutil.TempDir(os.TempDir(), "test") + if err != nil { + t.Fatal(err) + } + + config = map[string]interface{}{ + "VolumesFrom": []string{volName}, + "Binds": []string{bindPath + ":/tmp"}, + } + if _, err := sockRequest("POST", "/containers/"+name+"/start", config); err != nil && !strings.Contains(err.Error(), "204 No Content") { + t.Fatal(err) + } + + pth, err := inspectFieldMap(name, "Volumes", volPath) + if err != nil { + t.Fatal(err) + } + pth2, err := inspectFieldMap(volName, "Volumes", volPath) + if err != nil { + t.Fatal(err) + } + + if pth != pth2 { + t.Fatalf("expected volume host path to be %s, got %s", pth, pth2) + } + + logDone("container REST API - check VolumesFrom has priority") +}