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

Fix multi process kubeconfig access support #2380

Merged
merged 1 commit into from
Jun 29, 2020

Conversation

michaelbeaumont
Copy link
Contributor

@michaelbeaumont michaelbeaumont commented Jun 29, 2020

Description

Multi process support was broken on every platform, just even worse on windows.

The problem on Windows was that by locking the kubeconfig file itself, client-go/tools/clientmd.ModifyConfig is unable to write to the file.
The intended use of flock is to create a lock file to protect some critical section.
This is also what client-go does internally (hence the .eksctl prefix for the lock file).

Additionally, we must use the the same flock.Flock to lock and unlock the file. Doing otherwise leads to deadlocks and concurrent writes.

This wasn't caught because the test that was written with the PR didn't actually test anything, it just made a bunch of calls to os.Write, not kubeconfig.Write.

Fixes #2376

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Added labels for change area (e.g. area/nodegroup), target version (e.g. version/0.12.0) and kind (e.g. kind/improvement)
  • Make sure the title of the PR is a good description that can go into the release notes

Copy link
Contributor

@cPu1 cPu1 left a comment

Choose a reason for hiding this comment

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

Looks like the library was also being used incorrectly. Instead of unlocking the existing file handle, it was creating a new Flock instance and trying to unlock it (which is a no-op).

@michaelbeaumont
Copy link
Contributor Author

michaelbeaumont commented Jun 29, 2020

Looks like the library was also being used incorrectly. Instead of unlocking the existing file handle, it was creating a new Flock instance and trying to unlock it (which is a no-op).

Yes exactly, that was broken on every platform

@michaelbeaumont michaelbeaumont merged commit c7aee59 into eksctl-io:master Jun 29, 2020
@michaelbeaumont michaelbeaumont deleted the fix_cfg_lock_win branch June 29, 2020 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to write kubeconfig
2 participants