diff --git a/lib/ansible/module_common.py b/lib/ansible/module_common.py index 9d5efdef7f..a23b326967 100644 --- a/lib/ansible/module_common.py +++ b/lib/ansible/module_common.py @@ -807,8 +807,9 @@ class AnsibleModule(object): self.fail_json(msg='Could not make backup of %s to %s: %s' % (fn, backupdest, e)) return backupdest - def atomic_replace(self, src, dest): - '''atomically replace dest with src, copying attributes from dest''' + def atomic_move(self, src, dest): + '''atomically move src to dest, copying attributes from dest, returns true on success''' + rc = False if os.path.exists(dest): st = os.stat(dest) os.chmod(src, st.st_mode & 07777) @@ -824,7 +825,25 @@ class AnsibleModule(object): if self.selinux_enabled(): context = self.selinux_default_context(dest) self.set_context_if_different(src, context, False) - os.rename(src, dest) + # Ensure file is on same partition to make replacement atomic + dest_dir = os.path.dirname(dest) + dest_file = os.path.basename(dest) + tmp_dest = "%s/.%s.%s.%s" % (dest_dir,dest_file,os.getpid(),time.time()) + try: + shutil.move(src, tmp_dest) + os.rename(tmp_dest, dest) + rc = True + except (shutil.Error, OSError, IOError), e: + self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, e)) + finally: + # Clean up in case of failure (don't leave nasty temps around) + if os.path.exists(tmp_dest): + try: + #TODO: would be nice to respect 'keep_remote_files' + os.unlink(tmp_dest) + except OSError, e: + sys.stderr.write("could not cleanup %s: %s" % (tmp_dest, e)) + return rc def run_command(self, args, check_rc=False, close_fds=False, executable=None, data=None): ''' diff --git a/library/authorized_key b/library/authorized_key index e56912727e..0f9f4a6c89 100644 --- a/library/authorized_key +++ b/library/authorized_key @@ -162,7 +162,7 @@ def writekeys(module, filename, keys): except IOError, e: module.fail_json(msg="Failed to write to file %s: %s" % (tmp_path, str(e))) f.close() - module.atomic_replace(tmp_path, filename) + module.atomic_move(tmp_path, filename) def enforce_state(module, params): """ diff --git a/library/copy b/library/copy index 03578dbec3..26c05f7d7c 100644 --- a/library/copy +++ b/library/copy @@ -19,7 +19,6 @@ # along with Ansible. If not, see . import os -import shutil import time DOCUMENTATION = ''' @@ -144,17 +143,15 @@ def main(): if os.path.islink(dest): os.unlink(dest) open(dest, 'w').close() - # TODO:pid + epoch should avoid most collisions, hostname/mac for those using nfs? - # might be an issue with exceeding path length - dest_tmp = "%s.%s.%s.tmp" % (dest,os.getpid(),time.time()) - shutil.copyfile(src, dest_tmp) if validate: - (rc,out,err) = module.run_command(validate % dest_tmp) + (rc,out,err) = module.run_command(validate % src) if rc != 0: module.fail_json(msg="failed to validate: rc:%s error:%s" % (rc,err)) - module.atomic_replace(dest_tmp, dest) - except shutil.Error: - module.fail_json(msg="failed to copy: %s and %s are the same" % (src, dest)) + if not module.atomic_move(src, dest): + try: + os.unlink(src) # cleanup tmp files on failure + except OSError, e: + sys.stderr.write("failed to clean up tmp file %s: %s\n" % (src, e)) except IOError: module.fail_json(msg="failed to copy: %s to %s" % (src, dest)) changed = True diff --git a/library/cron b/library/cron index 73eca78903..9baa660118 100644 --- a/library/cron +++ b/library/cron @@ -145,15 +145,7 @@ def get_jobs_file(module, user, tmpfile, cron_file): def install_jobs(module, user, tmpfile, cron_file): if cron_file: cron_file = '/etc/cron.d/%s' % cron_file - try: - module.atomic_replace(tmpfile, cron_file) - except (OSError, IOError), e: - return (1, "", str(e)) - except: - return (1, "", str(e)) - else: - return (0, "", "") - + module.atomic_move(tmpfile, cron_file) else: cmd = "crontab %s %s" % (user, tmpfile) return module.run_command(cmd) diff --git a/library/service b/library/service index e972c35e81..be75728b2c 100644 --- a/library/service +++ b/library/service @@ -334,7 +334,7 @@ class Service(object): os.close(TMP_RCCONF) # Replace previous rc.conf. - self.module.atomic_replace(tmp_rcconf_file, self.rcconf_file) + self.module.atomic_move(tmp_rcconf_file, self.rcconf_file) # =========================================== # Subclass: Linux diff --git a/library/sysctl b/library/sysctl index c7f3af7128..874453ae04 100644 --- a/library/sysctl +++ b/library/sysctl @@ -110,10 +110,10 @@ def write_sysctl(module, lines, **sysctl_args): module.fail_json(msg="Failed to write to file %s: %s" % (tmp_path, str(e))) f.flush() f.close() - + # replace the real one - module.atomic_replace(tmp_path, sysctl_args['sysctl_file']) - + module.atomic_move(tmp_path, sysctl_args['sysctl_file']) + # end return sysctl_args