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

Add --random-fully to MASQ iptables rules to mitigate conntrack issues #958

Merged
merged 4 commits into from
Aug 4, 2020

Conversation

coufalja
Copy link
Contributor

@coufalja coufalja changed the title Random all Add --random-fully to MASQ iptables rules to mitigate conntrack issues Jul 28, 2020
@coufalja
Copy link
Contributor Author

coufalja commented Jul 28, 2020

Docker image for testing purposes: wanderadock/kube-router:amd64-random-all

@murali-reddy
Copy link
Member

@coufalja thanks for the PR. --random-fully option make sense. Changes look good but on upgrade we would end up with two rules. Old rule plus new rule with --random-fully option set. Are you expecting users to flush the iptables chains?

Gopkg.lock Show resolved Hide resolved
@coufalja
Copy link
Contributor Author

@murali-reddy The way I deployed was going through full rolling-update of a cluster so I didn't hit the issue. But yeah It would be great to actually allow users to update in place. What needs to be done? Adding the old one to pkg/controllers/routing/pod_egress.go:85 and pkg/controllers/proxy/network_services_controller.go:1268 ?

@coufalja coufalja requested a review from mrueg July 29, 2020 19:40
Copy link
Collaborator

@aauren aauren left a comment

Choose a reason for hiding this comment

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

This LGTM. Thanks for all of your work on this!

@murali-reddy I tested this locally on our nodes with both iptables that are able to support --random-full and iptables that are not. I also tested the upgrade procedure and found that it cleaned the nodes appropriately when upgrading from iptables that didn't support it to iptables that did support it.

@aauren aauren merged commit e35dc9d into cloudnativelabs:master Aug 4, 2020
@coufalja
Copy link
Contributor Author

coufalja commented Aug 5, 2020

@aauren Thanks, just a question when I can expect this to be released? in the 1.1 or you plan to release 1.0.2 with a round of minor fixes?

@coufalja coufalja deleted the random-all branch August 5, 2020 06:05
@mrueg
Copy link
Collaborator

mrueg commented Aug 5, 2020

This would be an additional feature, so it will be in 1.1

@murali-reddy
Copy link
Member

I tested this locally on our nodes with both iptables that are able to support --random-full and iptables that are not. I also tested the upgrade procedure and found that it cleaned the nodes appropriately when upgrading from iptables that didn't support it to iptables that did support it.

thanks @aauren

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.

4 participants