Refactor IPTable Rules #2697
Merged
Refactor IPTable Rules #2697
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
jdn5126
reviewed
Dec 1, 2023
jchen6585
force-pushed
the
iptable-refactor
branch
6 times, most recently
from
45b54e0
to
1e181a3
Compare
jdn5126
previously approved these changes
Dec 4, 2023
jchen6585
force-pushed
the
iptable-refactor
branch
2 times, most recently
from
c72b7e4
to
26edd44
Compare
jchen6585
commented
Dec 5, 2023
jdn5126
reviewed
Dec 5, 2023
jchen6585
force-pushed
the
iptable-refactor
branch
4 times, most recently
from
700c562
to
dc524ce
Compare
|
This will lead to non-zero chains being in iptables after upgrades even though we are not using them. I think it is better to delete the AWS chains during nodeInit and we anyway rebuild the chains so should be safe... Let's discuss it offline. |
jchen6585
force-pushed
the
iptable-refactor
branch
3 times, most recently
from
803671e
to
929c112
Compare
jdn5126
reviewed
Dec 11, 2023
jchen6585
force-pushed
the
iptable-refactor
branch
from
0bfe8a3
to
4a18ecc
Compare
jdn5126
reviewed
Dec 12, 2023
jchen6585
force-pushed
the
iptable-refactor
branch
from
4a18ecc
to
99d2b15
Compare
jdn5126
previously approved these changes
Dec 12, 2023
jayanthvn
reviewed
Dec 12, 2023
jchen6585
force-pushed
the
iptable-refactor
branch
from
99d2b15
to
871ef9f
Compare
jchen6585
force-pushed
the
iptable-refactor
branch
from
871ef9f
to
f52f1a5
Compare
Merged
jdn5126
added a commit
to jdn5126/amazon-vpc-cni-k8s
that referenced
this pull request
Dec 21, 2023
jdn5126
added a commit
that referenced
this pull request
Dec 21, 2023
jchen6585
pushed a commit
to jchen6585/amazon-vpc-cni-k8s
that referenced
this pull request
Jan 5, 2024
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.
What type of PR is this?
bug
Which issue does this PR fix:
#2373
What does this PR do / Why do we need it:
nf_tables mode restricts the jump stack size to at most 16. We currently have (one jump per VPC CIDR/Excluded CIDR) + 1, so having 15+ CIDRS will result in a jump stack size greater than 16, causing failure. This pr refactors our iptable rules to retain current behavior while keeping the jump stack size to a size of 2.
If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
Testing done on this change:
Manually checked iptable rules for 16 CIDRS, aws-node is up and running
Manually tested adding exclusion cidrs using
AWS_VPC_K8S_CNI_EXCLUDE_SNAT_CIDRSAlso manually checked up to VPC Limit (50 CIDRS).
Manually checked packet using tcpdump
Integration Tests:
Ensured traffic to the internet aren't dropped while rules are being updated (deleted and added 10+ cidrs while ping in a pod was running)
Scale Testing
130 Pod
730 pod
5000 pod
Will this PR introduce any new dependencies?:
No
Will this break upgrades or downgrades? Has updating a running cluster been tested?:
No, Yes
Does this change require updates to the CNI daemonset config files to work?:
No
Does this PR introduce any user-facing change?:
No
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.