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.

Sign up for GitHub

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

Jump to bottom

Add ENABLE_NFTABLES to VPC CNI #2155

Merged
merged 1 commit into from Dec 9, 2022
Merged

Add ENABLE_NFTABLES to VPC CNI #2155

merged 1 commit into from Dec 9, 2022

Conversation

jdn5126
Copy link
Contributor

@jdn5126 jdn5126 commented Dec 8, 2022

What type of PR is this?
feature

Which issue does this PR fix:
#2103
#1847

What does this PR do / Why do we need it:
This PR add a new environment variable, ENABLE_NFTABLES, that allows customers to switch from using iptables-legacy (default) to iptables-nft in aws-node pod.

It also updates cmd/aws-vpc-cni-init/main.go to use procsyswrapper and removes utils/sysctl. #2146 merged before I realized that procsyswrapper existed.

Output:

$ kubectl logs aws-node-xlqj9 -n kube-system -c aws-node
time="2022-12-08T00:37:34Z" level=info msg="Updating iptables mode to nft"
Installed /host/opt/cni/bin/aws-cni
Installed /host/opt/cni/bin/egress-v4-cni
time="2022-12-08T00:37:34Z" level=info msg="Starting IPAM daemon... "
time="2022-12-08T00:37:34Z" level=info msg="Checking for IPAM connectivity... "
time="2022-12-08T00:37:35Z" level=info msg="Copying config file... "
time="2022-12-08T00:37:35Z" level=info msg="Successfully copied CNI plugin binary and config file."

Exec Output:

$ kubectl exec aws-node-xlqj9 -n kube-system -c aws-node -- iptables -V
iptables v1.8.4 (nf_tables)

$ kubectl exec aws-node-vttvf -n kube-system -c aws-node -- iptables -V
iptables v1.8.4 (legacy)

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 integration tests pass (IPv4 and IPv6).

Automation added to e2e:
None

Will this PR introduce any new dependencies?:
No

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
This will not break upgrades or downgrades. A running cluster has been tested.

Does this change require updates to the CNI daemonset config files to work?:
Yes, this adds an environment variable to the CNI daemonset config.

Does this PR introduce any user-facing change?:
Yes

Add `ENABLE_NFTABLES` environment variable to VPC CNI to allow customers to use `iptables-nft` instead of default `iptables-legacy`

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 December 8, 2022 00:41
achevuru
achevuru reviewed Dec 8, 2022
View reviewed changes
cmd/aws-vpc-cni/main.go Outdated Show resolved Hide resolved
jayanthvn
jayanthvn reviewed Dec 8, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
@jdn5126 jdn5126 merged commit c81626d into aws:master Dec 9, 2022
@jdn5126 jdn5126 deleted the nftables branch December 9, 2022 16:13
jdn5126 added a commit that referenced this pull request Dec 12, 2022
@jaydeokar @jdn5126
@antoninbas @jayanthvn @sushrk @jerryhe1999 @bwagner5 @jcogilvie
Merge master to Release 1.12 for v1.12.1 release (#2156)
54ae7a7 
* create publisher with logger (#2119)

* Add missing rules when NodePort support is disabled (#2026)

* Add missing rules when NodePort support is disabled

* the rules that need to be installed for NodePort support and SNAT
  support are very similar. The same traffic mark is needed for both. As
  a result, rules that are currently installed only when NodePort
  support is enabled should also be installed when external SNAT is
  disabled, which is the case by default.
* remove "-m state --state NEW" from a rule in the nat table. This is
  always true for packets that traverse the nat table.
* fix typo in one rule's name (extra whitespace).

Fixes #2025

Co-authored-by: Quan Tian <qtian@vmware.com>

Signed-off-by: Antonin Bas <abas@vmware.com>

* Fix typos and unit tests

Signed-off-by: Antonin Bas <abas@vmware.com>

* Minor improvement to code comment

Signed-off-by: Antonin Bas <abas@vmware.com>

* Address review comments

* Delete legacy nat rule
* Fix an unrelated log message

Signed-off-by: Antonin Bas <abas@vmware.com>

Signed-off-by: Antonin Bas <abas@vmware.com>
Co-authored-by: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
Co-authored-by: Sushmitha Ravikumar <58063229+sushrk@users.noreply.github.com>

* downgrade test go.mod to align with root go.mod (#2128)

* skip addon installation when addon info is not available (#2131)

* Merging test/Makefile and test/go.mod to the root Makefil and go.mod, adjust the .github/workflows and integration test instructions (#2129)

* update troubleshooting docs for CNI image (#2132)

fix location where make command is run

* fix env name in test script (#2136)

* optionally allow CLUSTER_ENDPOINT to be used rather than the cluster-ip (#2138)

* optionally allow CLUSTER_ENDPOINT to be used rather than the kubernetes cluster ip

* remove check for kube-proxy

* add version to readme

* Add resources config option to cni metrics helper (#2141)

* Add resources config option to cni metrics helper

* Remove default-empty resources block; replace with conditional

* Add metrics for ec2 api calls made by CNI and expose via prometheus (#2142)

Co-authored-by: Jay Deokar <jsdeokar@amazon.com>

* increase workflow role duration to 4 hours (#2148)

* Update golang 1.19.2 EKS-D (#2147)

* Update golang

* Move to EKS distro builds

* [HELM]: Move CRD resources to a separate folder as per helm standard (#2144)

Co-authored-by: Jay Deokar <jsdeokar@amazon.com>

* VPC-CNI minimal image builds (#2146)

* VPC-CNI minimal image builds

* update dependencies for ginkgo when running integration tests

* address review comments and break up init main function

* review comments for sysctl

* Simplify binary installation, fix review comments

Since init container is required to always run, let binary installation
for external plugins happen in init container. This simplifies the main
container entrypoint and the dockerfile for each image.

* when IPAMD connection fails, try to teardown pod network using prevResult (#2145)

* add env var to enable nftables (#2155)

* fix failing weekly cron tests (#2154)

* Deprecate AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER and remove no-op setter (#2153)

* Deprecate AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER

* update release version comments

Signed-off-by: Antonin Bas <abas@vmware.com>
Co-authored-by: Jeffrey Nelson <jdnelson@amazon.com>
Co-authored-by: Antonin Bas <antonin.bas@gmail.com>
Co-authored-by: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
Co-authored-by: Sushmitha Ravikumar <58063229+sushrk@users.noreply.github.com>
Co-authored-by: Jerry He <37866862+jerryhe1999@users.noreply.github.com>
Co-authored-by: Brandon Wagner <wagnerbm@amazon.com>
Co-authored-by: Jonathan Ogilvie <679297+jcogilvie@users.noreply.github.com>
Co-authored-by: Jay Deokar <jsdeokar@amazon.com>
haouc pushed a commit to haouc/amazon-vpc-cni-k8s that referenced this pull request Dec 13, 2022
@jdn5126 @haouc
add env var to enable nftables (aws#2155)
39dd9a9
@jaxesn jaxesn mentioned this pull request Feb 6, 2023
Closed
@hakman hakman mentioned this pull request May 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jayanthvn jayanthvn jayanthvn approved these changes

@achevuru achevuru achevuru approved these changes

Assignees

@achevuru achevuru

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jdn5126 @jayanthvn @achevuru