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

Adds iptables-wrapper to iptables minimal image #885

Merged
merged 2 commits into from
Mar 6, 2023

Conversation

jaxesn
Copy link
Member

@jaxesn jaxesn commented Mar 3, 2023

Issue #, if available:

Description of changes:

This should resolve aws/eks-distro#1729.

@g-gaston has been working with upstream on a golang based iptables-wrapper which we could use in our minimal images to properly switch the kube-proxy image from iptables-legacy to iptables-nft. This PR pulls in the current state of this PR, which based on our testing seems pretty solid. This code is also very simple and easy enough to reason about that I feel comfortable getting a bit ahead here, esp considering our kube-proxy image does not work properly on hosts with iptables-nft as the default.

I have chosen to add the wrapper to the minimal iptables images, vs adding in the eks-d build so that we can utilize this in the various consumers of this base image (eks-d, eks minimal addon, eks-a kind image). This change introduces the binary to the final along with a new set-alternative which should be fully backward compatible with previous versions of this image, allowing consumers to "opt-in" to use the new wrapper. The default will continue to be iptables-legacy.

Once merged and built, we can make a simple change to eks-d, basically just this line to swap the default to the wrapper alternative, which should fix the legacy/nft issue.

/hold

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

@jaxesn
Copy link
Member Author

jaxesn commented Mar 3, 2023

Tested in a kops using a custom build of kube-proxy on both Ubuntu 22.04 and 20.04 and the wrapper correctly picked the mode.

eks-distro-base/Dockerfile.minimal-base-iptables Outdated Show resolved Hide resolved
eks-distro-base/Dockerfile.minimal-base-iptables Outdated Show resolved Hide resolved
eks-distro-base/Dockerfile.minimal-base-iptables Outdated Show resolved Hide resolved

cleanup-iptable-rules

# run upstream test binaries which tests a number of the same cases as above
Copy link
Member

Choose a reason for hiding this comment

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

is anything that you are testing above this line missing from the go tests?
If so, let me and I'll add it.

@g-gaston
Copy link
Member

g-gaston commented Mar 6, 2023

/lgtm

@jaxesn jaxesn force-pushed the jgw/add-iptables-wrapper branch from da73daa to 4307b8d Compare March 6, 2023 18:53
@jaxesn
Copy link
Member Author

jaxesn commented Mar 6, 2023

/unhold

@jaxesn
Copy link
Member Author

jaxesn commented Mar 6, 2023

The null in the tag file breaks the kind presubmit.

/override eks-distro-base-tooling-presubmits-kind-al-2 eks-distro-base-tooling-presubmits-kind-al-2022

@eks-distro-bot
Copy link
Collaborator

@jaxesn: Overrode contexts on behalf of jaxesn: eks-distro-base-tooling-presubmits-kind-al-2, eks-distro-base-tooling-presubmits-kind-al-2022

In response to this:

The null in the tag file breaks the kind presubmit.

/override eks-distro-base-tooling-presubmits-kind-al-2 eks-distro-base-tooling-presubmits-kind-al-2022

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jaxesn
Copy link
Member Author

jaxesn commented Mar 6, 2023

/approve

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jaxesn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting hosts configured to use iptables-nft in kube-proxy
4 participants