[DHPA] Upudate dockerPortMap() in task.go with dynamic host port range support - part 2

[chienhanlin]

Summary

DHPA - Dynamic Host Port Assignment

The target branch of this PR is feature/dynamicHostPortAssignment.

This is a follow-up PR for Upudate dockerPortMap() in task.go with dynamic host port range support - part 1 to implement dynamic host port assignment for default bridge mode service connect ingress listener ports in dockerPortMap().

*** For a better view of diffs between these 2 PR, please take a look on this PR. ***

Comparing to the current Upudate dockerPortMap() in task.go with dynamic host port range support - part 1 PR, main changes in this PR include:

1. Add function buildPortMapWithSCIngressConfig() to build a dockerPortMap and a containerPortSet for ingress listener ports under two service connect bridge mode cases:

• Case 1. Non-default bridge mode service connect experience: customers specify host ports (ingressPortOverride) for listeners in the ingress config.

"serviceConnectConfiguration": {
        "enabled": true,
        "namespace": "<Namespace>",
        "services": [
            {
                "portName": "webserver",
                "discoveryName": "<DiscoveryName>",
                "ingressPortOverride": 15000,
                "clientAliases": [
                    {
                        "port": <Client Port>,
                        "dnsName": "<Client DNS>"
                    }
                ]
            }
        ],
}               

• Case 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. If ECS Agent cannot find an available host port within the range, an error will be returned.

2. Update comments for function dockerPortMap() to include service connect ingress listener ports info
3. Refactor how we create port bindings for service connect ingress listener ports in function dockerPortMap()
4. Add a new function ResetTracker() in ephemeral_ports.go to reset the last assigned host port to 0. This function will be used in unit tests TestGetHostPort, TestGetHostPortRange and TestDockerHostConfigSCBridgeMode.
5. Update TestDockerHostConfigSCBridgeMode to test changes made in this PR.

Note that dynamicHostPortRange can be configured by customers using ECS Agent environment variable ECS_DYNAMIC_HOST_PORT_RANGE; if the customized value is not provided, ECS Agent will use the default value returned from GetDynamicHostPortRange().

Implementation details

agent/api/task/task.go

• Add function buildPortMapWithSCIngressConfig() to build a dockerPortMap and a containerPortSet for ingress listener ports
• Refactor how we create port bindings for service connect ingress listener ports and update comments for function dockerPortMap()

agent/api/task/task_test.go

• Update test cases in TestDockerHostConfigSCBridgeMode to test changes

agent/utils/ephemeral_ports.go

• Add ResetTracker() and use it in TestGetHostPort and TestGetHostPortRange

agent/utils/ephemeral_ports_test.go

• Use ResetTracker() in TestGetHostPort and TestGetHostPortRange
• Add TestResetTracker

Testing

New tests cover the changes: yes

Unit test

TestDockerHostConfigSCBridgeMode

--- PASS: TestDockerHostConfigSCBridgeMode (0.00s)
    --- PASS: TestDockerHostConfigSCBridgeMode/with_default_dynamic_host_port_range (0.00s)
    --- PASS: TestDockerHostConfigSCBridgeMode/with_user-specified_dynamic_host_port_range (0.00s)
    --- PASS: TestDockerHostConfigSCBridgeMode/with_user-specified_dynamic_host_port_range_but_no_available_host_port (0.00s)

TestResetTracker

--- PASS: TestResetTracker (0.00s)

Manual test

See Upudate dockerPortMap() in task.go with dynamic host port range support - part 1 for container port and container port range testing results.

Description for the changelog

[Enhancement] Support user-specified dynamic host port range for a singular port and default bridge mode service connect ingress listener port.

Related PRs

1. [DHPA] Add GetHostPort() and update unit tests #3570
2. Remove fallback to Docker for host port ranges assignment #3569
3. Add new config option for DynamicHostPortRange  #3522
4. [DHPA] Upudate dockerPortMap() in task.go with dynamic host port range support - part 1 #3584

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[mye956]

nit: maybe we could add a debug message here saying it's a regular task container and we're leaving the map empty (I also really like these comments as well)

[chienhanlin]

Thanks for the suggestion! I am not sure if adding a debug log here helps because

1. Customers don't really have to know what happened to application containers and service connect AppNet containers as port bindings are published by the associated pause containers.
2. Customers can check network bindings results for service connect AppNet container and application containers through describe-tasks.
3. Showing an empty map in logs probably has less value to customers and us to debug

[SreeeS]

Really like your explanations here.

[YashdalfTheGray]

Absolutely love this demystification of the process.