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

Annotate pods with IPv6 details from branch ENI when available #309

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Annotate pods with IPv6 details from branch ENI when available #309

merged 1 commit into from
Oct 9, 2023

Conversation

jdn5126
Copy link
Contributor

@jdn5126 jdn5126 commented Sep 29, 2023

Issue #, if available:
N/A

Description of changes:
In order to support Security Groups for Pods in IPv6 clusters, the VPC Resource Controller must create trunk and branch ENIs with IPv6 addresses assigned. This happens already based on the subnet used in an IPv6 cluster, so VPC Resource Controller's next role is to annotate pods with branch ENI information.

This PR annotates pods with the IPv6 address and CIDR that is configured for the branch ENI, if there is one. The reason that IPv6 fields are added in-addition, instead of in-place is two-fold:

  1. VPC Resource Controller must support older VPC CNI versions, so it cannot change existing JSON names.
  2. If dual-stack is eventually supported, the pod will need IPv4 and IPv6 fields, so this change facilitates that.

I tested this code against an IPv4 and an IPv6 cluster to verify that it works. In an IPv4 cluster, the annotation looks as follows:

k get pod nginx-deployment-6c4d879f6c-7ktc5 -n sg-test -o jsonpath='{.metadata.annotations}'
{"vpc.amazonaws.com/pod-eni":"[{\"eniId\":\"eni-0f11cb35d7d5bf4d6\",\"ifAddress\":\"0e:ee:16:bd:41:31\",\"privateIp\":\"192.168.19.148\",\"ipv6Addr\":\"\",\"vlanId\":1,\"subnetCidr\":\"192.16
8.0.0/19\",\"subnetV6Cidr\":\"\"}]"}

In an IPv6 cluster, it looks as follows:

{"vpc.amazonaws.com/pod-eni":"[{\"eniId\":\"eni-0f11cb35d7d5bf4d6\",\"ifAddress\":\"0e:ee:16:bd:41:31\",\"privateIp\":\"192.168.19.148\",\"ipv6Addr\":\"2600::\",\"vlanId\":1,\"subnetCidr\":\"192.16
8.0.0/19\",\"subnetV6Cidr\":\"2600::/64\"}]"}

There are no backward-incompatible changes here, but I am still awaiting results from integration tests for further coverage.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jdn5126 jdn5126 requested a review from a team as a code owner September 29, 2023 21:53
@jdn5126 jdn5126 requested a review from haouc September 29, 2023 21:53
@sushrk
Copy link
Contributor

sushrk commented Oct 2, 2023

Changes LGTM overall, thanks @jdn5126! I have kicked off integration tests on Gitlab and will update the results.

EDIT: Seeing errors in setup stage, re-running this.

Copy link
Contributor

@haouc haouc left a comment

Choose a reason for hiding this comment

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

lgtm

@jdn5126 jdn5126 merged commit 86d7623 into aws:master Oct 9, 2023
2 checks passed
@jdn5126 jdn5126 deleted the sgpp_v6 branch October 9, 2023 16:47
sushrk pushed a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Oct 13, 2023
sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Oct 13, 2023
sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Oct 13, 2023
sushrk added a commit that referenced this pull request Oct 13, 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