From a490ba1593602c965f63f55be673ffa40b1703b7 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Wed, 28 Feb 2018 17:00:24 -0800 Subject: [PATCH 1/3] handlers: add eni info to introspection endpoint --- agent/api/container.go | 8 ++++++ agent/handlers/types/v1/response.go | 7 +++++ agent/handlers/v1_handlers.go | 42 +++++++++++++++++++++++++-- agent/handlers/v1_handlers_test.go | 44 +++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/agent/api/container.go b/agent/api/container.go index ff190c0b3a0..780f5bc237a 100644 --- a/agent/api/container.go +++ b/agent/api/container.go @@ -493,6 +493,14 @@ func (c *Container) GetLabels() map[string]string { return c.labels } +// GetPorts gets the ports for a container +func (c *Container) GetPorts() []PortBinding { + c.lock.RLock() + defer c.lock.RUnlock() + + return c.Ports +} + // HealthStatusShouldBeReported returns true if the health check is defined in // the task definition func (c *Container) HealthStatusShouldBeReported() bool { diff --git a/agent/handlers/types/v1/response.go b/agent/handlers/types/v1/response.go index c59e5d69a32..81761278fa9 100644 --- a/agent/handlers/types/v1/response.go +++ b/agent/handlers/types/v1/response.go @@ -13,6 +13,11 @@ package v1 +import ( + "github.com/aws/amazon-ecs-agent/agent/containermetadata" + "github.com/aws/amazon-ecs-agent/agent/handlers/types/v2" +) + // MetadataResponse is the schema for the metadata response JSON object type MetadataResponse struct { Cluster string @@ -40,4 +45,6 @@ type ContainerResponse struct { DockerId string DockerName string Name string + Ports []v2.PortResponse + Networks []containermetadata.Network } diff --git a/agent/handlers/v1_handlers.go b/agent/handlers/v1_handlers.go index 20859c7dd1e..2643cd69d5c 100644 --- a/agent/handlers/v1_handlers.go +++ b/agent/handlers/v1_handlers.go @@ -23,9 +23,11 @@ import ( "github.com/aws/amazon-ecs-agent/agent/api" "github.com/aws/amazon-ecs-agent/agent/config" + "github.com/aws/amazon-ecs-agent/agent/containermetadata" "github.com/aws/amazon-ecs-agent/agent/engine" "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" "github.com/aws/amazon-ecs-agent/agent/handlers/types/v1" + "github.com/aws/amazon-ecs-agent/agent/handlers/types/v2" "github.com/aws/amazon-ecs-agent/agent/logger" "github.com/aws/amazon-ecs-agent/agent/utils" "github.com/aws/amazon-ecs-agent/agent/version" @@ -66,11 +68,12 @@ func metadataV1RequestHandlerMaker(containerInstanceArn *string, cfg *config.Con func newTaskResponse(task *api.Task, containerMap map[string]*api.DockerContainer) *v1.TaskResponse { containers := []v1.ContainerResponse{} - for containerName, container := range containerMap { + for _, container := range containerMap { if container.Container.IsInternal() { continue } - containers = append(containers, v1.ContainerResponse{DockerId: container.DockerID, DockerName: container.DockerName, Name: containerName}) + containerResponse := newContainerResponse(container, task.GetTaskENI()) + containers = append(containers, containerResponse) } knownStatus := task.GetKnownStatus() @@ -92,6 +95,41 @@ func newTaskResponse(task *api.Task, containerMap map[string]*api.DockerContaine } } +func newContainerResponse(dockerContainer *api.DockerContainer, eni *api.ENI) v1.ContainerResponse { + container := dockerContainer.Container + resp := v1.ContainerResponse{ + Name: container.Name, + DockerId: dockerContainer.DockerID, + DockerName: dockerContainer.DockerName, + } + + for _, binding := range container.GetPorts() { + port := v2.PortResponse{ + ContainerPort: binding.ContainerPort, + Protocol: binding.Protocol.String(), + } + + if eni == nil { + port.HostPort = binding.HostPort + } else { + port.HostPort = port.ContainerPort + } + + resp.Ports = append(resp.Ports, port) + } + + if eni != nil { + resp.Networks = []containermetadata.Network{ + { + NetworkMode: "awsvpc", + IPv4Addresses: eni.GetIPV4Addresses(), + IPv6Addresses: eni.GetIPV6Addresses(), + }, + } + } + return resp +} + func newTasksResponse(state dockerstate.TaskEngineState) *v1.TasksResponse { allTasks := state.AllTasks() taskResponses := make([]*v1.TaskResponse, len(allTasks)) diff --git a/agent/handlers/v1_handlers_test.go b/agent/handlers/v1_handlers_test.go index 76262b961a5..a9bab30025c 100644 --- a/agent/handlers/v1_handlers_test.go +++ b/agent/handlers/v1_handlers_test.go @@ -36,6 +36,7 @@ import ( const testContainerInstanceArn = "test_container_instance_arn" const testClusterArn = "test_cluster_arn" +const eniIPV4Address = "10.0.0.2" func TestMetadataHandler(t *testing.T) { metadataHandler := metadataV1RequestHandlerMaker(utils.Strptr(testContainerInstanceArn), &config.Config{Cluster: testClusterArn}) @@ -125,6 +126,24 @@ func TestGetTaskByTaskArn(t *testing.T) { taskDiffHelper(t, []*api.Task{testTasks[0]}, v1.TasksResponse{Tasks: []*v1.TaskResponse{&taskResponse}}) } +func TestGetAWSVPCTaskByTaskArn(t *testing.T) { + recorder := performMockRequest(t, "/v1/tasks?taskarn=awsvpcTask") + + var taskResponse v1.TaskResponse + err := json.Unmarshal(recorder.Body.Bytes(), &taskResponse) + if err != nil { + t.Fatal(err) + } + + resp := v1.TasksResponse{Tasks: []*v1.TaskResponse{&taskResponse}} + + assert.Equal(t, eniIPV4Address, resp.Tasks[0].Containers[0].Networks[0].IPv4Addresses[0]) + assert.Equal(t, uint16(80), resp.Tasks[0].Containers[0].Ports[0].ContainerPort) + assert.Equal(t, "tcp", resp.Tasks[0].Containers[0].Ports[0].Protocol) + + taskDiffHelper(t, []*api.Task{testTasks[3]}, resp) +} + func TestGetTaskByTaskArnNotFound(t *testing.T) { recorder := performMockRequest(t, "/v1/tasks?taskarn=doesnotexist") @@ -303,6 +322,31 @@ var testTasks = []*api.Task{ }, }, }, + { + Arn: "awsvpcTask", + DesiredStatusUnsafe: api.TaskRunning, + KnownStatusUnsafe: api.TaskRunning, + Family: "test", + Version: "1", + Containers: []*api.Container{ + { + Name: "awsvpc", + Ports: []api.PortBinding{ + { + ContainerPort: 80, + Protocol: api.TransportProtocolTCP, + }, + }, + }, + }, + ENI: &api.ENI{ + IPV4Addresses: []*api.ENIIPV4Address{ + { + Address: eniIPV4Address, + }, + }, + }, + }, } func stateSetupHelper(state dockerstate.TaskEngineState, tasks []*api.Task) { From d9d127f85132b03a0f0128db574ab92f7feb8312 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Fri, 2 Mar 2018 10:32:19 -0800 Subject: [PATCH 2/3] api: add set/get for KnownPortBindings field --- agent/api/container.go | 18 +++++++++++++----- agent/api/statechange.go | 2 +- agent/api/testutils/container_equal.go | 2 +- agent/engine/docker_task_engine.go | 4 ++-- agent/engine/docker_task_engine_test.go | 2 +- agent/engine/dockerstate/json_test.go | 8 ++++---- agent/engine/testdata/test_tasks/sleep5.json | 2 +- .../testdata/test_tasks/sleep5TaskCgroup.json | 2 +- agent/handlers/v1_handlers.go | 5 +++-- agent/handlers/v1_handlers_test.go | 2 +- 10 files changed, 28 insertions(+), 19 deletions(-) diff --git a/agent/api/container.go b/agent/api/container.go index 780f5bc237a..433bc436a42 100644 --- a/agent/api/container.go +++ b/agent/api/container.go @@ -177,8 +177,8 @@ type Container struct { // and `SetKnownExitCode`. KnownExitCodeUnsafe *int `json:"KnownExitCode"` - // KnownPortBindings is an array of port bindings for the container. - KnownPortBindings []PortBinding + // KnownPortBindingsUnsafe is an array of port bindings for the container. + KnownPortBindingsUnsafe []PortBinding `json:"KnownPortBindings"` // SteadyStateStatusUnsafe specifies the steady state status for the container // If uninitialized, it's assumed to be set to 'ContainerRunning'. Even though @@ -493,12 +493,20 @@ func (c *Container) GetLabels() map[string]string { return c.labels } -// GetPorts gets the ports for a container -func (c *Container) GetPorts() []PortBinding { +// SetKnownPortBindings gets the ports for a container +func (c *Container) SetKnownPortBindings(ports []PortBinding) { c.lock.RLock() defer c.lock.RUnlock() - return c.Ports + c.KnownPortBindingsUnsafe = ports +} + +// GetKnownPortBindings gets the ports for a container +func (c *Container) GetKnownPortBindings() []PortBinding { + c.lock.RLock() + defer c.lock.RUnlock() + + return c.KnownPortBindingsUnsafe } // HealthStatusShouldBeReported returns true if the health check is defined in diff --git a/agent/api/statechange.go b/agent/api/statechange.go index b447756f3e9..b49676e9ee8 100644 --- a/agent/api/statechange.go +++ b/agent/api/statechange.go @@ -128,7 +128,7 @@ func NewContainerStateChangeEvent(task *Task, cont *Container, reason string) (C ContainerName: cont.Name, Status: contKnownStatus.BackendStatus(cont.GetSteadyStateStatus()), ExitCode: cont.GetKnownExitCode(), - PortBindings: cont.KnownPortBindings, + PortBindings: cont.GetKnownPortBindings(), Reason: reason, Container: cont, } diff --git a/agent/api/testutils/container_equal.go b/agent/api/testutils/container_equal.go index 9241992d263..e8e82886e72 100644 --- a/agent/api/testutils/container_equal.go +++ b/agent/api/testutils/container_equal.go @@ -54,7 +54,7 @@ func ContainersEqual(lhs, rhs *api.Container) bool { if !utils.SlicesDeepEqual(lhs.Ports, rhs.Ports) { return false } - if !utils.SlicesDeepEqual(lhs.KnownPortBindings, rhs.KnownPortBindings) { + if !utils.SlicesDeepEqual(lhs.KnownPortBindingsUnsafe, rhs.KnownPortBindingsUnsafe) { return false } if lhs.Essential != rhs.Essential { diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 7f507247947..ecc52ec80c4 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -314,8 +314,8 @@ func updateContainerMetadata(metadata *DockerContainerMetadata, container *api.C } // Set port mappings - if len(metadata.PortBindings) != 0 && len(container.KnownPortBindings) == 0 { - container.KnownPortBindings = metadata.PortBindings + if len(metadata.PortBindings) != 0 && len(container.GetKnownPortBindings()) == 0 { + container.SetKnownPortBindings(metadata.PortBindings) } // update the container health information if container.HealthStatusShouldBeReported() { diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index 0edf7df942b..cc0f6af3611 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -1858,7 +1858,7 @@ func TestContainerMetadataUpdatedOnRestart(t *testing.T) { assert.Equal(t, "tmp", task.Volumes[0].Volume.SourcePath()) } if tc.stage == "started" { - assert.Equal(t, uint16(80), dockerContainer.Container.KnownPortBindings[0].ContainerPort) + assert.Equal(t, uint16(80), dockerContainer.Container.KnownPortBindingsUnsafe[0].ContainerPort) } if tc.stage == "finished" { assert.False(t, task.GetExecutionStoppedAt().IsZero()) diff --git a/agent/engine/dockerstate/json_test.go b/agent/engine/dockerstate/json_test.go index 04f32bf0446..a66a072021b 100644 --- a/agent/engine/dockerstate/json_test.go +++ b/agent/engine/dockerstate/json_test.go @@ -75,7 +75,7 @@ const ( "AppliedStatus": "NONE", "ApplyingError": null, "SentStatus": "NONE", - "KnownPortBindings": [] + "KnownPortBindingsUnsafe": [] }, { "Name": "~internal~ecs~pause", @@ -110,7 +110,7 @@ const ( "AppliedStatus": "NONE", "ApplyingError": null, "SentStatus": "NONE", - "KnownPortBindings": [], + "KnownPortBindingsUnsafe": [], "SteadyStateStatus": "RESOURCES_PROVISIONED" } ], @@ -171,7 +171,7 @@ const ( "AppliedStatus": "NONE", "ApplyingError": null, "SentStatus": "NONE", - "KnownPortBindings": [], + "KnownPortBindingsUnsafe": [], "SteadyStateStatus": "RESOURCES_PROVISIONED" } }, @@ -224,7 +224,7 @@ const ( "AppliedStatus": "NONE", "ApplyingError": null, "SentStatus": "NONE", - "KnownPortBindings": [] + "KnownPortBindingsUnsafe": [] } } }, diff --git a/agent/engine/testdata/test_tasks/sleep5.json b/agent/engine/testdata/test_tasks/sleep5.json index 0c44a649d88..8e0043b194a 100644 --- a/agent/engine/testdata/test_tasks/sleep5.json +++ b/agent/engine/testdata/test_tasks/sleep5.json @@ -26,7 +26,7 @@ "ApplyingError":null, "SentStatus":"NONE", "KnownExitCode":null, - "KnownPortBindings":null, + "KnownPortBindingsUnsafe":null, "StatusLock":{} } ], diff --git a/agent/engine/testdata/test_tasks/sleep5TaskCgroup.json b/agent/engine/testdata/test_tasks/sleep5TaskCgroup.json index 3f5551a30a5..d7745e53c3a 100644 --- a/agent/engine/testdata/test_tasks/sleep5TaskCgroup.json +++ b/agent/engine/testdata/test_tasks/sleep5TaskCgroup.json @@ -26,7 +26,7 @@ "ApplyingError":null, "SentStatus":"NONE", "KnownExitCode":null, - "KnownPortBindings":null, + "KnownPortBindingsUnsafe":null, "StatusLock":{} } ], diff --git a/agent/handlers/v1_handlers.go b/agent/handlers/v1_handlers.go index 2643cd69d5c..7d627e586d7 100644 --- a/agent/handlers/v1_handlers.go +++ b/agent/handlers/v1_handlers.go @@ -39,6 +39,7 @@ const ( dockerIdQueryField = "dockerid" taskArnQueryField = "taskarn" dockerShortIdLen = 12 + networkModeAwsvpc = "awsvpc" ) type rootResponse struct { @@ -103,7 +104,7 @@ func newContainerResponse(dockerContainer *api.DockerContainer, eni *api.ENI) v1 DockerName: dockerContainer.DockerName, } - for _, binding := range container.GetPorts() { + for _, binding := range container.GetKnownPortBindings() { port := v2.PortResponse{ ContainerPort: binding.ContainerPort, Protocol: binding.Protocol.String(), @@ -121,7 +122,7 @@ func newContainerResponse(dockerContainer *api.DockerContainer, eni *api.ENI) v1 if eni != nil { resp.Networks = []containermetadata.Network{ { - NetworkMode: "awsvpc", + NetworkMode: networkModeAwsvpc, IPv4Addresses: eni.GetIPV4Addresses(), IPv6Addresses: eni.GetIPV6Addresses(), }, diff --git a/agent/handlers/v1_handlers_test.go b/agent/handlers/v1_handlers_test.go index a9bab30025c..48fb4c9aaef 100644 --- a/agent/handlers/v1_handlers_test.go +++ b/agent/handlers/v1_handlers_test.go @@ -331,7 +331,7 @@ var testTasks = []*api.Task{ Containers: []*api.Container{ { Name: "awsvpc", - Ports: []api.PortBinding{ + KnownPortBindingsUnsafe: []api.PortBinding{ { ContainerPort: 80, Protocol: api.TransportProtocolTCP, From 64a0760493b6b6b0cd37ee38ccdfedeaf39d2c93 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Fri, 2 Mar 2018 11:45:26 -0800 Subject: [PATCH 3/3] handler: use container.Ports when KnownPortsBindings empty --- agent/api/container.go | 6 +-- agent/handlers/types/v1/response.go | 4 +- agent/handlers/v1_handlers.go | 40 ++++++++++----- agent/handlers/v1_handlers_test.go | 75 ++++++++++++++++++++++++++--- 4 files changed, 102 insertions(+), 23 deletions(-) diff --git a/agent/api/container.go b/agent/api/container.go index 433bc436a42..2a3901ff135 100644 --- a/agent/api/container.go +++ b/agent/api/container.go @@ -493,10 +493,10 @@ func (c *Container) GetLabels() map[string]string { return c.labels } -// SetKnownPortBindings gets the ports for a container +// SetKnownPortBindings sets the ports for a container func (c *Container) SetKnownPortBindings(ports []PortBinding) { - c.lock.RLock() - defer c.lock.RUnlock() + c.lock.Lock() + defer c.lock.Unlock() c.KnownPortBindingsUnsafe = ports } diff --git a/agent/handlers/types/v1/response.go b/agent/handlers/types/v1/response.go index 81761278fa9..1b710e24484 100644 --- a/agent/handlers/types/v1/response.go +++ b/agent/handlers/types/v1/response.go @@ -45,6 +45,6 @@ type ContainerResponse struct { DockerId string DockerName string Name string - Ports []v2.PortResponse - Networks []containermetadata.Network + Ports []v2.PortResponse `json:",omitempty"` + Networks []containermetadata.Network `json:",omitempty"` } diff --git a/agent/handlers/v1_handlers.go b/agent/handlers/v1_handlers.go index 7d627e586d7..7450396e895 100644 --- a/agent/handlers/v1_handlers.go +++ b/agent/handlers/v1_handlers.go @@ -104,7 +104,33 @@ func newContainerResponse(dockerContainer *api.DockerContainer, eni *api.ENI) v1 DockerName: dockerContainer.DockerName, } - for _, binding := range container.GetKnownPortBindings() { + resp.Ports = newPortBindingsResponse(dockerContainer, eni) + + if eni != nil { + resp.Networks = []containermetadata.Network{ + { + NetworkMode: networkModeAwsvpc, + IPv4Addresses: eni.GetIPV4Addresses(), + IPv6Addresses: eni.GetIPV6Addresses(), + }, + } + } + return resp +} + +func newPortBindingsResponse(dockerContainer *api.DockerContainer, eni *api.ENI) []v2.PortResponse { + container := dockerContainer.Container + resp := []v2.PortResponse{} + + bindings := container.GetKnownPortBindings() + + // if KnownPortBindings list is empty, then we use the port mapping + // information that was passed down from ACS. + if len(bindings) == 0 { + bindings = container.Ports + } + + for _, binding := range bindings { port := v2.PortResponse{ ContainerPort: binding.ContainerPort, Protocol: binding.Protocol.String(), @@ -116,17 +142,7 @@ func newContainerResponse(dockerContainer *api.DockerContainer, eni *api.ENI) v1 port.HostPort = port.ContainerPort } - resp.Ports = append(resp.Ports, port) - } - - if eni != nil { - resp.Networks = []containermetadata.Network{ - { - NetworkMode: networkModeAwsvpc, - IPv4Addresses: eni.GetIPV4Addresses(), - IPv6Addresses: eni.GetIPV6Addresses(), - }, - } + resp = append(resp, port) } return resp } diff --git a/agent/handlers/v1_handlers_test.go b/agent/handlers/v1_handlers_test.go index 48fb4c9aaef..9551807f0b0 100644 --- a/agent/handlers/v1_handlers_test.go +++ b/agent/handlers/v1_handlers_test.go @@ -138,10 +138,41 @@ func TestGetAWSVPCTaskByTaskArn(t *testing.T) { resp := v1.TasksResponse{Tasks: []*v1.TaskResponse{&taskResponse}} assert.Equal(t, eniIPV4Address, resp.Tasks[0].Containers[0].Networks[0].IPv4Addresses[0]) + taskDiffHelper(t, []*api.Task{testTasks[3]}, resp) +} + +func TestGetHostNeworkingTaskByTaskArn(t *testing.T) { + recorder := performMockRequest(t, "/v1/tasks?taskarn=hostModeNetworkingTask") + + var taskResponse v1.TaskResponse + err := json.Unmarshal(recorder.Body.Bytes(), &taskResponse) + if err != nil { + t.Fatal(err) + } + + resp := v1.TasksResponse{Tasks: []*v1.TaskResponse{&taskResponse}} + assert.Equal(t, uint16(80), resp.Tasks[0].Containers[0].Ports[0].ContainerPort) assert.Equal(t, "tcp", resp.Tasks[0].Containers[0].Ports[0].Protocol) - taskDiffHelper(t, []*api.Task{testTasks[3]}, resp) + taskDiffHelper(t, []*api.Task{testTasks[4]}, resp) +} + +func TestGetBridgeNeworkingTaskByTaskArn(t *testing.T) { + recorder := performMockRequest(t, "/v1/tasks?taskarn=bridgeModeNetworkingTask") + + var taskResponse v1.TaskResponse + err := json.Unmarshal(recorder.Body.Bytes(), &taskResponse) + if err != nil { + t.Fatal(err) + } + + resp := v1.TasksResponse{Tasks: []*v1.TaskResponse{&taskResponse}} + + assert.Equal(t, uint16(80), resp.Tasks[0].Containers[0].Ports[0].ContainerPort) + assert.Equal(t, "tcp", resp.Tasks[0].Containers[0].Ports[0].Protocol) + + taskDiffHelper(t, []*api.Task{testTasks[5]}, resp) } func TestGetTaskByTaskArnNotFound(t *testing.T) { @@ -331,18 +362,50 @@ var testTasks = []*api.Task{ Containers: []*api.Container{ { Name: "awsvpc", - KnownPortBindingsUnsafe: []api.PortBinding{ + }, + }, + ENI: &api.ENI{ + IPV4Addresses: []*api.ENIIPV4Address{ + { + Address: eniIPV4Address, + }, + }, + }, + }, + { + Arn: "hostModeNetworkingTask", + DesiredStatusUnsafe: api.TaskRunning, + KnownStatusUnsafe: api.TaskRunning, + Family: "test", + Version: "1", + Containers: []*api.Container{ + { + Name: "awsvpc", + Ports: []api.PortBinding{ { ContainerPort: 80, + HostPort: 80, Protocol: api.TransportProtocolTCP, }, }, }, }, - ENI: &api.ENI{ - IPV4Addresses: []*api.ENIIPV4Address{ - { - Address: eniIPV4Address, + }, + { + Arn: "bridgeModeNetworkingTask", + DesiredStatusUnsafe: api.TaskRunning, + KnownStatusUnsafe: api.TaskRunning, + Family: "test", + Version: "1", + Containers: []*api.Container{ + { + Name: "awsvpc", + KnownPortBindingsUnsafe: []api.PortBinding{ + { + ContainerPort: 80, + HostPort: 80, + Protocol: api.TransportProtocolTCP, + }, }, }, },