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

Gracefully handle race condition of 'makedirs' #21409

Merged
merged 1 commit into from Mar 9, 2015
Merged

Gracefully handle race condition of 'makedirs' #21409

merged 1 commit into from Mar 9, 2015

Conversation

jquast
Copy link
Contributor

@jquast jquast commented Mar 8, 2015

Problem

In the imperative sequence:

   if not os.path.isdir(folder_name):
       os.makedirs(folder_name)

There exists the possibility that another process may create folder_name between the system path test and the system folder construction. This is especially true when launching both the salt-minion and salt-master simultaneously:

# systemctl status -l salt-minion salt-master
* salt-minion.service - The Salt Minion
   Loaded: loaded (/usr/lib/systemd/system/salt-minion.service; enabled)
   Active: inactive (dead) since Sat 2015-03-07 21:38:53 UTC; 7h ago
  Process: 1168 ExecStart=/usr/bin/salt-minion (code=exited, status=0/SUCCESS)
 Main PID: 1168 (code=exited, status=0/SUCCESS)

Mar 07 21:36:29 local.dev systemd[1]: Started The Salt Minion.
Mar 07 21:38:53 local.dev salt-minion[1168]: [ERROR   ] Attempt to authenticate with the salt master failed

* salt-master.service - The Salt Master Server
   Loaded: loaded (/usr/lib/systemd/system/salt-master.service; enabled)
   Active: failed (Result: exit-code) since Sat 2015-03-07 21:37:53 UTC; 7h ago
  Process: 1166 ExecStart=/usr/bin/salt-master (code=exited, status=17)
 Main PID: 1166 (code=exited, status=17)

Mar 07 21:37:52 local.dev salt-master[1166]: Failed to create path "/var/log/salt/master" - [Errno 17] File exists: '/var/log/salt'
Mar 07 21:37:53 local.dev systemd[1]: salt-master.service: main process exited, code=exited, status=17/n/a
Mar 07 21:37:53 local.dev systemd[1]: Failed to start The Salt Master Server.
Mar 07 21:37:53 local.dev systemd[1]: Unit salt-master.service entered failed state.

In such cases, the salt-minion tested for the non-existence (true), salt-master also tested (true), salt-minion created the folder, and then so salt-master failed with Errno 17: File exists.

Solution

Do not test for path existence at all, always create, expecting errno.EEXISTS as an acceptable condition. This delegates responsibility of handling such "race conditions" to the kernel where locks exist to prevent it.

Problem
-------

In the imperative sequence::

   if not os.path.isdir(folder_name):
       os.makedirs(folder_name)

There exists the possibility that another process may create ``folder_name``
between the system path test and the system folder construction.  This is
especially true when launching both the salt-minion and salt-master
simultaneously::

    # systemctl status -l salt-minion salt-master
    * salt-minion.service - The Salt Minion
       Loaded: loaded (/usr/lib/systemd/system/salt-minion.service; enabled)
       Active: inactive (dead) since Sat 2015-03-07 21:38:53 UTC; 7h ago
      Process: 1168 ExecStart=/usr/bin/salt-minion (code=exited, status=0/SUCCESS)
     Main PID: 1168 (code=exited, status=0/SUCCESS)

    Mar 07 21:36:29 local.dev systemd[1]: Started The Salt Minion.
    Mar 07 21:38:53 local.dev salt-minion[1168]: [ERROR   ] Attempt to authenticate with the salt master failed

    * salt-master.service - The Salt Master Server
       Loaded: loaded (/usr/lib/systemd/system/salt-master.service; enabled)
       Active: failed (Result: exit-code) since Sat 2015-03-07 21:37:53 UTC; 7h ago
      Process: 1166 ExecStart=/usr/bin/salt-master (code=exited, status=17)
     Main PID: 1166 (code=exited, status=17)

    Mar 07 21:37:52 local.dev salt-master[1166]: Failed to create path "/var/log/salt/master" - [Errno 17] File exists: '/var/log/salt'
    Mar 07 21:37:53 local.dev systemd[1]: salt-master.service: main process exited, code=exited, status=17/n/a
    Mar 07 21:37:53 local.dev systemd[1]: Failed to start The Salt Master Server.
    Mar 07 21:37:53 local.dev systemd[1]: Unit salt-master.service entered failed state.

In such cases, the salt-minion tested for the non-existence (true), then
salt-master also tested (true).  salt-minion created the folder, but
salt-master failed (with Errno 17: File exists).

Solution
--------

Do not test for path existence at all, always create, expecting errno.EEXISTS
as an acceptable condition.  This delegates responsibility of handling such
"race conditions" to the kernel where locks exist to prevent it.
@jfindlay
Copy link
Contributor

jfindlay commented Mar 9, 2015

@jquast, excellent, thank you.

@jfindlay jfindlay added the bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch label Mar 9, 2015
@jfindlay
Copy link
Contributor

jfindlay commented Mar 9, 2015

@jquast, in the future, feel free to submit fixes directly to the oldest release branch, http://docs.saltstack.com/en/latest/topics/development/contributing.html#id1

thatch45 added a commit that referenced this pull request Mar 9, 2015
Gracefully handle race condition of 'makedirs'
@thatch45 thatch45 merged commit ab4d5b7 into saltstack:develop Mar 9, 2015
@rallytime rallytime added ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. and removed bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch labels Mar 9, 2015
thatch45 added a commit that referenced this pull request Mar 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants