From 3bd73a96333e011738136f6a9eda23642cc204ab Mon Sep 17 00:00:00 2001 From: Greg Thornton Date: Sat, 10 Aug 2013 04:55:23 +0000 Subject: [PATCH 1/2] Apply volumes-from before creating volumes Copies the volumes from the container specified in `Config.VolumesFrom` before creating volumes from `Config.Volumes`. Skips any preexisting volumes when processing `Config.Volumes`. Fixes #1351 --- container.go | 60 ++++++++++++++++++++++++----------------------- container_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 29 deletions(-) diff --git a/container.go b/container.go index 8721d45a55..44797e6724 100644 --- a/container.go +++ b/container.go @@ -574,40 +574,12 @@ func (container *Container) Start(hostConfig *HostConfig) error { binds[path.Clean(dst)] = bindMap } - // FIXME: evaluate volumes-from before individual volumes, so that the latter can override the former. - // Create the requested volumes volumes if container.Volumes == nil || len(container.Volumes) == 0 { container.Volumes = make(map[string]string) container.VolumesRW = make(map[string]bool) - - for volPath := range container.Config.Volumes { - volPath = path.Clean(volPath) - // If an external bind is defined for this volume, use that as a source - if bindMap, exists := binds[volPath]; exists { - container.Volumes[volPath] = bindMap.SrcPath - if strings.ToLower(bindMap.Mode) == "rw" { - container.VolumesRW[volPath] = true - } - // Otherwise create an directory in $ROOT/volumes/ and use that - } else { - c, err := container.runtime.volumes.Create(nil, container, "", "", nil) - if err != nil { - return err - } - srcPath, err := c.layer() - if err != nil { - return err - } - container.Volumes[volPath] = srcPath - container.VolumesRW[volPath] = true // RW by default - } - // Create the mountpoint - if err := os.MkdirAll(path.Join(container.RootfsPath(), volPath), 0755); err != nil { - return nil - } - } } + // Apply volumes from another container if requested if container.Config.VolumesFrom != "" { c := container.runtime.Get(container.Config.VolumesFrom) if c == nil { @@ -627,6 +599,36 @@ func (container *Container) Start(hostConfig *HostConfig) error { } } + // Create the requested volumes if they don't exist + for volPath := range container.Config.Volumes { + volPath = path.Clean(volPath) + // If an external bind is defined for this volume, use that as a source + if _, exists := container.Volumes[volPath]; exists { + // Skip existing mounts + } else if bindMap, exists := binds[volPath]; exists { + container.Volumes[volPath] = bindMap.SrcPath + if strings.ToLower(bindMap.Mode) == "rw" { + container.VolumesRW[volPath] = true + } + // Otherwise create an directory in $ROOT/volumes/ and use that + } else { + c, err := container.runtime.volumes.Create(nil, container, "", "", nil) + if err != nil { + return err + } + srcPath, err := c.layer() + if err != nil { + return err + } + container.Volumes[volPath] = srcPath + container.VolumesRW[volPath] = true // RW by default + } + // Create the mountpoint + if err := os.MkdirAll(path.Join(container.RootfsPath(), volPath), 0755); err != nil { + return nil + } + } + if err := container.generateLXCConfig(); err != nil { return err } diff --git a/container_test.go b/container_test.go index aca53e5eb3..c4f2193733 100644 --- a/container_test.go +++ b/container_test.go @@ -1276,6 +1276,65 @@ func TestRestartWithVolumes(t *testing.T) { } } +// Test for #1351 +func TestVolumesFromWithVolumes(t *testing.T) { + runtime := mkRuntime(t) + defer nuke(runtime) + + container, err := NewBuilder(runtime).Create(&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 := NewBuilder(runtime).Create( + &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() + } +} + func TestOnlyLoopbackExistsWhenUsingDisableNetworkOption(t *testing.T) { runtime := mkRuntime(t) defer nuke(runtime) From 57b49efc98d2f4605c95d5579a6cd952dfd6f124 Mon Sep 17 00:00:00 2001 From: Greg Thornton Date: Sat, 10 Aug 2013 06:37:57 +0000 Subject: [PATCH 2/2] Skip existing volumes in volumes-from Removes the error when a container already has a volume that would otherwise be created by `Config.VolumesFrom`. Allows restarting containers with a `Config.VolumesFrom` set. --- container.go | 10 ++++++---- container_test.go | 6 ++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/container.go b/container.go index 44797e6724..326c0c55fe 100644 --- a/container.go +++ b/container.go @@ -587,7 +587,7 @@ func (container *Container) Start(hostConfig *HostConfig) error { } for volPath, id := range c.Volumes { if _, exists := container.Volumes[volPath]; exists { - return fmt.Errorf("The requested volume %s overlap one of the volume of the container %s", volPath, c.ID) + continue } if err := os.MkdirAll(path.Join(container.RootfsPath(), volPath), 0755); err != nil { return nil @@ -602,10 +602,12 @@ func (container *Container) Start(hostConfig *HostConfig) error { // Create the requested volumes if they don't exist for volPath := range container.Config.Volumes { volPath = path.Clean(volPath) - // If an external bind is defined for this volume, use that as a source + // Skip existing volumes if _, exists := container.Volumes[volPath]; exists { - // Skip existing mounts - } else if bindMap, exists := binds[volPath]; exists { + continue + } + // If an external bind is defined for this volume, use that as a source + if bindMap, exists := binds[volPath]; exists { container.Volumes[volPath] = bindMap.SrcPath if strings.ToLower(bindMap.Mode) == "rw" { container.VolumesRW[volPath] = true diff --git a/container_test.go b/container_test.go index c4f2193733..644e1c058c 100644 --- a/container_test.go +++ b/container_test.go @@ -1333,6 +1333,12 @@ func TestVolumesFromWithVolumes(t *testing.T) { if container.Volumes["/test"] != container2.Volumes["/test"] { t.Fail() } + + // Ensure it restarts successfully + _, err = container2.Output() + if err != nil { + t.Fatal(err) + } } func TestOnlyLoopbackExistsWhenUsingDisableNetworkOption(t *testing.T) {