Skip to content

Initialize IP Comaptibility config before service clients are created #4568

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

Merged
merged 18 commits into from
Apr 14, 2025

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Apr 10, 2025

Summary

Currently IP compatibility config variables are initialized as a part of ecsAgent startup in its start method. This happens after some service clients such as ECS Client and ECR Client have been initialized. In future, service client configuration will depend on IP compatibility, so IP compatibility needs to be determined earlier.

So, this PR moves IP Compatibility config variables from global variables to fields of type Config struct and initializes them when a Config is created by NewConfig function. Note that IP Compatibility initialization depends on MAC resolution of primary ENI in case of EC2 Launch Type. So, config initialization now involves a call IMDS to fetch the MAC address of the primary ENI. NewConfig is already provided an ec2.EC2MetadataClient so the same client is used to call IMDS to fetch primary ENI's mac address. Note that this call to IMDS is not a net new dependency on IMDS because Agent already does this as a part of ecsAgent.start method. We are just doing it one more time during config initialization.

Implementation details

  • Introduce a helper type IPCompatibility struct for storing IPv4 and IPv6 compatibility status.
  • Remove IP Compatibility global variables from config package and add a new InstanceIPCompatibility ipcompatibility.IPCompatibility field to config.Config struct.
  • Existing logic to determine compatibility is moved from config package to utils/net package.
  • Update config.NewConfig function so that it calls config.determineIPCompatibility method to determine and set IP compatibility.
  • config.determineIPCompatibility method on Linux is set up to fetch MAC address of the primary ENI from IMDS (for EC2 Launch Type only) and then call DetermineIPCompatibility function from utils/net.
  • Remove IP Compatibility initialization logic from app/agent.go.
  • Tests that depend on config.NewConfig function typically pass ec2.NewBlackholeEC2MetadataClient() as a parameter which returns an error on every method call. This worked fine until now as NewConfig did not error out on IMDS call failures, however, with this change that no longer holds. So, a new fake implementation of IMDS client named FakeEC2MetadataClient is added and the tests are updated to use this implementation instead. The implementation returns a "not implemented" error for all API calls except for PrimaryENIMAC call that returns a non-error response.

Testing

New tests cover the changes: yes

Ran Agent on three instances - dual-stack, IPv4-only, and IPv6-only - and verified in the logs that instance IP compatibility is determined correctly in all three cases.

Description for the changelog

Additional Information

Does this PR include breaking model changes? If so, Have you added transformation functions?

No

Does this PR include the addition of new environment variables in the README?

No

Licensing

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

@amogh09 amogh09 changed the base branch from master to dev April 10, 2025 17:06
@amogh09 amogh09 force-pushed the ipv6-config-instance branch from f9caf82 to 2da7f74 Compare April 10, 2025 17:07
@amogh09 amogh09 changed the title Ipv6 config instance Move IP Compatibility config variables to Config struct Apr 10, 2025
@amogh09 amogh09 changed the title Move IP Compatibility config variables to Config struct Initialize IP Comaptibility config before service clients are created Apr 10, 2025
@amogh09 amogh09 marked this pull request as ready for review April 10, 2025 21:16
@amogh09 amogh09 requested a review from a team as a code owner April 10, 2025 21:16
ShelbyZ
ShelbyZ previously approved these changes Apr 11, 2025
@amogh09 amogh09 force-pushed the ipv6-config-instance branch from ba04f21 to 2815ad6 Compare April 14, 2025 19:37
@amogh09 amogh09 merged commit 5e47ab8 into aws:dev Apr 14, 2025
40 checks passed
xxx0624 pushed a commit to xxx0624/amazon-ecs-agent that referenced this pull request Apr 17, 2025
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.

4 participants