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

Migrate ECS client to aws-sdk-go-v2 #4447

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from
Open
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
46 changes: 23 additions & 23 deletions agent/api/statechange.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ import (
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs"
ecsmodel "github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs/model/ecs"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
"github.com/aws/amazon-ecs-agent/ecs-agent/logger/field"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
"github.com/aws/amazon-ecs-agent/ecs-agent/utils"
"github.com/aws/aws-sdk-go-v2/service/ecs/types"

"github.com/aws/aws-sdk-go/aws"
"github.com/docker/go-connections/nat"
Expand Down Expand Up @@ -327,7 +327,7 @@ func (c *ContainerStateChange) ToECSAgent() (*ecs.ContainerStateChange, error) {
Status: c.Status,
ImageDigest: aws.StringValue(pl.ImageDigest),
Reason: aws.StringValue(pl.Reason),
ExitCode: utils.Int64PtrToIntPtr(pl.ExitCode),
ExitCode: utils.Int32PtrToIntPtr(pl.ExitCode),
NetworkBindings: pl.NetworkBindings,
MetadataGetter: newContainerMetadataGetter(c.Container),
}, nil
Expand Down Expand Up @@ -424,11 +424,11 @@ func (change *TaskStateChange) ToECSAgent() (*ecs.TaskStateChange, error) {

for _, managedAgentEvent := range change.ManagedAgents {
if mgspl := buildManagedAgentStateChangePayload(managedAgentEvent); mgspl != nil {
output.ManagedAgents = append(output.ManagedAgents, mgspl)
output.ManagedAgents = append(output.ManagedAgents, *mgspl)
}
}

containerEvents := make([]*ecsmodel.ContainerStateChange, len(change.Containers))
containerEvents := make([]types.ContainerStateChange, len(change.Containers))
for i, containerEvent := range change.Containers {
payload, err := buildContainerStateChangePayload(containerEvent)
if err != nil {
Expand All @@ -438,7 +438,7 @@ func (change *TaskStateChange) ToECSAgent() (*ecs.TaskStateChange, error) {
})
return nil, err
}
containerEvents[i] = payload
containerEvents[i] = *payload
}
output.Containers = containerEvents

Expand Down Expand Up @@ -481,7 +481,7 @@ func (AttachmentStateChange) GetEventType() statechange.EventType {
return statechange.AttachmentEvent
}

func buildManagedAgentStateChangePayload(change ManagedAgentStateChange) *ecsmodel.ManagedAgentStateChange {
func buildManagedAgentStateChangePayload(change ManagedAgentStateChange) *types.ManagedAgentStateChange {
Copy link
Contributor

@danehlim danehlim Feb 3, 2025

Choose a reason for hiding this comment

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

Is there any specific reason why we don't just return type types.ManagedAgentStateChange here? Returning pointer type *types.ManagedAgentStateChange here seems unnecessary to me (i.e., we just end up dereferencing on L427 anyway), but I may be missing something. It seems like in most other places in this pull request we replace *ecsmodel.<INSERT TYPE HERE> with types.<INSERT TYPE HERE> as opposed to *types.<INSERT TYPE HERE>.

if !change.Status.ShouldReportToBackend() {
logger.Warn("Not submitting unsupported managed agent state", logger.Fields{
field.Status: change.Status.String(),
Expand All @@ -490,19 +490,19 @@ func buildManagedAgentStateChangePayload(change ManagedAgentStateChange) *ecsmod
})
return nil
}
return &ecsmodel.ManagedAgentStateChange{
ManagedAgentName: aws.String(change.Name),
return &types.ManagedAgentStateChange{
ManagedAgentName: types.ManagedAgentName(change.Name),
ContainerName: aws.String(change.Container.Name),
Status: aws.String(change.Status.String()),
Reason: aws.String(change.Reason),
}
}

func buildContainerStateChangePayload(change ContainerStateChange) (*ecsmodel.ContainerStateChange, error) {
func buildContainerStateChangePayload(change ContainerStateChange) (*types.ContainerStateChange, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as this previous comment applies here too.

if change.ContainerName == "" {
return nil, fmt.Errorf("container state change has no container name")
}
statechange := &ecsmodel.ContainerStateChange{
statechange := types.ContainerStateChange{
ContainerName: aws.String(change.ContainerName),
}
if change.RuntimeID != "" {
Expand Down Expand Up @@ -534,8 +534,8 @@ func buildContainerStateChangePayload(change ContainerStateChange) (*ecsmodel.Co
statechange.Status = aws.String(stat.BackendStatusString())

if change.ExitCode != nil {
exitCode := int64(aws.IntValue(change.ExitCode))
statechange.ExitCode = aws.Int64(exitCode)
exitCode := int32(aws.IntValue(change.ExitCode))
statechange.ExitCode = aws.Int32(exitCode)
}

networkBindings := getNetworkBindings(change)
Expand All @@ -547,7 +547,7 @@ func buildContainerStateChangePayload(change ContainerStateChange) (*ecsmodel.Co
}
statechange.NetworkBindings = networkBindings

return statechange, nil
return &statechange, nil
}

// ProtocolBindIP used to store protocol and bindIP information associated to a particular host port
Expand All @@ -557,8 +557,8 @@ type ProtocolBindIP struct {
}

// getNetworkBindings returns the list of networkingBindings, sent to ECS as part of the container state change payload
func getNetworkBindings(change ContainerStateChange) []*ecsmodel.NetworkBinding {
networkBindings := []*ecsmodel.NetworkBinding{}
func getNetworkBindings(change ContainerStateChange) []types.NetworkBinding {
networkBindings := []types.NetworkBinding{}
// hostPortToProtocolBindIPMap is a map to store protocol and bindIP information associated to host ports
// that belong to a range. This is used in case when there are multiple protocol/bindIP combinations associated to a
// port binding. example: when both IPv4 and IPv6 bindIPs are populated by docker.
Expand All @@ -571,22 +571,22 @@ func getNetworkBindings(change ContainerStateChange) []*ecsmodel.NetworkBinding
containerPortRangeMap := change.Container.GetContainerPortRangeMap()

for _, binding := range change.PortBindings {
hostPort := int64(binding.HostPort)
containerPort := int64(binding.ContainerPort)
containerPort := int32(binding.ContainerPort)
bindIP := binding.BindIP
protocol := binding.Protocol.String()

// create network binding for each containerPort that exists in the singular ContainerPortSet
// for container ports that belong to a range, we'll have 1 consolidated network binding for the range
if _, ok := containerPortSet[int(containerPort)]; ok {
networkBindings = append(networkBindings, &ecsmodel.NetworkBinding{
networkBindings = append(networkBindings, types.NetworkBinding{
BindIP: aws.String(bindIP),
ContainerPort: aws.Int64(containerPort),
HostPort: aws.Int64(hostPort),
Protocol: aws.String(protocol),
ContainerPort: aws.Int32(containerPort),
HostPort: aws.Int32(int32(binding.HostPort)),
Protocol: types.TransportProtocol(protocol),
})
} else {
// populate hostPortToProtocolBindIPMap – this is used below when we construct network binding for ranges.
hostPort := int64(binding.HostPort)
hostPortToProtocolBindIPMap[hostPort] = append(hostPortToProtocolBindIPMap[hostPort],
ProtocolBindIP{
protocol: protocol,
Expand All @@ -601,11 +601,11 @@ func getNetworkBindings(change ContainerStateChange) []*ecsmodel.NetworkBinding
hostPort, _, _ := nat.ParsePortRangeToInt(hostPortRange)
if val, ok := hostPortToProtocolBindIPMap[int64(hostPort)]; ok {
for _, v := range val {
networkBindings = append(networkBindings, &ecsmodel.NetworkBinding{
networkBindings = append(networkBindings, types.NetworkBinding{
BindIP: aws.String(v.bindIP),
ContainerPortRange: aws.String(containerPortRange),
HostPortRange: aws.String(hostPortRange),
Protocol: aws.String(v.protocol),
Protocol: types.TransportProtocol(v.protocol),
})
}
}
Expand Down
43 changes: 21 additions & 22 deletions agent/api/statechange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ import (
apitask "github.com/aws/amazon-ecs-agent/agent/api/task"
"github.com/aws/amazon-ecs-agent/agent/engine/execcmd"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs/model/ecs"
ecsmodel "github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs/model/ecs"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -327,36 +326,36 @@ func TestNewManagedAgentChangeEvent(t *testing.T) {

func TestGetNetworkBindings(t *testing.T) {
testContainerStateChange := getTestContainerStateChange()
expectedNetworkBindings := []*ecs.NetworkBinding{
expectedNetworkBindings := []types.NetworkBinding{
{
BindIP: aws.String("0.0.0.0"),
ContainerPort: aws.Int64(10),
HostPort: aws.Int64(10),
Protocol: aws.String("tcp"),
ContainerPort: aws.Int32(10),
HostPort: aws.Int32(10),
Protocol: "tcp",
},
{
BindIP: aws.String("1.2.3.4"),
ContainerPort: aws.Int64(12),
HostPort: aws.Int64(12),
Protocol: aws.String("udp"),
ContainerPort: aws.Int32(12),
HostPort: aws.Int32(12),
Protocol: "udp",
},
{
BindIP: aws.String("5.6.7.8"),
ContainerPort: aws.Int64(15),
HostPort: aws.Int64(20),
Protocol: aws.String("tcp"),
ContainerPort: aws.Int32(15),
HostPort: aws.Int32(20),
Protocol: "tcp",
},
{
BindIP: aws.String("::"),
ContainerPortRange: aws.String("21-22"),
HostPortRange: aws.String("60001-60002"),
Protocol: aws.String("udp"),
Protocol: "udp",
},
{
BindIP: aws.String("0.0.0.0"),
ContainerPortRange: aws.String("96-97"),
HostPortRange: aws.String("47001-47002"),
Protocol: aws.String("tcp"),
Protocol: "tcp",
},
}

Expand Down Expand Up @@ -748,7 +747,7 @@ func TestBuildContainerStateChangePayload(t *testing.T) {
tcs := []struct {
name string
change ContainerStateChange
expected *ecsmodel.ContainerStateChange
expected *types.ContainerStateChange
expectedError string
}{
{
Expand All @@ -772,10 +771,10 @@ func TestBuildContainerStateChangePayload(t *testing.T) {
Status: apicontainerstatus.ContainerManifestPulled,
ImageDigest: "digest",
},
expected: &ecsmodel.ContainerStateChange{
expected: &types.ContainerStateChange{
ContainerName: aws.String("container"),
ImageDigest: aws.String("digest"),
NetworkBindings: []*ecs.NetworkBinding{},
NetworkBindings: []types.NetworkBinding{},
Status: aws.String("PENDING"),
},
},
Expand All @@ -788,11 +787,11 @@ func TestBuildContainerStateChangePayload(t *testing.T) {
ImageDigest: "digest",
RuntimeID: "runtimeid",
},
expected: &ecsmodel.ContainerStateChange{
expected: &types.ContainerStateChange{
ContainerName: aws.String("container"),
ImageDigest: aws.String("digest"),
RuntimeId: aws.String("runtimeid"),
NetworkBindings: []*ecs.NetworkBinding{},
NetworkBindings: []types.NetworkBinding{},
Status: aws.String("RUNNING"),
},
},
Expand All @@ -806,12 +805,12 @@ func TestBuildContainerStateChangePayload(t *testing.T) {
RuntimeID: "runtimeid",
ExitCode: aws.Int(1),
},
expected: &ecsmodel.ContainerStateChange{
expected: &types.ContainerStateChange{
ContainerName: aws.String("container"),
ImageDigest: aws.String("digest"),
RuntimeId: aws.String("runtimeid"),
ExitCode: aws.Int64(1),
NetworkBindings: []*ecs.NetworkBinding{},
ExitCode: aws.Int32(1),
NetworkBindings: []types.NetworkBinding{},
Status: aws.String("STOPPED"),
},
},
Expand Down
Loading
Loading