node: Don't skip masquerading for External node IPs #18483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pchaigno
added
kind/bug
I think we should document this subtle difference in the masquerading.rst. |
Commit 49cb220 ("iptables: Don't masquerade traffic to cluster nodes") introduced ipsets in our iptables rules, to skip masquerading traffic to cluster nodes when iptables-based masquerading is used. BPF-based masquerading already implemented this logic. In practice, traffic to both Internal and External node IPs would skip masquerading. However, Kubernetes doesn't state that External Node IPs are routable within the cluster. Masquerading may thus be required for External Node IPs, as reported by several users. Hence, this commit changes the logic to only skip masquerading in iptables for Internal node IPs. This commit doesn't affect the BPF-based masquerading logic, as unfortunately we can't easily distinguish between Internal and External node IPs in our datapath today (both are identified as host or remote-node). Fixes: 49cb220 ("iptables: Don't masquerade traffic to cluster nodes") Signed-off-by: Paul Chaignon <paul@cilium.io>
This commit documents the difference in handling traffic to External IPs of cluster nodes between the BPF-based and iptables-based implementations of masquerading in Cilium. Suggested-by: Martynas Pumputis <m@lambda.lt> Signed-off-by: Paul Chaignon <paul@cilium.io>
@brb 👍 Done in the second commit. Please have a look. |
pchaigno
force-pushed
the
fix-ipt-masq-external-node-ips
branch
from
28a40dc
to
d3304c0
Compare
qmonnet
approved these changes
Jan 17, 2022
|
Several tests failed with known flakes:
Reviews are in, except for the agent team, but I'm not sure why that team was requested in the first place. Marking ready to merge. |
pchaigno
added
the
ready-to-merge
This was referenced Jan 21, 2022
glibsm
added
backport-done/1.11
joestringer
moved this from Needs backport from master
to Backport done to v1.11
in 1.11.2
zuzzas
added a commit
to deckhouse/deckhouse
that referenced
this pull request
Mar 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commits for details. The first has the fix, the second documents the different between our two implementations.
Fixes: #16603.
Related: #17177.