From 5e8307177d8561a54b5912070488aa398e653dc2 Mon Sep 17 00:00:00 2001 From: Amogh Rathore Date: Wed, 23 Apr 2025 01:03:21 +0000 Subject: [PATCH] Fix error handling bug in Service Connect --- agent/engine/serviceconnect/manager_linux.go | 7 ++- .../serviceconnect/manager_linux_test.go | 49 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/agent/engine/serviceconnect/manager_linux.go b/agent/engine/serviceconnect/manager_linux.go index 607ce9429cc..0d0ef3a4a0c 100644 --- a/agent/engine/serviceconnect/manager_linux.go +++ b/agent/engine/serviceconnect/manager_linux.go @@ -348,9 +348,12 @@ func (m *manager) AugmentTaskContainer( DNSConfigToDockerExtraHostsFormat(task.ServiceConnectConfig.DNSConfig)...) } if container == task.GetServiceConnectContainer() { - m.augmentAgentContainer(task, container, hostConfig, instanceIPCompatibility) + err = m.augmentAgentContainer(task, container, hostConfig, instanceIPCompatibility) } - return err + if err != nil { + return dockerapi.CannotCreateContainerError{FromError: err} + } + return nil } func (m *manager) CreateInstanceTask(cfg *config.Config) (*apitask.Task, error) { diff --git a/agent/engine/serviceconnect/manager_linux_test.go b/agent/engine/serviceconnect/manager_linux_test.go index 62dbcd2b236..5512dc6e35a 100644 --- a/agent/engine/serviceconnect/manager_linux_test.go +++ b/agent/engine/serviceconnect/manager_linux_test.go @@ -23,7 +23,10 @@ import ( apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" "github.com/aws/amazon-ecs-agent/agent/api/serviceconnect" + apitask "github.com/aws/amazon-ecs-agent/agent/api/task" "github.com/aws/amazon-ecs-agent/agent/config/ipcompatibility" + "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" + dockertypes "github.com/docker/docker/api/types" dockercontainer "github.com/docker/docker/api/types/container" "github.com/stretchr/testify/assert" ) @@ -311,3 +314,49 @@ func TestIsIsoRegion(t *testing.T) { }) } } + +// Tests that AugmentTaskContainer returns an error if it fails. +func TestAugmentTaskContainerError(t *testing.T) { + t.Run("returns an error if container IP mapping could not be generated", func(t *testing.T) { + // Task containers do not have an IPv6 address + task := &apitask.Task{ + NetworkMode: apitask.BridgeNetworkMode, + Containers: []*apicontainer.Container{ + { + Type: apicontainer.ContainerCNIPause, + Name: "~internal~ecs~pause-web", + NetworkSettingsUnsafe: &dockertypes.NetworkSettings{ + DefaultNetworkSettings: dockertypes.DefaultNetworkSettings{ + IPAddress: "1.2.3.4", + }, + }, + }, + { + Type: apicontainer.ContainerNormal, + Name: "web", + }, + { + Type: apicontainer.ContainerCNIPause, + Name: "~internal~ecs~pause-sc-container", + NetworkSettingsUnsafe: &dockertypes.NetworkSettings{ + DefaultNetworkSettings: dockertypes.DefaultNetworkSettings{ + IPAddress: "1.2.3.5", + }, + }, + }, + { + Type: apicontainer.ContainerNormal, + Name: "sc-container", + }, + }, + ServiceConnectConfig: &serviceconnect.Config{ContainerName: "sc-container"}, + } + scManager := &manager{} + + // Instance has IPv6-only compatibility + err := scManager.AugmentTaskContainer(task, task.Containers[3], nil, ipcompatibility.NewIPv6OnlyCompatibility()) + namedErr, ok := err.(dockerapi.CannotCreateContainerError) + assert.True(t, ok) + assert.EqualError(t, namedErr, "instance is IPv6-only but no IPv6 address found for container 'web'") + }) +}