Skip to content

Commit

Permalink
Revert "Add LogDriver, LogOptions, and LaunchType to TMDEv4"
Browse files Browse the repository at this point in the history 
This reverts commit a2299e2.

This needs to be in a feature branch for now.
  • Loading branch information
@sparrc
sparrc committed Sep 8, 2020
1 parent 372f248 commit 6cdb042
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 203 deletions.
24 changes: 2 additions & 22 deletions agent/api/container/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1072,33 +1072,13 @@ func (c *Container) GetLogDriver() string {
hostConfig := &dockercontainer.HostConfig{}
err := json.Unmarshal([]byte(*c.DockerConfig.HostConfig), hostConfig)
if err != nil {
seelog.Warnf("Encountered error when trying to get log driver for container %s: %v", c.RuntimeID, err)
seelog.Warnf("Encountered error when trying to get log driver for container %s: %v", err)
return ""
}

return hostConfig.LogConfig.Type
}

// GetLogOptions gets the log 'options' map passed into the task definition.
// see https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html
func (c *Container) GetLogOptions() map[string]string {
c.lock.RLock()
defer c.lock.RUnlock()

if c.DockerConfig.HostConfig == nil {
return map[string]string{}
}

hostConfig := &dockercontainer.HostConfig{}
err := json.Unmarshal([]byte(*c.DockerConfig.HostConfig), hostConfig)
if err != nil {
seelog.Warnf("Encountered error when trying to get log configuration for container %s: %v", c.RuntimeID, err)
return map[string]string{}
}

return hostConfig.LogConfig.Config
}

// GetNetworkModeFromHostConfig returns the network mode used by the container from the host config .
func (c *Container) GetNetworkModeFromHostConfig() string {
c.lock.RLock()
Expand All @@ -1112,7 +1092,7 @@ func (c *Container) GetNetworkModeFromHostConfig() string {
// TODO return error to differentiate between error and default mode .
err := json.Unmarshal([]byte(*c.DockerConfig.HostConfig), hostConfig)
if err != nil {
seelog.Warnf("Encountered error when trying to get network mode for container %s: %v", c.RuntimeID, err)
seelog.Warnf("Encountered error when trying to get network mode for container %s: %v", err)
return ""
}

Expand Down
42 changes: 4 additions & 38 deletions agent/handlers/task_server_setup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,25 +313,8 @@ var (
},
}
expectedV4TaskResponse = v4.TaskResponse{
TaskResponse: &v2.TaskResponse{
Cluster: clusterName,
TaskARN: taskARN,
Family: family,
Revision: version,
DesiredStatus: statusRunning,
KnownStatus: statusRunning,
Containers: []v2.ContainerResponse{expectedContainerResponse},
Limits: &v2.LimitsResponse{
CPU: aws.Float64(cpu),
Memory: aws.Int64(memory),
},
PullStartedAt: aws.Time(now.UTC()),
PullStoppedAt: aws.Time(now.UTC()),
ExecutionStoppedAt: aws.Time(now.UTC()),
AvailabilityZone: availabilityzone,
LaunchType: "EC2",
},
Containers: []v4.ContainerResponse{expectedV4ContainerResponse},
TaskResponse: &expectedTaskResponse,
Containers: []v4.ContainerResponse{expectedV4ContainerResponse},
}
expectedV4BridgeContainerResponse = v4.ContainerResponse{
ContainerResponse: &expectedBridgeContainerResponse,
Expand All @@ -350,25 +333,8 @@ var (
},
}
expectedV4BridgeTaskResponse = v4.TaskResponse{
TaskResponse: &v2.TaskResponse{
Cluster: clusterName,
TaskARN: taskARN,
Family: family,
Revision: version,
DesiredStatus: statusRunning,
KnownStatus: statusRunning,
Containers: []v2.ContainerResponse{expectedBridgeContainerResponse},
Limits: &v2.LimitsResponse{
CPU: aws.Float64(cpu),
Memory: aws.Int64(memory),
},
PullStartedAt: aws.Time(now.UTC()),
PullStoppedAt: aws.Time(now.UTC()),
ExecutionStoppedAt: aws.Time(now.UTC()),
AvailabilityZone: availabilityzone,
LaunchType: "EC2",
},
Containers: []v4.ContainerResponse{expectedV4BridgeContainerResponse},
TaskResponse: &expectedBridgeTaskResponse,
Containers: []v4.ContainerResponse{expectedV4BridgeContainerResponse},
}
)

Expand Down
34 changes: 8 additions & 26 deletions agent/handlers/v2/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ type TaskResponse struct {
AvailabilityZone string `json:"AvailabilityZone,omitempty"`
TaskTags map[string]string `json:"TaskTags,omitempty"`
ContainerInstanceTags map[string]string `json:"ContainerInstanceTags,omitempty"`
LaunchType string `json:"LaunchType,omitempty"`
}

// ContainerResponse defines the schema for the container response
Expand All @@ -68,8 +67,6 @@ type ContainerResponse struct {
Networks []containermetadata.Network `json:"Networks,omitempty"`
Health *apicontainer.HealthStatus `json:"Health,omitempty"`
Volumes []v1.VolumeResponse `json:"Volumes,omitempty"`
LogDriver string `json:"LogDriver,omitempty"`
LogOptions map[string]string `json:"LogOptions,omitempty"`
}

// LimitsResponse defines the schema for task/cpu limits response
Expand All @@ -80,16 +77,13 @@ type LimitsResponse struct {
}

// NewTaskResponse creates a new response object for the task
func NewTaskResponse(
taskARN string,
func NewTaskResponse(taskARN string,
state dockerstate.TaskEngineState,
ecsClient api.ECSClient,
cluster string,
az string,
containerInstanceArn string,
propagateTags bool,
includeV4Metadata bool,
) (*TaskResponse, error) {
propagateTags bool) (*TaskResponse, error) {
task, ok := state.TaskByArn(taskARN)
if !ok {
return nil, errors.Errorf("v2 task response: unable to find task '%s'", taskARN)
Expand Down Expand Up @@ -135,7 +129,7 @@ func NewTaskResponse(
}

for _, dockerContainer := range containerNameToDockerContainer {
containerResponse := newContainerResponse(dockerContainer, task.GetPrimaryENI(), state, includeV4Metadata)
containerResponse := newContainerResponse(dockerContainer, task.GetPrimaryENI(), state)
resp.Containers = append(resp.Containers, containerResponse)
}

Expand Down Expand Up @@ -169,11 +163,8 @@ func propagateTagsToMetadata(state dockerstate.TaskEngineState, ecsClient api.EC
}

// NewContainerResponse creates a new container response based on container id
func NewContainerResponse(
containerID string,
state dockerstate.TaskEngineState,
includeV4Metadata bool,
) (*ContainerResponse, error) {
func NewContainerResponse(containerID string,
state dockerstate.TaskEngineState) (*ContainerResponse, error) {
dockerContainer, ok := state.ContainerByID(containerID)
if !ok {
return nil, errors.Errorf(
Expand All @@ -185,16 +176,13 @@ func NewContainerResponse(
"v2 container response: unable to find task for container '%s'", containerID)
}

resp := newContainerResponse(dockerContainer, task.GetPrimaryENI(), state, includeV4Metadata)
resp := newContainerResponse(dockerContainer, task.GetPrimaryENI(), state)
return &resp, nil
}

func newContainerResponse(
dockerContainer *apicontainer.DockerContainer,
func newContainerResponse(dockerContainer *apicontainer.DockerContainer,
eni *apieni.ENI,
state dockerstate.TaskEngineState,
includeV4Metadata bool,
) ContainerResponse {
state dockerstate.TaskEngineState) ContainerResponse {
container := dockerContainer.Container
resp := ContainerResponse{
ID: dockerContainer.DockerID,
Expand All @@ -212,12 +200,6 @@ func newContainerResponse(
ExitCode: container.GetKnownExitCode(),
Labels: container.GetLabels(),
}
// V4 metadata endpoint calls this function for consistency across versions,
// but needs additional metadata only available at this scope.
if includeV4Metadata {
resp.LogDriver = container.GetLogDriver()
resp.LogOptions = container.GetLogOptions()
}

// Write the container health status inside the container
if dockerContainer.Container.HealthStatusShouldBeReported() {
Expand Down
112 changes: 4 additions & 108 deletions agent/handlers/v2/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,115 +123,11 @@ func TestTaskResponse(t *testing.T) {
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
)

taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, false, false)
taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, false)
assert.NoError(t, err)
_, err = json.Marshal(taskResponse)
assert.NoError(t, err)
assert.Equal(t, created.UTC().String(), taskResponse.Containers[0].CreatedAt.String())
// LaunchType should not be populated
assert.Equal(t, "", taskResponse.LaunchType)
// Log driver and Log options should not be populated
assert.Equal(t, "", taskResponse.Containers[0].LogDriver)
assert.Len(t, taskResponse.Containers[0].LogOptions, 0)

gomock.InOrder(
state.EXPECT().TaskByArn(taskARN).Return(task, true),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
)
// verify that 'v4' response without log driver or options returns blank fields as well
taskResponse, err = NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, false, true)
assert.NoError(t, err)
_, err = json.Marshal(taskResponse)
assert.NoError(t, err)
// LaunchType should not be populated
assert.Equal(t, "", taskResponse.LaunchType)
// Log driver and Log options should not be populated
assert.Equal(t, "", taskResponse.Containers[0].LogDriver)
assert.Len(t, taskResponse.Containers[0].LogOptions, 0)
}

func TestTaskResponseWithV4Metadata(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

state := mock_dockerstate.NewMockTaskEngineState(ctrl)
ecsClient := mock_api.NewMockECSClient(ctrl)
now := time.Now()
task := &apitask.Task{
Arn: taskARN,
Family: family,
Version: version,
DesiredStatusUnsafe: apitaskstatus.TaskRunning,
KnownStatusUnsafe: apitaskstatus.TaskRunning,
ENIs: []*apieni.ENI{
{
IPV4Addresses: []*apieni.ENIIPV4Address{
{
Address: eniIPv4Address,
},
},
},
},
CPU: cpu,
Memory: memory,
PullStartedAtUnsafe: now,
PullStoppedAtUnsafe: now,
ExecutionStoppedAtUnsafe: now,
}
container := &apicontainer.Container{
Name: containerName,
Image: imageName,
ImageID: imageID,
DesiredStatusUnsafe: apicontainerstatus.ContainerRunning,
KnownStatusUnsafe: apicontainerstatus.ContainerRunning,
CPU: cpu,
Memory: memory,
Type: apicontainer.ContainerNormal,
Ports: []apicontainer.PortBinding{
{
ContainerPort: 80,
Protocol: apicontainer.TransportProtocolTCP,
},
},
VolumesUnsafe: []types.MountPoint{
{
Name: volName,
Source: volSource,
Destination: volDestination,
},
},
DockerConfig: apicontainer.DockerConfig{
HostConfig: aws.String(`{"LogConfig":{"Type":"awslogs","Config":{"awslogs-group":"myLogGroup"}}}`),
},
}
created := time.Now()
container.SetCreatedAt(created)
labels := map[string]string{
"foo": "bar",
}
container.SetLabels(labels)
containerNameToDockerContainer := map[string]*apicontainer.DockerContainer{
taskARN: {
DockerID: containerID,
DockerName: containerName,
Container: container,
},
}
gomock.InOrder(
state.EXPECT().TaskByArn(taskARN).Return(task, true),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
)

taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, false, true)
assert.NoError(t, err)
_, err = json.Marshal(taskResponse)
assert.NoError(t, err)
assert.Equal(t, created.UTC().String(), taskResponse.Containers[0].CreatedAt.String())
// LaunchType is populated by the v4 handler
assert.Equal(t, "", taskResponse.LaunchType)
// Log driver and config should be populated
assert.Equal(t, "awslogs", taskResponse.Containers[0].LogDriver)
assert.Equal(t, map[string]string{"awslogs-group": "myLogGroup"}, taskResponse.Containers[0].LogOptions)
}

func TestContainerResponse(t *testing.T) {
Expand Down Expand Up @@ -311,7 +207,7 @@ func TestContainerResponse(t *testing.T) {
state.EXPECT().TaskByID(containerID).Return(task, true),
)

containerResponse, err := NewContainerResponse(containerID, state, false)
containerResponse, err := NewContainerResponse(containerID, state)
assert.NoError(t, err)
assert.Equal(t, containerResponse.Health == nil, tc.result)
_, err = json.Marshal(containerResponse)
Expand Down Expand Up @@ -448,7 +344,7 @@ func TestTaskResponseMarshal(t *testing.T) {
}, nil),
)

taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, true, false)
taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, true)
assert.NoError(t, err)

taskResponseJSON, err := json.Marshal(taskResponse)
Expand Down Expand Up @@ -552,7 +448,7 @@ func TestContainerResponseMarshal(t *testing.T) {
state.EXPECT().TaskByID(containerID).Return(task, true),
)

containerResponse, err := NewContainerResponse(containerID, state, false)
containerResponse, err := NewContainerResponse(containerID, state)
assert.NoError(t, err)

containerResponseJSON, err := json.Marshal(containerResponse)
Expand Down
4 changes: 2 additions & 2 deletions agent/handlers/v2/task_container_metadata_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TaskContainerMetadataHandler(state dockerstate.TaskEngineState, ecsClient a

// WriteContainerMetadataResponse writes the container metadata to response writer.
func WriteContainerMetadataResponse(w http.ResponseWriter, containerID string, state dockerstate.TaskEngineState) {
containerResponse, err := NewContainerResponse(containerID, state, false)
containerResponse, err := NewContainerResponse(containerID, state)
if err != nil {
errResponseJSON, err := json.Marshal("Unable to generate metadata for container '" + containerID + "'")
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
Expand All @@ -91,7 +91,7 @@ func WriteContainerMetadataResponse(w http.ResponseWriter, containerID string, s
// WriteTaskMetadataResponse writes the task metadata to response writer.
func WriteTaskMetadataResponse(w http.ResponseWriter, taskARN string, cluster string, state dockerstate.TaskEngineState, ecsClient api.ECSClient, az, containerInstanceArn string, propagateTags bool) {
// Generate a response for the task
taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, az, containerInstanceArn, propagateTags, false)
taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, az, containerInstanceArn, propagateTags)
if err != nil {
errResponseJSON, err := json.Marshal("Unable to generate metadata for task: '" + taskARN + "'")
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
Expand Down
2 changes: 1 addition & 1 deletion agent/handlers/v3/container_metadata_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo

// GetContainerResponse gets container response for v3 metadata
func GetContainerResponse(containerID string, state dockerstate.TaskEngineState) (*v2.ContainerResponse, error) {
containerResponse, err := v2.NewContainerResponse(containerID, state, false)
containerResponse, err := v2.NewContainerResponse(containerID, state)
if err != nil {
return nil, errors.Errorf("Unable to generate metadata for container '%s'", containerID)
}
Expand Down
2 changes: 1 addition & 1 deletion agent/handlers/v3/task_metadata_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli

seelog.Infof("V3 task metadata handler: writing response for task '%s'", taskARN)

taskResponse, err := v2.NewTaskResponse(taskARN, state, ecsClient, cluster, az, containerInstanceArn, propagateTags, false)
taskResponse, err := v2.NewTaskResponse(taskARN, state, ecsClient, cluster, az, containerInstanceArn, propagateTags)
if err != nil {
errResponseJSON, err := json.Marshal("Unable to generate metadata for task: '" + taskARN + "'")
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
Expand Down
Loading

0 comments on commit 6cdb042

Please sign in to comment.