Mount /run/xtables.lock to prevent unwanted race conditions. #1186
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
…grams are manipulating iptables at the same time.
|
Thanks @kgtw for the fix. This will help closing some of the issues. @mogren or @jayanthvn we might have to backport this to 1.7 release branch. |
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(s) this PR fixes:
#989 (comment)
What this PR does / why we need it:
As initially mentioned in #989 (comment) we should mount
/run/xtables.lockto prevent race conditions where multiple programs are manipulating iptables at the same time.The
--waiton iptables commands isn't actually respected unless we mount the iptables lock file in to the container.The downside of this is that it introduces a blocking action where the cni will wait until the lock is released.
I'm not sure how the kubernetes manifests under
v1.xare generated, can copy by hand if thats the case..If issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
Testing:
Running
flock -e /run/xtables.lock sleep 180on the host before starting the container you will observe that the cni will wait 3 minutes until the lock is released. In theory this could be indefinitely according to the iptables manual.Trying to replicate the exact bug I mentioned in the comment is extremely difficult because you have to be updating iptables at the exact same time as the cni is. We have only seen this bug observed in our production environments a handful of times so far.
Automation added to e2e:
n/a
Will this break upgrades and downgrades. Has it been tested?:
Will not break upgrades/downgrades.
Does this require config only upgrades?:
yes?
Does this PR introduce a user-facing change?:
yes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.