Merge pull request moby#1207 from crosbymichael/819-use-persistent-vo…

…lume

* Runtime: Do not overwrite container volumes from config

container.go

@@ -520,8 +520,6 @@ func (container *Container) Start(hostConfig *HostConfig) error {
 		log.Printf("WARNING: Your kernel does not support swap limit capabilities. Limitation discarded.\n")
 		container.Config.MemorySwap = -1
 	}
-	container.Volumes = make(map[string]string)
-	container.VolumesRW = make(map[string]bool)
 
 	// Create the requested bind mounts
 	binds := make(map[string]BindMap)
@@ -561,30 +559,35 @@ func (container *Container) Start(hostConfig *HostConfig) error {
 
 	// FIXME: evaluate volumes-from before individual volumes, so that the latter can override the former.
 	// Create the requested volumes volumes
-	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
+	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
 			}
-			srcPath, err := c.layer()
-			if err != nil {
-				return err
+			// Create the mountpoint
+			if err := os.MkdirAll(path.Join(container.RootfsPath(), volPath), 0755); err != nil {
+				return nil
 			}
-			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
 		}
 	}
 

container_test.go

@@ -1299,3 +1299,46 @@ func TestVolumesFromReadonlyMount(t *testing.T) {
 		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)
+	defer nuke(runtime)
+
+	container, err := NewBuilder(runtime).Create(&Config{
+		Image:   GetTestImage(runtime).ID,
+		Cmd:     []string{"echo", "-n", "foobar"},
+		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()
+	}
+	// Run the container again to verify the volume path persists
+	_, err = container.Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	actual := container.Volumes["/test"]
+	if expected != actual {
+		t.Fatalf("Expected volume path: %s Actual path: %s", expected, actual)
+	}
+}