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

Migrate Agent to use vpc-eni plugin for awsvpc mode instead of ecs-eni plugin on Linux #3873

Merged
merged 5 commits into from
Sep 1, 2023

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Aug 28, 2023

Summary

We recently added Linux support to vpc-eni CNI plugin (aws/amazon-vpc-cni-plugins#101). So, vpc-eni plugin is now capable of setting up network for awsvpc tasks on Linux rendering the currently used ecs-eni plugin redundant. Agent on Windows already uses vpc-eni plugin to set up task network for awsvpc mode.

This PR updates vpc-cni submodule in this repository to be5214353252f8315a1341f4df9ffbd8cf69000c from a83b66349768e020487a00e31767fc2e6fc88136 (diff) and makes Agent use vpc-eni plugin for awsvpc tasks on Linux so that the same plugin is used to setup awsvpc network across Windows and Linux platforms.

There is no change in functionality.

Implementation details

Agent code changes -

  • NewENINetworkConfig function that is used to create a CNI configuration for ecs-eni plugin for Linux in agent/ecscni/netconfig_linux.go file has been replaced with NewVPCENINetworkConfig function that creates a CNI configuration for vpc-eni plugin. Function call to build CNI configuration for awsvpc mode for Linux (for environments with ENI Trunking disabled) in BuildCNIConfigAwsvpc function in task_linux.go file is changed accordingly.
  • The configuration for vpc-eni plugin is very similar to that of ecs-eni plugin. Main differences are in the field names.
  • Configuration struct for vpc-eni plugin is moved out of Windows specific ecscni/types_windows.go file to the more general ecscni/types.go file so that the same configuration struct can be used for both Windows and Linux.
  • Constant ECSVPCENIPluginName for the name of vpc-eni plugin is removed from ecscni/types_windows.go file and is replaced with a new VPCENIPluginName constant in ecscni/types.go file.
  • Configuration struct and constant for the name of for ecs-eni plugin are removed from ecscni/types_linux.go file as they are now unused.
  • Rest of the changes are just reference and test updates.

Build script changes -

  • scripts/build-cni-plugins script is updated to include building vpc-eni plugin.
  • scripts/build-agent-image script is updated to copy vpc-eni plugin instead of ecs-eni plugin to Agent image.
  • scripts/dockerfiles/Dockerfile.buildVPCCNIPlugins Dockerfile is updated to include building vpc-eni plugin.

Testing

A new functional test was added for testing awsvpc mode on Linux for instances with ENI trunking disabled.

Comprehensive manual testing was done -

  • Tested that security group changes to task ENI are reflected dynamically.
  • Tested that IMDS access over IPv4 and IPv6 is blocked for tasks when configuration instructs.
  • Tested that containers in the same task are able to communicate with each other over localhost.
  • Tested that logs from vpc-eni plugin are collected.
  • Tested that an awsvpc task is reachable over its IPv4 and IPv6 addresses.
  • Tested that task is able to reach Agent Task Metadata Endpoint and Credentials Endpoint.
  • Tested that an awsvpc task registered to an Application Load Balancer is reachable through the Load Balancer public IP.
  • Tested that Service Connect ingress and egress metrics are updated in CloudWatch for Service Connect clients and servers.
  • Tested that a test app built with AppMesh works as expected.
  • Tested that network metrics are returned by Task Metadata Endpoint for awsvpc tasks.
  • Tested that network metrics for task's ENI are updated in CloudWatch.
  • Tested that Agent cleans up task network namespaces.
  • Tested that Agent restarts do not impact network setup and handling.

New tests cover the changes: yes

Description for the changelog

Migrate Agent to use vpc-eni plugin for awsvpc mode instead of ecs-eni plugin on Linux

Licensing

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

@amogh09 amogh09 requested a review from a team as a code owner August 28, 2023 19:28
@amogh09 amogh09 changed the base branch from master to dev August 28, 2023 19:29
@amogh09 amogh09 changed the title Vpc eni Migrate Agent to use vpc-eni plugin for awsvpc mode instead of ecs-eni plugin on Linux Aug 29, 2023
@@ -61,9 +61,9 @@ make clean

# buildvcs=false excludes version control information in golang >= 1.18. This is required for compiling agent with included repositories
if [[ $goversion < "1.18" ]]; then
cd ${GITPATH}/amazon-vpc-cni-plugins && GO111MODULE=on GOFLAGS="-mod=vendor" make aws-appmesh vpc-branch-eni ecs-serviceconnect
cd ${GITPATH}/amazon-vpc-cni-plugins && GO111MODULE=on GOFLAGS="-mod=vendor" make aws-appmesh vpc-branch-eni ecs-serviceconnect vpc-eni
Copy link
Member

Choose a reason for hiding this comment

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

were you able to validate that the new make commands all work with this update -- I'm specifically looking at release-agent-internal

Copy link
Contributor Author

@amogh09 amogh09 Aug 31, 2023

Choose a reason for hiding this comment

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

Yes for all the manual testing I built Agent with make release-agent which delegates image building to release-agent-internal. There were no issues in getting the Agent image built and running Agent with it.

Some functional tests are executed on images built with docker-release target. This target is also updated in this PR (change is in scripts/dockerfiles/Dockerfile.buildVPCCNIPlugins). Those tests are also passing and I have verified in the logs that vpc-eni plugin is invoked indeed.

@fierlion
Copy link
Member

Overall comment --
This was a great PR to read through. Thank you for the clear description and outline of testing done. These CNI changes have bit us in the past, but your work here gives me confidence in this particular (not insignificant) change.

@amogh09 amogh09 merged commit a6b6076 into aws:dev Sep 1, 2023
7 checks passed
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