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

Ignore the exit code of modprobe always #468

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Ignore the exit code of modprobe always #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da22584
Ignore the exit code of `modprobe` always
tianon Dec 18, 2023
d5d05eb
Use `/proc/net/ip_tables_names` (and friends) to pre-detect whether w…
tianon Dec 19, 2023
eb4c40e
Add `DOCKER_IPTABLES_LEGACY` for letting users explicitly pick `iptab…
tianon Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions 24/dind/dockerd-entrypoint.sh
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 33 additions & 4 deletions 25-rc/dind/dockerd-entrypoint.sh
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 33 additions & 4 deletions dockerd-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,46 @@ if [ "$1" = 'dockerd' ]; then
# XXX inject "docker-init" (tini) as pid1 to workaround https://github.com/docker-library/docker/issues/318 (zombie container-shim processes)
set -- docker-init -- "$@"

if ! iptables -nL > /dev/null 2>&1; then
iptablesLegacy=
if [ -n "${DOCKER_IPTABLES_LEGACY+x}" ]; then
# let users choose explicitly to legacy or not to legacy
iptablesLegacy="$DOCKER_IPTABLES_LEGACY"
Comment on lines +147 to +149
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't actually test this before I pushed it, but now I have: 😅

$ docker run --rm 3f3f60609d5a |& grep '^iptables '
iptables v1.8.10 (nf_tables)
$ docker run --rm --env DOCKER_IPTABLES_LEGACY= 3f3f60609d5a |& grep '^iptables '
iptables v1.8.10 (nf_tables)
$ docker run --rm --env DOCKER_IPTABLES_LEGACY=1 3f3f60609d5a |& grep '^iptables '
iptables v1.8.10 (legacy)

Copy link

Choose a reason for hiding this comment

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

Giving users the opportunity to decide for themselves is the best option I think.

if [ -n "$iptablesLegacy" ]; then
modprobe ip_tables || :
else
modprobe nf_tables || :
fi
elif (
# https://git.netfilter.org/iptables/tree/iptables/nft-shared.c?id=f5cf76626d95d2c491a80288bccc160c53b44e88#n420
# https://github.com/docker-library/docker/pull/468#discussion_r1442131459
for f in /proc/net/ip_tables_names /proc/net/ip6_tables_names /proc/net/arp_tables_names; do
if b="$(cat "$f")" && [ -n "$b" ]; then
exit 0
fi
done
exit 1
); then
# if we already have any "legacy" iptables rules, we should always use legacy
iptablesLegacy=1
elif ! iptables -nL > /dev/null 2>&1; then
# if iptables fails to run, chances are high the necessary kernel modules aren't loaded (perhaps the host is using xtables, for example)
# https://github.com/docker-library/docker/issues/350
# https://github.com/moby/moby/issues/26824
# https://github.com/docker-library/docker/pull/437#issuecomment-1854900620
if ! modprobe nf_tables; then
modprobe nf_tables || :
if ! iptables -nL > /dev/null 2>&1; then
# might be host has no nf_tables, but Alpine is all-in now (so let's try a legacy fallback)
modprobe ip_tables || :
# see https://github.com/docker-library/docker/issues/463 (and the dind Dockerfile where this directory is set up)
export PATH="/usr/local/sbin/.iptables-legacy:$PATH"
if /usr/local/sbin/.iptables-legacy/iptables -nL > /dev/null 2>&1; then
iptablesLegacy=1
fi
fi
Copy link

Choose a reason for hiding this comment

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

When host has both nf_tables and ip_tables but is using ip_tables, iptables is giving this warning:

iptables -nL >/dev/null
# Warning: iptables-legacy tables present, use iptables-legacy to see them

Something like this worked for me to detect that case and use iptables-legacy:

        elif iptables -nL 2>&1 >/dev/null | grep -q tables-legacy; then
                export PATH="/usr/local/sbin/.iptables-legacy:$PATH"

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, interesting! Any idea in what situations/use cases a fresh network namespace might have legacy tables set up in it already?

Copy link
Member Author

Choose a reason for hiding this comment

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

I found the code in iptables itself responsible for that check: 😄

https://git.netfilter.org/iptables/tree/iptables/nft-shared.c?id=f5cf76626d95d2c491a80288bccc160c53b44e88#n420

It's effectively "read from /proc/net/ip_tables_names and if it's non-empty, print the warning", so we can do better than grepping the output of the tool and can instead do a simple [ -s /proc/net/ip_tables_names ] (although we probably ought to do all three of those "legacy" files the upstream code references, for good measure).

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, interesting! Any idea in what situations/use cases a fresh network namespace might have legacy tables set up in it already?

Did some more testing and figured this out -- it turns out that on CentOS 7, /proc/net/ip_tables_names is not empty in a brand new network namespace, even though there aren't any actual rules. What's even funnier is that I was confused by that upstream code reading from the files and then throwing away everything it reads, but it turns out that's done because these special kernel files have contents, but if you stat them their size is 0 bytes. 🙃

8637e230c491:/# test -s /proc/net/ip_tables_names; echo $?
1
8637e230c491:/# stat /proc/net/ip_tables_names
  File: /proc/net/ip_tables_names
  Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
Device: 29h/41d	Inode: 4026532469  Links: 1
Access: (0440/-r--r-----)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2024-01-04 18:43:59.112212543 +0000
Modify: 2024-01-04 18:43:59.112212543 +0000
Change: 2024-01-04 18:43:59.112212543 +0000
8637e230c491:/# cat /proc/net/ip_tables_names
nat
mangle
security
raw
filter

(so my -s test isn't sufficient 😞)

fi
if [ -n "$iptablesLegacy" ]; then
# see https://github.com/docker-library/docker/issues/463 (and the dind Dockerfile where this directory is set up)
export PATH="/usr/local/sbin/.iptables-legacy:$PATH"
fi
iptables --version # so users can see whether it's legacy or not

uid="$(id -u)"
if [ "$uid" != '0' ]; then
Expand Down