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.

Sign up for GitHub

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

Move v2 metadata models to ecs-agent module #3705

Merged
merged 2 commits into from
May 22, 2023
Merged

Move v2 metadata models to ecs-agent module #3705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3f3123
Add v2 metadata models to ecs-agent
amogh09 May 17, 2023
27db712
Consume common TMDS models from ecs-agent
amogh09 May 19, 2023
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
26 changes: 13 additions & 13 deletions agent/handlers/task_server_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/aws/amazon-ecs-agent/agent/ecs_client/model/ecs"
mock_dockerstate "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate/mocks"
task_protection_v1 "github.com/aws/amazon-ecs-agent/agent/handlers/agentapi/taskprotection/v1/handlers"
v2 "github.com/aws/amazon-ecs-agent/agent/handlers/v2"
v3 "github.com/aws/amazon-ecs-agent/agent/handlers/v3"
v4 "github.com/aws/amazon-ecs-agent/agent/handlers/v4"
"github.com/aws/amazon-ecs-agent/agent/stats"
Expand All @@ -51,6 +50,7 @@ import (
tmdsresponse "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/response"
"github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/utils"
tmdsv1 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v1"
v2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
"github.com/aws/aws-sdk-go/aws"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -839,21 +839,21 @@ func TestV2TaskWithTagsMetadata(t *testing.T) {
state.EXPECT().TaskByArn(taskARN).Return(task, true),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
ecsClient.EXPECT().GetResourceTags(containerInstanceArn).Return([]*ecs.Tag{
&ecs.Tag{
{
Key: &contInstTag1Key,
Value: &contInstTag1Val,
},
&ecs.Tag{
{
Key: &contInstTag2Key,
Value: &contInstTag2Val,
},
}, nil),
ecsClient.EXPECT().GetResourceTags(taskARN).Return([]*ecs.Tag{
&ecs.Tag{
{
Key: &taskTag1Key,
Value: &taskTag1Val,
},
&ecs.Tag{
{
Key: &taskTag2Key,
Value: &taskTag2Val,
},
Expand Down Expand Up @@ -1127,21 +1127,21 @@ func TestV3TaskMetadataWithTags(t *testing.T) {
state.EXPECT().TaskByArn(taskARN).Return(task, true),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
ecsClient.EXPECT().GetResourceTags(containerInstanceArn).Return([]*ecs.Tag{
&ecs.Tag{
{
Key: &contInstTag1Key,
Value: &contInstTag1Val,
},
&ecs.Tag{
{
Key: &contInstTag2Key,
Value: &contInstTag2Val,
},
}, nil),
ecsClient.EXPECT().GetResourceTags(taskARN).Return([]*ecs.Tag{
&ecs.Tag{
{
Key: &taskTag1Key,
Value: &taskTag1Val,
},
&ecs.Tag{
{
Key: &taskTag2Key,
Value: &taskTag2Val,
},
Expand Down Expand Up @@ -1469,21 +1469,21 @@ func TestV4TaskMetadataWithTags(t *testing.T) {
state.EXPECT().TaskByArn(taskARN).Return(task, true).AnyTimes(),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
ecsClient.EXPECT().GetResourceTags(containerInstanceArn).Return([]*ecs.Tag{
&ecs.Tag{
{
Key: &contInstTag1Key,
Value: &contInstTag1Val,
},
&ecs.Tag{
{
Key: &contInstTag2Key,
Value: &contInstTag2Val,
},
}, nil),
ecsClient.EXPECT().GetResourceTags(taskARN).Return([]*ecs.Tag{
&ecs.Tag{
{
Key: &taskTag1Key,
Value: &taskTag1Val,
},
&ecs.Tag{
{
Key: &taskTag2Key,
Value: &taskTag2Val,
},
Expand Down
107 changes: 29 additions & 78 deletions agent/handlers/v2/response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,86 +14,22 @@
package v2

import (
"time"

"github.com/aws/aws-sdk-go/aws/awserr"

"github.com/aws/amazon-ecs-agent/agent/api"
apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
apicontainerstatus "github.com/aws/amazon-ecs-agent/agent/api/container/status"
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
v1 "github.com/aws/amazon-ecs-agent/agent/handlers/v1"
apieni "github.com/aws/amazon-ecs-agent/ecs-agent/api/eni"
tmdsresponse "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/response"
"github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/utils"
tmdsv2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
"github.com/aws/aws-sdk-go/aws"
"github.com/cihub/seelog"
"github.com/pkg/errors"
)

// TaskResponse defines the schema for the task response JSON object
type TaskResponse struct {
Cluster string `json:"Cluster"`
TaskARN string `json:"TaskARN"`
Family string `json:"Family"`
Revision string `json:"Revision"`
DesiredStatus string `json:"DesiredStatus,omitempty"`
KnownStatus string `json:"KnownStatus"`
Containers []ContainerResponse `json:"Containers,omitempty"`
Limits *LimitsResponse `json:"Limits,omitempty"`
PullStartedAt *time.Time `json:"PullStartedAt,omitempty"`
PullStoppedAt *time.Time `json:"PullStoppedAt,omitempty"`
ExecutionStoppedAt *time.Time `json:"ExecutionStoppedAt,omitempty"`
AvailabilityZone string `json:"AvailabilityZone,omitempty"`
TaskTags map[string]string `json:"TaskTags,omitempty"`
ContainerInstanceTags map[string]string `json:"ContainerInstanceTags,omitempty"`
LaunchType string `json:"LaunchType,omitempty"`
Errors []ErrorResponse `json:"Errors,omitempty"`
}

// ContainerResponse defines the schema for the container response
// JSON object
type ContainerResponse struct {
ID string `json:"DockerId"`
Name string `json:"Name"`
DockerName string `json:"DockerName"`
Image string `json:"Image"`
ImageID string `json:"ImageID"`
Ports []tmdsresponse.PortResponse `json:"Ports,omitempty"`
Labels map[string]string `json:"Labels,omitempty"`
DesiredStatus string `json:"DesiredStatus"`
KnownStatus string `json:"KnownStatus"`
ExitCode *int `json:"ExitCode,omitempty"`
Limits LimitsResponse `json:"Limits"`
CreatedAt *time.Time `json:"CreatedAt,omitempty"`
StartedAt *time.Time `json:"StartedAt,omitempty"`
FinishedAt *time.Time `json:"FinishedAt,omitempty"`
Type string `json:"Type"`
Networks []tmdsresponse.Network `json:"Networks,omitempty"`
Health *apicontainer.HealthStatus `json:"Health,omitempty"`
Volumes []tmdsresponse.VolumeResponse `json:"Volumes,omitempty"`
LogDriver string `json:"LogDriver,omitempty"`
LogOptions map[string]string `json:"LogOptions,omitempty"`
ContainerARN string `json:"ContainerARN,omitempty"`
}

// LimitsResponse defines the schema for task/cpu limits response
// JSON object
type LimitsResponse struct {
CPU *float64 `json:"CPU,omitempty"`
Memory *int64 `json:"Memory,omitempty"`
}

// ErrorResponse defined the schema for error response
// JSON object
type ErrorResponse struct {
ErrorField string `json:"ErrorField,omitempty"`
ErrorCode string `json:"ErrorCode,omitempty"`
ErrorMessage string `json:"ErrorMessage,omitempty"`
StatusCode int `json:"StatusCode,omitempty"`
RequestId string `json:"RequestId,omitempty"`
ResourceARN string `json:"ResourceARN,omitempty"`
}

// Agent versions >= 1.2.0: Null, zero, and CPU values of 1
// are passed to Docker as two CPU shares
const minimumCPUUnit = 2
Expand All @@ -108,13 +44,13 @@ func NewTaskResponse(
containerInstanceArn string,
propagateTags bool,
includeV4Metadata bool,
) (*TaskResponse, error) {
) (*tmdsv2.TaskResponse, error) {
task, ok := state.TaskByArn(taskARN)
if !ok {
return nil, errors.Errorf("v2 task response: unable to find task '%s'", taskARN)
}

resp := &TaskResponse{
resp := &tmdsv2.TaskResponse{
Cluster: cluster,
TaskARN: task.Arn,
Family: task.Family,
Expand All @@ -130,7 +66,7 @@ func NewTaskResponse(
taskCPU := task.CPU
taskMemory := task.Memory
if taskCPU != 0 || taskMemory != 0 {
taskLimits := &LimitsResponse{}
taskLimits := &tmdsv2.LimitsResponse{}
if taskCPU != 0 {
taskLimits.CPU = &taskCPU
}
Expand Down Expand Up @@ -169,7 +105,7 @@ func NewTaskResponse(
}

// propagateTagsToMetadata retrieves container instance and task tags from ECS
func propagateTagsToMetadata(ecsClient api.ECSClient, containerInstanceARN, taskARN string, resp *TaskResponse, includeV4Metadata bool) {
func propagateTagsToMetadata(ecsClient api.ECSClient, containerInstanceARN, taskARN string, resp *tmdsv2.TaskResponse, includeV4Metadata bool) {
containerInstanceTags, err := ecsClient.GetResourceTags(containerInstanceARN)

if err == nil {
Expand Down Expand Up @@ -198,7 +134,7 @@ func NewContainerResponseFromState(
containerID string,
state dockerstate.TaskEngineState,
includeV4Metadata bool,
) (*ContainerResponse, error) {
) (*tmdsv2.ContainerResponse, error) {
dockerContainer, ok := state.ContainerByID(containerID)
if !ok {
return nil, errors.Errorf(
Expand All @@ -220,17 +156,17 @@ func NewContainerResponse(
dockerContainer *apicontainer.DockerContainer,
eni *apieni.ENI,
includeV4Metadata bool,
) ContainerResponse {
) tmdsv2.ContainerResponse {
container := dockerContainer.Container
resp := ContainerResponse{
resp := tmdsv2.ContainerResponse{
ID: dockerContainer.DockerID,
Name: container.Name,
DockerName: dockerContainer.DockerName,
Image: container.Image,
ImageID: container.ImageID,
DesiredStatus: container.GetDesiredStatus().String(),
KnownStatus: container.GetKnownStatus().String(),
Limits: LimitsResponse{
Limits: tmdsv2.LimitsResponse{
CPU: aws.Float64(float64(container.CPU)),
Memory: aws.Int64(int64(container.Memory)),
},
Expand All @@ -255,7 +191,7 @@ func NewContainerResponse(
// Write the container health status inside the container
if dockerContainer.Container.HealthStatusShouldBeReported() {
health := dockerContainer.Container.GetHealthStatus()
resp.Health = &health
resp.Health = dockerContainerHealthToV2Health(health)
}

if createdAt := container.GetCreatedAt(); !createdAt.IsZero() {
Expand Down Expand Up @@ -302,9 +238,24 @@ func NewContainerResponse(
return resp
}

// Converts apicontainer HealthStatus type to v2 Metadata HealthStatus type
func dockerContainerHealthToV2Health(health apicontainer.HealthStatus) *tmdsv2.HealthStatus {
status := health.Status.String()
if health.Status == apicontainerstatus.ContainerHealthUnknown {
// Skip sending status if it is unknown
Copy link
Contributor

@chienhanlin chienhanlin May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q. did we previously send heath status ContainerHealthUnknown? If yes, would this be a new change introduced to v2? Also curious how would Since, ExitCode and Output look like when status is ContainerHealthUnknown : )

Copy link
Contributor Author

@amogh09 amogh09 May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. No we do not send Status when container's health status is ContainerHealthUnknown because the Status field in apicontainer.HealthStatus struct is defined with json:"status,omitempty".

ContainerHealthUnknown is the zero value of Status's type so Status = ContainerHealthUnknown is skipped when apicontainer.HealthStatus is marshaled to JSON.

The TestDockerContainerHealthToV2HealthJSON test in this PR covers this case.

amazon-ecs-agent/agent/api/container/container.go

Lines 111 to 120 in 883211e

type HealthStatus struct {
// Status is the container health status
Status apicontainerstatus.ContainerHealthStatus `json:"status,omitempty"`
// Since is the timestamp when container health status changed
Since *time.Time `json:"statusSince,omitempty"`
// ExitCode is the exitcode of health check if failed
ExitCode int `json:"exitCode,omitempty"`
// Output is the output of health check
Output string `json:"output,omitempty"`
}

amazon-ecs-agent/agent/api/container/status/containerstatus.go

Lines 43 to 50 in 883211e

const (
// ContainerHealthUnknown is the initial status of container health
ContainerHealthUnknown ContainerHealthStatus = iota
// ContainerHealthy represents the status of container health check when returned healthy
ContainerHealthy
// ContainerUnhealthy represents the status of container health check when returned unhealthy
ContainerUnhealthy
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining this in detail!

status = ""
}
return &tmdsv2.HealthStatus{
Status: status,
Since: health.Since,
ExitCode: health.ExitCode,
Output: health.Output,
}
}

// metadataErrorHandling writes an error to the logger, and append an error response
// to V4 metadata endpoint task response
func metadataErrorHandling(resp *TaskResponse, err error, field, resourceARN string, includeV4Metadata bool) {
func metadataErrorHandling(resp *tmdsv2.TaskResponse, err error, field, resourceARN string, includeV4Metadata bool) {
seelog.Errorf("Task Metadata error: unable to get '%s' for '%s': %s", field, resourceARN, err.Error())
if includeV4Metadata {
errResp := newErrorResponse(err, field, resourceARN)
Expand All @@ -313,8 +264,8 @@ func metadataErrorHandling(resp *TaskResponse, err error, field, resourceARN str
}

// newErrorResponse creates a new error response
func newErrorResponse(err error, field, resourceARN string) *ErrorResponse {
errResp := &ErrorResponse{
func newErrorResponse(err error, field, resourceARN string) *tmdsv2.ErrorResponse {
errResp := &tmdsv2.ErrorResponse{
ErrorField: field,
ErrorMessage: err.Error(),
ResourceARN: resourceARN,
Expand Down
Loading