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

Add IPv6 egress support to eks IPv4 cluster #2361

Merged
merged 3 commits into from
May 3, 2023

Conversation

wanyufe
Copy link
Member

@wanyufe wanyufe commented Apr 26, 2023

What type of PR is this?
feature

Which issue does this PR fix:
#1925

What does this PR do / Why do we need it:
IPv6 egress support for IPv4 EKS cluster

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

Testing done on this change:
Manual testing - unit testing has finished

bash-5.1# ip -6 a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 state UNKNOWN qlen 1000
    inet6 ::1/128 scope host 
       valid_lft forever preferred_lft forever
5: v6if0@if130: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9001 state UP 
    inet6 fd00::34/8 scope global 
       valid_lft forever preferred_lft forever
    inet6 fe80::e0ac:89ff:fe27:4bdb/64 scope link 
       valid_lft forever preferred_lft forever
bash-5.1# ip -6 r
fd00::/8 dev v6if0 proto kernel metric 256 pref medium
fe80::/64 dev v6if0 proto kernel metric 256 pref medium
default via fe80::5c51:9ff:fe55:3ab2 dev v6if0 metric 1024 pref medium
bash-5.1# ping6 2600::
PING 2600::(2600::) 56 data bytes
64 bytes from 2600::: icmp_seq=1 ttl=43 time=37.6 ms
64 bytes from 2600::: icmp_seq=2 ttl=43 time=37.6 ms
64 bytes from 2600::: icmp_seq=3 ttl=43 time=37.6 ms
^C
--- 2600:: ping statistics ---
3 packets transmitted, 3 received, 0% packet loss, time 2003ms
rtt min/avg/max/mdev = 37.598/37.619/37.633/0.015 ms

Automation added to e2e:

Will this PR introduce any new dependencies?:
No

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

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

Does this PR introduce any user-facing change?:


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

Copy link
Contributor

@jdn5126 jdn5126 left a comment

Choose a reason for hiding this comment

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

Changes look good me overall, thanks @wanyufe ! Just a few nits

cmd/aws-vpc-cni-init/main.go Outdated Show resolved Hide resolved
cmd/aws-vpc-cni/main.go Outdated Show resolved Hide resolved
cmd/aws-vpc-cni/main.go Outdated Show resolved Hide resolved
cmd/egress-cni-plugin/egress-plugin.log Outdated Show resolved Hide resolved
pkg/utils/cniutils/cni_utils.go Show resolved Hide resolved
@wanyufe wanyufe marked this pull request as ready for review April 27, 2023 01:41
@wanyufe wanyufe requested a review from a team as a code owner April 27, 2023 01:41
Copy link
Contributor

@jdn5126 jdn5126 left a comment

Choose a reason for hiding this comment

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

Did a pass focusing on optimizations, and I am realizing we may be able to get some optimizations on the delete path. Hoping so, at least.

cmd/aws-k8s-agent/main.go Outdated Show resolved Hide resolved
cmd/aws-vpc-cni/main.go Outdated Show resolved Hide resolved
cmd/aws-vpc-cni/main.go Outdated Show resolved Hide resolved
cmd/egress-cni-plugin/egress-plugin.log Outdated Show resolved Hide resolved
cmd/egress-cni-plugin/egressContext.go Outdated Show resolved Hide resolved
cmd/egress-cni-plugin/egressContext.go Outdated Show resolved Hide resolved
cmd/egress-cni-plugin/egressContext.go Outdated Show resolved Hide resolved
cmd/egress-cni-plugin/main_test.go Show resolved Hide resolved
cmd/egress-cni-plugin/snat/snat.go Outdated Show resolved Hide resolved
pkg/utils/cniutils/cni_utils.go Show resolved Hide resolved
jdn5126
jdn5126 previously approved these changes May 1, 2023
cmd/egress-cni-plugin/main.go Show resolved Hide resolved
cmd/egress-cni-plugin/main.go Outdated Show resolved Hide resolved
jdn5126
jdn5126 previously approved these changes May 2, 2023
cmd/aws-vpc-cni/main.go Outdated Show resolved Hide resolved
pkg/networkutils/network.go Outdated Show resolved Hide resolved
jdn5126
jdn5126 previously approved these changes May 2, 2023
README.md Outdated Show resolved Hide resolved
@jdn5126 jdn5126 merged commit 233dc9a into aws:master May 3, 2023
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