Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DHPA] Add GetHostPort() and update unit tests #3570

Merged

Conversation

chienhanlin
Copy link
Contributor

@chienhanlin chienhanlin commented Feb 14, 2023

Summary

DHPA - Dynamic Host Port Assignment

The target branch of this PR is feature/dynamicHostPortAssignment.

A new function GetHostPort() is added to ephemeral_ports.go to get a singular host port within the given dynamicHostPortRange range. This change is a part of works to make ECS Agent selects a host port/host port range and passes it to Docker as default behavior.

dynamicHostPortRange can be configured by users 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() in the utils package.

Implementation details

  1. agent/utils/ephemeral_ports.go
  • Add a new function GetHostPort()
  • Move the shared port finding logic for both host port and host port range to a function named getNumOfHostPorts()
  • Update comments, error messages, and add log entries at the debug level
  1. agent/utils/ephemeral_ports_test.go
  • Add unit tests to test GetHostPort()

Testing

New tests cover the changes: yes

--- PASS: TestGetHostPort (0.00s)
    --- PASS: TestGetHostPort/tcp_protocol,_a_host_port_found (0.00s)
    --- PASS: TestGetHostPort/udp protocol, a host port found (0.00s)
    --- PASS: TestGetHostPort/5 requests for host port in succession, success (0.00s)
    --- PASS: TestGetHostPort/5 requests for host port in succession, success (0.00s)

An example of debug log when the port is unavailable

1676356595945429552 [Debug] Port 100 is unavailable or an error occurred while listening on the local tcp network
1676356595945467805 [Debug] Port 101 is unavailable or an error occurred while listening on the local tcp network

Description for the changelog

N/A

Licensing

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

@chienhanlin chienhanlin changed the title [DynamicHostPortAssignment] Add GetHostPort() and update unit tests [DHPA] Add GetHostPort() and update unit tests Feb 14, 2023
@chienhanlin chienhanlin marked this pull request as ready for review February 14, 2023 19:56
@chienhanlin chienhanlin requested a review from a team as a code owner February 14, 2023 19:56
Comment on lines 115 to 120
numberOfPorts := 1
result, err := getHostPortsWithRange(numberOfPorts, protocol, dynamicHostPortRange)
if err == nil {
result = strings.Split(result, "-")[0]
}
return result, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Also, wanted to call out that you gave the 1 a name so that it's clear what it is.

agent/utils/ephemeral_ports.go Outdated Show resolved Hide resolved
agent/utils/ephemeral_ports.go Outdated Show resolved Hide resolved
agent/utils/ephemeral_ports.go Outdated Show resolved Hide resolved
@chienhanlin chienhanlin merged commit de967e1 into aws:feature/dynamicHostPortAssignment Feb 15, 2023
chienhanlin added a commit that referenced this pull request Mar 2, 2023
1. Add GetHostPort() and update unit tests #3570

2. Upudate dockerPortMap() in task.go with dynamic host port range support part 1 #3584

3. Upudate dockerPortMap() in task.go with dynamic host port range support part 2 #3589

4. Validate the host port/host port range found by ECS Agent before returning it #3589
chienhanlin added a commit to chienhanlin/amazon-ecs-agent that referenced this pull request Mar 4, 2023
1. Add GetHostPort() and update unit tests aws#3570

2. Upudate dockerPortMap() in task.go with dynamic host port range support part 1 aws#3584

3. Upudate dockerPortMap() in task.go with dynamic host port range support part 2 aws#3589

4. Validate the host port/host port range found by ECS Agent before returning it aws#3589

5. Refactor buildPortMapWithSCIngressConfig() in task.go aws#3600
chienhanlin added a commit that referenced this pull request Mar 6, 2023
1. Add GetHostPort() and update unit tests #3570

2. Upudate dockerPortMap() in task.go with dynamic host port range support part 1 #3584

3. Upudate dockerPortMap() in task.go with dynamic host port range support part 2 #3589

4. Validate the host port/host port range found by ECS Agent before returning it #3589

5. Refactor buildPortMapWithSCIngressConfig() in task.go #3600
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants