Fix volumes-from/bind-mounts passed in on start
Fixes #9628
Slightly reverts #8683, HostConfig on start is _not_ deprecated.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
diff --git a/daemon/volumes.go b/daemon/volumes.go
index 46ae558..ad2dd3a 100644
--- a/daemon/volumes.go
+++ b/daemon/volumes.go
@@ -24,6 +24,7 @@
volume *volumes.Volume
Writable bool
copyData bool
+ from *Container
}
func (mnt *Mount) Export(resource string) (io.ReadCloser, error) {
@@ -42,9 +43,6 @@
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 @@
}
}
- 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 @@
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 530b15d..03613d9 100644
--- a/docs/sources/reference/api/docker_remote_api.md
+++ b/docs/sources/reference/api/docker_remote_api.md
@@ -61,12 +61,6 @@
**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 f02f619..8b0b8fd 100644
--- a/integration-cli/docker_api_containers_test.go
+++ b/integration-cli/docker_api_containers_test.go
@@ -4,7 +4,10 @@
"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 @@
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")
+}
