Skip to content

Commit 59fa3b7

Browse files
chienhanlinchienhanlin
committed
Upudate dockerPortMap() in task.go with dynamic host port range support part 1
1 parent de967e1 commit 59fa3b7

File tree

5 files changed

+243
-69
lines changed

5 files changed

+243
-69
lines changed

‎agent/api/task/task.go

+49-10
Original file line numberDiff line numberDiff line change
@@ -2338,8 +2338,21 @@ func (task *Task) dockerLinks(container *apicontainer.Container, dockerContainer
23382338
}
23392339

23402340
var getHostPortRange = utils.GetHostPortRange
2341+
var getHostPort = utils.GetHostPort
23412342

2343+
// dockerPortMap creates a port binding map for
2344+
// (1) Ingress listeners for the service connect AppNet container in the service connect enabled bridge network mode task.
2345+
// (2) Port mapping definied by customers in the task definition.
2346+
//
2347+
// For non-service connect bridge network mode task, ECS Agent will assign a host port or a host port range
2348+
// within the default/user-specified dynamic host port range. If no available host port or host port range can be
2349+
// found by ECS Agent, an error will be returned.
2350+
//
2351+
// Note that,
2352+
// (a) ECS Agent will not assign a new host port within the dynamic host port range for awsvpc network mode task
2353+
// (b) ECS Agent will not assign a new host port within the dynamic host port range if the user-specified host port exists
23422354
func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPortRange string) (nat.PortMap, error) {
2355+
hostPortStr := ""
23432356
dockerPortMap := nat.PortMap{}
23442357
scContainer := task.GetServiceConnectContainer()
23452358
containerToCheck := container
@@ -2382,15 +2395,41 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo
23822395
}
23832396
}
23842397

2398+
// For each port binding config, either one of containerPort or containerPortRange is set.
2399+
// (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.
2400+
// (2) containerPortRange is the port number range on the container that's bound to the mapped host port range found by ECS Agent.
2401+
var err error
23852402
for _, portBinding := range containerToCheck.Ports {
2386-
// for each port binding config, either one of containerPort or containerPortRange is set
23872403
if portBinding.ContainerPort != 0 {
23882404
containerPort := int(portBinding.ContainerPort)
2405+
protocolStr := portBinding.Protocol.String()
2406+
dockerPort := nat.Port(strconv.Itoa(containerPort) + "/" + protocolStr)
23892407

2390-
dockerPort := nat.Port(strconv.Itoa(containerPort) + "/" + portBinding.Protocol.String())
2391-
dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: strconv.Itoa(int(portBinding.HostPort))})
2408+
if portBinding.HostPort != 0 {
2409+
// An User-specified host port exists
2410+
hostPortStr = strconv.Itoa(int(portBinding.HostPort))
2411+
} else {
2412+
// If there is no user-specified host port, ECS Agent will find an available host port
2413+
// within the given dynamic host port range. And if no host port is available within the range,
2414+
// an error will be returned.
2415+
logger.Debug("No user-specified host port, ECS Agent will find an available host port within the given dynamic host port range", logger.Fields{
2416+
field.Container: containerToCheck.Name,
2417+
"dynamicHostPortRange": dynamicHostPortRange,
2418+
})
2419+
hostPortStr, err = getHostPort(protocolStr, dynamicHostPortRange)
2420+
if err != nil {
2421+
logger.Error("Unable to find a host port for container within the given dynamic host port range", logger.Fields{
2422+
field.TaskID: task.GetID(),
2423+
field.Container: container.Name,
2424+
"dynamicHostPortRange": dynamicHostPortRange,
2425+
field.Error: err,
2426+
})
2427+
return nil, err
2428+
}
2429+
}
2430+
dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: hostPortStr})
23922431

2393-
// append non-range, singular container port to the containerPortSet
2432+
// For the containerPort case, append a non-range, singular container port to the containerPortSet.
23942433
containerPortSet[containerPort] = struct{}{}
23952434
} else if portBinding.ContainerPortRange != "" {
23962435
containerToCheck.SetContainerHasPortRange(true)
@@ -2404,8 +2443,8 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo
24042443

24052444
numberOfPorts := endContainerPort - startContainerPort + 1
24062445
protocol := portBinding.Protocol.String()
2407-
// we will try to get a contiguous set of host ports from the ephemeral host port range.
2408-
// this is to ensure that docker maps host ports in a contiguous manner, and
2446+
// We will try to get a contiguous set of host ports from the ephemeral host port range.
2447+
// This is to ensure that docker maps host ports in a contiguous manner, and
24092448
// we are guaranteed to have the entire hostPortRange in a single network binding while sending this info to ECS;
24102449
// therefore, an error will be returned if we cannot find a contiguous set of host ports.
24112450
hostPortRange, err := getHostPortRange(numberOfPorts, protocol, dynamicHostPortRange)
@@ -2419,7 +2458,7 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo
24192458
return nil, err
24202459
}
24212460

2422-
// append ranges to the dockerPortMap
2461+
// For the ContainerPortRange case, append ranges to the dockerPortMap.
24232462
// nat.ParsePortSpec returns a list of port mappings in a format that docker likes
24242463
mappings, err := nat.ParsePortSpec(hostPortRange + ":" + containerPortRange + "/" + protocol)
24252464
if err != nil {
@@ -2430,13 +2469,13 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo
24302469
dockerPortMap[mapping.Port] = append(dockerPortMap[mapping.Port], mapping.Binding)
24312470
}
24322471

2433-
// append containerPortRange and associated hostPortRange to the containerPortRangeMap
2434-
// this will ensure that we consolidate range into 1 network binding while sending it to ECS
2472+
// For the ContainerPortRange case, append containerPortRange and associated hostPortRange to the containerPortRangeMap.
2473+
// This will ensure that we consolidate range into 1 network binding while sending it to ECS
24352474
containerPortRangeMap[containerPortRange] = hostPortRange
24362475
}
24372476
}
24382477

2439-
// set Container.ContainerPortSet and Container.ContainerPortRangeMap to be used during network binding creation
2478+
// Set Container.ContainerPortSet and Container.ContainerPortRangeMap to be used during network binding creation
24402479
containerToCheck.SetContainerPortSet(containerPortSet)
24412480
containerToCheck.SetContainerPortRangeMap(containerPortRangeMap)
24422481
return dockerPortMap, nil

‎agent/api/task/task_test.go

+103-32
Original file line numberDiff line numberDiff line change
@@ -240,13 +240,13 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
240240
Ports: []apicontainer.PortBinding{
241241
{
242242
ContainerPort: 10,
243-
HostPort: 10,
243+
HostPort: 20,
244244
BindIP: "",
245245
Protocol: apicontainer.TransportProtocolTCP,
246246
},
247247
{
248248
ContainerPort: 20,
249-
HostPort: 20,
249+
HostPort: 30,
250250
BindIP: "",
251251
Protocol: apicontainer.TransportProtocolUDP,
252252
},
@@ -256,6 +256,31 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
256256
}
257257

258258
testTask2 := &Task{
259+
Containers: []*apicontainer.Container{
260+
{
261+
Name: "c1",
262+
Ports: []apicontainer.PortBinding{
263+
{
264+
ContainerPort: 10,
265+
BindIP: "",
266+
Protocol: apicontainer.TransportProtocolTCP,
267+
},
268+
{
269+
ContainerPort: 20,
270+
BindIP: "",
271+
Protocol: apicontainer.TransportProtocolUDP,
272+
},
273+
{
274+
ContainerPort: 30,
275+
BindIP: "",
276+
Protocol: apicontainer.TransportProtocolUDP,
277+
},
278+
},
279+
},
280+
},
281+
}
282+
283+
testTask3 := &Task{
259284
Containers: []*apicontainer.Container{
260285
{
261286
Name: "c1",
@@ -278,17 +303,20 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
278303
testCases := []struct {
279304
testName string
280305
testTask *Task
281-
getHostPortRange func(numberOfPorts int, protocol string, dynamicHostPortRange string) (string, error)
306+
testDynamicHostPortRange string
307+
testContainerPortRange string
282308
expectedPortBinding nat.PortMap
283309
expectedContainerPortSet map[int]struct{}
284310
expectedContainerPortRangeMap map[string]string
311+
expectedError bool
285312
}{
286313
{
287-
testName: "2 port bindings, each with singular container port - host port",
288-
testTask: testTask1,
314+
testName: "user-specified container ports and host ports",
315+
testTask: testTask1,
316+
testDynamicHostPortRange: "40000-60000",
289317
expectedPortBinding: nat.PortMap{
290-
nat.Port("10/tcp"): []nat.PortBinding{{HostPort: "10"}},
291-
nat.Port("20/udp"): []nat.PortBinding{{HostPort: "20"}},
318+
nat.Port("10/tcp"): []nat.PortBinding{{HostPort: "20"}},
319+
nat.Port("20/udp"): []nat.PortBinding{{HostPort: "30"}},
292320
},
293321
expectedContainerPortSet: map[int]struct{}{
294322
10: {},
@@ -297,23 +325,37 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
297325
expectedContainerPortRangeMap: map[string]string{},
298326
},
299327
{
300-
testName: "2 port bindings, one with container port range, other with singular container port",
301-
testTask: testTask2,
302-
getHostPortRange: func(numberOfPorts int, protocol string, dynamicHostPortRange string) (string, error) {
303-
return "155-157", nil
304-
},
305-
expectedPortBinding: nat.PortMap{
306-
nat.Port("55/udp"): []nat.PortBinding{{HostPort: "155"}},
307-
nat.Port("56/udp"): []nat.PortBinding{{HostPort: "156"}},
308-
nat.Port("57/udp"): []nat.PortBinding{{HostPort: "157"}},
309-
nat.Port("80/tcp"): []nat.PortBinding{{HostPort: "0"}},
328+
testName: "user-specified container ports with a ideal dynamicHostPortRange",
329+
testTask: testTask2,
330+
testDynamicHostPortRange: "40000-60000",
331+
expectedContainerPortSet: map[int]struct{}{
332+
10: {},
333+
20: {},
334+
30: {},
310335
},
336+
expectedContainerPortRangeMap: map[string]string{},
337+
},
338+
{
339+
testName: "user-specified container ports with a bad dynamicHostPortRange",
340+
testTask: testTask2,
341+
testDynamicHostPortRange: "100-101",
342+
expectedError: true,
343+
},
344+
{
345+
testName: "user-specified container port and container port range with a ideal dynamicHostPortRange",
346+
testTask: testTask3,
347+
testDynamicHostPortRange: "40000-60000",
348+
testContainerPortRange: "55-57",
311349
expectedContainerPortSet: map[int]struct{}{
312350
80: {},
313351
},
314-
expectedContainerPortRangeMap: map[string]string{
315-
"55-57": "155-157",
316-
},
352+
},
353+
{
354+
testName: "user-specified container port and container port range with a bad user-specified dynamicHostPortRange",
355+
testTask: testTask3,
356+
testDynamicHostPortRange: "40000-40001",
357+
testContainerPortRange: "55-57",
358+
expectedError: true,
317359
},
318360
}
319361

@@ -322,22 +364,51 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
322364
defer func() {
323365
getHostPortRange = utils.GetHostPortRange
324366
}()
325-
getHostPortRange = tc.getHostPortRange
326367

327-
config, err := tc.testTask.DockerHostConfig(tc.testTask.Containers[0], dockerMap(tc.testTask), defaultDockerClientAPIVersion,
328-
&config.Config{})
329-
assert.Nil(t, err)
368+
// Get the Docker host config for the task container
369+
config, err := tc.testTask.DockerHostConfig(tc.testTask.Containers[0], dockerMap(tc.testTask),
370+
defaultDockerClientAPIVersion, &config.Config{DynamicHostPortRange: tc.testDynamicHostPortRange})
371+
if !tc.expectedError {
372+
assert.Nil(t, err)
330373

331-
if !reflect.DeepEqual(config.PortBindings, tc.expectedPortBinding) {
332-
t.Error("Expected port bindings to be resolved, was: ", config.PortBindings)
333-
}
374+
// Verify PortBindings
375+
if tc.expectedPortBinding != nil {
376+
if !reflect.DeepEqual(config.PortBindings, tc.expectedPortBinding) {
377+
t.Error("Expected port bindings to be resolved, was: ", config.PortBindings)
378+
}
379+
} else {
380+
// Verify ECS Agent assigned host ports are within the dynamic host port range
381+
eStartPort, eEndPort, _ := nat.ParsePortRangeToInt(tc.testDynamicHostPortRange)
382+
for _, hostPortBinding := range config.PortBindings {
383+
hostPort, _ := strconv.Atoi(hostPortBinding[0].HostPort)
384+
result := utils.PortIsInRange(hostPort, eStartPort, eEndPort)
385+
if !result {
386+
t.Error("Actual host port is not in the dynamicHostPortRange: ", hostPort)
387+
break
388+
}
389+
}
390+
}
334391

335-
if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortSet, tc.expectedContainerPortSet) {
336-
t.Error("Expected container port set to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortSet())
337-
}
392+
// Verify ContainerPortSet
393+
if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortSet, tc.expectedContainerPortSet) {
394+
t.Error("Expected container port set to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortSet())
395+
}
338396

339-
if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortRangeMap, tc.expectedContainerPortRangeMap) {
340-
t.Error("Expected container port range map to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortRangeMap())
397+
// Verify ContainerPortRangeMap
398+
if tc.expectedContainerPortRangeMap != nil {
399+
if !reflect.DeepEqual(tc.testTask.Containers[0].ContainerPortRangeMap, tc.expectedContainerPortRangeMap) {
400+
t.Error("Expected container port range map to be resolved, was: ", tc.testTask.Containers[0].GetContainerPortRangeMap())
401+
}
402+
} else {
403+
// Verify ECS Agent assigned host port range are within the dynamic host port range
404+
hostPortRange := tc.testTask.Containers[0].ContainerPortRangeMap[tc.testContainerPortRange]
405+
result := utils.VerifyPortsWithinRange(hostPortRange, tc.testDynamicHostPortRange)
406+
if !result {
407+
t.Error("Expected host port range should be in the dynamicHostPortRange, but the actual host port range is: ", hostPortRange)
408+
}
409+
}
410+
} else {
411+
assert.NotNil(t, err)
341412
}
342413
})
343414
}

‎agent/config/config.go

+2
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@ func (cfg *Config) String() string {
630630
"DependentContainersPullUpfront: %v, "+
631631
"TaskCPUMemLimit: %v, "+
632632
"ShouldExcludeIPv6PortBinding: %v, "+
633+
"DynamicHostPortRange: %v"+
633634
"%s",
634635
cfg.Cluster,
635636
cfg.AWSRegion,
@@ -648,6 +649,7 @@ func (cfg *Config) String() string {
648649
cfg.DependentContainersPullUpfront,
649650
cfg.TaskCPUMemLimit,
650651
cfg.ShouldExcludeIPv6PortBinding,
652+
cfg.DynamicHostPortRange,
651653
cfg.platformString(),
652654
)
653655
}

‎agent/utils/ephemeral_ports.go

+29
Original file line numberDiff line numberDiff line change
@@ -214,3 +214,32 @@ func getHostPortRange(numberOfPorts, start, end int, protocol string) (string, i
214214

215215
return fmt.Sprintf("%d-%d", resultStartPort, resultEndPort), resultEndPort, nil
216216
}
217+
218+
// PortIsInRange returns true if the given port is within the start-end port range;
219+
// otherwise, returns false.
220+
func PortIsInRange(port, start, end int) bool {
221+
if (port >= start) && (port <= end) {
222+
return true
223+
}
224+
return false
225+
}
226+
227+
// VerifyPortsWithinRange returns true if the actualPortRange is within the expectedPortRange;
228+
// otherwise, returns false.
229+
func VerifyPortsWithinRange(actualPortRange, expectedPortRange string) bool {
230+
// Get the actual start port and end port
231+
aStartPort, aEndPort, _ := nat.ParsePortRangeToInt(actualPortRange)
232+
// Get the expected start port and end port
233+
eStartPort, eEndPort, _ := nat.ParsePortRangeToInt(expectedPortRange)
234+
// Check the actual start port is in the expected range or not
235+
aStartIsInRange := PortIsInRange(aStartPort, eStartPort, eEndPort)
236+
// Check the actual end port is in the expected range or not
237+
aEndIsInRange := PortIsInRange(aEndPort, eStartPort, eEndPort)
238+
239+
// Return true if both actual start port and end port are in the expected range
240+
if aStartIsInRange && aEndIsInRange {
241+
return true
242+
}
243+
244+
return false
245+
}

0 commit comments

Comments
 (0)