From 441bb4d969a009015f2212b4b922813123648b5d Mon Sep 17 00:00:00 2001
From: Utsa Bhattacharjya <utsa@amazon.com>
Date: Thu, 3 Sep 2020 16:19:49 -0700
Subject: [PATCH] Solve unordered container dependency problem

---
 agent/engine/dependencygraph/graph.go |  7 ++-
 agent/engine/ordering_integ_test.go   | 65 +++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go
index c0cc1f1218b..03e866c0c69 100644
--- a/agent/engine/dependencygraph/graph.go
+++ b/agent/engine/dependencygraph/graph.go
@@ -252,6 +252,8 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi
 		return nil, nil
 	}
 
+	var blockedDependency *apicontainer.DependsOn
+
 	targetDependencies := target.GetDependsOn()
 	for _, dependency := range targetDependencies {
 		dependencyContainer, ok := existingContainers[dependency.ContainerName]
@@ -276,9 +278,12 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi
 		}
 
 		if !resolves(target, dependencyContainer, dependency.Condition) {
-			return &dependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target)
+			blockedDependency = &dependency
 		}
 	}
+	if blockedDependency != nil {
+		return blockedDependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", blockedDependency, target)
+	}
 	return nil, nil
 }
 
diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go
index 49491ae96a3..4348fcb11c9 100644
--- a/agent/engine/ordering_integ_test.go
+++ b/agent/engine/ordering_integ_test.go
@@ -433,6 +433,71 @@ func TestShutdownOrder(t *testing.T) {
 	waitFinished(t, finished, shutdownOrderingTimeout)
 }
 
+func TestMultipleContainerDependency(t *testing.T) {
+	taskEngine, done, _ := setupWithDefaultConfig(t)
+	defer done()
+
+	stateChangeEvents := taskEngine.StateChangeEvents()
+
+	taskArn := "testMultipleContainerDependency"
+	testTask := createTestTask(taskArn)
+
+	exit := createTestContainerWithImageAndName(baseImageForOS, "exit")
+	A := createTestContainerWithImageAndName(baseImageForOS, "A")
+	B := createTestContainerWithImageAndName(baseImageForOS, "B")
+
+	exit.EntryPoint = &entryPointForOS
+	exit.Command = []string{"exit 1"}
+	exit.Essential = true
+
+	A.EntryPoint = &entryPointForOS
+	A.Command = []string{"sleep 10"}
+	A.DependsOnUnsafe = []apicontainer.DependsOn{
+		{
+			ContainerName: "exit",
+			Condition:     "SUCCESS",
+		},
+	}
+
+	B.EntryPoint = &entryPointForOS
+	B.Command = []string{"sleep 10"}
+	B.DependsOnUnsafe = []apicontainer.DependsOn{
+		{
+			ContainerName: "A",
+			Condition:     "START",
+		},
+		{
+			ContainerName: "exit",
+			Condition:     "SUCCESS",
+		},
+	}
+
+	testTask.Containers = []*apicontainer.Container{
+		exit,
+		A,
+		B,
+	}
+
+	go taskEngine.AddTask(testTask)
+
+	finished := make(chan interface{})
+	go func() {
+		// Only exit should first progress to running
+		verifyContainerRunningStateChange(t, taskEngine)
+
+		// Exit container should stop with exit code 1
+		event := <-stateChangeEvents
+		assert.Equal(t, event.(api.ContainerStateChange).Status, status.ContainerStopped)
+		assert.Equal(t, event.(api.ContainerStateChange).ContainerName, "exit")
+
+		// The task should be now stopped as dependencies of A and B are not resolved
+		verifyTaskIsStopped(stateChangeEvents, testTask)
+		close(finished)
+	}()
+
+	waitFinished(t, finished, orderingTimeout)
+}
+
 func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Duration) {
 	select {
 	case <-finished: