From eac27ad46d7df4f11a28b84130ff484224f8b63d Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 12 Dec 2014 11:15:31 -0800 Subject: [PATCH 1/3] Add test to enforce volume build content This tests ensures that the content from a dir within a build is carried over even if VOLUME for that dir is specified in the Dockerfile. This test ensures this long standing functionality. Signed-off-by: Michael Crosby --- integration-cli/docker_cli_build_test.go | 34 ++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 0fd5b1363d..49905d204a 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -3914,3 +3914,37 @@ RUN [ ! -e /injected ]`, logDone("build - xz host is being used") } + +func TestBuildVolumesRetainContents(t *testing.T) { + var ( + name = "testbuildvolumescontent" + expected = "some text" + ) + defer deleteImages(name) + ctx, err := fakeContext(` +FROM busybox +COPY content /foo/file +VOLUME /foo +CMD cat /foo/file`, + map[string]string{ + "content": expected, + }) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + if _, err := buildImageFromContext(name, ctx, false); err != nil { + t.Fatal(err) + } + + out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "run", "--rm", name)) + if err != nil { + t.Fatal(err) + } + if out != expected { + t.Fatalf("expected file contents for /foo/file to be %q but received %q", expected, out) + } + + logDone("build - volumes retain contents in build") +} From a83dadbeafd7933bbe541b99732978b508f1c768 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 12 Dec 2014 11:01:05 -0500 Subject: [PATCH 2/3] 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 --- daemon/volumes.go | 25 +++- .../reference/api/docker_remote_api.md | 6 - integration-cli/docker_api_containers_test.go | 131 ++++++++++++++++++ 3 files changed, 150 insertions(+), 12 deletions(-) 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") +} From 5bc2ff8a36e9a768e8b479de4fe3ea9c9daf4121 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Mon, 15 Dec 2014 15:55:12 -0800 Subject: [PATCH 3/3] Bump to version 1.4.1 Signed-off-by: Michael Crosby --- CHANGELOG.md | 5 +++++ VERSION | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6db1a2a372..e5dfab8236 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 1.4.1 (2014-12-15) + +#### Runtime +- Fix issue with volumes-from and bind mounts not being honored after create + ## 1.4.0 (2014-12-11) #### Notable Features since 1.3.0 diff --git a/VERSION b/VERSION index 88c5fb891d..347f5833ee 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.4.0 +1.4.1