Skip to content

Commit 9855c1b

Browse files
chienhanlinchienhanlin
committed
Upudate dockerPortMap() in task.go with dynamic host port range support part 2
1 parent 339e375 commit 9855c1b

File tree

4 files changed

+201
-75
lines changed

4 files changed

+201
-75
lines changed

‎agent/api/task/task.go

+77-22
Original file line numberDiff line numberDiff line change
@@ -2340,10 +2340,53 @@ func (task *Task) dockerLinks(container *apicontainer.Container, dockerContainer
23402340
var getHostPortRange = utils.GetHostPortRange
23412341
var getHostPort = utils.GetHostPort
23422342

2343+
// In buildPortMapWithSCIngressConfig, the dockerPortMap and the containerPortSet will be constructed
2344+
// for ingress listeners under two service connect bridge mode cases:
2345+
// (1) non-default bridge mode service connect experience: customers specify host ports for listeners in the ingress config.
2346+
// (2) default bridge mode service connect experience: customers do not specify host ports for listeners in the ingress config.
2347+
// Instead, ECS Agent finds host ports within the given dynamic host port range.
2348+
// An error will be returned for case (2) if ECS Agent cannot find an available host port within range.
2349+
func (task *Task) buildPortMapWithSCIngressConfig(dynamicHostPortRange string) (nat.PortMap, map[int]struct{}, error) {
2350+
var err error
2351+
ingressDockerPortMap := nat.PortMap{}
2352+
ingressContainerPortSet := make(map[int]struct{})
2353+
protocolStr := "tcp"
2354+
for _, ic := range task.ServiceConnectConfig.IngressConfig {
2355+
listenerPortInt := int(ic.ListenerPort)
2356+
dockerPort := nat.Port(strconv.Itoa(listenerPortInt) + "/" + protocolStr)
2357+
hostPortStr := ""
2358+
if ic.HostPort != nil {
2359+
// For non-default bridge mode service connect experience, a host port is specified by customers
2360+
hostPortStr = strconv.Itoa(int(*ic.HostPort))
2361+
} else {
2362+
// For default bridge mode service connect experience, customers do not specify a host port
2363+
// thus the host port will be assigned by ECS Agent.
2364+
// ECS Agent will find an available host port within the given dynamic host port range,
2365+
// or return an error if no host port is available within the range.
2366+
hostPortStr, err = getHostPort(protocolStr, dynamicHostPortRange)
2367+
if err != nil {
2368+
return nil, nil, err
2369+
}
2370+
}
2371+
2372+
ingressDockerPortMap[dockerPort] = append(ingressDockerPortMap[dockerPort], nat.PortBinding{HostPort: hostPortStr})
2373+
// Append non-range, singular container port to the ingressContainerPortSet
2374+
ingressContainerPortSet[listenerPortInt] = struct{}{}
2375+
}
2376+
return ingressDockerPortMap, ingressContainerPortSet, err
2377+
}
2378+
23432379
// dockerPortMap creates a port binding map for
23442380
// (1) Ingress listeners for the service connect AppNet container in the service connect enabled bridge network mode task.
23452381
// (2) Port mapping definied by customers in the task definition.
23462382
//
2383+
// For service connect bridge mode task, we will create port bindings for customers' application containers
2384+
// and service connect AppNet container, and let them to be published by the associated pause containers.
2385+
// (a) For default bridge service connect experience, ECS Agent will assign a host port within the
2386+
// default/user-specified dynamic host port range for the ingress listener. If no available host port can be
2387+
// found by ECS Agent, an error will be returned.
2388+
// (b) For non-default bridge service connect experience, ECS Agent will use the user-defined host port for the ingress listener.
2389+
//
23472390
// For non-service connect bridge network mode task, ECS Agent will assign a host port or a host port range
23482391
// within the default/user-specified dynamic host port range. If no available host port or host port range can be
23492392
// found by ECS Agent, an error will be returned.
@@ -2354,43 +2397,49 @@ var getHostPort = utils.GetHostPort
23542397
func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPortRange string) (nat.PortMap, error) {
23552398
hostPortStr := ""
23562399
dockerPortMap := nat.PortMap{}
2357-
scContainer := task.GetServiceConnectContainer()
23582400
containerToCheck := container
23592401
containerPortSet := make(map[int]struct{})
23602402
containerPortRangeMap := make(map[string]string)
2403+
2404+
// For service connect bridge network mode task, we will create port bindings for task containers,
2405+
// including both application containers and service connect AppNet container, and let them to be published
2406+
// by the associated pause containers.
23612407
if task.IsServiceConnectEnabled() && task.IsNetworkModeBridge() {
23622408
if container.Type == apicontainer.ContainerCNIPause {
2363-
// we will create bindings for task containers (including both customer containers and SC Appnet container)
2364-
// and let them be published by the associated pause container.
2365-
// Note - for SC bridge mode we do not allow customer to specify a host port for their containers. Additionally,
2366-
// When an ephemeral host port is assigned, Appnet will NOT proxy traffic to that port
2409+
// Find the task container associated with this particular pause container
23672410
taskContainer, err := task.getBridgeModeTaskContainerForPauseContainer(container)
23682411
if err != nil {
23692412
return nil, err
23702413
}
2414+
2415+
scContainer := task.GetServiceConnectContainer()
23712416
if taskContainer == scContainer {
2372-
// create bindings for all ingress listener ports
2373-
// no need to create binding for egress listener port as it won't be access from host level or from outside
2374-
for _, ic := range task.ServiceConnectConfig.IngressConfig {
2375-
listenerPortInt := int(ic.ListenerPort)
2376-
dockerPort := nat.Port(strconv.Itoa(listenerPortInt)) + "/tcp"
2377-
hostPort := 0 // default bridge-mode SC experience - host port will be an ephemeral port assigned by docker
2378-
if ic.HostPort != nil { // non-default bridge-mode SC experience - host port specified by customer
2379-
hostPort = int(*ic.HostPort)
2380-
}
2381-
dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: strconv.Itoa(hostPort)})
2382-
// append non-range, singular container port to the containerPortSet
2383-
containerPortSet[listenerPortInt] = struct{}{}
2384-
// set taskContainer.ContainerPortSet to be used during network binding creation
2385-
taskContainer.SetContainerPortSet(containerPortSet)
2417+
// If the associated task container is the service connect AppNet container,
2418+
// create port binding(s) for ingress listener ports based on its ingress config.
2419+
// Note that, there is no need to do this for egress listener port as it won't be accessed
2420+
// from host level or from outside.
2421+
dockerPortMap, containerPortSet, err := task.buildPortMapWithSCIngressConfig(dynamicHostPortRange)
2422+
if err != nil {
2423+
logger.Error("Failed to build a port map with service connect ingress config", logger.Fields{
2424+
field.TaskID: task.GetID(),
2425+
field.Container: taskContainer.Name,
2426+
"dynamicHostPortRange": dynamicHostPortRange,
2427+
field.Error: err,
2428+
})
2429+
return nil, err
23862430
}
2431+
// Set taskContainer.ContainerPortSet to be used during network binding creation
2432+
taskContainer.SetContainerPortSet(containerPortSet)
23872433
return dockerPortMap, nil
23882434
}
23892435
containerToCheck = taskContainer
23902436
} else {
2391-
// If container is neither SC container nor pause container, it's a regular task container. Its port bindings(s)
2392-
// are published by the associated pause container, and we leave the map empty here (docker would actually complain
2393-
// otherwise).
2437+
// If the container is neither service connect AppNet container nor pause container, and it is a regular task container.
2438+
// Its port bindings(s) are published by the associated pause container, and we will leave the map empty here
2439+
// (docker would actually complain otherwise).
2440+
//
2441+
// Note - for service connect bridge mode, we do not allow customers to specify a host port for their application containers.
2442+
// Additionally, AppNet will NOT proxy traffic to that port when an ephemeral host port is assigned.
23942443
return dockerPortMap, nil
23952444
}
23962445
}
@@ -2478,6 +2527,12 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo
24782527
// Set Container.ContainerPortSet and Container.ContainerPortRangeMap to be used during network binding creation
24792528
containerToCheck.SetContainerPortSet(containerPortSet)
24802529
containerToCheck.SetContainerPortRangeMap(containerPortRangeMap)
2530+
logger.Debug("containerToCheck is", logger.Fields{
2531+
field.Container: containerToCheck.Name,
2532+
"dockerPortMap": dockerPortMap,
2533+
"containerPortSet": containerPortSet,
2534+
"containerPortRangeMap": containerPortRangeMap,
2535+
})
24812536
return dockerPortMap, nil
24822537
}
24832538

‎agent/api/task/task_test.go

+108-51
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,14 @@ var (
424424
defaultSCProtocol = "/tcp"
425425
)
426426

427+
func getDefaultDynamicHostPortRange() (start int, end int) {
428+
startHostPortRange, endHostPortRange, err := utils.GetDynamicHostPortRange()
429+
if err != nil {
430+
return utils.DefaultPortRangeStart, utils.DefaultPortRangeEnd
431+
}
432+
return startHostPortRange, endHostPortRange
433+
}
434+
427435
func getTestTaskServiceConnectBridgeMode() *Task {
428436
testTask := &Task{
429437
NetworkMode: BridgeNetworkMode,
@@ -479,61 +487,110 @@ func convertSCPort(port uint16) nat.Port {
479487
}
480488

481489
// TestDockerHostConfigSCBridgeMode verifies port bindings and network mode overrides for each
482-
// container in an SC-enabled bridge mode task. The test task is consisted of the SC container, a regular container,
490+
// container in an SC-enabled bridge mode task with default/user-specified dynamic host port range.
491+
// The test task is consisted of the SC container, a regular container,
483492
// and two pause containers associated with each.
484493
func TestDockerHostConfigSCBridgeMode(t *testing.T) {
485494
testTask := getTestTaskServiceConnectBridgeMode()
486-
// task container and SC container should both get empty port binding map and "container" network mode
487-
actualConfig, err := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion,
488-
&config.Config{})
489-
assert.Nil(t, err)
490-
assert.NotNil(t, actualConfig)
491-
assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1"
492-
dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, "C1")), actualConfig.NetworkMode)
493-
assert.Empty(t, actualConfig.PortBindings, "Task container port binding should be empty")
494-
495-
actualConfig, err = testTask.DockerHostConfig(testTask.Containers[2], dockerMap(testTask), defaultDockerClientAPIVersion,
496-
&config.Config{})
497-
assert.Nil(t, err)
498-
assert.NotNil(t, actualConfig)
499-
assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1"
500-
dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, serviceConnectContainerTestName)), actualConfig.NetworkMode)
501-
assert.Empty(t, actualConfig.PortBindings, "SC container port binding should be empty")
495+
testCases := []struct {
496+
testStartHostPort int
497+
testEndHostPort int
498+
testName string
499+
testError bool
500+
}{
501+
{
502+
testStartHostPort: 0,
503+
testEndHostPort: 0,
504+
testName: "with default dynamic host port range",
505+
},
506+
{
507+
testStartHostPort: 50000,
508+
testEndHostPort: 60000,
509+
testName: "with user-specified dynamic host port range",
510+
},
511+
{
512+
testStartHostPort: 1,
513+
testEndHostPort: 2,
514+
testName: "with user-specified dynamic host port range but no available host port",
515+
testError: true,
516+
},
517+
}
502518

503-
// task pause container should get port binding map of the task container
504-
actualConfig, err = testTask.DockerHostConfig(testTask.Containers[1], dockerMap(testTask), defaultDockerClientAPIVersion,
505-
&config.Config{})
506-
assert.Nil(t, err)
507-
assert.NotNil(t, actualConfig)
508-
assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode)
509-
bindings, ok := actualConfig.PortBindings[convertSCPort(SCTaskContainerPort1)]
510-
assert.True(t, ok, "Could not get port bindings")
511-
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
512-
assert.Equal(t, "0", bindings[0].HostPort, "Wrong hostport")
513-
bindings, ok = actualConfig.PortBindings[convertSCPort(SCTaskContainerPort2)]
514-
assert.True(t, ok, "Could not get port bindings")
515-
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
516-
assert.Equal(t, "0", bindings[0].HostPort, "Wrong hostport")
517-
518-
// SC pause container should get port binding map of all ingress listeners
519-
actualConfig, err = testTask.DockerHostConfig(testTask.Containers[3], dockerMap(testTask), defaultDockerClientAPIVersion,
520-
&config.Config{})
521-
assert.Nil(t, err)
522-
assert.NotNil(t, actualConfig)
523-
assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode)
524-
// SC - ingress listener 1 - default experience
525-
bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener1ContainerPort)]
526-
assert.True(t, ok, "Could not get port bindings")
527-
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
528-
assert.Equal(t, "0", bindings[0].HostPort, "Wrong hostport")
529-
// SC - ingress listener 2 - non-default host port
530-
bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener2ContainerPort)]
531-
assert.True(t, ok, "Could not get port bindings")
532-
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
533-
assert.Equal(t, strconv.Itoa(int(SCIngressListener2HostPort)), bindings[0].HostPort, "Wrong hostport")
534-
// SC - egress listener - should not have port binding
535-
bindings, ok = actualConfig.PortBindings[convertSCPort(SCEgressListenerContainerPort)]
536-
assert.False(t, ok, "egress listener has port binding but it shouldn't")
519+
for _, tc := range testCases {
520+
t.Run(tc.testName, func(t *testing.T) {
521+
// need to reset the tracker to avoid getting data from previous test cases
522+
utils.ResetTracker()
523+
if tc.testStartHostPort == 0 && tc.testEndHostPort == 0 {
524+
tc.testStartHostPort, tc.testEndHostPort = getDefaultDynamicHostPortRange()
525+
}
526+
testDynamicHostPortRange := fmt.Sprintf("%d-%d", tc.testStartHostPort, tc.testEndHostPort)
527+
testConfig := &config.Config{DynamicHostPortRange: testDynamicHostPortRange}
528+
529+
// task container and SC container should both get empty port binding map and "container" network mode
530+
531+
// the task container
532+
actualConfig, err := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig)
533+
assert.Nil(t, err)
534+
assert.NotNil(t, actualConfig)
535+
assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1"
536+
dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, "C1")), actualConfig.NetworkMode)
537+
assert.Empty(t, actualConfig.PortBindings, "Task container port binding should be empty")
538+
539+
// the service connect container
540+
actualConfig, err = testTask.DockerHostConfig(testTask.Containers[2], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig)
541+
assert.Nil(t, err)
542+
assert.NotNil(t, actualConfig)
543+
assert.Equal(t, dockercontainer.NetworkMode(fmt.Sprintf("%s-%s", // e.g. "container:dockerid-~internal~ecs~pause-C1"
544+
dockerMappingContainerPrefix+dockerIDPrefix+NetworkPauseContainerName, serviceConnectContainerTestName)), actualConfig.NetworkMode)
545+
assert.Empty(t, actualConfig.PortBindings, "SC container port binding should be empty")
546+
547+
// task pause container should get port binding map of the task container
548+
actualConfig, err = testTask.DockerHostConfig(testTask.Containers[1], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig)
549+
if !tc.testError {
550+
assert.Nil(t, err)
551+
assert.NotNil(t, actualConfig)
552+
assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode)
553+
bindings, ok := actualConfig.PortBindings[convertSCPort(SCTaskContainerPort1)]
554+
assert.True(t, ok, "Could not get port bindings")
555+
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
556+
hostPort, _ := strconv.Atoi(bindings[0].HostPort)
557+
result := utils.PortIsInRange(hostPort, tc.testStartHostPort, tc.testEndHostPort)
558+
assert.True(t, result, "hostport is not in the dynamic host port range")
559+
560+
bindings, ok = actualConfig.PortBindings[convertSCPort(SCTaskContainerPort2)]
561+
assert.True(t, ok, "Could not get port bindings")
562+
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
563+
hostPort, _ = strconv.Atoi(bindings[0].HostPort)
564+
result = utils.PortIsInRange(hostPort, tc.testStartHostPort, tc.testEndHostPort)
565+
assert.True(t, result, "hostport is not in the dynamic host port range")
566+
567+
// SC pause container should get port binding map of all ingress listeners
568+
actualConfig, err = testTask.DockerHostConfig(testTask.Containers[3], dockerMap(testTask), defaultDockerClientAPIVersion, testConfig)
569+
assert.Nil(t, err)
570+
assert.NotNil(t, actualConfig)
571+
assert.Equal(t, dockercontainer.NetworkMode(BridgeNetworkMode), actualConfig.NetworkMode)
572+
573+
// SC - ingress listener 1 - default experience
574+
bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener1ContainerPort)]
575+
assert.True(t, ok, "Could not get port bindings")
576+
hostPort, _ = strconv.Atoi(bindings[0].HostPort)
577+
result = utils.PortIsInRange(hostPort, tc.testStartHostPort, tc.testEndHostPort)
578+
assert.True(t, result, "hostport is not in the dynamic host port range")
579+
580+
// SC - ingress listener 2 - non-default host port
581+
bindings, ok = actualConfig.PortBindings[convertSCPort(SCIngressListener2ContainerPort)]
582+
assert.True(t, ok, "Could not get port bindings")
583+
assert.Equal(t, 1, len(bindings), "Wrong number of bindings")
584+
assert.Equal(t, strconv.Itoa(int(SCIngressListener2HostPort)), bindings[0].HostPort, "Wrong hostport")
585+
586+
// SC - egress listener - should not have port binding
587+
bindings, ok = actualConfig.PortBindings[convertSCPort(SCEgressListenerContainerPort)]
588+
assert.False(t, ok, "egress listener has port binding but it shouldn't")
589+
} else {
590+
assert.NotNil(t, err)
591+
}
592+
})
593+
}
537594
}
538595

539596
// TestDockerHostConfigSCBridgeMode_getPortBindingFailure verifies that when we can't find the task

‎agent/utils/ephemeral_ports.go

+5
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ func (pt *safePortTracker) GetLastAssignedHostPort() int {
9696

9797
var tracker safePortTracker
9898

99+
// ResetTracker resets the last assigned host port to 0.
100+
func ResetTracker() {
101+
tracker.SetLastAssignedHostPort(0)
102+
}
103+
99104
// GetHostPortRange gets N contiguous host ports from the ephemeral host port range defined on the host.
100105
// dynamicHostPortRange can be set by customers using ECS Agent environment variable ECS_DYNAMIC_HOST_PORT_RANGE;
101106
// otherwise, ECS Agent will use the default value returned from GetDynamicHostPortRange() in the utils package.

‎agent/utils/ephemeral_ports_test.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func TestGetHostPortRange(t *testing.T) {
144144
assert.Equal(t, tc.expectedLastAssignedPort[i], actualLastAssignedHostPort)
145145
} else {
146146
// need to reset the tracker to avoid getting data from previous test cases
147-
tracker.SetLastAssignedHostPort(0)
147+
ResetTracker()
148148

149149
hostPortRange, err := GetHostPortRange(tc.numberOfPorts, tc.protocol, tc.testDynamicHostPortRange)
150150
assert.Equal(t, tc.expectedError, err)
@@ -196,7 +196,8 @@ func TestGetHostPort(t *testing.T) {
196196

197197
for _, tc := range testCases {
198198
if tc.resetLastAssignedHostPort {
199-
tracker.SetLastAssignedHostPort(0)
199+
// need to reset the tracker to avoid getting data from previous test cases
200+
ResetTracker()
200201
}
201202

202203
t.Run(tc.testName, func(t *testing.T) {
@@ -275,6 +276,14 @@ func TestVerifyPortsWithinRange(t *testing.T) {
275276
}
276277
}
277278

279+
func TestResetTracker(t *testing.T) {
280+
tracker.SetLastAssignedHostPort(100)
281+
ResetTracker()
282+
expectedResetVal := 0
283+
actualResult := tracker.GetLastAssignedHostPort()
284+
assert.Equal(t, expectedResetVal, actualResult)
285+
}
286+
278287
func getPortRangeLength(portRange string) (int, error) {
279288
startPort, endPort, err := nat.ParsePortRangeToInt(portRange)
280289
if err != nil {

0 commit comments

Comments
 (0)