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

IPv6 Security Groups for Pods Support #2639

Merged
merged 1 commit into from
Nov 30, 2023
Merged

IPv6 Security Groups for Pods Support #2639

merged 1 commit into from
Nov 30, 2023

Conversation

jdn5126
Copy link
Contributor

@jdn5126 jdn5126 commented Oct 31, 2023

What type of PR is this?

feature

Which issue does this PR fix:
N/A

What does this PR do / Why do we need it:
This PR adds VPC CNI support for IPv6 Security Groups for Pods. It is dependent on VPC Resource Controller support added in aws/amazon-vpc-resource-controller-k8s#309.

This PR is unfortunately quite large, so to help reviewers, I will call out major areas to focus on:
Initialization

  • cmd/aws-vpc-cni-init/main.go - default sysctls for IPv6 have been modified
  • pkg/ipamd/ipamd.go - configuration of IPv6 Security Groups for Pods needs to be allowed

ENI Management

  • pkg/awsutils/awsutils.go - when attaching an IPv6 trunk ENI, the metadata associated with the ENI needs to incorporate IPv6.
  • pkg/ipamd/ipamd.go - when IPv6 Security Groups for Pods is enabled, IPAMD must wait for a trunk ENI to be attached
  • pkg/networkutils/network.go - support for attaching an IPv6 trunk ENI, which includes getting the IPv6 gateway address and configuring relevant routes and rules

Pod Interface Management

  • cmd/routed-eni-cni-plugin/driver/driver.go - sysctls for branch interfaces and IPv6 veth interfaces are set
  • pkg/ipamd/rpc_handler.go - support for deriving IPv6 branch ENI information from annotation

Caching

  • We do not want to constantly derive the IPv6 gateway address on an instance. When the IPv6 trunk ENI is configured, we cache the gateway address in IPAMD for use by CNI ADD.

Testing

  • test/integration/pod-eni has been made compatible for IPv6

Other
I realized that make unit-test was not invoking unit tests defined in cmd/.... Some of these unit tests were failing, so I made sure all were passing in this PR.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
N/A

Testing done on this change:

Manually verified that test/integration/pod-eni passes. Also manually verified that all existing integration tests pass.

Further validation is in progress for upgrade/downgrade, scale, performance, etc.

Will this PR introduce any new dependencies?:

No

Will this break upgrades or downgrades? Has updating a running cluster been tested?:
No, Yes

Does this change require updates to the CNI daemonset config files to work?:

No

Does this PR introduce any user-facing change?:

Yes

IPv6 Security Groups for Pods support

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

@jdn5126 jdn5126 requested a review from a team as a code owner October 31, 2023 17:06
@jdn5126 jdn5126 force-pushed the sgpp_v6 branch 4 times, most recently from 2f07cdb to 1c86175 Compare November 2, 2023 17:42
@jdn5126 jdn5126 force-pushed the sgpp_v6 branch 3 times, most recently from bd236a8 to e57c43e Compare November 13, 2023 21:31
@jdn5126 jdn5126 force-pushed the sgpp_v6 branch 3 times, most recently from 103614d to 3dd6004 Compare November 17, 2023 17:09
@jdn5126 jdn5126 force-pushed the sgpp_v6 branch 3 times, most recently from 4bcdeef to 8bccb40 Compare November 27, 2023 21:46
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
@achevuru
Copy link
Contributor

lgtm

@jdn5126 jdn5126 merged commit 9adefc7 into aws:master Nov 30, 2023
6 checks passed
@jdn5126 jdn5126 deleted the sgpp_v6 branch November 30, 2023 17:36
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.

3 participants