-
Notifications
You must be signed in to change notification settings - Fork 678
Reload router iptables rules if they get cleared #3802
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Manual testing:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Few new thoughts below.
@bboreham PTAL at the last 2 commits when you have a moment! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good but a few nits in how things are described.
- Instead of EnsureBridge - Adds ipset.ExistEntry method - Refactor ipset method names Exist <-> EntryExists - Turn on iptable-refresh by default (10s) - Adds a test that checks iptables are refreshed, check EXPOSE recovers
- Should always be on. Simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; one nit about a comment / function name but we can fix that another day if you don't have time now.
- Makes it all a bit more explicit
Adds a
CANARY
iptable chain and reloads all the weavenet iptable chains if theCANARY
is removed by something (usuallyfirewalld
).Partially addresses #3586
Details
ConfigureIPTables
and tries to make it safe to call again and again.ResetIPTables
and are called at launch so iptable state should remain mostly the same as prior to this PR.