Skip to content

Commit

Permalink
Added function to check task dependencies before processing the task
Browse files Browse the repository at this point in the history
  • Loading branch information
richardpen committed May 24, 2017
1 parent 4b82f8d commit fc7b501
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 3 deletions.
20 changes: 17 additions & 3 deletions agent/engine/docker_task_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
25 changes: 25 additions & 0 deletions agent/engine/docker_task_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
15 changes: 15 additions & 0 deletions agent/engine/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions agent/engine/testdata/test_tasks/circular_dependency.json
Original file line number Diff line number Diff line change
@@ -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"
}

0 comments on commit fc7b501

Please sign in to comment.