Jump to DOCKER-INGRESS from DOCKER-FORWARD #49538
Conversation
robmry
added
area/networking
area/swarm
kind/bugfix
A jump to DOCKER-INGRESS chain is only created when Swarm needs it. That's always after jumps to DOCKER-USER and DOCKER-FORWARD have been inserted at the top of the FORWARD chain. The DOCKER-INGRESS rule needs to be between those two other jumps. Placing the jump to DOCKER-INGRESS at the top of the DOCKER-FORWARD chain puts it in the right place, without needing to shuffle any other rules around when it's added. Signed-off-by: Rob Murray <rob.murray@docker.com>
robmry
force-pushed
the
docker_ingress
branch
from
bd6fe0d to
558da63
Compare
| // the daemon older than 28.0.1. | ||
| // FIXME(robmry) - should only do this once, on startup. | ||
| if iptable.Exists(iptables.Filter, "FORWARD", "-j", ingressChain) { |
There was a problem hiding this comment.
VERY unlikely to be relevant in this spot and no need to update 👈 , but Go (GoDoc) has some extra semantics for "special" comments (most notably Deprecated: but ISTR also to some extent TODO / FIXME comments); to prevent such comments from being considered part of the comment itself, they expect there to be an empty (comment-)line before them, and for them to be last in the comment block, e.g.;
| // the daemon older than 28.0.1. | |
| // FIXME(robmry) - should only do this once, on startup. | |
| if iptable.Exists(iptables.Filter, "FORWARD", "-j", ingressChain) { | |
| // the daemon older than 28.0.1. | |
| // | |
| // FIXME(robmry) - should only do this once, on startup. | |
| if iptable.Exists(iptables.Filter, "FORWARD", "-j", ingressChain) { |
In this case it unlikely matters as it's a comment internal to the function (which Go itself doesn't really bother with), but I try to stick to that convention when writing these, also to make sure IDEs can spot them (which tend to try to follow Go conventions, although usually are slightly more permissive).
There was a problem hiding this comment.
Oh, thank you ... will do for next time.
I should have found somewhere better to put that delete, but this seemed safest right now - it mustn't happen on a live-restore (if swarm supports that), and I didn't want to miss any other special case. It can be tidied up as we shift over to nftables.
- What I did
In 28.0.0, the iptables jump to
DOCKER-INGRESScould get out of place, fixed that.Introduced in commit 0ef2b24 - the intention was that jumps to
DOCKER-USER,DOCKER-ISOLATION-STAGE-1andDOCKER-INGRESSno longer needed to be deleted and re-created at the top of theFORWARDchain each time a new network was added. That works for the other chains, because on daemon restart they're re-created. However, the jump toDOCKER-INGRESSis not re-created (shuffled back to the top of the table) if theDOCKER-INGRESSchain already existed. So, it ended up in the wrong place in theFORWARDchain. It worked in 27.x, becausearrangeIngressFilterRulewas called when the ingress network was re-created on startup.- How I did it
A jump to DOCKER-INGRESS chain is only created when Swarm needs it. That's always after jumps to DOCKER-USER and DOCKER-FORWARD have been inserted at the top of the FORWARD chain. The DOCKER-INGRESS rule needs to be between those two other jumps.
Placing the jump to DOCKER-INGRESS at the top of the DOCKER-FORWARD chain puts it in the right place, without needing to shuffle any other rules around when it's added.
- How to verify it
With 27.5.1, created a swarm service with a published port (
docker service create -p 8080:80 nginx), checked that the jump toDOCKER-INGRESSfollowed the jump toDOCKER-USERin theFORWARDchain. Stopped the daemon and started one built with this change - waited until the swarm service restarted, checked that the jump toDOCKER-INGRESShad been removed from theFORWARDchain and added to the top ofDOCKER-FORWARD.I'll follow up with a regression test, but that doesn't need to block 28.0.1.
- Human readable description for the release notes