Various performance streamlining and making the file features usable in all…

Various performance streamlining and making the file features usable in all modules without daisy chaining.

lib/ansible/module_common.py

@@ -50,6 +50,16 @@ import types
 import time
 import shutil
 import stat
+import stat
+import grp
+import pwd
+
+HAVE_SELINUX=False
+try:
+    import selinux
+    HAVE_SELINUX=True
+except ImportError:
+    pass
 
 try:
     from hashlib import md5 as _md5
@@ -63,11 +73,22 @@ except ImportError:
   import syslog
   has_journal = False
 
+FILE_COMMON_ARGUMENTS=dict(
+    src = dict(),
+    mode = dict(),
+    owner = dict(),
+    group = dict(),
+    seuser = dict(),
+    serole = dict(),
+    selevel = dict(),
+    setype = dict(),
+)
+
 class AnsibleModule(object):
 
     def __init__(self, argument_spec, bypass_checks=False, no_log=False,
         check_invalid_arguments=True, mutually_exclusive=None, required_together=None,
-        required_one_of=None):
+        required_one_of=None, add_file_common_args=True):
 
         '''
         common code for quickly building an ansible module in Python
@@ -76,12 +97,15 @@ class AnsibleModule(object):
         '''
 
         self.argument_spec = argument_spec
+
+        if add_file_common_args:
+            self.argument_spec.update(FILE_COMMON_ARGUMENTS)
+
         (self.params, self.args) = self._load_params()
 
         self._legal_inputs = []
         self._handle_aliases()
 
-        # this may be disabled where modules are going to daisy chain into others
         if check_invalid_arguments:
             self._check_invalid_arguments()
 
@@ -98,6 +122,256 @@ class AnsibleModule(object):
         if not no_log:
             self._log_invocation()
 
+    def load_file_common_arguments(self, params):
+        ''' 
+        many modules deal with files, this encapsulates common
+        options that the file module accepts such that it is directly
+        available to all modules and they can share code.
+        '''
+
+        path = params.get('path', params.get('dest', None))
+        if path is None:
+            return {}
+
+        mode   = params.get('mode', None)
+        owner  = params.get('owner', None)
+        group  = params.get('group', None)
+
+        # selinux related options
+        seuser    = params.get('seuser', None)
+        serole    = params.get('serole', None)
+        setype    = params.get('setype', None)
+        selevel   = params.get('serange', 's0')
+        secontext = [seuser, serole, setype]
+    
+        if self.selinux_mls_enabled():
+            secontext.append(selevel)
+
+        default_secontext = self.selinux_default_context(path)
+        for i in range(len(default_secontext)):
+            if i is not None and secontext[i] == '_default':
+                secontext[i] = default_secontext[i]
+
+        return dict(
+            path=path, mode=mode, owner=owner, group=group, 
+            seuser=seuser, serole=serole, setype=setype,
+            selevel=selevel, secontext=secontext, 
+        )
+
+
+    # Detect whether using selinux that is MLS-aware.
+    # While this means you can set the level/range with
+    # selinux.lsetfilecon(), it may or may not mean that you
+    # will get the selevel as part of the context returned
+    # by selinux.lgetfilecon().
+
+    def selinux_mls_enabled(self):
+        if not HAVE_SELINUX:
+            return False
+        if selinux.is_selinux_mls_enabled() == 1:
+            return True
+        else:
+            return False
+
+    def selinux_enabled(self):
+        if not HAVE_SELINUX:
+            return False
+        if selinux.is_selinux_enabled() == 1:
+            return True
+        else:
+            return False
+
+    # Determine whether we need a placeholder for selevel/mls
+    def selinux_initial_context(self):
+        context = [None, None, None]
+        if self.selinux_mls_enabled():
+            context.append(None)
+        return context
+
+    # If selinux fails to find a default, return an array of None
+    def selinux_default_context(self, path, mode=0):
+        context = self.selinux_initial_context()
+        if not HAVE_SELINUX or not self.selinux_enabled():
+            return context
+        try:
+            ret = selinux.matchpathcon(path, mode)
+        except OSError:
+            return context
+        if ret[0] == -1:
+            return context
+        context = ret[1].split(':')
+        return context
+
+    def selinux_context(self, path):
+        context = self.selinux_initial_context()
+        if not HAVE_SELINUX or not self.selinux_enabled():
+            return context
+        try:
+            ret = selinux.lgetfilecon(path)
+        except:
+            self.fail_json(path=path, msg='failed to retrieve selinux context')
+        if ret[0] == -1:
+            return context
+        context = ret[1].split(':')
+        return context
+
+    def user_and_group(self, filename):
+        st = os.stat(filename)
+        uid = st.st_uid
+        gid = st.st_gid
+        try:    
+            user = pwd.getpwuid(uid)[0]
+        except KeyError:
+            user = str(uid)
+        try:    
+            group = grp.getgrgid(gid)[0]
+        except KeyError:
+            group = str(gid)
+        return (user, group)
+
+    def set_context_if_different(self, path, context, changed):
+
+        if not HAVE_SELINUX or not self.selinux_enabled():
+            return changed 
+        cur_context = self.selinux_context(path)
+        new_context = list(cur_context)
+        # Iterate over the current context instead of the
+        # argument context, which may have selevel.
+
+        for i in range(len(cur_context)):
+            if context[i] is not None and context[i] != cur_context[i]:
+                new_context[i] = context[i]
+            if context[i] is None:
+                new_context[i] = cur_context[i]
+        if cur_context != new_context:
+            try:    
+                rc = selinux.lsetfilecon(path, ':'.join(new_context))
+            except OSError:
+                self.fail_json(path=path, msg='invalid selinux context', new_context=new_context, cur_context=cur_context, input_was=context)
+            if rc != 0:
+                self.fail_json(path=path, msg='set selinux context failed')
+            changed = True
+        return changed
+
+    def set_owner_if_different(self, path, owner, changed):
+        if owner is None:
+            return changed
+        user, group = self.user_and_group(path)
+        if owner != user:
+            try:
+                uid = pwd.getpwnam(owner).pw_uid
+            except KeyError:
+                self.fail_json(path=path, msg='chown failed: failed to look up user %s' % owner)
+            try:
+                os.chown(path, uid, -1)
+            except OSError:
+                self.fail_json(path=path, msg='chown failed')
+            changed = True
+        return changed
+
+    def set_group_if_different(self, path, group, changed):
+        if group is None:
+            return changed
+        old_user, old_group = self.user_and_group(path)
+        if old_group != group:
+            try:
+                gid = grp.getgrnam(group).gr_gid
+            except KeyError:
+                self.fail_json(path=path, msg='chgrp failed: failed to look up group %s' % group)
+            try:
+                os.chown(path, -1, gid)
+            except OSError:
+                self.fail_json(path=path, msg='chgrp failed')
+            changed = True
+        return changed
+
+    def set_mode_if_different(self, path, mode, changed):
+        if mode is None:
+            return changed
+        try:
+            # FIXME: support English modes
+            mode = int(mode, 8)
+        except Exception, e:
+            self.fail_json(path=path, msg='mode needs to be something octalish', details=str(e))
+
+        st = os.stat(path)
+        prev_mode = stat.S_IMODE(st[stat.ST_MODE])
+
+        if prev_mode != mode:
+            # FIXME: comparison against string above will cause this to be executed
+            # every time
+            try:
+                os.chmod(path, mode)
+            except Exception, e:
+                self.fail_json(path=path, msg='chmod failed', details=str(e))
+
+            st = os.stat(path)
+            new_mode = stat.S_IMODE(st[stat.ST_MODE])
+
+            if new_mode != prev_mode:
+                changed = True
+        return changed
+
+    def set_file_attributes_if_different(self, file_args, changed):
+        # set modes owners and context as needed
+        changed = self.set_context_if_different(
+            file_args['path'], file_args['secontext'], changed
+        )
+        changed = self.set_owner_if_different(
+            file_args['path'], file_args['owner'], changed
+        )
+        changed = self.set_group_if_different(
+            file_args['path'], file_args['group'], changed
+        )
+        changed = self.set_mode_if_different(
+            file_args['path'], file_args['mode'], changed
+        )
+        return changed
+
+    def set_directory_attributes_if_different(self, file_args, changed):
+        changed = self.set_context_if_different(
+            file_args['path'], file_args['secontext'], changed
+        )
+        changed = self.set_owner_if_different(
+            file_args['path'], file_args['owner'], changed
+        )
+        changed = self.set_group_if_different(
+            file_args['path'], file_args['group'], changed
+        )
+        changed = self.set_mode_if_different(
+            file_args['path'], file_args['mode'], changed
+        )
+        return changed
+
+    def add_path_info(self, kwargs):
+        ''' 
+        for results that are files, supplement the info about the file
+        in the return path with stats about the file path. 
+        '''
+
+        path = kwargs.get('path', kwargs.get('dest', None))
+        if path is None:
+            return kwargs
+        if os.path.exists(path):
+            (user, group) = self.user_and_group(path)
+            kwargs['owner']  = user
+            kwargs['group'] = group
+            st = os.stat(path)
+            kwargs['mode']  = oct(stat.S_IMODE(st[stat.ST_MODE]))
+            # secontext not yet supported
+            if os.path.islink(path):
+                kwargs['state'] = 'link'
+            elif os.path.isdir(path):
+                kwargs['state'] = 'directory'
+            else:
+                kwargs['state'] = 'file'
+            if HAVE_SELINUX and self.selinux_enabled():
+                kwargs['secontext'] = ':'.join(self.selinux_context(path))
+        else:
+            kwargs['state'] = 'absent'
+        return kwargs
+
+
     def _handle_aliases(self):
         for (k,v) in self.argument_spec.iteritems():
             self._legal_inputs.append(k)
@@ -274,11 +548,13 @@ class AnsibleModule(object):
 
     def exit_json(self, **kwargs):
         ''' return from the module, without error '''
+        self.add_path_info(kwargs)
         print self.jsonify(kwargs)
         sys.exit(0)
 
     def fail_json(self, **kwargs):
         ''' return from the module, with an error message '''
+        self.add_path_info(kwargs)
         assert 'msg' in kwargs, "implementation error -- msg to explain the error is required"
         kwargs['failed'] = True
         print self.jsonify(kwargs)

lib/ansible/runner/__init__.py

@@ -179,22 +179,6 @@ class Runner(object):
 
     # *****************************************************
 
-    def _delete_remote_files(self, conn, files):
-        ''' deletes one or more remote files '''
-
-        if os.getenv("ANSIBLE_KEEP_REMOTE_FILES","0") == "1":
-            # ability to turn off temp file deletion for debug purposes
-            return
-
-        if type(files) in [ str, unicode ]:
-            files = [ files ]
-        for filename in files:
-            if filename.find('/tmp/') == -1:
-                raise Exception("safeguard deletion, removal of %s is not going to happen" % filename)
-            self._low_level_exec_command(conn, "rm -rf %s" % filename, None)
-
-    # *****************************************************
-
     def _transfer_str(self, conn, tmp, name, data):
         ''' transfer string to remote file '''
 
@@ -230,12 +214,12 @@ class Runner(object):
             if 'port' not in args:
                 args += " port=%s" % C.ZEROMQ_PORT
 
-        (remote_module_path, is_new_style) = self._copy_module(conn, tmp, module_name, args, inject)
-        cmd = "chmod u+x %s" % remote_module_path
+        (remote_module_path, is_new_style, shebang) = self._copy_module(conn, tmp, module_name, args, inject)
+
+        cmd_mod = ""
         if self.sudo and self.sudo_user != 'root':
             # deal with possible umask issues once sudo'ed to other user
-            cmd = "chmod a+rx %s" % remote_module_path
-        self._low_level_exec_command(conn, cmd, tmp)
+            cmd_mod = "chmod a+r %s; " % remote_module_path
 
         cmd = ""
         if not is_new_style:
@@ -251,6 +235,13 @@ class Runner(object):
             else:
                 cmd = " ".join([str(x) for x in [remote_module_path, async_jid, async_limit, async_module]])
 
+        if not shebang:
+            raise errors.AnsibleError("module is missing interpreter line")
+
+        cmd = shebang.replace("#!","") + " " + cmd
+        if tmp.find("tmp") != -1:
+            cmd = cmd + "; rm -rf %s > /tmp/del.log 2>&1" % tmp
+        cmd = cmd_mod + cmd
         res = self._low_level_exec_command(conn, cmd, tmp, sudoable=True)
         return ReturnData(conn=conn, result=res)
 
@@ -429,19 +420,6 @@ class Runner(object):
             else:
                 result = self.action_plugins['async'].run(conn, tmp, module_name, module_args, inject)
 
-        if result.is_successful() and 'daisychain' in result.result:
-            result2 = self._executor_internal_inner(host, result.result['daisychain'], result.result.get('daisychain_args', {}), inject, port, is_chained=True)
-
-            changed = False
-            if result.result.get('changed',False) or result2.result.get('changed',False):
-                changed = True
-            result.result.update(result2.result)
-            result.result['changed'] = changed
-
-            del result.result['daisychain']
-
-        if self.module_name != 'raw':
-            self._delete_remote_files(conn, tmp)
         conn.close()
 
         if not result.comm_ok:
@@ -576,7 +554,13 @@ class Runner(object):
             module_data = "\n".join(module_lines)
 
         self._transfer_str(conn, tmp, module_name, module_data)
-        return (out_path, is_new_style)
+
+        lines = module_data.split("\n")
+        shebang = None
+        if lines[0].startswith("#!"):
+            shebang = lines[0]
+
+        return (out_path, is_new_style, shebang)
 
     # *****************************************************
 

lib/ansible/runner/action_plugins/async.py

@@ -40,7 +40,7 @@ class ActionModule(object):
             module_name = 'command'
             module_args += " #USE_SHELL"
 
-        (module_path, is_new_style) = self.runner._copy_module(conn, tmp, module_name, module_args, inject)
+        (module_path, is_new_style, shebang) = self.runner._copy_module(conn, tmp, module_name, module_args, inject)
         self.runner._low_level_exec_command(conn, "chmod a+rx %s" % module_path, tmp)
 
         return self.runner._execute_module(conn, tmp, 'async_wrapper', module_args,

lib/ansible/runner/action_plugins/copy.py

@@ -84,14 +84,13 @@ class ActionModule(object):
 
             # run the copy module
             module_args = "%s src=%s" % (module_args, tmp_src)
-            return self.runner._execute_module(conn, tmp, 'copy', module_args, inject=inject).daisychain('file', module_args)
+            return self.runner._execute_module(conn, tmp, 'copy', module_args, inject=inject)
 
         else:
-            # no need to transfer the file, already correct md5, but still need to set src so the file module
-            # does not freak out.  It's just the basename of the file.
+            # no need to transfer the file, already correct md5, but still need to call
+            # the file module in case we want to change attributes
 
             tmp_src = tmp + os.path.basename(source)
             module_args = "%s src=%s" % (module_args, tmp_src)
-            result = dict(changed=False, md5sum=remote_md5, transferred=False)
-            return ReturnData(conn=conn, result=result).daisychain('file', module_args)
+            return self.runner._execute_module(conn, tmp, 'file', module_args, inject=inject)
 

lib/ansible/runner/action_plugins/template.py

@@ -80,8 +80,9 @@ class ActionModule(object):
         if self.runner.sudo and self.runner.sudo_user != 'root':
             self.runner._low_level_exec_command(conn, "chmod a+r %s" % xfered, 
                 tmp)
-        # run the copy module, queue the file module
+
+        # run the copy module
         module_args = "%s src=%s dest=%s" % (module_args, xfered, dest)
-        return self.runner._execute_module(conn, tmp, 'copy', module_args, inject=inject).daisychain('file', module_args)
+        return self.runner._execute_module(conn, tmp, 'copy', module_args, inject=inject)
 
 

lib/ansible/runner/return_data.py

@@ -53,10 +53,3 @@ class ReturnData(object):
     def is_successful(self):
         return self.comm_ok and ('failed' not in self.result) and (self.result.get('rc',0) == 0)
 
-    def daisychain(self, module_name, module_args):
-        ''' request a module call follow this one '''
-        if self.is_successful():
-            self.result['daisychain'] = module_name
-            self.result['daisychain_args'] = module_args
-        return self
-

library/async_status

@@ -83,7 +83,7 @@ def main():
             # file not written yet?  That means it is running
             module.exit_json(results_file=log_path, ansible_job_id=jid, started=1)
         else:
-            module_fail_json(ansible_job_id=jid, results_file=log_path,
+            module.fail_json(ansible_job_id=jid, results_file=log_path,
                 msg="Could not parse job output: %s" % data)
 
     if not 'started' in data:

library/copy

@@ -64,17 +64,18 @@ def main():
 
     module = AnsibleModule(
         # not checking because of daisy chain to file module
-        check_invalid_arguments = False,
         argument_spec = dict(
             src=dict(required=True),
             dest=dict(required=True),
             backup=dict(default=False, choices=BOOLEANS),
-        )
+        ),
+        add_file_common_args=True
     )
 
     src  = os.path.expanduser(module.params['src'])
     dest = os.path.expanduser(module.params['dest'])
     backup = module.boolean(module.params.get('backup', False))
+    file_args = module.load_file_common_arguments(module.params)    
 
     if not os.path.exists(src):
         module.fail_json(msg="Source %s failed to transfer" % (src))
@@ -122,15 +123,16 @@ def main():
     else:
         changed = False
 
-    res_args = {
-        'dest':dest,
-        'src':src,
-        'md5sum':md5sum_src,
-        'changed':changed
-    }
+    res_args = dict(
+        dest = dest, src = src, md5sum = md5sum_src, changed = changed
+    )
     if backup_file:
         res_args['backup_file'] = backup_file
 
+    module.params['dest'] = dest
+    file_args = module.load_file_common_arguments(module.params)
+    res_args['changed'] = module.set_file_attributes_if_different(file_args, res_args['changed'])
+
     module.exit_json(**res_args)
 
 # this is magic, see lib/ansible/module_common.py

library/file

@@ -123,191 +123,6 @@ requirements: [ ]
 author: Michael DeHaan
 '''
 
-def add_path_info(kwargs):
-    path = kwargs['path']
-    if os.path.exists(path):
-        (user, group) = user_and_group(path)
-        kwargs['owner']  = user
-        kwargs['group'] = group
-        st = os.stat(path)
-        kwargs['mode']  = oct(stat.S_IMODE(st[stat.ST_MODE]))
-        # secontext not yet supported
-        if os.path.islink(path):
-            kwargs['state'] = 'link'
-        elif os.path.isdir(path):
-            kwargs['state'] = 'directory'
-        else:
-            kwargs['state'] = 'file'
-        if HAVE_SELINUX and selinux_enabled():
-            kwargs['secontext'] = ':'.join(selinux_context(path))
-    else:
-        kwargs['state'] = 'absent'
-    return kwargs
-
-def module_exit_json(**kwargs):
-    add_path_info(kwargs)
-    module.exit_json(**kwargs)
-
-def module_fail_json(**kwargs):
-    add_path_info(kwargs)
-    module.fail_json(**kwargs)
-
-# Detect whether using selinux that is MLS-aware.
-# While this means you can set the level/range with
-# selinux.lsetfilecon(), it may or may not mean that you
-# will get the selevel as part of the context returned
-# by selinux.lgetfilecon().
-def selinux_mls_enabled():
-    if not HAVE_SELINUX:
-        return False
-    if selinux.is_selinux_mls_enabled() == 1:
-        return True
-    else:
-        return False
-
-def selinux_enabled():
-    if not HAVE_SELINUX:
-        return False
-    if selinux.is_selinux_enabled() == 1:
-        return True
-    else:
-        return False
-
-# Determine whether we need a placeholder for selevel/mls
-def selinux_initial_context():
-    context = [None, None, None]
-    if selinux_mls_enabled():
-        context.append(None)
-    return context
-
-# If selinux fails to find a default, return an array of None
-def selinux_default_context(path, mode=0):
-    context = selinux_initial_context()
-    if not HAVE_SELINUX or not selinux_enabled():
-        return context
-    try:
-        ret = selinux.matchpathcon(path, mode)
-    except OSError:
-        return context
-    if ret[0] == -1:
-        return context
-    context = ret[1].split(':')
-    return context
-
-def selinux_context(path):
-    context = selinux_initial_context()
-    if not HAVE_SELINUX or not selinux_enabled():
-        return context
-    try:
-        ret = selinux.lgetfilecon(path)
-    except:
-        module_fail_json(path=path, msg='failed to retrieve selinux context')
-    if ret[0] == -1:
-        return context
-    context = ret[1].split(':')
-    return context
-
-# ===========================================
-# support functions
-
-def user_and_group(filename):
-    st = os.stat(filename)
-    uid = st.st_uid
-    gid = st.st_gid
-    try:
-        user = pwd.getpwuid(uid)[0]
-    except KeyError:
-        user = str(uid)
-    try:
-        group = grp.getgrgid(gid)[0]
-    except KeyError:
-        group = str(gid)
-    return (user, group)
-
-def set_context_if_different(path, context, changed):
-    if not HAVE_SELINUX or not selinux_enabled():
-        return changed
-    cur_context = selinux_context(path)
-    new_context = list(cur_context)
-    # Iterate over the current context instead of the
-    # argument context, which may have selevel.
-    for i in range(len(cur_context)):
-        if context[i] is not None and context[i] != cur_context[i]:
-            new_context[i] = context[i]
-    if cur_context != new_context:
-        try:
-            rc = selinux.lsetfilecon(path, ':'.join(new_context))
-        except OSError:
-            module_fail_json(path=path, msg='invalid selinux context')
-        if rc != 0:
-            module_fail_json(path=path, msg='set selinux context failed')
-        changed = True
-    return changed
-
-def set_owner_if_different(path, owner, changed):
-    if owner is None:
-        return changed
-    user, group = user_and_group(path)
-    if owner != user:
-        try:
-            uid = pwd.getpwnam(owner).pw_uid
-        except KeyError:
-            module_fail_json(path=path, msg='chown failed: failed to look up user %s' % owner)
-        try:
-            os.chown(path, uid, -1)
-        except OSError:
-            module_fail_json(path=path, msg='chown failed')
-        return True
-
-    return changed
-
-def set_group_if_different(path, group, changed):
-    if group is None:
-        return changed
-    old_user, old_group = user_and_group(path)
-    if old_group != group:
-        try:
-            gid = grp.getgrnam(group).gr_gid
-        except KeyError:
-            module_fail_json(path=path, msg='chgrp failed: failed to look up group %s' % group)
-        try:
-            os.chown(path, -1, gid)
-        except OSError:
-            module_fail_json(path=path, msg='chgrp failed')
-        return True
-    return changed
-
-def set_mode_if_different(path, mode, changed):
-    if mode is None:
-        return changed
-    try:
-        # FIXME: support English modes
-        mode = int(mode, 8)
-    except Exception, e:
-        module_fail_json(path=path, msg='mode needs to be something octalish', details=str(e))
-
-    st = os.stat(path)
-    prev_mode = stat.S_IMODE(st[stat.ST_MODE])
-
-    if prev_mode != mode:
-        # FIXME: comparison against string above will cause this to be executed
-        # every time
-        try:
-            os.chmod(path, mode)
-        except Exception, e:
-            module_fail_json(path=path, msg='chmod failed', details=str(e))
- 
-        st = os.stat(path)
-        new_mode = stat.S_IMODE(st[stat.ST_MODE])
-
-        if new_mode != prev_mode:
-            return True
-    return changed
-
-
-def rmtree_error(func, path, exc_info):
-    module_fail_json(path=path, msg='failed to remove directory')
-
 def main():
 
     # FIXME: pass this around, should not use global
@@ -318,15 +133,8 @@ def main():
         argument_spec = dict(
             state = dict(choices=['file','directory','link','absent'], default='file'),
             path  = dict(aliases=['dest', 'name'], required=True),
-            src   = dict(),
-            mode  = dict(),
-            owner = dict(),
-            group = dict(),
-            seuser = dict(),
-            serole = dict(),
-            selevel = dict(),
-            setype = dict(),
-        )
+        ),
+        add_file_common_args=True
     )
 
     params = module.params
@@ -343,28 +151,12 @@ def main():
     if src is not None and os.path.isdir(path) and state != "link":
         params['path'] = path = os.path.join(path, os.path.basename(src))
 
-    mode   = params.get('mode', None)
-    owner  = params.get('owner', None)
-    group  = params.get('group', None)
-
-    # selinux related options
-    seuser    = params.get('seuser', None)
-    serole    = params.get('serole', None)
-    setype    = params.get('setype', None)
-    selevel   = params.get('serange', 's0')
-    secontext = [seuser, serole, setype]
-    if selinux_mls_enabled():
-        secontext.append(selevel)
-
-    default_secontext = selinux_default_context(path)
-    for i in range(len(default_secontext)):
-        if i is not None and secontext[i] == '_default':
-            secontext[i] = default_secontext[i]
+    file_args = module.load_file_common_arguments(params)
 
     if state == 'link' and (src is None or path is None):
-        module_fail_json(msg='src and dest are required for "link" state')
+        module.fail_json(msg='src and dest are required for "link" state')
     elif path is None:
-        module_fail_json(msg='path is required')
+        module.fail_json(msg='path is required')
 
     changed = False
 
@@ -384,31 +176,29 @@ def main():
                 if os.path.islink(path):
                     os.unlink(path)
                 else:
-                    shutil.rmtree(path, ignore_errors=False, onerror=rmtree_error)
+                    try:
+                        shutil.rmtree(path, ignore_errors=False)
+                    except:
+                        module.exit_json(msg="rmtree failed")
             else:
                 os.unlink(path)
         except Exception, e:
-            module_fail_json(path=path, msg=str(e))
-        module_exit_json(path=path, changed=True)
+            module.fail_json(path=path, msg=str(e))
+        module.exit_json(path=path, changed=True)
 
     if prev_state != 'absent' and prev_state != state:
-        module_fail_json(path=path, msg='refusing to convert between %s and %s for %s' % (prev_state, state, src))
+        module.fail_json(path=path, msg='refusing to convert between %s and %s for %s' % (prev_state, state, src))
 
     if prev_state == 'absent' and state == 'absent':
-        module_exit_json(path=path, changed=False)
+        module.exit_json(path=path, changed=False)
 
     if state == 'file':
 
         if prev_state != 'file':
-            module_fail_json(path=path, msg='file (%s) does not exist, use copy or template module to create' % path) 
+            module.fail_json(path=path, msg='file (%s) does not exist, use copy or template module to create' % path) 
 
-        # set modes owners and context as needed
-        changed = set_context_if_different(path, secontext, changed)
-        changed = set_owner_if_different(path, owner, changed)
-        changed = set_group_if_different(path, group, changed)
-        changed = set_mode_if_different(path, mode, changed)
-
-        module_exit_json(path=path, changed=changed)
+        changed = module.set_file_attributes_if_different(file_args, changed)
+        module.exit_json(path=path, changed=changed)
 
     elif state == 'directory':
 
@@ -416,13 +206,8 @@ def main():
             os.makedirs(path)
             changed = True
 
-        # set modes owners and context as needed
-        changed = set_context_if_different(path, secontext, changed)
-        changed = set_owner_if_different(path, owner, changed)
-        changed = set_group_if_different(path, group, changed)
-        changed = set_mode_if_different(path, mode, changed)
-
-        module_exit_json(path=path, changed=changed)
+        changed = module.set_directory_attributes_if_different(file_args, changed)
+        module.exit_json(path=path, changed=changed)
 
     elif state == 'link':
 
@@ -431,7 +216,7 @@ def main():
         else:
             module.fail_json(msg="absolute paths are required")
         if not os.path.exists(abs_src):
-            module_fail_json(path=path, src=src, msg='src file does not exist')
+            module.fail_json(path=path, src=src, msg='src file does not exist')
 
         if prev_state == 'absent':
             os.symlink(src, path)
@@ -445,17 +230,19 @@ def main():
                 os.symlink(src, path)
                 changed = True
         else:
-            module_fail_json(dest=path, src=src, msg='unexpected position reached')
+            module.fail_json(dest=path, src=src, msg='unexpected position reached')
 
         # set modes owners and context as needed
-        changed = set_context_if_different(path, secontext, changed)
-        changed = set_owner_if_different(path, owner, changed)
-        changed = set_group_if_different(path, group, changed)
-        changed = set_mode_if_different(path, mode, changed)
+
+        file_args = module.load_file_common_arguments(module.params)
+        changed = module.set_context_if_different(path, file_args['secontext'], changed)
+        changed = module.set_owner_if_different(path, file_args['owner'], changed)
+        changed = module.set_group_if_different(path, file_args['group'], changed)
+        changed = module.set_mode_if_different(path, file_args['mode'], changed)
 
         module.exit_json(dest=path, src=src, changed=changed)
 
-    module_fail_json(path=path, msg='unexpected position reached')
+    module.fail_json(path=path, msg='unexpected position reached')
 
 # this is magic, see lib/ansible/module_common.py
 #<<INCLUDE_ANSIBLE_MODULE_COMMON>>

library/get_url

@@ -108,9 +108,7 @@ def url_do_get(module, url, dest):
         module.params['path'] = actualdest
     else:
         actualdest = dest
-    info['daisychain_args'] = module.params
-    info['daisychain_args']['state'] = 'file'
-    info['daisychain_args']['dest'] = actualdest
+
     info['actualdest'] = actualdest
 
     request = urllib2.Request(url)
@@ -177,12 +175,12 @@ def main():
 
     module = AnsibleModule(
         # not checking because of daisy chain to file module
-        check_invalid_arguments = False,
         argument_spec = dict(
             url = dict(required=True),
             dest = dict(required=True),
             thirsty = dict(default='no', choices=BOOLEANS)
-        )
+        ),
+        add_file_common_args=True
     )
 
     url  = module.params['url']
@@ -237,6 +235,12 @@ def main():
 
     os.remove(tmpsrc)
 
+    # allow file attribute changes
+    module.params['path'] = dest
+    file_args = module.load_file_common_arguments(module.params)
+    file_args['path'] = dest
+    changed = module.set_file_attributes_if_different(file_args, changed)
+
     # Mission complete
     module.exit_json(url=url, dest=dest, src=tmpsrc, md5sum=md5sum_src,
         changed=changed, msg=info.get('msg',''),

library/template

 # this is a virtual module that is entirely implemented server side
 
-
 DOCUMENTATION = '''
 ---
 module: template

test/TestRunner.py

@@ -181,7 +181,9 @@ class TestRunner(unittest.TestCase):
         assert self._run('file', ['dest=' + filedemo, 'src=/dev/null', 'state=link'])['failed']
         assert os.path.isfile(filedemo)
 
-        assert self._run('file', ['dest=' + filedemo, 'mode=604', 'state=file'])['changed']
+        res = self._run('file', ['dest=' + filedemo, 'mode=604', 'state=file'])
+        print res
+        assert res['changed']
         assert os.path.isfile(filedemo) and os.stat(filedemo).st_mode == 0100604
 
         assert self._run('file', ['dest=' + filedemo, 'state=absent'])['changed']
@@ -271,6 +273,7 @@ class TestRunner(unittest.TestCase):
             "src=%s" % input,
             "dest=%s" % output,
         ])
+        print result
         assert os.path.exists(output)
         out = file(output).read()
         assert out.find("first") != -1
@@ -283,4 +286,5 @@ class TestRunner(unittest.TestCase):
             "src=%s" % input,
             "dest=%s" % output,
         ])
+        print result
         assert result['changed'] == False