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.

Sign up for GitHub

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

Jump to bottom

Add new config option for DynamicHostPortRange #3522

Merged
merged 1 commit into from
Dec 22, 2022
Merged

Add new config option for DynamicHostPortRange #3522

merged 1 commit into from
Dec 22, 2022

Conversation

SreeeS
Copy link
Contributor

@SreeeS SreeeS commented Dec 16, 2022 •
edited
Loading

Summary

Added a new config option to configure ephemeral host port range

Implementation details

  1. Added a new config option to configure ephemeral host port range by adding a new environment variable to the config struct which would be called from the NewConfig function.
  2. Modified the current fallback logic in the case of no host port range and added this fallback logic to work while parsing the environment variable for DynamicHostPortRange.
  3. Added the unit tests for the changes

Testing

  1. Tested the new dynamic host port range config option by running the taskDefinitions and checking the portbindings for the hostPortRange by describing the tasks.
  • Case1: Passed the wrong dynamic host port range value and received the default value.
  • Case2: Passed the expected dynamic host port range value and received the hostPortRange with in user defined DynamicHostPortRange
  • Case3: Passed no value and still received the default value.
  1. Pass the unit tests in the GitHub workflow

New tests cover the changes: yes

Description for the changelog

Enhancement: added new agent configuration to specify ephemeral host port range for port range mapping.

Licensing

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

@SreeeS SreeeS marked this pull request as ready for review December 16, 2022 17:21
@SreeeS SreeeS requested a review from a team as a code owner December 16, 2022 17:21
@SreeeS SreeeS changed the title Added a new configuration option to configure ephimeral host port range with a new environment variable Added a new configuration option to configure ephemeral host port range with a new environment variable Dec 16, 2022
singholt
singholt reviewed Dec 19, 2022
View reviewed changes
agent/config/parse.go Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
agent/config/config_test.go Outdated Show resolved Hide resolved
agent/config/parse.go Show resolved Hide resolved
agent/config/types.go Outdated Show resolved Hide resolved
agent/config/types.go Outdated Show resolved Hide resolved
@SreeeS SreeeS changed the title Added a new configuration option to configure ephemeral host port range with a new environment variable Add new config option for DynamicHostPortRange Dec 21, 2022
singholt
singholt previously approved these changes Dec 21, 2022
View reviewed changes
Copy link
Contributor

@singholt singholt left a comment

Choose a reason for hiding this comment

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

🚀

agent/config/config_test.go Outdated Show resolved Hide resolved
@SreeeS SreeeS dismissed stale reviews from prateekchaudhry and singholt via e2aa6fe December 21, 2022 21:42
@SreeeS SreeeS merged commit f96a061 into aws:dev Dec 22, 2022
@SreeeS SreeeS mentioned this pull request Jan 4, 2023
Merged
@fierlion fierlion added this to the 1.68.0 milestone Jan 9, 2023
This was referenced Feb 16, 2023
Closed
Merged
Closed
Closed
This was referenced Feb 24, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prateekchaudhry prateekchaudhry prateekchaudhry approved these changes

@singholt singholt singholt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.68.0
Development

Successfully merging this pull request may close these issues.
5 participants
@SreeeS @prateekchaudhry @singholt @fierlion @amazon-ecs-bot