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

Mount /run/xtables.lock to prevent unwanted race conditions. #1186

Merged
merged 1 commit into from Sep 1, 2020

Conversation

kgtw
Copy link
Contributor

@kgtw kgtw commented Sep 1, 2020

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.lock to prevent race conditions where multiple programs are manipulating iptables at the same time.

The --wait on 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.x are 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 180 on 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

Users are recommended to mount `/run/xtables.lock` to prevent unwanted race conditions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…grams are manipulating iptables at the same time.
@anguslees anguslees self-requested a review September 1, 2020 23:23
Copy link
Contributor

@anguslees anguslees left a comment

Choose a reason for hiding this comment

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

(Thanks!)

@anguslees anguslees added the bug label Sep 1, 2020
@SaranBalaji90 SaranBalaji90 merged commit 8fffd38 into aws:master Sep 1, 2020
@SaranBalaji90
Copy link
Contributor

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants