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

debian has iptables-legacy and iptables-nft now #2285

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

myobie
Copy link
Contributor

@myobie myobie commented Oct 31, 2018

🎉

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "iptables-legacy" git@github.com:myobie/libnetwork.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@jessfraz
Copy link

cc @thaJeztah I had this problem on my computer and this worked for me, debian started using iptables-legacy and iptables returns an error..... so I aliased iptables to iptables-legacy to start docker.

Signed-off-by: Nathan Herald <me@nathanherald.com>
@thaJeztah
Copy link
Member

@jessfraz oh! thanks for the ping; looks like this is related to moby/moby#38099 ? (was looking at that one earlier this week, but hadn't time yet to reproduce it) 🤗

ping @fcrisciani ^^

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

unfortunate that this is needed for a specific distro 😕

@myobie
Copy link
Contributor Author

myobie commented Oct 31, 2018

@thaJeztah yes, moby/moby#38099 is related.

Copy link

@fcrisciani fcrisciani left a comment

Choose a reason for hiding this comment

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

LGTM

@fcrisciani fcrisciani merged commit 4962716 into moby:master Oct 31, 2018
@thaJeztah
Copy link
Member

Whoop! that went fast. Thank you so much for your contribution @myobie 🤗 🎉

@tianon
Copy link
Member

tianon commented Nov 1, 2018

I think this is probably just delaying the issue; see the latest link on #1998. Now both of iptables and firewalld prefer nft (and for good reason in my experience), so I imagine at the very least this legacy iptables kernel code is going to experience bitrot and possibly eventual removal, so it's probably time to start considering direct nft support more earnestly. 😅

@thaJeztah
Copy link
Member

Tracking issues for that; #1998 and moby/moby#26824

@thaJeztah
Copy link
Member

Oh, erm if nftables is the same as iptables-nft that is 😅 ☺️

@tianon
Copy link
Member

tianon commented Nov 1, 2018

Not exactly -- iptables-nft is an "iptables compatible wrapper for nftables" (so in other words, using nftables to reimplement the iptables functionality, which is way better). It's honestly likely a bug in that translator that's caused moby/moby#38099 but I wouldn't have any idea where to file that upstream. 😅

@ijc
Copy link

ijc commented Nov 14, 2018

This seems like a rather odd change to me.

Debian is now using their "alternatives" mechanism to manage /usr/sbin/iptables to point to whichever of iptable-legacy and iptables-nft is in active use on the system:

# update-alternatives --display iptables
iptables - auto mode
  link best version is /usr/sbin/iptables-nft
  link currently points to /usr/sbin/iptables-nft
  link iptables is /usr/sbin/iptables
  slave iptables-restore is /usr/sbin/iptables-restore
  slave iptables-save is /usr/sbin/iptables-save
/usr/sbin/iptables-legacy - priority 10
  slave iptables-restore: /usr/sbin/iptables-legacy-restore
  slave iptables-save: /usr/sbin/iptables-legacy-save
/usr/sbin/iptables-nft - priority 20
  slave iptables-restore: /usr/sbin/iptables-nft-restore
  slave iptables-save: /usr/sbin/iptables-nft-save
# readlink  /usr/sbin/iptables
/etc/alternatives/iptables
# readlink  /etc/alternatives/iptables
/usr/sbin/iptables-nft
# update-alternatives --config iptables
There are 2 choices for the alternative iptables (providing /usr/sbin/iptables).

  Selection    Path                       Priority   Status
------------------------------------------------------------
* 0            /usr/sbin/iptables-nft      20        auto mode
  1            /usr/sbin/iptables-legacy   10        manual mode
  2            /usr/sbin/iptables-nft      20        manual mode

Press <enter> to keep the current choice[*], or type selection number: 1
update-alternatives: using /usr/sbin/iptables-legacy to provide /usr/sbin/iptables (iptables) in manual mode
# readlink /usr/sbin/iptables
/etc/alternatives/iptables
# readlink /etc/alternatives/iptables
/usr/sbin/iptables-legacy

Going around that and calling into iptable-legacy directly seems odd and will do the wrong thing when iptables → iptables-nft (and is unnecessary when iptables → iptables-legacy).

Some here in this thread have mentioned errors when running iptables I noticed that the package changelog has an entry under version 1.8.1-1 which reads:

  * [85fe398] d/iptables.postinst: fix typo in update-alternative command.

Noone has mentioned any versions anywhere but perhaps these were just during that window of brokenness?

@ijc
Copy link

ijc commented Nov 14, 2018

IOW moby/moby#38099 (comment) was correct in their recommendation.

@TobiX
Copy link

TobiX commented Mar 4, 2019

FWIW, this broke Docker network connectivity on my Debian unstable system, since everything else is using iptables-nft and the docker rules don't integrate with my firewall setup anymore... "Forcing" docker to use iptables-nft (by just deleting the iptables-legacy binary) restored network functionality.

@myobie
Copy link
Contributor Author

myobie commented Mar 5, 2019

@TobiX this PR has been reverted in #2343 after an iptables bug was fixed. Can you try master?

@nurupo
Copy link

nurupo commented Mar 30, 2019

Yep, it has been reverted and is fixed now, so just update. I had Docker 18.09.3 that was broken, 18.09.4 is fixed.

@TobiX
Copy link

TobiX commented Apr 17, 2019

@myobie Sorry for the late reply: Yeah, it's fixed now, thanks!

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.

None yet

9 participants