From 2590df6df1e3e4317f3247185be2940d95bd2c7b Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 2 Jun 2015 11:41:30 -0400 Subject: [PATCH 1/2] created makedirs_safe function for use in cases of multiprocess should fix #11126 and most race conditions --- lib/ansible/plugins/action/fetch.py | 4 ++-- lib/ansible/plugins/connections/paramiko_ssh.py | 7 +++---- lib/ansible/plugins/connections/winrm.py | 7 +++---- lib/ansible/plugins/lookup/password.py | 10 +++++----- lib/ansible/utils/path.py | 10 ++++++++++ 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index c242c8739d0..6a903ae5a27 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -29,6 +29,7 @@ from ansible.errors import * from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean from ansible.utils.hashing import checksum, checksum_s, md5, secure_hash +from ansible.utils.path import makedirs_safe class ActionModule(ActionBase): @@ -125,8 +126,7 @@ class ActionModule(ActionBase): if remote_checksum != local_checksum: # create the containing directories, if needed - if not os.path.isdir(os.path.dirname(dest)): - os.makedirs(os.path.dirname(dest)) + makedirs_safe(os.path.dirname(dest)) # fetch the file and check for changes if remote_data is None: diff --git a/lib/ansible/plugins/connections/paramiko_ssh.py b/lib/ansible/plugins/connections/paramiko_ssh.py index 797eeea9e02..0d7a82c34b5 100644 --- a/lib/ansible/plugins/connections/paramiko_ssh.py +++ b/lib/ansible/plugins/connections/paramiko_ssh.py @@ -42,6 +42,7 @@ from binascii import hexlify from ansible import constants as C from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNotFound from ansible.plugins.connections import ConnectionBase +from ansible.utils.path import makedirs_safe AUTHENTICITY_MSG=""" paramiko: The authenticity of host '%s' can't be established. @@ -309,8 +310,7 @@ class Connection(ConnectionBase): return False path = os.path.expanduser("~/.ssh") - if not os.path.exists(path): - os.makedirs(path) + makedirs_safe(path) f = open(filename, 'w') @@ -347,8 +347,7 @@ class Connection(ConnectionBase): # add any new SSH host keys -- warning -- this could be slow lockfile = self.keyfile.replace("known_hosts",".known_hosts.lock") dirname = os.path.dirname(self.keyfile) - if not os.path.exists(dirname): - os.makedirs(dirname) + makedirs_safe(dirname) KEY_LOCK = open(lockfile, 'w') fcntl.lockf(KEY_LOCK, fcntl.LOCK_EX) diff --git a/lib/ansible/plugins/connections/winrm.py b/lib/ansible/plugins/connections/winrm.py index 8a42da2534b..dbdf7cd6789 100644 --- a/lib/ansible/plugins/connections/winrm.py +++ b/lib/ansible/plugins/connections/winrm.py @@ -44,6 +44,7 @@ from ansible import constants as C from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNotFound from ansible.plugins.connections import ConnectionBase from ansible.plugins import shell_loader +from ansible.utils import makedirs_safe class Connection(ConnectionBase): '''WinRM connections over HTTP/HTTPS.''' @@ -213,8 +214,7 @@ class Connection(ConnectionBase): out_path = out_path.replace('\\', '/') self._display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self._connection_info.remote_addr) buffer_size = 2**19 # 0.5MB chunks - if not os.path.exists(os.path.dirname(out_path)): - os.makedirs(os.path.dirname(out_path)) + makedirs_safe(os.path.dirname(out_path)) out_file = None try: offset = 0 @@ -251,8 +251,7 @@ class Connection(ConnectionBase): else: data = base64.b64decode(result.std_out.strip()) if data is None: - if not os.path.exists(out_path): - os.makedirs(out_path) + makedirs_safe(out_path) break else: if not out_file: diff --git a/lib/ansible/plugins/lookup/password.py b/lib/ansible/plugins/lookup/password.py index 2e7633a067a..9506274e5f8 100644 --- a/lib/ansible/plugins/lookup/password.py +++ b/lib/ansible/plugins/lookup/password.py @@ -30,6 +30,7 @@ from ansible import constants as C from ansible.errors import AnsibleError from ansible.plugins.lookup import LookupBase from ansible.utils.encrypt import do_encrypt +from ansible.utils import makedirs_safe DEFAULT_LENGTH = 20 @@ -98,11 +99,10 @@ class LookupModule(LookupBase): path = self._loader.path_dwim(relpath) if not os.path.exists(path): pathdir = os.path.dirname(path) - if not os.path.isdir(pathdir): - try: - os.makedirs(pathdir, mode=0o700) - except OSError as e: - raise AnsibleError("cannot create the path for the password lookup: %s (error was %s)" % (pathdir, str(e))) + try: + makedirs_safe(pathdir, mode=0o700) + except OSError as e: + raise AnsibleError("cannot create the path for the password lookup: %s (error was %s)" % (pathdir, str(e))) chars = "".join([getattr(string,c,c) for c in use_chars]).replace('"','').replace("'",'') password = ''.join(random.choice(chars) for _ in range(length)) diff --git a/lib/ansible/utils/path.py b/lib/ansible/utils/path.py index e49a2f7d553..534226984be 100644 --- a/lib/ansible/utils/path.py +++ b/lib/ansible/utils/path.py @@ -19,6 +19,7 @@ __metaclass__ = type import os import stat +from time import sleep __all__ = ['is_executable', 'unfrackpath'] @@ -35,3 +36,12 @@ def unfrackpath(path): ''' return os.path.normpath(os.path.realpath(os.path.expandvars(os.path.expanduser(path)))) +def makedirs_safe(path, mode=None): + '''Safe way to create dirs in muliprocess/thread environments''' + while not os.path.exists(path): + try: + os.makedirs(path, mode) + except OSError, e: + if e.errno != 17: + raise + sleep(1) From ba02e5e3bf7d03a8c64713cebb5f851b2f5396ce Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 2 Jun 2015 13:01:02 -0400 Subject: [PATCH 2/2] minor adjustments as per code review --- lib/ansible/utils/path.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ansible/utils/path.py b/lib/ansible/utils/path.py index 534226984be..ac5160402b2 100644 --- a/lib/ansible/utils/path.py +++ b/lib/ansible/utils/path.py @@ -20,6 +20,7 @@ __metaclass__ = type import os import stat from time import sleep +from errno import EEXIST __all__ = ['is_executable', 'unfrackpath'] @@ -38,10 +39,9 @@ def unfrackpath(path): def makedirs_safe(path, mode=None): '''Safe way to create dirs in muliprocess/thread environments''' - while not os.path.exists(path): + if not os.path.exists(path): try: os.makedirs(path, mode) except OSError, e: - if e.errno != 17: + if e.errno != EEXIST: raise - sleep(1)