From eabe7bb1feb585ff97c6e1337e0dfe5915fe7d36 Mon Sep 17 00:00:00 2001 From: Chien-Han Lin Date: Wed, 15 Feb 2023 17:57:03 +0000 Subject: [PATCH] ECS Agent dynamic host port assignment 1. Add GetHostPort() and update unit tests https://github.com/aws/amazon-ecs-agent/pull/3570 2. Upudate dockerPortMap() in task.go with dynamic host port range support part 1 https://github.com/aws/amazon-ecs-agent/pull/3584 3. Upudate dockerPortMap() in task.go with dynamic host port range support part 2 https://github.com/aws/amazon-ecs-agent/pull/3589 4. Validate the host port/host port range found by ECS Agent before returning it https://github.com/aws/amazon-ecs-agent/pull/3589 --- agent/api/task/task.go | 166 ++++++++++++++---- agent/api/task/task_test.go | 255 +++++++++++++++++++--------- agent/config/config.go | 2 + agent/utils/ephemeral_ports.go | 81 ++++++++- agent/utils/ephemeral_ports_test.go | 130 +++++++++++++- 5 files changed, 513 insertions(+), 121 deletions(-) diff --git a/agent/api/task/task.go b/agent/api/task/task.go index 64f08e5aa8b..731e71572cc 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -2338,65 +2338,161 @@ func (task *Task) dockerLinks(container *apicontainer.Container, dockerContainer } var getHostPortRange = utils.GetHostPortRange +var getHostPort = utils.GetHostPort +// In buildPortMapWithSCIngressConfig, the dockerPortMap and the containerPortSet will be constructed +// for ingress listeners under two service connect bridge mode cases: +// (1) non-default bridge mode service connect experience: customers specify host ports for listeners in the ingress config. +// (2) default bridge mode service connect experience: customers do not specify host ports for listeners in the ingress config. +// +// Instead, ECS Agent finds host ports within the given dynamic host port range. An error will be returned for case (2) if +// ECS Agent cannot find an available host port within range. +func (task *Task) buildPortMapWithSCIngressConfig(dynamicHostPortRange string) (nat.PortMap, map[int]struct{}, error) { + var err error + ingressDockerPortMap := nat.PortMap{} + ingressContainerPortSet := make(map[int]struct{}) + protocolStr := "tcp" + for _, ic := range task.ServiceConnectConfig.IngressConfig { + listenerPortInt := int(ic.ListenerPort) + dockerPort := nat.Port(strconv.Itoa(listenerPortInt) + "/" + protocolStr) + hostPortStr := "" + if ic.HostPort != nil { + // For non-default bridge mode service connect experience, a host port is specified by customers + // Note that service connect ingress config has been validated in service_connect_validator.go, + // where host ports will be validated to ensure user-definied ports are within a valid port range (1 to 65535) + // and do not have port collisions. + hostPortStr = strconv.Itoa(int(*ic.HostPort)) + } else { + // For default bridge mode service connect experience, customers do not specify a host port + // thus the host port will be assigned by ECS Agent. + // ECS Agent will find an available host port within the given dynamic host port range, + // or return an error if no host port is available within the range. + hostPortStr, err = getHostPort(protocolStr, dynamicHostPortRange) + if err != nil { + return nil, nil, err + } + } + + ingressDockerPortMap[dockerPort] = append(ingressDockerPortMap[dockerPort], nat.PortBinding{HostPort: hostPortStr}) + // Append non-range, singular container port to the ingressContainerPortSet + ingressContainerPortSet[listenerPortInt] = struct{}{} + } + return ingressDockerPortMap, ingressContainerPortSet, err +} + +// dockerPortMap creates a port binding map for +// (1) Ingress listeners for the service connect AppNet container in the service connect bridge network mode task. +// (2) Port mapping configured by customers in the task definition. +// +// For service connect bridge mode task, we will create port bindings for customers' application containers +// and service connect AppNet container, and let them to be published by the associated pause containers. +// (a) For default bridge service connect experience, ECS Agent will assign a host port within the +// default/user-specified dynamic host port range for the ingress listener. If no available host port can be +// found by ECS Agent, an error will be returned. +// (b) For non-default bridge service connect experience, ECS Agent will use the user-defined host port for the ingress listener. +// +// For non-service connect bridge network mode task, ECS Agent will assign a host port or a host port range +// within the default/user-specified dynamic host port range. If no available host port or host port range can be +// found by ECS Agent, an error will be returned. +// +// Note that +// (a) ECS Agent will not assign a new host port within the dynamic host port range for awsvpc network mode task +// (b) ECS Agent will not assign a new host port within the dynamic host port range if the user-specified host port exists func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPortRange string) (nat.PortMap, error) { + hostPortStr := "" dockerPortMap := nat.PortMap{} - scContainer := task.GetServiceConnectContainer() containerToCheck := container containerPortSet := make(map[int]struct{}) containerPortRangeMap := make(map[string]string) + + // For service connect bridge network mode task, we will create port bindings for task containers, + // including both application containers and service connect AppNet container, and let them to be published + // by the associated pause containers. if task.IsServiceConnectEnabled() && task.IsNetworkModeBridge() { if container.Type == apicontainer.ContainerCNIPause { - // we will create bindings for task containers (including both customer containers and SC Appnet container) - // and let them be published by the associated pause container. - // Note - for SC bridge mode we do not allow customer to specify a host port for their containers. Additionally, - // When an ephemeral host port is assigned, Appnet will NOT proxy traffic to that port + // Find the task container associated with this particular pause container taskContainer, err := task.getBridgeModeTaskContainerForPauseContainer(container) if err != nil { return nil, err } + + scContainer := task.GetServiceConnectContainer() if taskContainer == scContainer { - // create bindings for all ingress listener ports - // no need to create binding for egress listener port as it won't be access from host level or from outside - for _, ic := range task.ServiceConnectConfig.IngressConfig { - listenerPortInt := int(ic.ListenerPort) - dockerPort := nat.Port(strconv.Itoa(listenerPortInt)) + "/tcp" - hostPort := 0 // default bridge-mode SC experience - host port will be an ephemeral port assigned by docker - if ic.HostPort != nil { // non-default bridge-mode SC experience - host port specified by customer - hostPort = int(*ic.HostPort) - } - dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: strconv.Itoa(hostPort)}) - // append non-range, singular container port to the containerPortSet - containerPortSet[listenerPortInt] = struct{}{} - // set taskContainer.ContainerPortSet to be used during network binding creation - taskContainer.SetContainerPortSet(containerPortSet) + // If the associated task container to this pause container is the service connect AppNet container, + // create port binding(s) for ingress listener ports based on its ingress config. + // Note that there is no need to do this for egress listener ports as they won't be accessed + // from host level or from outside. + dockerPortMap, containerPortSet, err := task.buildPortMapWithSCIngressConfig(dynamicHostPortRange) + if err != nil { + logger.Error("Failed to build a port map with service connect ingress config", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: taskContainer.Name, + "dynamicHostPortRange": dynamicHostPortRange, + field.Error: err, + }) + return nil, err } + // Set taskContainer.ContainerPortSet to be used during network binding creation + taskContainer.SetContainerPortSet(containerPortSet) return dockerPortMap, nil } + // If the associated task container to this pause container is NOT the service connect AppNet container, + // we will continue to update the dockerPortMap for the pause container using the port bindings + // configured for the application container since port bindings will be published by the pasue container. containerToCheck = taskContainer } else { - // If container is neither SC container nor pause container, it's a regular task container. Its port bindings(s) - // are published by the associated pause container, and we leave the map empty here (docker would actually complain - // otherwise). + // If the container is not a pause container, then it is a regular customers' application container + // or a service connect AppNet container. We will leave the map empty and return it as its port bindings(s) + // are published by the associated pause container. return dockerPortMap, nil } } + // For each port binding config, either one of containerPort or containerPortRange is set. + // (1) containerPort is the port number on the container that's bound to the user-specified host port or the + // host port assigned by ECS Agent. + // (2) containerPortRange is the port number range on the container that's bound to the mapped host port range + // found by ECS Agent. + var err error for _, portBinding := range containerToCheck.Ports { - // for each port binding config, either one of containerPort or containerPortRange is set if portBinding.ContainerPort != 0 { containerPort := int(portBinding.ContainerPort) + protocolStr := portBinding.Protocol.String() + dockerPort := nat.Port(strconv.Itoa(containerPort) + "/" + protocolStr) + + if portBinding.HostPort != 0 { + // An user-specified host port exists. + // Note that the host port value has been validated by ECS front end service; + // thus only an valid host port value will be streamed down to ECS Agent. + hostPortStr = strconv.Itoa(int(portBinding.HostPort)) + } else { + // If there is no user-specified host port, ECS Agent will find an available host port + // within the given dynamic host port range. And if no host port is available within the range, + // an error will be returned. + logger.Debug("No user-specified host port, ECS Agent will find an available host port within the given dynamic host port range", logger.Fields{ + field.Container: containerToCheck.Name, + "dynamicHostPortRange": dynamicHostPortRange, + }) + hostPortStr, err = getHostPort(protocolStr, dynamicHostPortRange) + if err != nil { + logger.Error("Unable to find a host port for container within the given dynamic host port range", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: container.Name, + "dynamicHostPortRange": dynamicHostPortRange, + field.Error: err, + }) + return nil, err + } + } + dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: hostPortStr}) - dockerPort := nat.Port(strconv.Itoa(containerPort) + "/" + portBinding.Protocol.String()) - dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: strconv.Itoa(int(portBinding.HostPort))}) - - // append non-range, singular container port to the containerPortSet + // For the containerPort case, append a non-range, singular container port to the containerPortSet. containerPortSet[containerPort] = struct{}{} } else if portBinding.ContainerPortRange != "" { containerToCheck.SetContainerHasPortRange(true) containerPortRange := portBinding.ContainerPortRange - // nat.ParsePortRangeToInt validates a port range; if valid, it returns start and end ports as integers + // nat.ParsePortRangeToInt validates a port range; if valid, it returns start and end ports as integers. startContainerPort, endContainerPort, err := nat.ParsePortRangeToInt(containerPortRange) if err != nil { return nil, err @@ -2404,8 +2500,8 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo numberOfPorts := endContainerPort - startContainerPort + 1 protocol := portBinding.Protocol.String() - // we will try to get a contiguous set of host ports from the ephemeral host port range. - // this is to ensure that docker maps host ports in a contiguous manner, and + // We will try to get a contiguous set of host ports from the ephemeral host port range. + // This is to ensure that docker maps host ports in a contiguous manner, and // we are guaranteed to have the entire hostPortRange in a single network binding while sending this info to ECS; // therefore, an error will be returned if we cannot find a contiguous set of host ports. hostPortRange, err := getHostPortRange(numberOfPorts, protocol, dynamicHostPortRange) @@ -2419,8 +2515,8 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo return nil, err } - // append ranges to the dockerPortMap - // nat.ParsePortSpec returns a list of port mappings in a format that docker likes + // For the ContainerPortRange case, append ranges to the dockerPortMap. + // nat.ParsePortSpec returns a list of port mappings in a format that Docker likes. mappings, err := nat.ParsePortSpec(hostPortRange + ":" + containerPortRange + "/" + protocol) if err != nil { return nil, err @@ -2430,13 +2526,13 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo dockerPortMap[mapping.Port] = append(dockerPortMap[mapping.Port], mapping.Binding) } - // append containerPortRange and associated hostPortRange to the containerPortRangeMap - // this will ensure that we consolidate range into 1 network binding while sending it to ECS + // For the ContainerPortRange case, append containerPortRange and associated hostPortRange to the containerPortRangeMap. + // This will ensure that we consolidate range into 1 network binding while sending it to ECS. containerPortRangeMap[containerPortRange] = hostPortRange } } - // set Container.ContainerPortSet and Container.ContainerPortRangeMap to be used during network binding creation + // Set Container.ContainerPortSet and Container.ContainerPortRangeMap to be used during network binding creation. containerToCheck.SetContainerPortSet(containerPortSet) containerToCheck.SetContainerPortRangeMap(containerPortRangeMap) return dockerPortMap, nil diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index c148eed50cb..97441600fef 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -240,13 +240,13 @@ func TestDockerHostConfigPortBinding(t *testing.T) { Ports: []apicontainer.PortBinding{ { ContainerPort: 10, - HostPort: 10, + HostPort: 20, BindIP: "", Protocol: apicontainer.TransportProtocolTCP, }, { ContainerPort: 20, - HostPort: 20, + HostPort: 30, BindIP: "", Protocol: apicontainer.TransportProtocolUDP, }, @@ -256,6 +256,31 @@ func TestDockerHostConfigPortBinding(t *testing.T) { } testTask2 := &Task{ + Containers: []*apicontainer.Container{ + { + Name: "c1", + Ports: []apicontainer.PortBinding{ + { + ContainerPort: 10, + BindIP: "", + Protocol: apicontainer.TransportProtocolTCP, + }, + { + ContainerPort: 20, + BindIP: "", + Protocol: apicontainer.TransportProtocolUDP, + }, + { + ContainerPort: 30, + BindIP: "", + Protocol: apicontainer.TransportProtocolUDP, + }, + }, + }, + }, + } + + testTask3 := &Task{ Containers: []*apicontainer.Container{ { Name: "c1", @@ -278,17 +303,20 @@ func TestDockerHostConfigPortBinding(t *testing.T) { testCases := []struct { testName string testTask *Task - getHostPortRange func(numberOfPorts int, protocol string, dynamicHostPortRange string) (string, error) + testDynamicHostPortRange string + testContainerPortRange string expectedPortBinding nat.PortMap expectedContainerPortSet map[int]struct{} expectedContainerPortRangeMap map[string]string + expectedError bool }{ { - testName: "2 port bindings, each with singular container port - host port", - testTask: testTask1, + testName: "user-specified container ports and host ports", + testTask: testTask1, + testDynamicHostPortRange: "40000-60000", expectedPortBinding: nat.PortMap{ - nat.Port("10/tcp"): []nat.PortBinding{{HostPort: "10"}}, - nat.Port("20/udp"): []nat.PortBinding{{HostPort: "20"}}, + nat.Port("10/tcp"): []nat.PortBinding{{HostPort: "20"}}, + nat.Port("20/udp"): []nat.PortBinding{{HostPort: "30"}}, }, expectedContainerPortSet: map[int]struct{}{ 10: {}, @@ -297,23 +325,37 @@ func TestDockerHostConfigPortBinding(t *testing.T) { expectedContainerPortRangeMap: map[string]string{}, }, { - testName: "2 port bindings, one with container port range, other with singular container port", - testTask: testTask2, - getHostPortRange: func(numberOfPorts int, protocol string, dynamicHostPortRange string) (string, error) { - return "155-157", nil - }, - expectedPortBinding: nat.PortMap{ - nat.Port("55/udp"): []nat.PortBinding{{HostPort: "155"}}, - nat.Port("56/udp"): []nat.PortBinding{{HostPort: "156"}}, - nat.Port("57/udp"): []nat.PortBinding{{HostPort: "157"}}, - nat.Port("80/tcp"): []nat.PortBinding{{HostPort: "0"}}, + testName: "user-specified container ports with a ideal dynamicHostPortRange", + testTask: testTask2, + testDynamicHostPortRange: "40000-60000", + expectedContainerPortSet: map[int]struct{}{ + 10: {}, + 20: {}, + 30: {}, }, + expectedContainerPortRangeMap: map[string]string{}, + }, + { + testName: "user-specified container ports with a bad dynamicHostPortRange", + testTask: testTask2, + testDynamicHostPortRange: "100-101", + expectedError: true, + }, + { + testName: "user-specified container port and container port range with a ideal dynamicHostPortRange", + testTask: testTask3, + testDynamicHostPortRange: "40000-60000", + testContainerPortRange: "55-57", expectedContainerPortSet: map[int]struct{}{ 80: {}, }, - expectedContainerPortRangeMap: map[string]string{ - "55-57": "155-157", - }, + }, + { + testName: "user-specified container port and container port range with a bad user-specified dynamicHostPortRange", + testTask: testTask3, + testDynamicHostPortRange: "40000-40001", + testContainerPortRange: "55-57", + expectedError: true, }, } @@ -322,22 +364,33 @@ func TestDockerHostConfigPortBinding(t *testing.T) { defer func() { getHostPortRange = utils.GetHostPortRange }() - getHostPortRange = tc.getHostPortRange - config, err := tc.testTask.DockerHostConfig(tc.testTask.Containers[0], dockerMap(tc.testTask), defaultDockerClientAPIVersion, - &config.Config{}) - assert.Nil(t, err) + // Get the Docker host config for the task container + config, err := tc.testTask.DockerHostConfig(tc.testTask.Containers[0], dockerMap(tc.testTask), + defaultDockerClientAPIVersion, &config.Config{DynamicHostPortRange: tc.testDynamicHostPortRange}) + if !tc.expectedError { + assert.Nil(t, err) - if !reflect.DeepEqual(config.PortBindings, tc.expectedPortBinding) { - t.Error("Expected port bindings to be resolved, was: ", config.PortBindings) - } + // Verify PortBindings + if tc.expectedPortBinding != nil { + if !reflect.DeepEqual(config.PortBindings, tc.expectedPortBinding) { + t.Error("Expected port bindings to be resolved, was: ", config.PortBindings) + } + } - if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortSet, tc.expectedContainerPortSet) { - t.Error("Expected container port set to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortSet()) - } + // Verify ContainerPortSet + if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortSet, tc.expectedContainerPortSet) { + t.Error("Expected container port set to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortSet()) + } - if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortRangeMap, tc.expectedContainerPortRangeMap) { - t.Error("Expected container port range map to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortRangeMap()) + // Verify ContainerPortRangeMap + if tc.expectedContainerPortRangeMap != nil { + if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortRangeMap, tc.expectedContainerPortRangeMap) { + t.Error("Expected container port range map to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortRangeMap()) + } + } + } else { + assert.NotNil(t, err) } }) } @@ -353,6 +406,14 @@ var ( defaultSCProtocol = "/tcp" ) +func getDefaultDynamicHostPortRange() (start int, end int) { + startHostPortRange, endHostPortRange, err := utils.GetDynamicHostPortRange() + if err != nil { + return utils.DefaultPortRangeStart, utils.DefaultPortRangeEnd + } + return startHostPortRange, endHostPortRange +} + func getTestTaskServiceConnectBridgeMode() *Task { testTask := &Task{ NetworkMode: BridgeNetworkMode, @@ -408,61 +469,91 @@ func convertSCPort(port uint16) nat.Port { } // TestDockerHostConfigSCBridgeMode verifies port bindings and network mode overrides for each -// container in an SC-enabled bridge mode task. The test task is consisted of the SC container, a regular container, +// container in an SC-enabled bridge mode task with default/user-specified dynamic host port range. +// The test task is consisted of the SC container, a regular container, // and two pause containers associated with each. func TestDockerHostConfigSCBridgeMode(t *testing.T) { testTask := getTestTaskServiceConnectBridgeMode() - // task container and SC container should both get empty port binding map and "container" network mode - actualConfig, err := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion, - &config.Config{}) - assert.Nil(t, err) - assert.NotNil(t, actualConfig) - assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1" - dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, "C1")), actualConfig.NetworkMode) - assert.Empty(t, actualConfig.PortBindings, "Task container port binding should be empty") + testCases := []struct { + testStartHostPort int + testEndHostPort int + testName string + testError bool + }{ + { + testStartHostPort: 0, + testEndHostPort: 0, + testName: "with default dynamic host port range", + }, + { + testStartHostPort: 50000, + testEndHostPort: 60000, + testName: "with user-specified dynamic host port range", + }, + } - actualConfig, err = testTask.DockerHostConfig(testTask.Containers[2], dockerMap(testTask), defaultDockerClientAPIVersion, - &config.Config{}) - assert.Nil(t, err) - assert.NotNil(t, actualConfig) - assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1" - dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, serviceConnectContainerTestName)), actualConfig.NetworkMode) - assert.Empty(t, actualConfig.PortBindings, "SC container port binding should be empty") + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + // need to reset the tracker to avoid getting data from previous test cases + utils.ResetTracker() + if tc.testStartHostPort == 0 && tc.testEndHostPort == 0 { + tc.testStartHostPort, tc.testEndHostPort = getDefaultDynamicHostPortRange() + } + testDynamicHostPortRange := fmt.Sprintf("%d-%d", tc.testStartHostPort, tc.testEndHostPort) + testConfig := &config.Config{DynamicHostPortRange: testDynamicHostPortRange} - // task pause container should get port binding map of the task container - actualConfig, err = testTask.DockerHostConfig(testTask.Containers[1], dockerMap(testTask), defaultDockerClientAPIVersion, - &config.Config{}) - assert.Nil(t, err) - assert.NotNil(t, actualConfig) - assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode) - bindings, ok := actualConfig.PortBindings[convertSCPort(SCTaskContainerPort1)] - assert.True(t, ok, "Could not get port bindings") - assert.Equal(t, 1, len(bindings), "Wrong number of bindings") - assert.Equal(t, "0", bindings[0].HostPort, "Wrong hostport") - bindings, ok = actualConfig.PortBindings[convertSCPort(SCTaskContainerPort2)] - assert.True(t, ok, "Could not get port bindings") - assert.Equal(t, 1, len(bindings), "Wrong number of bindings") - assert.Equal(t, "0", bindings[0].HostPort, "Wrong hostport") - - // SC pause container should get port binding map of all ingress listeners - actualConfig, err = testTask.DockerHostConfig(testTask.Containers[3], dockerMap(testTask), defaultDockerClientAPIVersion, - &config.Config{}) - assert.Nil(t, err) - assert.NotNil(t, actualConfig) - assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode) - // SC - ingress listener 1 - default experience - bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener1ContainerPort)] - assert.True(t, ok, "Could not get port bindings") - assert.Equal(t, 1, len(bindings), "Wrong number of bindings") - assert.Equal(t, "0", bindings[0].HostPort, "Wrong hostport") - // SC - ingress listener 2 - non-default host port - bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener2ContainerPort)] - assert.True(t, ok, "Could not get port bindings") - assert.Equal(t, 1, len(bindings), "Wrong number of bindings") - assert.Equal(t, strconv.Itoa(int(SCIngressListener2HostPort)), bindings[0].HostPort, "Wrong hostport") - // SC - egress listener - should not have port binding - bindings, ok = actualConfig.PortBindings[convertSCPort(SCEgressListenerContainerPort)] - assert.False(t, ok, "egress listener has port binding but it shouldn't") + // task container and SC container should both get empty port binding map and "container" network mode + + // the task container + actualConfig, err := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig) + assert.Nil(t, err) + assert.NotNil(t, actualConfig) + assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1" + dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, "C1")), actualConfig.NetworkMode) + assert.Empty(t, actualConfig.PortBindings, "Task container port binding should be empty") + + // the service connect container + actualConfig, err = testTask.DockerHostConfig(testTask.Containers[2], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig) + assert.Nil(t, err) + assert.NotNil(t, actualConfig) + assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1" + dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, serviceConnectContainerTestName)), actualConfig.NetworkMode) + assert.Empty(t, actualConfig.PortBindings, "SC container port binding should be empty") + + // task pause container should get port binding map of the task container + actualConfig, err = testTask.DockerHostConfig(testTask.Containers[1], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig) + assert.Nil(t, err) + assert.NotNil(t, actualConfig) + assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode) + bindings, ok := actualConfig.PortBindings[convertSCPort(SCTaskContainerPort1)] + assert.True(t, ok, "Could not get port bindings") + assert.Equal(t, 1, len(bindings), "Wrong number of bindings") + + bindings, ok = actualConfig.PortBindings[convertSCPort(SCTaskContainerPort2)] + assert.True(t, ok, "Could not get port bindings") + assert.Equal(t, 1, len(bindings), "Wrong number of bindings") + + // SC pause container should get port binding map of all ingress listeners + actualConfig, err = testTask.DockerHostConfig(testTask.Containers[3], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig) + assert.Nil(t, err) + assert.NotNil(t, actualConfig) + assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode) + + // SC - ingress listener 1 - default experience + bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener1ContainerPort)] + assert.True(t, ok, "Could not get port bindings") + + // SC - ingress listener 2 - non-default host port + bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener2ContainerPort)] + assert.True(t, ok, "Could not get port bindings") + assert.Equal(t, 1, len(bindings), "Wrong number of bindings") + assert.Equal(t, strconv.Itoa(int(SCIngressListener2HostPort)), bindings[0].HostPort, "Wrong hostport") + + // SC - egress listener - should not have port binding + bindings, ok = actualConfig.PortBindings[convertSCPort(SCEgressListenerContainerPort)] + assert.False(t, ok, "egress listener has port binding but it shouldn't") + }) + } } // TestDockerHostConfigSCBridgeMode_getPortBindingFailure verifies that when we can't find the task diff --git a/agent/config/config.go b/agent/config/config.go index d8c3d98f697..38f0d0f4b5b 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -630,6 +630,7 @@ func (cfg *Config) String() string { "DependentContainersPullUpfront: %v, "+ "TaskCPUMemLimit: %v, "+ "ShouldExcludeIPv6PortBinding: %v, "+ + "DynamicHostPortRange: %v"+ "%s", cfg.Cluster, cfg.AWSRegion, @@ -648,6 +649,7 @@ func (cfg *Config) String() string { cfg.DependentContainersPullUpfront, cfg.TaskCPUMemLimit, cfg.ShouldExcludeIPv6PortBinding, + cfg.DynamicHostPortRange, cfg.platformString(), ) } diff --git a/agent/utils/ephemeral_ports.go b/agent/utils/ephemeral_ports.go index aff87cdf473..b374942e5ee 100644 --- a/agent/utils/ephemeral_ports.go +++ b/agent/utils/ephemeral_ports.go @@ -18,6 +18,7 @@ import ( "math/rand" "net" "strconv" + "strings" "sync" "time" @@ -30,6 +31,11 @@ const ( EphemeralPortMin = 32768 EphemeralPortMax = 60999 maxPortSelectionAttempts = 100 + portUnavailableMsg = "Port %v is unavailable or an error occurred while listening on the local %v network" + portNotFoundErrMsg = "a host port is unavailable" + portsNotFoundErrMsg = "%v contiguous host ports are unavailable" + portRangeErrMsg = "The host port range: %s found by ECS Agent is not within the expected host port range: %s" + portErrMsg = "The host port: %s found by ECS Agent is not within the expected host port range: %s" ) var ( @@ -93,11 +99,50 @@ func (pt *safePortTracker) GetLastAssignedHostPort() int { var tracker safePortTracker +// ResetTracker resets the last assigned host port to 0. +func ResetTracker() { + tracker.SetLastAssignedHostPort(0) +} + // GetHostPortRange gets N contiguous host ports from the ephemeral host port range defined on the host. +// dynamicHostPortRange can be set by customers using ECS Agent environment variable ECS_DYNAMIC_HOST_PORT_RANGE; +// otherwise, ECS Agent will use the default value returned from GetDynamicHostPortRange() in the utils package. func GetHostPortRange(numberOfPorts int, protocol string, dynamicHostPortRange string) (string, error) { portLock.Lock() defer portLock.Unlock() + result, err := getNumOfHostPorts(numberOfPorts, protocol, dynamicHostPortRange) + if err == nil { + // Verify the found host port range is within the given dynamic host port range + if isInRange := verifyPortsWithinRange(result, dynamicHostPortRange); !isInRange { + errMsg := fmt.Errorf(portRangeErrMsg, result, dynamicHostPortRange) + return "", errMsg + } + } + return result, err +} + +// GetHostPort gets 1 host port from the ephemeral host port range defined on the host. +// dynamicHostPortRange can be set by customers using ECS Agent environment variable ECS_DYNAMIC_HOST_PORT_RANGE; +// otherwise, ECS Agent will use the default value returned from GetDynamicHostPortRange() in the utils package. +func GetHostPort(protocol string, dynamicHostPortRange string) (string, error) { + portLock.Lock() + defer portLock.Unlock() + numberOfPorts := 1 + result, err := getNumOfHostPorts(numberOfPorts, protocol, dynamicHostPortRange) + foundHostPort := strings.Split(result, "-")[0] + if err == nil { + // Verify the found host port is within the given dynamic host port range + if isInRange := verifyPortsWithinRange(result, dynamicHostPortRange); !isInRange { + errMsg := fmt.Errorf(portErrMsg, foundHostPort, dynamicHostPortRange) + return "", errMsg + } + } + return foundHostPort, err +} +// getNumOfHostPorts returns the requested number of host ports using the given dynamic host port range +// and protocol. If no host port(s) was/were found, an empty string along with the error message will be returned. +func getNumOfHostPorts(numberOfPorts int, protocol, dynamicHostPortRange string) (string, error) { // get ephemeral port range, either default or if custom-defined startHostPortRange, endHostPortRange, _ := nat.ParsePortRangeToInt(dynamicHostPortRange) start := startHostPortRange @@ -127,7 +172,6 @@ func GetHostPortRange(numberOfPorts int, protocol string, dynamicHostPortRange s } else { tracker.SetLastAssignedHostPort(lastCheckedPort) } - return result, err } @@ -157,7 +201,11 @@ func getHostPortRange(numberOfPorts, start, end int, protocol string) (string, i } if n != numberOfPorts { - return "", resultEndPort, fmt.Errorf("%v contiguous host ports unavailable", numberOfPorts) + errMsg := fmt.Errorf(portNotFoundErrMsg) + if numberOfPorts > 1 { + errMsg = fmt.Errorf(portsNotFoundErrMsg, numberOfPorts) + } + return "", resultEndPort, errMsg } return fmt.Sprintf("%d-%d", resultStartPort, resultEndPort), resultEndPort, nil @@ -195,3 +243,32 @@ func isPortAvailable(port int, protocol string) (bool, error) { return false, errors.New("invalid protocol") } } + +// PortIsInRange returns true if the given port is within the start-end port range; +// otherwise, returns false. +func portIsInRange(port, start, end int) bool { + if (port >= start) && (port <= end) { + return true + } + return false +} + +// VerifyPortsWithinRange returns true if the actualPortRange is within the expectedPortRange; +// otherwise, returns false. +func verifyPortsWithinRange(actualPortRange, expectedPortRange string) bool { + // Get the actual start port and end port + aStartPort, aEndPort, _ := nat.ParsePortRangeToInt(actualPortRange) + // Get the expected start port and end port + eStartPort, eEndPort, _ := nat.ParsePortRangeToInt(expectedPortRange) + // Check the actual start port is in the expected range or not + aStartIsInRange := portIsInRange(aStartPort, eStartPort, eEndPort) + // Check the actual end port is in the expected range or not + aEndIsInRange := portIsInRange(aEndPort, eStartPort, eEndPort) + + // Return true if both actual start port and end port are in the expected range + if aStartIsInRange && aEndIsInRange { + return true + } + + return false +} diff --git a/agent/utils/ephemeral_ports_test.go b/agent/utils/ephemeral_ports_test.go index 54e9c21e895..aaa05f1a7da 100644 --- a/agent/utils/ephemeral_ports_test.go +++ b/agent/utils/ephemeral_ports_test.go @@ -130,7 +130,7 @@ func TestGetHostPortRange(t *testing.T) { protocol: testTCPProtocol, isPortAvailableFunc: func(port int, protocol string) (bool, error) { return true, nil }, numberOfRequests: 1, - expectedError: errors.New("20 contiguous host ports unavailable"), + expectedError: errors.New("20 contiguous host ports are unavailable"), }, { testName: "contiguous hostPortRange not found, no ports available on the host", @@ -166,7 +166,7 @@ func TestGetHostPortRange(t *testing.T) { assert.Equal(t, tc.expectedLastAssignedPort[i], actualLastAssignedHostPort) } else { // need to reset the tracker to avoid getting data from previous test cases - tracker.SetLastAssignedHostPort(0) + ResetTracker() hostPortRange, err := GetHostPortRange(tc.numberOfPorts, tc.protocol, tc.testDynamicHostPortRange) assert.Equal(t, tc.expectedError, err) @@ -177,6 +177,132 @@ func TestGetHostPortRange(t *testing.T) { } } +func TestGetHostPort(t *testing.T) { + numberOfPorts := 1 + testCases := []struct { + testName string + testDynamicHostPortRange string + protocol string + numberOfRequests int + resetLastAssignedHostPort bool + }{ + { + testName: "tcp protocol, a host port found", + testDynamicHostPortRange: "40090-40099", + protocol: testTCPProtocol, + resetLastAssignedHostPort: true, + numberOfRequests: 1, + }, + { + testName: "udp protocol, a host port found", + testDynamicHostPortRange: "40090-40099", + protocol: testUDPProtocol, + resetLastAssignedHostPort: true, + numberOfRequests: 1, + }, + { + testName: "5 requests for host port in succession, success", + testDynamicHostPortRange: "50090-50099", + protocol: testTCPProtocol, + resetLastAssignedHostPort: true, + numberOfRequests: 5, + }, + { + testName: "5 requests for host port in succession, success", + testDynamicHostPortRange: "50090-50099", + protocol: testUDPProtocol, + resetLastAssignedHostPort: false, + numberOfRequests: 5, + }, + } + + for _, tc := range testCases { + if tc.resetLastAssignedHostPort { + // need to reset the tracker to avoid getting data from previous test cases + ResetTracker() + } + + t.Run(tc.testName, func(t *testing.T) { + for i := 0; i < tc.numberOfRequests; i++ { + hostPortRange, err := GetHostPort(tc.protocol, tc.testDynamicHostPortRange) + assert.NoError(t, err) + numberOfHostPorts, err := getPortRangeLength(hostPortRange) + assert.NoError(t, err) + assert.Equal(t, numberOfPorts, numberOfHostPorts) + } + }) + } +} + +func TestPortIsInRange(t *testing.T) { + testCases := []struct { + testName string + testPort int + testStartPort int + testEndPort int + expectedResult bool + }{ + { + testName: "in the range", + testPort: 100, + testStartPort: 1, + testEndPort: 9999, + expectedResult: true, + }, + { + testName: "not in the range", + testPort: 10000, + testStartPort: 1, + testEndPort: 9999, + expectedResult: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + result := portIsInRange(tc.testPort, tc.testStartPort, tc.testEndPort) + assert.Equal(t, tc.expectedResult, result) + }) + } +} + +func TestVerifyPortsWithinRange(t *testing.T) { + testCases := []struct { + testName string + testActualRange string + testExpectedRange string + expectedResult bool + }{ + { + testName: "in the expected range", + testActualRange: "1000-1005", + testExpectedRange: "900-2000", + expectedResult: true, + }, + { + testName: "not in the expected range", + testActualRange: "1-2", + testExpectedRange: "2-100", + expectedResult: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + result := verifyPortsWithinRange(tc.testActualRange, tc.testExpectedRange) + assert.Equal(t, tc.expectedResult, result) + }) + } +} + +func TestResetTracker(t *testing.T) { + tracker.SetLastAssignedHostPort(100) + ResetTracker() + expectedResetVal := 0 + actualResult := tracker.GetLastAssignedHostPort() + assert.Equal(t, expectedResetVal, actualResult) +} + func getPortRangeLength(portRange string) (int, error) { startPort, endPort, err := nat.ParsePortRangeToInt(portRange) if err != nil {