From 967f80f3cceb96f85a4795d42eeb7b84ae0ce24a Mon Sep 17 00:00:00 2001
From: Thomas Orozco <thomas@orozco.fr>
Date: Thu, 23 Oct 2014 14:50:06 +0200
Subject: [PATCH] Fix: Failed Start breaks VolumesFrom

Running parseVolumesFromSpec on all VolumesFrom specs before initialize
any mounts endures that we don't leave container.Volumes in an
inconsistent (partially initialized) if one of out mount groups is not
available (e.g. the container we're trying to mount from does not
exist).

Keeping container.Volumes in a consistent state ensures that next time
we Start() the container, it'll run prepareVolumes() again.

The attached test demonstrates that when a container fails to start due
to a missing container specified in VolumesFrom, it "remembers" a Volume
that worked.

Fixes: #8726

Signed-off-by: Thomas Orozco <thomas@orozco.fr>

Conflicts:
	integration-cli/docker_cli_start_test.go
		cli integration test
---
 daemon/volumes.go                        |  9 ++++++--
 integration-cli/docker_cli_start_test.go | 29 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/daemon/volumes.go b/daemon/volumes.go
index ec0ccd0239580c934f7d7f2b3a3ea68911aa6ba2..f98baa1c78267faa289c3c0aa18cf03c8f2b0a09 100644
--- a/daemon/volumes.go
+++ b/daemon/volumes.go
@@ -191,15 +191,20 @@ func parseBindMountSpec(spec string) (string, string, bool, error) {
 func (container *Container) applyVolumesFrom() error {
 	volumesFrom := container.hostConfig.VolumesFrom
 
+	mountGroups := make([]map[string]*Mount, 0, len(volumesFrom))
+
 	for _, spec := range volumesFrom {
-		mounts, err := parseVolumesFromSpec(container.daemon, spec)
+		mountGroup, err := parseVolumesFromSpec(container.daemon, spec)
 		if err != nil {
 			return err
 		}
+		mountGroups = append(mountGroups, mountGroup)
+	}
 
+	for _, mounts := range mountGroups {
 		for _, mnt := range mounts {
 			mnt.container = container
-			if err = mnt.initialize(); err != nil {
+			if err := mnt.initialize(); err != nil {
 				return err
 			}
 		}
diff --git a/integration-cli/docker_cli_start_test.go b/integration-cli/docker_cli_start_test.go
index 18ad96aef13bd13d4127b02778c5358b1e8eb90f..addc781ca98859450a78d4f1880c6a77b605b459 100644
--- a/integration-cli/docker_cli_start_test.go
+++ b/integration-cli/docker_cli_start_test.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"os/exec"
+	"strings"
 	"testing"
 	"time"
 )
@@ -36,3 +37,31 @@ func TestStartAttachReturnsOnError(t *testing.T) {
 
 	logDone("start - error on start with attach exits")
 }
+
+// gh#8726: a failed Start() breaks --volumes-from on subsequent Start()'s
+func TestStartVolumesFromFailsCleanly(t *testing.T) {
+	defer deleteAllContainers()
+
+	// Create the first data volume
+	cmd(t, "run", "-d", "--name", "data_before", "-v", "/foo", "busybox")
+
+	// Expect this to fail because the data test after contaienr doesn't exist yet
+	if _, err := runCommand(exec.Command(dockerBinary, "run", "-d", "--name", "consumer", "--volumes-from", "data_before", "--volumes-from", "data_after", "busybox")); err == nil {
+		t.Fatal("Expected error but got none")
+	}
+
+	// Create the second data volume
+	cmd(t, "run", "-d", "--name", "data_after", "-v", "/bar", "busybox")
+
+	// Now, all the volumes should be there
+	cmd(t, "start", "consumer")
+
+	// Check that we have the volumes we want
+	out, _, _ := cmd(t, "inspect", "--format='{{ len .Volumes }}'", "consumer")
+	n_volumes := strings.Trim(out, " \r\n'")
+	if n_volumes != "2" {
+		t.Fatalf("Missing volumes: expected 2, got %s", n_volumes)
+	}
+
+	logDone("start - missing containers in --volumes-from did not affect subsequent runs")
+}