Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds support for lookup a task using the short docker Id #813

Merged
merged 1 commit into from
May 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions agent/engine/dockerstate/docker_task_engine_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package dockerstate

import (
"encoding/json"
"strings"
"sync"

"github.com/aws/amazon-ecs-agent/agent/api"
Expand All @@ -31,10 +32,14 @@ type TaskEngineState interface {
AllTasks() []*api.Task
// AllImageStates returns all of the image.ImageStates
AllImageStates() []*image.ImageState
// GetAllContainerIDs returns all of the Container Ids
GetAllContainerIDs() []string
// ContainerByID returns an api.DockerContainer for a given container ID
ContainerByID(id string) (*api.DockerContainer, bool)
// ContainerMapByArn returns a map of containers belonging to a particular task ARN
ContainerMapByArn(arn string) (map[string]*api.DockerContainer, bool)
// TaskByShortID retrieves the task of a given docker short container id
TaskByShortID(cid string) ([]*api.Task, bool)
// TaskByID returns an api.Task for a given container ID
TaskByID(cid string) (*api.Task, bool)
// TaskByArn returns a task for a given ARN
Expand Down Expand Up @@ -125,6 +130,19 @@ func (state *DockerTaskEngineState) allImageStates() []*image.ImageState {
return allImageStates
}

// GetAllContainerIDs returns all of the Container Ids
func (state *DockerTaskEngineState) GetAllContainerIDs() []string {
state.lock.RLock()
defer state.lock.RUnlock()

var ids []string
for id := range state.idToTask {
ids = append(ids, id)
}

return ids
}

// ContainerByID returns an api.DockerContainer for a given container ID
func (state *DockerTaskEngineState) ContainerByID(id string) (*api.DockerContainer, bool) {
state.lock.RLock()
Expand All @@ -143,6 +161,20 @@ func (state *DockerTaskEngineState) ContainerMapByArn(arn string) (map[string]*a
return ret, ok
}

// TaskByShortID retrieves the task of a given docker short container id
func (state *DockerTaskEngineState) TaskByShortID(cid string) ([]*api.Task, bool) {
containerIDs := state.GetAllContainerIDs()
var tasks []*api.Task
for _, id := range containerIDs {
if strings.HasPrefix(id, cid) {
if task, ok := state.TaskByID(id); ok {
tasks = append(tasks, task)
}
}
}
return tasks, len(tasks) > 0
}

// TaskByID retrieves the task of a given docker container id
func (state *DockerTaskEngineState) TaskByID(cid string) (*api.Task, bool) {
state.lock.RLock()
Expand Down
12 changes: 12 additions & 0 deletions agent/engine/dockerstate/dockerstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/aws/amazon-ecs-agent/agent/api"
"github.com/aws/amazon-ecs-agent/agent/engine/image"
"github.com/stretchr/testify/assert"
)

func TestCreateDockerTaskEngineState(t *testing.T) {
Expand All @@ -32,6 +33,10 @@ func TestCreateDockerTaskEngineState(t *testing.T) {
t.Error("Empty state should not have a test task")
}

if _, ok := state.TaskByShortID("test"); ok {
t.Error("Empty state should not have a test taskid")
}

if _, ok := state.TaskByID("test"); ok {
t.Error("Empty state should not have a test taskid")
}
Expand All @@ -43,6 +48,13 @@ func TestCreateDockerTaskEngineState(t *testing.T) {
if len(state.AllImageStates()) != 0 {
t.Error("Empty state should have no image states")
}

task, ok := state.TaskByShortID("test")
if assert.Empty(t, ok, "Empty state should have no tasks") {
assert.Empty(t, task, "Empty state should have no tasks")
}

assert.Empty(t, state.GetAllContainerIDs(), "Empty state should have no containers")
}

func TestAddTask(t *testing.T) {
Expand Down
21 changes: 21 additions & 0 deletions agent/engine/dockerstate/mocks/dockerstate_mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ func (_mr *_MockTaskEngineStateRecorder) AllTasks() *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "AllTasks")
}

func (_m *MockTaskEngineState) GetAllContainerIDs() []string {
ret := _m.ctrl.Call(_m, "GetAllContainerIDs")
ret0, _ := ret[0].([]string)
return ret0
}

func (_mr *_MockTaskEngineStateRecorder) GetAllContainerIDs() *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "GetAllContainerIDs")
}

func (_m *MockTaskEngineState) ContainerByID(_param0 string) (*api.DockerContainer, bool) {
ret := _m.ctrl.Call(_m, "ContainerByID", _param0)
ret0, _ := ret[0].(*api.DockerContainer)
Expand Down Expand Up @@ -158,6 +168,17 @@ func (_mr *_MockTaskEngineStateRecorder) TaskByID(arg0 interface{}) *gomock.Call
return _mr.mock.ctrl.RecordCall(_mr.mock, "TaskByID", arg0)
}

func (_mr *_MockTaskEngineStateRecorder) TaskByShortID(arg0 interface{}) *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "TaskByShortID", arg0)
}

func (_m *MockTaskEngineState) TaskByShortID(_param0 string) ([]*api.Task, bool) {
ret := _m.ctrl.Call(_m, "TaskByShortID", _param0)
ret0, _ := ret[0].([]*api.Task)
ret1, _ := ret[1].(bool)
return ret0, ret1
}

func (_m *MockTaskEngineState) UnmarshalJSON(_param0 []byte) error {
ret := _m.ctrl.Call(_m, "UnmarshalJSON", _param0)
ret0, _ := ret[0].(error)
Expand Down
21 changes: 20 additions & 1 deletion agent/handlers/v1_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var log = logger.ForModule("Handlers")
const (
dockerIdQueryField = "dockerid"
taskArnQueryField = "taskarn"
dockerShortIdLen = 12
)

type rootResponse struct {
Expand Down Expand Up @@ -134,7 +135,25 @@ func tasksV1RequestHandlerMaker(taskEngine DockerStateResolver) func(http.Respon
}
if dockerIdExists {
// Create TaskResponse for the docker id in the query.
task, found := dockerTaskEngineState.TaskByID(dockerId)
var task *api.Task
var found bool
if len(dockerId) > dockerShortIdLen {
task, found = dockerTaskEngineState.TaskByID(dockerId)
} else {
tasks, _ := dockerTaskEngineState.TaskByShortID(dockerId)
if len(tasks) == 0 {
task = nil
found = false
} else if len(tasks) == 1 {
task = tasks[0]
found = true
} else {
log.Info("Multiple tasks found for requsted dockerId: " + dockerId)
w.WriteHeader(http.StatusBadRequest)
w.Write(responseJSON)
return
}
}
responseJSON, status = createTaskJSONResponse(task, found, dockerId, dockerTaskEngineState)
w.WriteHeader(status)
} else if taskArnExists {
Expand Down
38 changes: 38 additions & 0 deletions agent/handlers/v1_handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"github.com/aws/amazon-ecs-agent/agent/utils"
"github.com/aws/amazon-ecs-agent/agent/utils/mocks"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const testContainerInstanceArn = "test_container_instance_arn"
Expand Down Expand Up @@ -78,6 +80,30 @@ func TestGetTaskByDockerID(t *testing.T) {
taskDiffHelper(t, []*api.Task{testTasks[1]}, TasksResponse{Tasks: []*TaskResponse{&taskResponse}})
}

func TestGetTaskByShortDockerIDMultiple(t *testing.T) {
recorder := performMockRequest(t, "/v1/tasks?dockerid=dockerid-tas")

assert.Equal(t, http.StatusBadRequest, recorder.Code, "Expected http 400 for dockerid with multiple matches")
}

func TestGetTaskShortByDockerID404(t *testing.T) {
recorder := performMockRequest(t, "/v1/tasks?dockerid=notfound")

assert.Equal(t, http.StatusNotFound, recorder.Code, "API did not return 404 for bad dockerid")
}

func TestGetTaskByShortDockerID(t *testing.T) {
// stateSetupHelper uses the convention of dockerid-$arn-$containerName; the
// first task has a container name prefix of dockerid-tas
recorder := performMockRequest(t, "/v1/tasks?dockerid=dockerid-by")

var taskResponse TaskResponse
err := json.Unmarshal(recorder.Body.Bytes(), &taskResponse)
require.NoError(t, err, "unmarshal failed for get task by short docker id")

taskDiffHelper(t, []*api.Task{testTasks[2]}, TasksResponse{Tasks: []*TaskResponse{&taskResponse}})
}

func TestGetTaskByDockerID404(t *testing.T) {
recorder := performMockRequest(t, "/v1/tasks?dockerid=does-not-exist")

Expand Down Expand Up @@ -264,6 +290,18 @@ var testTasks = []*api.Task{
},
},
},
{
Arn: "byShortId",
DesiredStatusUnsafe: api.TaskRunning,
KnownStatusUnsafe: api.TaskRunning,
Family: "test",
Version: "2",
Containers: []*api.Container{
{
Name: "shortId",
},
},
},
}

func stateSetupHelper(state dockerstate.TaskEngineState, tasks []*api.Task) {
Expand Down