From fc7b501e39d430ed2279763e12e4f36f16e2f1a8 Mon Sep 17 00:00:00 2001
From: richardpen <penyin@amazon.com>
Date: Tue, 9 May 2017 23:01:45 +0000
Subject: [PATCH] Added function to check task dependencies before processing
 the task

---
 agent/engine/docker_task_engine.go            | 20 ++++++++++++---
 agent/engine/docker_task_engine_test.go       | 25 +++++++++++++++++++
 agent/engine/errors.go                        | 15 +++++++++++
 .../test_tasks/circular_dependency.json       | 25 +++++++++++++++++++
 4 files changed, 82 insertions(+), 3 deletions(-)
 create mode 100644 agent/engine/testdata/test_tasks/circular_dependency.json

diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go
index c599b39f774..d13ec4dcdeb 100644
--- a/agent/engine/docker_task_engine.go
+++ b/agent/engine/docker_task_engine.go
@@ -25,6 +25,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/api"
 	"github.com/aws/amazon-ecs-agent/agent/config"
 	"github.com/aws/amazon-ecs-agent/agent/credentials"
+	"github.com/aws/amazon-ecs-agent/agent/engine/dependencygraph"
 	"github.com/aws/amazon-ecs-agent/agent/engine/dockerclient"
 	"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
 	"github.com/aws/amazon-ecs-agent/agent/eventstream"
@@ -446,12 +447,25 @@ func (engine *DockerTaskEngine) AddTask(task *api.Task) error {
 
 	existingTask, exists := engine.state.TaskByArn(task.Arn)
 	if !exists {
+		// This will update the container desired status
+		task.UpdateDesiredStatus()
+
 		engine.state.AddTask(task)
-		engine.startTask(task)
-	} else {
-		engine.updateTask(existingTask, task)
+		if dependencygraph.ValidDependencies(task) {
+			engine.startTask(task)
+		} else {
+			seelog.Errorf("Task cannot move forward due to the dependencies of containers cannot be resolved, task: %s", task.String())
+			task.SetKnownStatus(api.TaskStopped)
+			task.SetDesiredStatus(api.TaskStopped)
+			err := TaskDependencyError{task.Arn}
+			engine.emitTaskEvent(task, err.Error())
+		}
+		return nil
 	}
 
+	// Update task
+	engine.updateTask(existingTask, task)
+
 	return nil
 }
 
diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go
index db68b83c034..ab71dd69010 100644
--- a/agent/engine/docker_task_engine_test.go
+++ b/agent/engine/docker_task_engine_test.go
@@ -1146,3 +1146,28 @@ func TestEngineDisableConcurrentPull(t *testing.T) {
 	assert.False(t, dockerTaskEngine.enableConcurrentPull,
 		"Task engine should not be able to perform concurrent pulling for version < 1.11.1")
 }
+
+// TestTaskWithCircularDependency tests the task with containers of which the
+// dependencies can't be resolved
+func TestTaskWithCircularDependency(t *testing.T) {
+	ctrl, client, _, taskEngine, _, _ := mocks(t, &defaultConfig)
+	defer ctrl.Finish()
+
+	client.EXPECT().Version().Return("1.12.6", nil)
+	client.EXPECT().ContainerEvents(gomock.Any())
+
+	task := testdata.LoadTask("circular_dependency")
+
+	taskEngine.Init()
+	events := taskEngine.StateChangeEvents()
+
+	go taskEngine.AddTask(task)
+
+	event := <-events
+	assert.Equal(t, event.(api.TaskStateChange).Status, api.TaskStopped, "Expected task to move to stopped directly")
+	_, ok := taskEngine.(*DockerTaskEngine).state.TaskByArn(task.Arn)
+	assert.True(t, ok, "Task state should be added to the agent state")
+
+	_, ok = taskEngine.(*DockerTaskEngine).managedTasks[task.Arn]
+	assert.False(t, ok, "Task should not be added to task manager for processing")
+}
diff --git a/agent/engine/errors.go b/agent/engine/errors.go
index 20df377f332..6d1bfd61a64 100644
--- a/agent/engine/errors.go
+++ b/agent/engine/errors.go
@@ -112,6 +112,21 @@ func (CannotGetDockerClientError) ErrorName() string {
 	return "CannotGetDockerclientError"
 }
 
+// TaskDependencyError is the error for task that dependencies can't
+// be resolved
+type TaskDependencyError struct {
+	taskArn string
+}
+
+func (err TaskDependencyError) Error() string {
+	return "Task dependencies cannot be resolved" + err.taskArn
+}
+
+// ErrorName is the name of the error
+func (err TaskDependencyError) ErrorName() string {
+	return "TaskDependencyError"
+}
+
 // TaskStoppedBeforePullBeginError is a type for task errors involving pull
 type TaskStoppedBeforePullBeginError struct {
 	taskArn string
diff --git a/agent/engine/testdata/test_tasks/circular_dependency.json b/agent/engine/testdata/test_tasks/circular_dependency.json
new file mode 100644
index 00000000000..5cb8e312fd8
--- /dev/null
+++ b/agent/engine/testdata/test_tasks/circular_dependency.json
@@ -0,0 +1,25 @@
+{
+  "Arn":"arn:aws:ecs:us-west-2:123456789012:task/12345678-90ab-cdef-1234-56780abcdef1-circular-dependency",
+  "Family":"circular-dependency",
+  "Version":"2",
+  "Containers":
+  [{
+    "Name": "web",
+    "Image": "busybox",
+    "Command": ["sleep", "1000"],
+    "Links":["web-db:web-db"]
+  },
+  {
+    "Name": "web-db",
+    "Image": "busybox",
+    "Command": ["sleep", "1000"],
+    "volumesFrom": [{
+        "sourceContainer": "web"
+    }]
+  }],
+  "volumes": [],
+  "DesiredStatus": "RUNNING",
+  "KnownStatus": "NONE",
+  "KnownTime": "0001-01-01T00:00:00Z",
+  "SentStatus": "NONE"
+}