update for KUBE-IPTABLES-HINT (and other 2022-ness) #3
Conversation
iptables 1.8.3 has a bug that makes it hang forever in certain cases, requiring us to use "timeout" in the script. However, 1.8.4 is over two years old at this point, and even Debian buster-backports has 1.8.5, so there's no reason to keep supporting 1.8.3. So remove that. Also update the README and test Dockerfiles to use newer versions of Fedora and Alpine to get a new-enough version.
Systems these days are more likely to be using iptables-nft than iptables-legacy, so if there are no rules in either table, guess "nft" rather than "legacy".
…CANARY first Check for the (1.24+) KUBE-IPTABLES-HINT or (1.17+) KUBE-KUBELET-CANARY chain first, and only fall back to counting total number of legacy vs nft rules if we don't find that.
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship 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 |
k8s-ci-robot
added
the
size/L
k8s-ci-robot
added
the
approved
|
@danwinship: GitHub didn't allow me to request PR reviews from the following users: squeed. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: 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. |
| @@ -16,7 +16,7 @@ | |||
|
|
|||
| FROM debian:buster | |||
|
|
|||
| ARG INSTALL_ARGS | |||
| ARG INSTALL_ARGS= | |||
There was a problem hiding this comment.
is this = right?
I've never seen this format, but doesn't mean isnot right
There was a problem hiding this comment.
Yeah, it's just setting the default value to "". Which is what it was before, but podman complains now if you declare an ARG without giving it an explicit default value and then also don't set it from the command line:
danw@p1:iptables-wrappers (modernize)> make check-debian
./test/run-test.sh --build-fail debian
docker build --no-cache -q -t iptables-wrapper-test-debian -f test/Dockerfile.test-debian .
time="2022-04-11T09:12:43-04:00" level=warning msg="missing \"INSTALL_ARGS\" build argument. Try adding \"--build-arg INSTALL_ARGS=<VALUE>\" to the command line"
| # Check for kubernetes 1.17-or-later with iptables-legacy. We | ||
| # can't pass "-t mangle" to iptables-legacy-save because it would | ||
| # cause the kernel to create that table if it didn't already | ||
| # exist, which we don't want. So we have to grab all the rules |
| mode=legacy | ||
| else | ||
| mode=nft | ||
| # With older kubernetes releases there may not be any _specific_ |
There was a problem hiding this comment.
I would say it's safe to assume KUBE-MARK-DROP will always exist -- it was added in, AFAICT, v1.4.
That said, it probably makes sense to just fall back to "literally any rules exist at all".
There was a problem hiding this comment.
KUBE-MARK-DROP is going away 🙂
kubernetes/enhancements#3179
|
looks good to me. Thanks for thinking to maintain this. |
KEP-3178 will (eventually) make kubelet create fewer iptables rules so we need to update iptables-wrappers to cope with that.
Also, misc updates for the last few years of iptables evolution...
/assign @dcbw