From c5ad6a357c1fab78f204f4ce9b00f766a8dbf9ee Mon Sep 17 00:00:00 2001 From: Yinyi Chen Date: Thu, 11 May 2023 16:54:07 -0700 Subject: [PATCH 1/4] Support firelens for bridge mode ServiceConnect task --- agent/engine/docker_task_engine.go | 45 ++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index a9349650435..696b66cbd61 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -1377,7 +1377,11 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a fluentNetworkPort: FluentNetworkPortValue, }) } else if container.GetNetworkModeFromHostConfig() == "" || container.GetNetworkModeFromHostConfig() == apitask.BridgeNetworkMode { - ipAddress, ok := getContainerHostIP(task.GetFirelensContainer().GetNetworkSettings()) + targetContainer := task.GetFirelensContainer() + if task.IsServiceConnectEnabled() { + targetContainer, _ = task.GetBridgeModePauseContainerForTaskContainer(targetContainer) + } + ipAddress, ok := getContainerHostIP(targetContainer.GetNetworkSettings()) if !ok { err := apierrors.DockerClientConfigError{Msg: "unable to get BridgeIP for task in bridge mode"} return dockerapi.DockerContainerMetadata{Error: apierrors.NamedError(&err)} @@ -1630,25 +1634,42 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap // For the supported network mode - bridge and awsvpc, the awsvpc take the host 127.0.0.1 but in bridge mode, // there is a need to wait for the IP to be present before the container using the firelens can be created. if container.GetFirelensConfig() != nil { - if !task.IsNetworkModeAWSVPC() && (container.GetNetworkModeFromHostConfig() == "" || container.GetNetworkModeFromHostConfig() == apitask.BridgeNetworkMode) { + if !task.IsNetworkModeAWSVPC() && + (container.GetNetworkModeFromHostConfig() == "" || + container.GetNetworkModeFromHostConfig() == apitask.BridgeNetworkMode || + (container.GetNetworkModeFromHostConfig() == "container" && task.IsServiceConnectEnabled())) { + _, gotContainerIP := getContainerHostIP(dockerContainerMD.NetworkSettings) + if task.IsServiceConnectEnabled() { + targetContainer, _ := task.GetBridgeModePauseContainerForTaskContainer(container) + _, gotContainerIP = getContainerHostIP(targetContainer.GetNetworkSettings()) + } + if !gotContainerIP { getIPBridgeBackoff := retry.NewExponentialBackoff(minGetIPBridgeTimeout, maxGetIPBridgeTimeout, getIPBridgeRetryJitterMultiplier, getIPBridgeRetryDelayMultiplier) contextWithTimeout, cancel := context.WithTimeout(engine.ctx, time.Minute) defer cancel() err := retry.RetryWithBackoffCtx(contextWithTimeout, getIPBridgeBackoff, func() error { - inspectOutput, err := engine.client.InspectContainer(engine.ctx, dockerContainerMD.DockerID, - dockerclient.InspectContainerTimeout) - if err != nil { - return err - } - _, gotIPBridge := getContainerHostIP(inspectOutput.NetworkSettings) - if gotIPBridge { - dockerContainerMD.NetworkSettings = inspectOutput.NetworkSettings - return nil + gotIPBridge := false + if task.IsServiceConnectEnabled() { + targetContainer, _ := task.GetBridgeModePauseContainerForTaskContainer(container) + _, gotIPBridge = getContainerHostIP(targetContainer.GetNetworkSettings()) + if gotIPBridge { + return nil + } } else { - return errors.New("Bridge IP not available to use for firelens") + inspectOutput, err := engine.client.InspectContainer(engine.ctx, dockerContainerMD.DockerID, + dockerclient.InspectContainerTimeout) + if err != nil { + return err + } + _, gotIPBridge = getContainerHostIP(inspectOutput.NetworkSettings) + if gotIPBridge { + dockerContainerMD.NetworkSettings = inspectOutput.NetworkSettings + return nil + } } + return errors.New("Bridge IP not available to use for firelens") }) if err != nil { return dockerapi.DockerContainerMetadata{ From f977c521165e8cc7ab2bcdbec5fa6ccfddda2649 Mon Sep 17 00:00:00 2001 From: Yinyi Chen Date: Mon, 15 May 2023 09:15:51 -0700 Subject: [PATCH 2/4] add comments and error handling --- agent/engine/docker_task_engine.go | 31 ++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 696b66cbd61..40b7ab23998 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -1378,8 +1378,18 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a }) } else if container.GetNetworkModeFromHostConfig() == "" || container.GetNetworkModeFromHostConfig() == apitask.BridgeNetworkMode { targetContainer := task.GetFirelensContainer() + // For bridge-mode ServiceConnect-enabled tasks, we inject pause container for each application container + // including the firelens container. Therefore, when resolving the container IP, we should be checking that + // of the associated pause container. if task.IsServiceConnectEnabled() { - targetContainer, _ = task.GetBridgeModePauseContainerForTaskContainer(targetContainer) + var err error + targetContainer, err = task.GetBridgeModePauseContainerForTaskContainer(targetContainer) + if err != nil { + return dockerapi.DockerContainerMetadata{ + Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf("failed to start firelens"+ + "container: %v", err))}, + } + } } ipAddress, ok := getContainerHostIP(targetContainer.GetNetworkSettings()) if !ok { @@ -1633,15 +1643,25 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap // If container is a firelens container, fluent host is needed to be added to the environment variable for the task. // For the supported network mode - bridge and awsvpc, the awsvpc take the host 127.0.0.1 but in bridge mode, // there is a need to wait for the IP to be present before the container using the firelens can be created. + // + // For bridge-mode ServiceConnect-enabled tasks, we inject pause container for each application container + // including the firelens container. Therefore, when resolving the container IP, we should be checking that + // of the associated pause container. In such case, the firelens container has network mode "container" since it's + //launched into its pause container's network namespace. if container.GetFirelensConfig() != nil { if !task.IsNetworkModeAWSVPC() && (container.GetNetworkModeFromHostConfig() == "" || container.GetNetworkModeFromHostConfig() == apitask.BridgeNetworkMode || (container.GetNetworkModeFromHostConfig() == "container" && task.IsServiceConnectEnabled())) { - _, gotContainerIP := getContainerHostIP(dockerContainerMD.NetworkSettings) if task.IsServiceConnectEnabled() { - targetContainer, _ := task.GetBridgeModePauseContainerForTaskContainer(container) + targetContainer, err := task.GetBridgeModePauseContainerForTaskContainer(container) + if err != nil { + return dockerapi.DockerContainerMetadata{ + Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf( + "failed to start firelens container: %v", err))}, + } + } _, gotContainerIP = getContainerHostIP(targetContainer.GetNetworkSettings()) } @@ -1652,7 +1672,10 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap err := retry.RetryWithBackoffCtx(contextWithTimeout, getIPBridgeBackoff, func() error { gotIPBridge := false if task.IsServiceConnectEnabled() { - targetContainer, _ := task.GetBridgeModePauseContainerForTaskContainer(container) + targetContainer, err := task.GetBridgeModePauseContainerForTaskContainer(container) + if err != nil { + return err + } _, gotIPBridge = getContainerHostIP(targetContainer.GetNetworkSettings()) if gotIPBridge { return nil From 10002ef2ee557707d36483ce685c396f549361a5 Mon Sep 17 00:00:00 2001 From: Yinyi Chen Date: Tue, 16 May 2023 14:43:38 -0700 Subject: [PATCH 3/4] fix error message --- agent/engine/docker_task_engine.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 40b7ab23998..03f398d9909 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -1386,8 +1386,8 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a targetContainer, err = task.GetBridgeModePauseContainerForTaskContainer(targetContainer) if err != nil { return dockerapi.DockerContainerMetadata{ - Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf("failed to start firelens"+ - "container: %v", err))}, + Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf( + "failed to start firelens container: %v", err))}, } } } From 9e7dfbe01691533df652ada0e94d54b4a6dd80f3 Mon Sep 17 00:00:00 2001 From: Yinyi Chen Date: Wed, 17 May 2023 17:34:49 -0700 Subject: [PATCH 4/4] Clean up code, add logs and tests --- agent/api/task/task.go | 4 + agent/engine/docker_task_engine.go | 111 ++++---- agent/engine/docker_task_engine_test.go | 352 +++++++++++++++++------- 3 files changed, 315 insertions(+), 152 deletions(-) diff --git a/agent/api/task/task.go b/agent/api/task/task.go index 49fd6618f68..935bd07f572 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -3443,6 +3443,10 @@ func (task *Task) IsServiceConnectEnabled() bool { return task.GetServiceConnectContainer() != nil } +func (task *Task) IsServiceConnectBridgeModeApplicationContainer(container *apicontainer.Container) bool { + return container.GetNetworkModeFromHostConfig() == "container" && task.IsServiceConnectEnabled() +} + // PopulateServiceConnectContainerMappingEnvVar populates APPNET_CONTAINER_IP_MAPPING env var for AppNet Agent container // aka SC container func (task *Task) PopulateServiceConnectContainerMappingEnvVar() error { diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 03f398d9909..d9079ab03c8 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -1385,9 +1385,16 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a var err error targetContainer, err = task.GetBridgeModePauseContainerForTaskContainer(targetContainer) if err != nil { + logger.Error("Failed to create container", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: container.Name, + field.Error: errors.New(fmt.Sprintf( + "container uses awsfirelens log driver but we failed to resolve Firelens bridge IP: %v", err)), + }) return dockerapi.DockerContainerMetadata{ - Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf( - "failed to start firelens container: %v", err))}, + Error: dockerapi.CannotCreateContainerError{FromError: errors.New(fmt.Sprintf( + "failed to create container - container uses awsfirelens log driver but we failed to "+ + "resolve Firelens bridge IP: %v", err))}, } } } @@ -1647,60 +1654,64 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap // For bridge-mode ServiceConnect-enabled tasks, we inject pause container for each application container // including the firelens container. Therefore, when resolving the container IP, we should be checking that // of the associated pause container. In such case, the firelens container has network mode "container" since it's - //launched into its pause container's network namespace. - if container.GetFirelensConfig() != nil { - if !task.IsNetworkModeAWSVPC() && - (container.GetNetworkModeFromHostConfig() == "" || - container.GetNetworkModeFromHostConfig() == apitask.BridgeNetworkMode || - (container.GetNetworkModeFromHostConfig() == "container" && task.IsServiceConnectEnabled())) { - _, gotContainerIP := getContainerHostIP(dockerContainerMD.NetworkSettings) - if task.IsServiceConnectEnabled() { - targetContainer, err := task.GetBridgeModePauseContainerForTaskContainer(container) - if err != nil { - return dockerapi.DockerContainerMetadata{ - Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf( - "failed to start firelens container: %v", err))}, - } + // launched into its pause container's network namespace. + if container.GetFirelensConfig() != nil && task.IsNetworkModeBridge() { + _, gotContainerIP := getContainerHostIP(dockerContainerMD.NetworkSettings) + if task.IsServiceConnectEnabled() { + targetContainer, err := task.GetBridgeModePauseContainerForTaskContainer(container) + if err != nil { + logger.Error("Failed to start Firelens container", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: container.Name, + field.Error: err, + }) + return dockerapi.DockerContainerMetadata{ + Error: dockerapi.CannotStartContainerError{FromError: errors.New(fmt.Sprintf( + "failed to start firelens container: %v", err))}, } - _, gotContainerIP = getContainerHostIP(targetContainer.GetNetworkSettings()) } - - if !gotContainerIP { - getIPBridgeBackoff := retry.NewExponentialBackoff(minGetIPBridgeTimeout, maxGetIPBridgeTimeout, getIPBridgeRetryJitterMultiplier, getIPBridgeRetryDelayMultiplier) - contextWithTimeout, cancel := context.WithTimeout(engine.ctx, time.Minute) - defer cancel() - err := retry.RetryWithBackoffCtx(contextWithTimeout, getIPBridgeBackoff, func() error { - gotIPBridge := false - if task.IsServiceConnectEnabled() { - targetContainer, err := task.GetBridgeModePauseContainerForTaskContainer(container) - if err != nil { - return err - } - _, gotIPBridge = getContainerHostIP(targetContainer.GetNetworkSettings()) - if gotIPBridge { - return nil - } - } else { - inspectOutput, err := engine.client.InspectContainer(engine.ctx, dockerContainerMD.DockerID, - dockerclient.InspectContainerTimeout) - if err != nil { - return err - } - _, gotIPBridge = getContainerHostIP(inspectOutput.NetworkSettings) - if gotIPBridge { - dockerContainerMD.NetworkSettings = inspectOutput.NetworkSettings - return nil - } + _, gotContainerIP = getContainerHostIP(targetContainer.GetNetworkSettings()) + } + + if !gotContainerIP { + getIPBridgeBackoff := retry.NewExponentialBackoff(minGetIPBridgeTimeout, maxGetIPBridgeTimeout, getIPBridgeRetryJitterMultiplier, getIPBridgeRetryDelayMultiplier) + contextWithTimeout, cancel := context.WithTimeout(engine.ctx, time.Minute) + defer cancel() + err := retry.RetryWithBackoffCtx(contextWithTimeout, getIPBridgeBackoff, func() error { + gotIPBridge := false + if task.IsServiceConnectEnabled() { + targetContainer, err := task.GetBridgeModePauseContainerForTaskContainer(container) + if err != nil { + return err } - return errors.New("Bridge IP not available to use for firelens") - }) - if err != nil { - return dockerapi.DockerContainerMetadata{ - Error: dockerapi.CannotStartContainerError{FromError: err}, + _, gotIPBridge = getContainerHostIP(targetContainer.GetNetworkSettings()) + if gotIPBridge { + return nil + } + } else { + inspectOutput, err := engine.client.InspectContainer(engine.ctx, dockerContainerMD.DockerID, + dockerclient.InspectContainerTimeout) + if err != nil { + return err } + _, gotIPBridge = getContainerHostIP(inspectOutput.NetworkSettings) + if gotIPBridge { + dockerContainerMD.NetworkSettings = inspectOutput.NetworkSettings + return nil + } + } + return errors.New("Bridge IP not available to use for firelens") + }) + if err != nil { + logger.Error("Failed to start Firelens container", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: container.Name, + field.Error: err, + }) + return dockerapi.DockerContainerMetadata{ + Error: dockerapi.CannotStartContainerError{FromError: err}, } } - } } if execcmd.IsExecEnabledContainer(container) { diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index b41a50a1aa5..910587985f1 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -2806,7 +2806,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { envVarPort := "FLUENT_PORT=24224" envVarAWSVPCMode := "FLUENT_HOST=127.0.0.1" eniIPv4Address := "10.0.0.2" - getTask := func(logDriverType string, networkMode string) *apitask.Task { + getTask := func(logDriverType string, networkMode string, enableServiceConnect bool) *apitask.Task { rawHostConfigInput := dockercontainer.HostConfig{ LogConfig: dockercontainer.LogConfig{ Type: logDriverType, @@ -2819,7 +2819,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { } rawHostConfig, err := json.Marshal(&rawHostConfigInput) require.NoError(t, err) - return &apitask.Task{ + task := apitask.Task{ Arn: taskARN, Version: taskVersion, Family: taskFamily, @@ -2835,20 +2835,67 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { }, NetworkModeUnsafe: networkMode, }, - { - Name: "test-container", - FirelensConfig: &apicontainer.FirelensConfig{ - Type: "fluentd", + }, + } + + appContainerBridgeIp := bridgeIPAddr + appContainerNetworkMode := networkMode + firelensContainerName := "test-firelens" + + if enableServiceConnect { + appContainerBridgeIp = "" + appContainerNetworkMode = "container" + } + firelensContainer := &apicontainer.Container{ + Name: firelensContainerName, + FirelensConfig: &apicontainer.FirelensConfig{ + Type: "fluentd", + }, + NetworkModeUnsafe: appContainerNetworkMode, + NetworkSettingsUnsafe: &types.NetworkSettings{ + DefaultNetworkSettings: types.DefaultNetworkSettings{ + IPAddress: appContainerBridgeIp, + }, + }, + } + task.Containers = append(task.Containers, firelensContainer) + + if enableServiceConnect { + // add pause container for application container + applicationPauseContainer := &apicontainer.Container{ + Name: fmt.Sprintf("~internal~ecs~pause-%s", taskName), + NetworkModeUnsafe: networkMode, + NetworkSettingsUnsafe: &types.NetworkSettings{ + DefaultNetworkSettings: types.DefaultNetworkSettings{ + IPAddress: bridgeIPAddr, }, - NetworkModeUnsafe: networkMode, - NetworkSettingsUnsafe: &types.NetworkSettings{ - DefaultNetworkSettings: types.DefaultNetworkSettings{ - IPAddress: bridgeIPAddr, - }, + }, + } + + // add pause container for firelensContainer + firelensPauseContainer := &apicontainer.Container{ + Name: fmt.Sprintf("~internal~ecs~pause-%s", firelensContainerName), + NetworkModeUnsafe: networkMode, + NetworkSettingsUnsafe: &types.NetworkSettings{ + DefaultNetworkSettings: types.DefaultNetworkSettings{ + IPAddress: bridgeIPAddr, }, }, - }, + } + task.Containers = append(task.Containers, firelensPauseContainer) + task.Containers = append(task.Containers, applicationPauseContainer) + + // dummy service connect config + task.ServiceConnectConfig = &serviceconnect.Config{ + ContainerName: "service-connect", + } + scContainer := &apicontainer.Container{ + Name: "service-connect", + } + task.Containers = append(task.Containers, scContainer) } + + return &task } getTaskWithENI := func(logDriverType string, networkMode string) *apitask.Task { rawHostConfigInput := dockercontainer.HostConfig{ @@ -2905,6 +2952,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { testCases := []struct { name string task *apitask.Task + enableServiceConnect bool expectedLogConfigType string expectedLogConfigTag string expectedLogConfigFluentAddress string @@ -2916,7 +2964,8 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { }{ { name: "test container that uses firelens log driver with default mode", - task: getTask(logDriverTypeFirelens, ""), + task: getTask(logDriverTypeFirelens, "", false), + enableServiceConnect: false, expectedLogConfigType: logDriverTypeFluentd, expectedLogConfigTag: taskName + "-firelens-" + taskID, expectedFluentdAsyncConnect: strconv.FormatBool(true), @@ -2928,7 +2977,21 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { }, { name: "test container that uses firelens log driver with bridge mode", - task: getTask(logDriverTypeFirelens, networkModeBridge), + task: getTask(logDriverTypeFirelens, networkModeBridge, false), + enableServiceConnect: false, + expectedLogConfigType: logDriverTypeFluentd, + expectedLogConfigTag: taskName + "-firelens-" + taskID, + expectedFluentdAsyncConnect: strconv.FormatBool(true), + expectedSubSecondPrecision: strconv.FormatBool(true), + expectedBufferLimit: "10000", + expectedLogConfigFluentAddress: socketPathPrefix + filepath.Join(defaultConfig.DataDirOnHost, dataLogDriverPath, taskID, dataLogDriverSocketPath), + expectedIPAddress: envVarBridgeMode, + expectedPort: envVarPort, + }, + { + name: "test container that uses firelens log driver with bridge mode with Service Connect", + task: getTask(logDriverTypeFirelens, networkModeBridge, true), + enableServiceConnect: true, expectedLogConfigType: logDriverTypeFluentd, expectedLogConfigTag: taskName + "-firelens-" + taskID, expectedFluentdAsyncConnect: strconv.FormatBool(true), @@ -2941,6 +3004,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { { name: "test container that uses firelens log driver with awsvpc mode", task: getTaskWithENI(logDriverTypeFirelens, networkModeAWSVPC), + enableServiceConnect: false, expectedLogConfigType: logDriverTypeFluentd, expectedLogConfigTag: taskName + "-firelens-" + taskID, expectedFluentdAsyncConnect: strconv.FormatBool(true), @@ -2956,10 +3020,13 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { t.Run(tc.name, func(t *testing.T) { ctx, cancel := context.WithCancel(context.TODO()) defer cancel() - ctrl, client, _, taskEngine, _, _, _, _ := mocks(t, ctx, &defaultConfig) + ctrl, client, _, taskEngine, _, _, _, serviceConnectManager := mocks(t, ctx, &defaultConfig) defer ctrl.Finish() client.EXPECT().APIVersion().Return(defaultDockerClientAPIVersion, nil).AnyTimes() + if tc.enableServiceConnect { + serviceConnectManager.EXPECT().AugmentTaskContainer(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) + } client.EXPECT().CreateContainer(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do( func(ctx context.Context, config *dockercontainer.Config, @@ -3073,104 +3140,185 @@ func TestGetBridgeIP(t *testing.T) { } func TestStartFirelensContainerRetryForContainerIP(t *testing.T) { - dockerMetaDataWithoutNetworkSettings := dockerapi.DockerContainerMetadata{ - DockerID: containerID, - Volumes: []types.MountPoint{ - { - Name: "volume", - Source: "/src/vol", - Destination: "/vol", - }, - }, - } - rawHostConfigInput := dockercontainer.HostConfig{ - LogConfig: dockercontainer.LogConfig{ - Type: "fluentd", - Config: map[string]string{ - "key1": "value1", - "key2": "value2", + applicationContainerName := "logSenderTask" + firelensContainerName := "test-firelens" + bridgeIPAddr := "bridgeIP" + + getTask := func(enableServiceConnect bool) *apitask.Task { + rawHostConfigInput := dockercontainer.HostConfig{ + LogConfig: dockercontainer.LogConfig{ + Type: "fluentd", + Config: map[string]string{ + "key1": "value1", + "key2": "value2", + }, }, - }, - } - jsonBaseWithoutNetwork := &types.ContainerJSON{ - ContainerJSONBase: &types.ContainerJSONBase{ - ID: containerID, - State: &types.ContainerState{Pid: containerPid}, - HostConfig: &dockercontainer.HostConfig{ - NetworkMode: containerNetworkMode, + } + rawHostConfig, err := json.Marshal(&rawHostConfigInput) + require.NoError(t, err) + task := &apitask.Task{ + Arn: "arn:aws:ecs:region:account-id:task/task-id", + Version: "1", + Family: "logSenderTaskFamily", + Containers: []*apicontainer.Container{ + { + Name: applicationContainerName, + DockerConfig: apicontainer.DockerConfig{ + HostConfig: func() *string { + s := string(rawHostConfig) + return &s + }(), + }, + NetworkModeUnsafe: apitask.BridgeNetworkMode, + }, + { + Name: firelensContainerName, + FirelensConfig: &apicontainer.FirelensConfig{ + Type: "fluentd", + }, + NetworkModeUnsafe: apitask.BridgeNetworkMode, + }, }, - }, + NetworkMode: networkModeBridge, + } + if enableServiceConnect { + task.Containers[0].NetworkModeUnsafe = "container" + task.Containers[1].NetworkModeUnsafe = "container" + + // add pause container for application container + applicationPauseContainer := &apicontainer.Container{ + Name: fmt.Sprintf("~internal~ecs~pause-%s", applicationContainerName), + NetworkModeUnsafe: apitask.BridgeNetworkMode, + NetworkSettingsUnsafe: &types.NetworkSettings{ + DefaultNetworkSettings: types.DefaultNetworkSettings{ + IPAddress: bridgeIPAddr, + }, + }, + } + + // add pause container for firelensContainer + firelensPauseContainer := &apicontainer.Container{ + Name: fmt.Sprintf("~internal~ecs~pause-%s", firelensContainerName), + NetworkModeUnsafe: apitask.BridgeNetworkMode, + NetworkSettingsUnsafe: &types.NetworkSettings{ + DefaultNetworkSettings: types.DefaultNetworkSettings{ + IPAddress: bridgeIPAddr, + }, + }, + } + task.Containers = append(task.Containers, applicationPauseContainer) + task.Containers = append(task.Containers, firelensPauseContainer) + + // dummy service connect config + task.ServiceConnectConfig = &serviceconnect.Config{ + ContainerName: "service-connect", + } + scContainer := &apicontainer.Container{ + Name: "service-connect", + } + task.Containers = append(task.Containers, scContainer) + } + return task } - jsonBaseWithNetwork := &types.ContainerJSON{ - ContainerJSONBase: &types.ContainerJSONBase{ - ID: containerID, - State: &types.ContainerState{Pid: containerPid}, - HostConfig: &dockercontainer.HostConfig{ - NetworkMode: containerNetworkMode, - }, + testCases := []struct { + name string + enableServiceConnect bool + testTask *apitask.Task + }{ + { + name: "ServiceConnect_Enabled", + enableServiceConnect: true, + testTask: getTask(true), }, - NetworkSettings: &types.NetworkSettings{ - DefaultNetworkSettings: types.DefaultNetworkSettings{ - IPAddress: networkBridgeIP, - }, - Networks: map[string]*network.EndpointSettings{ - apitask.BridgeNetworkMode: &network.EndpointSettings{ - IPAddress: networkBridgeIP, - }, - }, + { + name: "ServiceConnect_Not_Enabled", + enableServiceConnect: false, + testTask: getTask(false), }, } - rawHostConfig, err := json.Marshal(&rawHostConfigInput) - require.NoError(t, err) - testTask := &apitask.Task{ - Arn: "arn:aws:ecs:region:account-id:task/task-id", - Version: "1", - Family: "logSenderTaskFamily", - Containers: []*apicontainer.Container{ - { - Name: "logSenderTask", - DockerConfig: apicontainer.DockerConfig{ - HostConfig: func() *string { - s := string(rawHostConfig) - return &s - }(), + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dockerMetaDataWithoutNetworkSettings := dockerapi.DockerContainerMetadata{ + DockerID: containerID, + Volumes: []types.MountPoint{ + { + Name: "volume", + Source: "/src/vol", + Destination: "/vol", + }, }, - NetworkModeUnsafe: apitask.BridgeNetworkMode, - }, - { - Name: "test-container", - FirelensConfig: &apicontainer.FirelensConfig{ - Type: "fluentd", + } + jsonBaseWithoutNetwork := &types.ContainerJSON{ + ContainerJSONBase: &types.ContainerJSONBase{ + ID: containerID, + State: &types.ContainerState{Pid: containerPid}, + HostConfig: &dockercontainer.HostConfig{ + NetworkMode: containerNetworkMode, + }, }, - NetworkModeUnsafe: apitask.BridgeNetworkMode, - }, - }, + } + + jsonBaseWithNetwork := &types.ContainerJSON{ + ContainerJSONBase: &types.ContainerJSONBase{ + ID: containerID, + State: &types.ContainerState{Pid: containerPid}, + HostConfig: &dockercontainer.HostConfig{ + NetworkMode: containerNetworkMode, + }, + }, + NetworkSettings: &types.NetworkSettings{ + DefaultNetworkSettings: types.DefaultNetworkSettings{ + IPAddress: networkBridgeIP, + }, + Networks: map[string]*network.EndpointSettings{ + apitask.BridgeNetworkMode: &network.EndpointSettings{ + IPAddress: networkBridgeIP, + }, + }, + }, + } + task := tc.testTask + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + ctrl, client, _, taskEngine, _, _, _, _ := mocks(t, ctx, &defaultConfig) + defer ctrl.Finish() + taskEngine.(*DockerTaskEngine).state.AddTask(task) + taskEngine.(*DockerTaskEngine).state.AddContainer(&apicontainer.DockerContainer{ + Container: task.Containers[1], + DockerName: dockerContainerName, + DockerID: containerID, + }, task) + if tc.enableServiceConnect { + taskEngine.(*DockerTaskEngine).state.AddContainer(&apicontainer.DockerContainer{ + Container: task.Containers[3], + DockerName: fmt.Sprintf("~internal~ecs~pause-%s", dockerContainerName), + DockerID: "pauseContainerID", + }, task) + } + + client.EXPECT().APIVersion().Return(defaultDockerClientAPIVersion, nil).AnyTimes() + client.EXPECT().StartContainer(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerMetaDataWithoutNetworkSettings).AnyTimes() + if !tc.enableServiceConnect { + gomock.InOrder( + client.EXPECT().InspectContainer(gomock.Any(), containerID, gomock.Any()). + Return(jsonBaseWithoutNetwork, nil), + client.EXPECT().InspectContainer(gomock.Any(), containerID, gomock.Any()). + Return(jsonBaseWithoutNetwork, nil), + client.EXPECT().InspectContainer(gomock.Any(), containerID, gomock.Any()). + Return(jsonBaseWithNetwork, nil), + ) + } + ret := taskEngine.(*DockerTaskEngine).startContainer(task, task.Containers[1]) + assert.NoError(t, ret.Error) + if !tc.enableServiceConnect { + assert.Equal(t, jsonBaseWithNetwork.NetworkSettings, ret.NetworkSettings) + } else { + assert.Equal(t, jsonBaseWithoutNetwork.NetworkSettings, ret.NetworkSettings) + } + }) } - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - ctrl, client, _, taskEngine, _, _, _, _ := mocks(t, ctx, &defaultConfig) - defer ctrl.Finish() - taskEngine.(*DockerTaskEngine).state.AddTask(testTask) - taskEngine.(*DockerTaskEngine).state.AddContainer(&apicontainer.DockerContainer{ - Container: testTask.Containers[1], - DockerName: dockerContainerName, - DockerID: containerID, - }, testTask) - client.EXPECT().APIVersion().Return(defaultDockerClientAPIVersion, nil).AnyTimes() - client.EXPECT().StartContainer(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerMetaDataWithoutNetworkSettings).AnyTimes() - gomock.InOrder( - client.EXPECT().InspectContainer(gomock.Any(), containerID, gomock.Any()). - Return(jsonBaseWithoutNetwork, nil), - client.EXPECT().InspectContainer(gomock.Any(), containerID, gomock.Any()). - Return(jsonBaseWithoutNetwork, nil), - client.EXPECT().InspectContainer(gomock.Any(), containerID, gomock.Any()). - Return(jsonBaseWithNetwork, nil), - ) - ret := taskEngine.(*DockerTaskEngine).startContainer(testTask, testTask.Containers[1]) - assert.NoError(t, ret.Error) - assert.Equal(t, jsonBaseWithNetwork.NetworkSettings, ret.NetworkSettings) } func TestStartExecAgent(t *testing.T) {