Changed atomic_replace to atomic_move, now ti DOES move atomically in the last

step
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>

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):
+    def atomic_move(self, src, dest):
-        '''atomically replace dest with src, copying attributes from 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):
         '''

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):
     """

library/copy

@@ -19,7 +19,6 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
 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)
+            if not module.atomic_move(src, dest):
-        except shutil.Error:
+                try:
-            module.fail_json(msg="failed to copy: %s and %s are the same" % (src, dest))
+                    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

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_move(tmpfile, cron_file)
-            module.atomic_replace(tmpfile, cron_file)
-        except (OSError, IOError), e:
-            return (1, "", str(e))   
-        except:
-            return (1, "", str(e))
-        else:
-            return (0, "", "")
-
     else:
         cmd = "crontab %s %s" % (user, tmpfile)
         return module.run_command(cmd)

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

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