From 6b6d03290a783690624ebad60d03322421af7827 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 22 Oct 2015 08:25:20 -0700 Subject: [PATCH 1/4] Restore the automatic addition of invocation to the output Revert "Remove auto-added invocation return value as it is not used by v2 and could leak sensitive data." This reverts commit 6ce6b202684e35313deea54f89bbb8e40e48cc79. Remove the note that invocation was removed as we've now restored it. Revert "keyword not in ubuntu 14.04" This reverts commit 5c016224570fc9d68f9e3d02ed8b0d4999d20457. Revert "remove invocation keyword check" This reverts commit 5177cb3f746caa104bf8e29e3469b5682d466534. --- CHANGELOG.md | 1 - lib/ansible/plugins/action/__init__.py | 7 +++++++ lib/ansible/plugins/callback/log_plays.py | 3 +++ test/integration/roles/test_apt/tasks/apt.yml | 1 + test/integration/roles/test_yum/tasks/yum.yml | 1 + 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fc63db9fe6..bc5b59571b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -301,7 +301,6 @@ you avoid ever writing sensitive plaintext to disk. * Configuration items defined as paths (local only) now all support shell style interpolations. * Many fixes and new options added to modules, too many to list here. * Now you can see task file and line number when using verbosity of 3 or above. -* Removed the automatically added invocation field from output of modules. It's no longer needed by the code. ## 1.9.4 "Dancing In the Street" - Oct 9, 2015 diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 561763696d4..6e3cb6f3f84 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -441,6 +441,13 @@ class ActionBase: if 'stdout' in data and 'stdout_lines' not in data: data['stdout_lines'] = data.get('stdout', u'').splitlines() + # store the module invocation details back into the result + if self._task.async != 0: + data['invocation'] = dict( + module_args = module_args, + module_name = module_name, + ) + self._display.debug("done with _execute_module (%s, %s)" % (module_name, module_args)) return data diff --git a/lib/ansible/plugins/callback/log_plays.py b/lib/ansible/plugins/callback/log_plays.py index d8a834cadce..f816951fe7a 100644 --- a/lib/ansible/plugins/callback/log_plays.py +++ b/lib/ansible/plugins/callback/log_plays.py @@ -58,7 +58,10 @@ class CallbackModule(CallbackBase): data = 'omitted' else: data = data.copy() + invocation = data.pop('invocation', None) data = json.dumps(data) + if invocation is not None: + data = json.dumps(invocation) + " => %s " % data path = os.path.join("/var/log/ansible/hosts", host) now = time.strftime(self.TIME_FORMAT, time.localtime()) diff --git a/test/integration/roles/test_apt/tasks/apt.yml b/test/integration/roles/test_apt/tasks/apt.yml index 5d331b192da..c02a1bd4a09 100644 --- a/test/integration/roles/test_apt/tasks/apt.yml +++ b/test/integration/roles/test_apt/tasks/apt.yml @@ -63,6 +63,7 @@ - name: verify apt module outputs assert: that: + - "'invocation' in apt_result" - "'changed' in apt_result" - "'stderr' in apt_result" - "'stdout' in apt_result" diff --git a/test/integration/roles/test_yum/tasks/yum.yml b/test/integration/roles/test_yum/tasks/yum.yml index 95b4477739b..923717552b5 100644 --- a/test/integration/roles/test_yum/tasks/yum.yml +++ b/test/integration/roles/test_yum/tasks/yum.yml @@ -67,6 +67,7 @@ - name: verify yum module outputs assert: that: + - "'invocation' in yum_result" - "'changed' in yum_result" - "'msg' in yum_result" - "'rc' in yum_result" From ed6aa75d635ca2dc95c02aa30aa24339854f6567 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 22 Oct 2015 08:52:21 -0700 Subject: [PATCH 2/4] Add parsing test to travis to catch that invocation is missing --- .travis.yml | 2 +- .../integration/roles/test_good_parsing/tasks/main.yml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 25ffe5ae7cd..1ff0ca118d4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,6 +27,6 @@ script: - if test x"$TOXENV" != x'py24' ; then tox ; fi - if test x"$TOXENV" = x'py24' ; then python2.4 -V && python2.4 -m compileall -fq -x 'module_utils/(a10|rax|openstack|ec2|gce).py' lib/ansible/module_utils ; fi #- make -C docsite all -- if test x"$INTEGRATION" = x'yes' ; then source ./hacking/env-setup && cd test/integration/ && make test_var_precedence && make unicode ; fi +- if test x"$INTEGRATION" = x'yes' ; then source ./hacking/env-setup && cd test/integration/ && make parsing && make test_var_precedence && make unicode ; fi after_success: - coveralls diff --git a/test/integration/roles/test_good_parsing/tasks/main.yml b/test/integration/roles/test_good_parsing/tasks/main.yml index 03afb99295c..2297442b733 100644 --- a/test/integration/roles/test_good_parsing/tasks/main.yml +++ b/test/integration/roles/test_good_parsing/tasks/main.yml @@ -161,11 +161,11 @@ # #- debug: var=result -#- name: assert the task with variable module name ran -# assert: -# that: -# - result.invocation.module_name == "debug" -# - result.msg == "this should be debugged" +- name: assert the task with variable module name ran + assert: + that: + - result.invocation.module_name == "debug" + - result.msg == "this should be debugged" - name: test conditional includes include: test_include_conditional.yml From 75cff7129ca10548543926a7cab06e2700a72e77 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 22 Oct 2015 09:03:35 -0700 Subject: [PATCH 3/4] Fix for invocation not being added to output. We want invocation to be omitted when we are running async, not when we aren't running async. --- lib/ansible/plugins/action/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 6e3cb6f3f84..6d83e5afd8a 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -442,7 +442,7 @@ class ActionBase: data['stdout_lines'] = data.get('stdout', u'').splitlines() # store the module invocation details back into the result - if self._task.async != 0: + if self._task.async == 0: data['invocation'] = dict( module_args = module_args, module_name = module_name, From 2e87c1f74e0fb7a656e3c94b525a620bb54ea3a3 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 22 Oct 2015 16:07:26 -0700 Subject: [PATCH 4/4] Two fixes to action plugins * Fix the task_vars parameter to not default to a mutable type (dict) * Implement invocation in the base class's run() method have each action module call the run() method's implemention in the base class. * Return values from the action plugins' run() method takes the return value from the base class run() method into account so that invocation makes its way to the output. Fixes #12869 --- lib/ansible/plugins/action/__init__.py | 46 ++++++++++---- lib/ansible/plugins/action/add_host.py | 30 +++++---- lib/ansible/plugins/action/assemble.py | 29 ++++++--- lib/ansible/plugins/action/assert.py | 26 ++++---- lib/ansible/plugins/action/async.py | 15 +++-- lib/ansible/plugins/action/copy.py | 54 ++++++++++------ lib/ansible/plugins/action/debug.py | 15 +++-- lib/ansible/plugins/action/fail.py | 12 +++- lib/ansible/plugins/action/fetch.py | 61 ++++++++++++++----- lib/ansible/plugins/action/group_by.py | 18 ++++-- lib/ansible/plugins/action/include_vars.py | 16 +++-- lib/ansible/plugins/action/normal.py | 8 ++- lib/ansible/plugins/action/package.py | 25 +++++--- lib/ansible/plugins/action/patch.py | 18 ++++-- lib/ansible/plugins/action/pause.py | 25 +++++--- lib/ansible/plugins/action/raw.py | 12 +++- lib/ansible/plugins/action/script.py | 19 ++++-- lib/ansible/plugins/action/service.py | 17 +++--- lib/ansible/plugins/action/set_fact.py | 17 ++++-- lib/ansible/plugins/action/synchronize.py | 10 ++- lib/ansible/plugins/action/template.py | 35 +++++++---- lib/ansible/plugins/action/unarchive.py | 28 ++++++--- lib/ansible/plugins/action/win_copy.py | 1 + lib/ansible/plugins/action/win_template.py | 1 + lib/ansible/plugins/callback/minimal.py | 12 +++- .../roles/test_good_parsing/tasks/main.yml | 8 +-- test/units/plugins/action/test_action.py | 8 ++- 27 files changed, 387 insertions(+), 179 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 6d83e5afd8a..30303961d49 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -27,8 +27,9 @@ import random import stat import tempfile import time +from abc import ABCMeta, abstractmethod -from ansible.compat.six import binary_type, text_type, iteritems +from ansible.compat.six import binary_type, text_type, iteritems, with_metaclass from ansible import constants as C from ansible.errors import AnsibleError, AnsibleConnectionFailure @@ -42,7 +43,7 @@ except ImportError: from ansible.utils.display import Display display = Display() -class ActionBase: +class ActionBase(with_metaclass(ABCMeta, object)): ''' This class is the base class for all action plugins, and defines @@ -62,11 +63,39 @@ class ActionBase: self._supports_check_mode = True - def _configure_module(self, module_name, module_args, task_vars=dict()): + @abstractmethod + def run(self, tmp=None, task_vars=None): + """ Action Plugins should implement this method to perform their + tasks. Everything else in this base class is a helper method for the + action plugin to do that. + + :kwarg tmp: Temporary directory. Sometimes an action plugin sets up + a temporary directory and then calls another module. This parameter + allows us to reuse the same directory for both. + :kwarg task_vars: The variables (host vars, group vars, config vars, + etc) associated with this task. + :returns: dictionary of results from the module + + Implementors of action modules may find the following variables especially useful: + + * Module parameters. These are stored in self._task.args + """ + # store the module invocation details into the results + results = {} + if self._task.async == 0: + results['invocation'] = dict( + module_name = self._task.action, + module_args = self._task.args, + ) + return results + + def _configure_module(self, module_name, module_args, task_vars=None): ''' Handles the loading and templating of the module code through the modify_module() function. ''' + if task_vars is None: + task_vars = dict() # Search module path(s) for named module. for mod_type in self._connection.module_implementation_preferences: @@ -329,10 +358,12 @@ class ActionBase: return data[idx:] - def _execute_module(self, module_name=None, module_args=None, tmp=None, task_vars=dict(), persist_files=False, delete_remote_tmp=True): + def _execute_module(self, module_name=None, module_args=None, tmp=None, task_vars=None, persist_files=False, delete_remote_tmp=True): ''' Transfer and run a module along with its arguments. ''' + if task_vars is None: + task_vars = dict() # if a module name was not specified for this execution, use # the action from the task @@ -441,13 +472,6 @@ class ActionBase: if 'stdout' in data and 'stdout_lines' not in data: data['stdout_lines'] = data.get('stdout', u'').splitlines() - # store the module invocation details back into the result - if self._task.async == 0: - data['invocation'] = dict( - module_args = module_args, - module_name = module_name, - ) - self._display.debug("done with _execute_module (%s, %s)" % (module_name, module_args)) return data diff --git a/lib/ansible/plugins/action/add_host.py b/lib/ansible/plugins/action/add_host.py index 0e7d3187e56..65026eb513f 100644 --- a/lib/ansible/plugins/action/add_host.py +++ b/lib/ansible/plugins/action/add_host.py @@ -20,27 +20,32 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import re - from ansible.plugins.action import ActionBase from ansible.parsing.utils.addresses import parse_address -from ansible.errors import AnsibleError, AnsibleParserError +from ansible.errors import AnsibleError + class ActionModule(ActionBase): ''' Create inventory hosts and groups in the memory inventory''' - ### We need to be able to modify the inventory + # We need to be able to modify the inventory BYPASS_HOST_LOOP = True TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if self._play_context.check_mode: - return dict(skipped=True, msg='check mode not supported for this module') + result['skipped'] = True + result['msg'] = 'check mode not supported for this module' + return result # Parse out any hostname:port patterns new_name = self._task.args.get('name', self._task.args.get('hostname', None)) - #vv("creating host via 'add_host': hostname=%s" % new_name) + self._display.vv("creating host via 'add_host': hostname=%s" % new_name) name, port = parse_address(new_name, allow_ranges=False) if not name: @@ -48,7 +53,7 @@ class ActionModule(ActionBase): if port: self._task.args['ansible_ssh_port'] = port - groups = self._task.args.get('groupname', self._task.args.get('groups', self._task.args.get('group', ''))) + groups = self._task.args.get('groupname', self._task.args.get('groups', self._task.args.get('group', ''))) # add it to the group if that was specified new_groups = [] if groups: @@ -58,8 +63,11 @@ class ActionModule(ActionBase): # Add any variables to the new_host host_vars = dict() + special_args = frozenset(('name', 'hostname', 'groupname', 'groups')) for k in self._task.args.keys(): - if not k in [ 'name', 'hostname', 'groupname', 'groups' ]: - host_vars[k] = self._task.args[k] + if k not in special_args: + host_vars[k] = self._task.args[k] - return dict(changed=True, add_host=dict(host_name=name, groups=new_groups, host_vars=host_vars)) + result['changed'] = True + result['add_host'] = dict(host_name=name, groups=new_groups, host_vars=host_vars) + return result diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index 34dc98be06d..64fb2f45cce 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -20,16 +20,14 @@ __metaclass__ = type import os import os.path -import pipes -import shutil import tempfile -import base64 import re from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean from ansible.utils.hashing import checksum_s + class ActionModule(ActionBase): TRANSFERS_FILES = True @@ -75,10 +73,16 @@ class ActionModule(ActionBase): tmp.close() return temp_path - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if self._play_context.check_mode: - return dict(skipped=True, msg=("skipped, this module does not support check_mode.")) + result['skipped'] = True + result['msg'] = "skipped, this module does not support check_mode." + return result src = self._task.args.get('src', None) dest = self._task.args.get('dest', None) @@ -87,12 +91,15 @@ class ActionModule(ActionBase): regexp = self._task.args.get('regexp', None) ignore_hidden = self._task.args.get('ignore_hidden', False) - if src is None or dest is None: - return dict(failed=True, msg="src and dest are required") + result['failed'] = True + result['msg'] = "src and dest are required" + return result if boolean(remote_src): - return self._execute_module(tmp=tmp, task_vars=task_vars) + result.update(self._execute_module(tmp=tmp, task_vars=task_vars)) + return result + elif self._task._role is not None: src = self._loader.path_dwim_relative(self._task._role._role_path, 'files', src) else: @@ -136,7 +143,8 @@ class ActionModule(ActionBase): res = self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, tmp=tmp) if diff: res['diff'] = diff - return res + result.update(res) + return result else: new_module_args = self._task.args.copy() new_module_args.update( @@ -147,4 +155,5 @@ class ActionModule(ActionBase): ) ) - return self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp) + result.update(self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp)) + return result diff --git a/lib/ansible/plugins/action/assert.py b/lib/ansible/plugins/action/assert.py index d39484f3663..effe5ba96b1 100644 --- a/lib/ansible/plugins/action/assert.py +++ b/lib/ansible/plugins/action/assert.py @@ -21,14 +21,19 @@ from ansible.errors import AnsibleError from ansible.playbook.conditional import Conditional from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): ''' Fail with custom message ''' TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) - if not 'that' in self._task.args: + if 'that' not in self._task.args: raise AnsibleError('conditional required in "that" string') msg = None @@ -38,7 +43,7 @@ class ActionModule(ActionBase): # make sure the 'that' items are a list thats = self._task.args['that'] if not isinstance(thats, list): - thats = [ thats ] + thats = [thats] # Now we iterate over the that items, temporarily assigning them # to the task's when value so we can evaluate the conditional using @@ -47,19 +52,18 @@ class ActionModule(ActionBase): # that value now cond = Conditional(loader=self._loader) for that in thats: - cond.when = [ that ] + cond.when = [that] test_result = cond.evaluate_conditional(templar=self._templar, all_vars=task_vars) if not test_result: - result = dict( - failed = True, - evaluated_to = test_result, - assertion = that, - ) + result['failed'] = True + result['evaluated_to'] = test_result + result['assertion'] = that if msg: result['msg'] = msg return result - return dict(changed=False, msg='all assertions passed') - + result['changed'] = False + result['msg'] = 'all assertions passed' + return result diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index 1012f83a9f2..51e2413af27 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -23,13 +23,20 @@ import random from ansible import constants as C from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' transfer the given module name, plus the async module, then run it ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if self._play_context.check_mode: - return dict(skipped=True, msg='check mode not supported for this module') + result['skipped'] = True + result['msg'] = 'check mode not supported for this module' + return result if not tmp: tmp = self._make_tmp_path() @@ -60,7 +67,7 @@ class ActionModule(ActionBase): async_jid = str(random.randint(0, 999999999999)) async_cmd = " ".join([str(x) for x in [env_string, async_module_path, async_jid, async_limit, remote_module_path, argsfile]]) - result = self._low_level_execute_command(cmd=async_cmd) + result.update(self._low_level_execute_command(cmd=async_cmd)) # clean up after if tmp and "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES: @@ -69,5 +76,3 @@ class ActionModule(ActionBase): result['changed'] = True return result - - diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index aca7bd3eb57..447cdda2784 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -21,7 +21,6 @@ __metaclass__ = type import json import os -import pipes import tempfile from ansible import constants as C @@ -30,10 +29,15 @@ from ansible.utils.boolean import boolean from ansible.utils.hashing import checksum from ansible.utils.unicode import to_bytes + class ActionModule(ActionBase): - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' handler for file transfer operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) source = self._task.args.get('src', None) content = self._task.args.get('content', None) @@ -44,11 +48,17 @@ class ActionModule(ActionBase): remote_src = boolean(self._task.args.get('remote_src', False)) if (source is None and content is None and faf is None) or dest is None: - return dict(failed=True, msg="src (or content) and dest are required") + result['failed'] = True + result['msg'] = "src (or content) and dest are required" + return result elif (source is not None or faf is not None) and content is not None: - return dict(failed=True, msg="src and content are mutually exclusive") + result['failed'] = True + result['msg'] = "src and content are mutually exclusive" + return result elif content is not None and dest is not None and dest.endswith("/"): - return dict(failed=True, msg="dest must be a file if content is defined") + result['failed'] = True + result['msg'] = "dest must be a file if content is defined" + return result # Check if the source ends with a "/" source_trailing_slash = False @@ -69,19 +79,24 @@ class ActionModule(ActionBase): content_tempfile = self._create_content_tempfile(content) source = content_tempfile except Exception as err: - return dict(failed=True, msg="could not write content temp file: %s" % err) + result['failed'] = True + result['msg'] = "could not write content temp file: %s" % err + return result # if we have first_available_file in our vars # look up the files and use the first one we find as src elif faf: source = self._get_first_available_file(faf, task_vars.get('_original_file', None)) if source is None: - return dict(failed=True, msg="could not find src in first_available_file list") + result['failed'] = True + result['msg'] = "could not find src in first_available_file list" + return result elif remote_src: new_module_args = self._task.args.copy() del new_module_args['remote_src'] - return self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, delete_remote_tmp=False) + result.update(self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, delete_remote_tmp=False)) + return result else: if self._task._role is not None: @@ -117,7 +132,7 @@ class ActionModule(ActionBase): source_files.append((source, os.path.basename(source))) changed = False - module_result = {"changed": False} + module_return = dict(changed=False) # A register for if we executed a module. # Used to cut down on command calls when not recursive. @@ -142,7 +157,9 @@ class ActionModule(ActionBase): # If local_checksum is not defined we can't find the file so we should fail out. if local_checksum is None: - return dict(failed=True, msg="could not find src=%s" % source_full) + result['failed'] = True + result['msg'] = "could not find src=%s" % source_full + return result # This is kind of optimization - if user told us destination is # dir, do path manipulation right away, otherwise we still check @@ -160,7 +177,9 @@ class ActionModule(ActionBase): if content is not None: # If source was defined as content remove the temporary file and fail out. self._remove_tempfile_if_content_defined(content, content_tempfile) - return dict(failed=True, msg="can not use content with a dir as dest") + result['failed'] = True + result['msg'] = "can not use content with a dir as dest" + return result else: # Append the relative source location to the destination and retry remote_checksum dest_file = self._connection._shell.join_path(dest, source_rel) @@ -250,9 +269,10 @@ class ActionModule(ActionBase): if not module_return.get('checksum'): module_return['checksum'] = local_checksum - if module_return.get('failed') == True: - return module_return - if module_return.get('changed') == True: + if module_return.get('failed'): + result.update(module_return) + return result + if module_return.get('changed'): changed = True # the file module returns the file path as 'path', but @@ -265,9 +285,9 @@ class ActionModule(ActionBase): self._remove_tmp_path(tmp) if module_executed and len(source_files) == 1: - result = module_return + result.update(module_return) else: - result = dict(dest=dest, src=source, changed=changed) + result.update(dict(dest=dest, src=source, changed=changed)) if diffs: result['diff'] = diffs @@ -288,8 +308,6 @@ class ActionModule(ActionBase): f.close() return content_tempfile - def _remove_tempfile_if_content_defined(self, content, content_tempfile): if content is not None: os.remove(content_tempfile) - diff --git a/lib/ansible/plugins/action/debug.py b/lib/ansible/plugins/action/debug.py index 01a59d1ad2d..5a5a805e746 100644 --- a/lib/ansible/plugins/action/debug.py +++ b/lib/ansible/plugins/action/debug.py @@ -20,27 +20,32 @@ __metaclass__ = type from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean + class ActionModule(ActionBase): ''' Print statements during execution ''' TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if 'msg' in self._task.args: if 'fail' in self._task.args and boolean(self._task.args['fail']): - result = dict(failed=True, msg=self._task.args['msg']) + result['failed'] = True + result['msg'] = self._task.args['msg'] else: - result = dict(msg=self._task.args['msg']) + result['msg'] = self._task.args['msg'] # FIXME: move the LOOKUP_REGEX somewhere else elif 'var' in self._task.args: # and not utils.LOOKUP_REGEX.search(self._task.args['var']): results = self._templar.template(self._task.args['var'], convert_bare=True) if results == self._task.args['var']: results = "VARIABLE IS NOT DEFINED!" - result = dict() result[self._task.args['var']] = results else: - result = dict(msg='here we are') + result['msg'] = 'here we are' # force flag to make debug output module always verbose result['_ansible_verbose_always'] = True diff --git a/lib/ansible/plugins/action/fail.py b/lib/ansible/plugins/action/fail.py index b7845c95c5c..c78f1683a95 100644 --- a/lib/ansible/plugins/action/fail.py +++ b/lib/ansible/plugins/action/fail.py @@ -20,16 +20,22 @@ __metaclass__ = type from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): ''' Fail with custom message ''' TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) msg = 'Failed as requested from task' if self._task.args and 'msg' in self._task.args: msg = self._task.args.get('msg') - return dict(failed=True, msg=msg) - + result['failed'] = True + result['msg'] = msg + return result diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index bbdedbdc7f6..478eac3f823 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -26,13 +26,20 @@ 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): - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' handler for fetch operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if self._play_context.check_mode: - return dict(skipped=True, msg='check mode not (yet) supported for this module') + result['skipped'] = True + result['msg'] = 'check mode not (yet) supported for this module' + return result source = self._task.args.get('src', None) dest = self._task.args.get('dest', None) @@ -41,10 +48,14 @@ class ActionModule(ActionBase): validate_checksum = boolean(self._task.args.get('validate_checksum', self._task.args.get('validate_md5'))) if 'validate_md5' in self._task.args and 'validate_checksum' in self._task.args: - return dict(failed=True, msg="validate_checksum and validate_md5 cannot both be specified") + result['failed'] = True + result['msg'] = "validate_checksum and validate_md5 cannot both be specified" + return result if source is None or dest is None: - return dict(failed=True, msg="src and dest are required") + result['failed'] = True + result['msg'] = "src and dest are required" + return result source = self._connection._shell.join_path(source) source = self._remote_expand_user(source) @@ -58,8 +69,12 @@ class ActionModule(ActionBase): slurpres = self._execute_module(module_name='slurp', module_args=dict(src=source), task_vars=task_vars, tmp=tmp) if slurpres.get('failed'): if remote_checksum == '1' and not fail_on_missing: - return dict(msg="the remote file does not exist, not transferring, ignored", file=source, changed=False) - return slurpres + result['msg'] = "the remote file does not exist, not transferring, ignored" + result['file'] = source + result['changed'] = False + return result + result.update(slurpres) + return result else: if slurpres['encoding'] == 'base64': remote_data = base64.b64decode(slurpres['content']) @@ -103,18 +118,30 @@ class ActionModule(ActionBase): # these don't fail because you may want to transfer a log file that possibly MAY exist # but keep going to fetch other log files if remote_checksum == '0': - result = dict(msg="unable to calculate the checksum of the remote file", file=source, changed=False) + result['msg'] = "unable to calculate the checksum of the remote file" + result['file'] = source + result['changed'] = False elif remote_checksum == '1': if fail_on_missing: - result = dict(failed=True, msg="the remote file does not exist", file=source) + result['failed'] = True + result['msg'] = "the remote file does not exist" + result['file'] = source else: - result = dict(msg="the remote file does not exist, not transferring, ignored", file=source, changed=False) + result['msg'] = "the remote file does not exist, not transferring, ignored" + result['file'] = source + result['changed'] = False elif remote_checksum == '2': - result = dict(msg="no read permission on remote file, not transferring, ignored", file=source, changed=False) + result['msg'] = "no read permission on remote file, not transferring, ignored" + result['file'] = source + result['changed'] = False elif remote_checksum == '3': - result = dict(msg="remote file is a directory, fetch cannot work on directories", file=source, changed=False) + result['msg'] = "remote file is a directory, fetch cannot work on directories" + result['file'] = source + result['changed'] = False elif remote_checksum == '4': - result = dict(msg="python isn't present on the system. Unable to compute checksum", file=source, changed=False) + result['msg'] = "python isn't present on the system. Unable to compute checksum" + result['file'] = source + result['changed'] = False return result # calculate checksum for the local file @@ -143,8 +170,10 @@ class ActionModule(ActionBase): new_md5 = None if validate_checksum and new_checksum != remote_checksum: - return dict(failed=True, md5sum=new_md5, msg="checksum mismatch", file=source, dest=dest, remote_md5sum=None, checksum=new_checksum, remote_checksum=remote_checksum) - return dict(changed=True, md5sum=new_md5, dest=dest, remote_md5sum=None, checksum=new_checksum, remote_checksum=remote_checksum) + result.update(dict(failed=True, md5sum=new_md5, msg="checksum mismatch", file=source, dest=dest, remote_md5sum=None, checksum=new_checksum, remote_checksum=remote_checksum)) + return result + result.update(dict(changed=True, md5sum=new_md5, dest=dest, remote_md5sum=None, checksum=new_checksum, remote_checksum=remote_checksum)) + return result else: # For backwards compatibility. We'll return None on FIPS enabled # systems @@ -153,5 +182,5 @@ class ActionModule(ActionBase): except ValueError: local_md5 = None - return dict(changed=False, md5sum=local_md5, file=source, dest=dest, checksum=local_checksum) - + result.update(dict(changed=False, md5sum=local_md5, file=source, dest=dest, checksum=local_checksum)) + return result diff --git a/lib/ansible/plugins/action/group_by.py b/lib/ansible/plugins/action/group_by.py index 126d04dd6fc..a891d3c70d5 100644 --- a/lib/ansible/plugins/action/group_by.py +++ b/lib/ansible/plugins/action/group_by.py @@ -19,19 +19,27 @@ __metaclass__ = type from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): ''' Create inventory groups based on variables ''' ### We need to be able to modify the inventory TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) - if not 'key' in self._task.args: - return dict(failed=True, msg="the 'key' param is required when using group_by") + if 'key' not in self._task.args: + result['failed'] = True + result['msg'] = "the 'key' param is required when using group_by" + return result group_name = self._task.args.get('key') group_name = group_name.replace(' ','-') - return dict(changed=True, add_group=group_name) - + result['changed'] = True + result['add_group'] = group_name + return result diff --git a/lib/ansible/plugins/action/include_vars.py b/lib/ansible/plugins/action/include_vars.py index e0c1c088b80..cada2f53a03 100644 --- a/lib/ansible/plugins/action/include_vars.py +++ b/lib/ansible/plugins/action/include_vars.py @@ -20,14 +20,18 @@ __metaclass__ = type import os from ansible.errors import AnsibleError -from ansible.parsing import DataLoader from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) source = self._task.args.get('_raw_params') @@ -43,7 +47,11 @@ class ActionModule(ActionBase): data = {} if not isinstance(data, dict): raise AnsibleError("%s must be stored as a dictionary/hash" % source) - return dict(ansible_facts=data, _ansible_no_log=not show_content) + result['ansible_facts'] = data + result['_ansible_no_log'] = not show_content else: - return dict(failed=True, msg="Source file not found.", file=source) + result['failed'] = True + result['msg'] = "Source file not found." + result['file'] = source + return result diff --git a/lib/ansible/plugins/action/normal.py b/lib/ansible/plugins/action/normal.py index 9ea962a240f..bf93fdad2d7 100644 --- a/lib/ansible/plugins/action/normal.py +++ b/lib/ansible/plugins/action/normal.py @@ -19,11 +19,15 @@ __metaclass__ = type from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() - results = self._execute_module(tmp=tmp, task_vars=task_vars) + results = super(ActionModule, self).run(tmp, task_vars) + results.update(self._execute_module(tmp=tmp, task_vars=task_vars)) # Remove special fields from the result, which can only be set # internally by the executor engine. We do this only here in diff --git a/lib/ansible/plugins/action/package.py b/lib/ansible/plugins/action/package.py index fce79ee22e1..ca9dde43df2 100644 --- a/lib/ansible/plugins/action/package.py +++ b/lib/ansible/plugins/action/package.py @@ -17,18 +17,20 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type - from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' handler for package operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) - name = self._task.args.get('name', None) - state = self._task.args.get('state', None) module = self._task.args.get('use', 'auto') if module == 'auto': @@ -40,13 +42,15 @@ class ActionModule(ActionBase): if module == 'auto': facts = self._execute_module(module_name='setup', module_args=dict(filter='ansible_pkg_mgr'), task_vars=task_vars) self._display.debug("Facts %s" % facts) - if not 'failed' in facts: + if 'failed' not in facts: module = getattr(facts['ansible_facts'], 'ansible_pkg_mgr', 'auto') if module != 'auto': if module not in self._shared_loader_obj.module_loader: - return {'failed': True, 'msg': 'Could not find a module for %s.' % module} + result['failed'] = True + result['msg'] = 'Could not find a module for %s.' % module + return result # run the 'package' module new_module_args = self._task.args.copy() @@ -54,8 +58,9 @@ class ActionModule(ActionBase): del new_module_args['use'] self._display.vvvv("Running %s" % module) - return self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars) - + result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars)) + return result else: - - return {'failed': True, 'msg': 'Could not detect which package manager to use. Try gathering facts or setting the "use" option.'} + result['failed'] = True + result['msg'] = 'Could not detect which package manager to use. Try gathering facts or setting the "use" option.' + return result diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py index b411fcdc355..c08f901cf58 100644 --- a/lib/ansible/plugins/action/patch.py +++ b/lib/ansible/plugins/action/patch.py @@ -23,20 +23,27 @@ import os from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean + class ActionModule(ActionBase): - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) src = self._task.args.get('src', None) - dest = self._task.args.get('dest', None) remote_src = boolean(self._task.args.get('remote_src', 'no')) if src is None: - return dict(failed=True, msg="src is required") + result['failed'] = True + result['msg'] = "src is required" + return result elif remote_src: # everything is remote, so we just execute the module # without changing any of the module arguments - return self._execute_module(task_vars=task_vars) + result.update(self._execute_module(task_vars=task_vars)) + return result if self._task._role is not None: src = self._loader.path_dwim_relative(self._task._role._role_path, 'files', src) @@ -61,4 +68,5 @@ class ActionModule(ActionBase): ) ) - return self._execute_module('patch', module_args=new_module_args, task_vars=task_vars) + result.update(self._execute_module('patch', module_args=new_module_args, task_vars=task_vars)) + return result diff --git a/lib/ansible/plugins/action/pause.py b/lib/ansible/plugins/action/pause.py index 0620a805a7e..c261b37758e 100644 --- a/lib/ansible/plugins/action/pause.py +++ b/lib/ansible/plugins/action/pause.py @@ -27,25 +27,32 @@ from os import isatty from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase + class AnsibleTimeoutExceeded(Exception): pass + def timeout_handler(signum, frame): raise AnsibleTimeoutExceeded + class ActionModule(ActionBase): ''' pauses execution for a length or time, or until input is received ''' PAUSE_TYPES = ['seconds', 'minutes', 'prompt', ''] BYPASS_HOST_LOOP = True - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' run the pause action module ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) duration_unit = 'minutes' prompt = None seconds = None - result = dict( + result.update(dict( changed = False, rc = 0, stderr = '', @@ -53,37 +60,37 @@ class ActionModule(ActionBase): start = None, stop = None, delta = None, - ) + )) # Is 'args' empty, then this is the default prompted pause if self._task.args is None or len(self._task.args.keys()) == 0: - pause_type = 'prompt' prompt = "[%s]\nPress enter to continue:" % self._task.get_name().strip() # Are 'minutes' or 'seconds' keys that exist in 'args'? elif 'minutes' in self._task.args or 'seconds' in self._task.args: try: if 'minutes' in self._task.args: - pause_type = 'minutes' # The time() command operates in seconds so we need to # recalculate for minutes=X values. seconds = int(self._task.args['minutes']) * 60 else: - pause_type = 'seconds' seconds = int(self._task.args['seconds']) duration_unit = 'seconds' except ValueError as e: - return dict(failed=True, msg="non-integer value given for prompt duration:\n%s" % str(e)) + result['failed'] = True + result['msg'] = "non-integer value given for prompt duration:\n%s" % str(e) + return result # Is 'prompt' a key in 'args'? elif 'prompt' in self._task.args: - pause_type = 'prompt' prompt = "[%s]\n%s:" % (self._task.get_name().strip(), self._task.args['prompt']) else: # I have no idea what you're trying to do. But it's so wrong. - return dict(failed=True, msg="invalid pause type given. must be one of: %s" % ", ".join(self.PAUSE_TYPES)) + result['failed'] = True + result['msg'] = "invalid pause type given. must be one of: %s" % ", ".join(self.PAUSE_TYPES) + return result ######################################################################## # Begin the hard work! diff --git a/lib/ansible/plugins/action/raw.py b/lib/ansible/plugins/action/raw.py index 8fd32c53426..d6fa2f35599 100644 --- a/lib/ansible/plugins/action/raw.py +++ b/lib/ansible/plugins/action/raw.py @@ -21,17 +21,23 @@ from ansible.plugins.action import ActionBase import re + class ActionModule(ActionBase): TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if self._play_context.check_mode: # in --check mode, always skip this module execution - return dict(skipped=True) + result['skipped'] = True + return result executable = self._task.args.get('executable') - result = self._low_level_execute_command(self._task.args.get('_raw_params'), executable=executable) + result.update(self._low_level_execute_command(self._task.args.get('_raw_params'), executable=executable)) # for some modules (script, raw), the sudo success key # may leak into the stdout due to the way the sudo/su diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index 31e0a1736c0..5b0f324dfcf 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -22,14 +22,21 @@ import os from ansible import constants as C from ansible.plugins.action import ActionBase + class ActionModule(ActionBase): TRANSFERS_FILES = True def run(self, tmp=None, task_vars=None): ''' handler for file transfer operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) if self._play_context.check_mode: - return dict(skipped=True, msg='check mode not supported for this module') + result['skipped'] = True + result['msg'] = 'check mode not supported for this module' + return result if not tmp: tmp = self._make_tmp_path() @@ -39,8 +46,8 @@ class ActionModule(ActionBase): # do not run the command if the line contains creates=filename # and the filename already exists. This allows idempotence # of command executions. - result = self._execute_module(module_name='stat', module_args=dict(path=creates), task_vars=task_vars, tmp=tmp, persist_files=True) - stat = result.get('stat', None) + res = self._execute_module(module_name='stat', module_args=dict(path=creates), task_vars=task_vars, tmp=tmp, persist_files=True) + stat = res.get('stat', None) if stat and stat.get('exists', False): return dict(skipped=True, msg=("skipped, since %s exists" % creates)) @@ -49,8 +56,8 @@ class ActionModule(ActionBase): # do not run the command if the line contains removes=filename # and the filename does not exist. This allows idempotence # of command executions. - result = self._execute_module(module_name='stat', module_args=dict(path=removes), task_vars=task_vars, tmp=tmp, persist_files=True) - stat = result.get('stat', None) + res = self._execute_module(module_name='stat', module_args=dict(path=removes), task_vars=task_vars, tmp=tmp, persist_files=True) + stat = res.get('stat', None) if stat and not stat.get('exists', False): return dict(skipped=True, msg=("skipped, since %s does not exist" % removes)) @@ -84,7 +91,7 @@ class ActionModule(ActionBase): env_string = self._compute_environment_string() script_cmd = ' '.join([env_string, tmp_src, args]) - result = self._low_level_execute_command(cmd=script_cmd, sudoable=True) + result.update(self._low_level_execute_command(cmd=script_cmd, sudoable=True)) # clean up after if tmp and "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES: diff --git a/lib/ansible/plugins/action/service.py b/lib/ansible/plugins/action/service.py index 8cceb85e945..9871cb5fc73 100644 --- a/lib/ansible/plugins/action/service.py +++ b/lib/ansible/plugins/action/service.py @@ -25,11 +25,13 @@ class ActionModule(ActionBase): TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' handler for package operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) - name = self._task.args.get('name', None) - state = self._task.args.get('state', None) module = self._task.args.get('use', 'auto') if module == 'auto': @@ -41,7 +43,7 @@ class ActionModule(ActionBase): if module == 'auto': facts = self._execute_module(module_name='setup', module_args=dict(filter='ansible_service_mgr'), task_vars=task_vars) self._display.debug("Facts %s" % facts) - if not 'failed' in facts: + if 'failed' not in facts: module = getattr(facts['ansible_facts'], 'ansible_service_mgr', 'auto') if not module or module == 'auto' or module not in self._shared_loader_obj.module_loader: @@ -54,8 +56,9 @@ class ActionModule(ActionBase): del new_module_args['use'] self._display.vvvv("Running %s" % module) - return self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars) - + result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars)) else: + result['failed'] = True + result['msg'] = 'Could not detect which service manager to use. Try gathering facts or setting the "use" option.' - return {'failed': True, 'msg': 'Could not detect which service manager to use. Try gathering facts or setting the "use" option.'} + return result diff --git a/lib/ansible/plugins/action/set_fact.py b/lib/ansible/plugins/action/set_fact.py index 84c7d765326..f6e4a7e67e9 100644 --- a/lib/ansible/plugins/action/set_fact.py +++ b/lib/ansible/plugins/action/set_fact.py @@ -20,7 +20,6 @@ __metaclass__ = type from ansible.compat.six import iteritems -from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean from ansible.utils.vars import isidentifier @@ -30,16 +29,26 @@ class ActionModule(ActionBase): TRANSFERS_FILES = False - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) + facts = dict() if self._task.args: for (k, v) in iteritems(self._task.args): k = self._templar.template(k) if not isidentifier(k): - return dict(failed=True, msg="The variable name '%s' is not valid. Variables must start with a letter or underscore character, and contain only letters, numbers and underscores." % k) + result['failed'] = True + result['msg'] = "The variable name '%s' is not valid. Variables must start with a letter or underscore character, and contain only letters, numbers and underscores." % k + return result if isinstance(v, basestring) and v.lower() in ('true', 'false', 'yes', 'no'): v = boolean(v) facts[k] = v - return dict(changed=False, ansible_facts=facts) + + result['changed'] = False + result['ansible_facts'] = facts + return result diff --git a/lib/ansible/plugins/action/synchronize.py b/lib/ansible/plugins/action/synchronize.py index cbc4d2ce472..b0cdcb1b11f 100644 --- a/lib/ansible/plugins/action/synchronize.py +++ b/lib/ansible/plugins/action/synchronize.py @@ -101,8 +101,12 @@ class ActionModule(ActionBase): if key.startswith("ansible_") and key.endswith("_interpreter"): task_vars[key] = localhost[key] - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' generates params and passes them on to the rsync module ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) original_transport = task_vars.get('ansible_connection') or self._play_context.connection remote_transport = False @@ -124,7 +128,7 @@ class ActionModule(ActionBase): # ansible's delegate_to mechanism to determine which host rsync is # running on so localhost could be a non-controller machine if # delegate_to is used) - src_host = '127.0.0.1' + src_host = '127.0.0.1' inventory_hostname = task_vars.get('inventory_hostname') dest_host_inventory_vars = task_vars['hostvars'].get(inventory_hostname) dest_host = dest_host_inventory_vars.get('ansible_ssh_host', inventory_hostname) @@ -236,7 +240,7 @@ class ActionModule(ActionBase): self._task.args['ssh_args'] = C.ANSIBLE_SSH_ARGS # run the module and store the result - result = self._execute_module('synchronize', task_vars=task_vars) + result.update(self._execute_module('synchronize', task_vars=task_vars)) if 'SyntaxError' in result['msg']: # Emit a warning about using python3 because synchronize is diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 583f45ef229..f3d7bc3c4bd 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -17,7 +17,6 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import base64 import datetime import os import pwd @@ -29,6 +28,7 @@ from ansible.utils.hashing import checksum_s from ansible.utils.boolean import boolean from ansible.utils.unicode import to_bytes, to_unicode + class ActionModule(ActionBase): TRANSFERS_FILES = True @@ -52,8 +52,12 @@ class ActionModule(ActionBase): return remote_checksum - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' handler for template operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) source = self._task.args.get('src', None) dest = self._task.args.get('dest', None) @@ -61,7 +65,9 @@ class ActionModule(ActionBase): force = boolean(self._task.args.get('force', False)) if (source is None and faf is not None) or dest is None: - return dict(failed=True, msg="src and dest are required") + result['failed'] = True + result['msg'] = "src and dest are required" + return result if tmp is None: tmp = self._make_tmp_path() @@ -69,7 +75,9 @@ class ActionModule(ActionBase): if faf: source = self._get_first_available_file(faf, task_vars.get('_original_file', None, 'templates')) if source is None: - return dict(failed=True, msg="could not find src in first_available_file list") + result['failed'] = True + result['msg'] = "could not find src in first_available_file list" + return result else: if self._task._role is not None: source = self._loader.path_dwim_relative(self._task._role._role_path, 'templates', source) @@ -128,20 +136,21 @@ class ActionModule(ActionBase): resultant = self._templar.template(template_data, preserve_trailing_newlines=True, escape_backslashes=False, convert_data=False) self._templar.set_available_variables(old_vars) except Exception as e: - return dict(failed=True, msg=type(e).__name__ + ": " + str(e)) + result['failed'] = True + result['msg'] = type(e).__name__ + ": " + str(e) + return result local_checksum = checksum_s(resultant) remote_checksum = self.get_checksum(dest, task_vars, not directory_prepended, source=source) if isinstance(remote_checksum, dict): # Error from remote_checksum is a dict. Valid return is a str - return remote_checksum + result.update(remote_checksum) + return result diff = {} new_module_args = self._task.args.copy() if local_checksum != remote_checksum: - dest_contents = '' - # if showing diffs, we need to get the remote value if self._play_context.diff: diff = self._get_diff_data(dest, resultant, task_vars, source_file=False) @@ -162,12 +171,12 @@ class ActionModule(ActionBase): follow=True, ), ) - result = self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars) + result.update(self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars)) else: if remote_checksum == '1' or force: - result = dict(changed=True) + result['changed'] = True else: - result = dict(changed=False) + result['changed'] = False if result.get('changed', False) and self._play_context.diff: result['diff'] = diff @@ -189,5 +198,5 @@ class ActionModule(ActionBase): ), ) - return self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars) - + result.update(self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars)) + return result diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 06c7c102dd8..40b72837c7c 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -19,7 +19,6 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import os -import pipes from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean @@ -29,8 +28,12 @@ class ActionModule(ActionBase): TRANSFERS_FILES = True - def run(self, tmp=None, task_vars=dict()): + def run(self, tmp=None, task_vars=None): ''' handler for unarchive operations ''' + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(tmp, task_vars) source = self._task.args.get('src', None) dest = self._task.args.get('dest', None) @@ -38,7 +41,9 @@ class ActionModule(ActionBase): creates = self._task.args.get('creates', None) if source is None or dest is None: - return dict(failed=True, msg="src (or content) and dest are required") + result['failed'] = True + result['msg'] = "src (or content) and dest are required" + return result if not tmp: tmp = self._make_tmp_path() @@ -47,11 +52,12 @@ class ActionModule(ActionBase): # do not run the command if the line contains creates=filename # and the filename already exists. This allows idempotence # of command executions. - module_args_tmp = "path=%s" % creates result = self._execute_module(module_name='stat', module_args=dict(path=creates), task_vars=task_vars) stat = result.get('stat', None) if stat and stat.get('exists', False): - return dict(skipped=True, msg=("skipped, since %s exists" % creates)) + result['skipped'] = True + result['msg'] = "skipped, since %s exists" % creates + return result dest = self._remote_expand_user(dest) # CCTODO: Fix path for Windows hosts. source = os.path.expanduser(source) @@ -68,9 +74,13 @@ class ActionModule(ActionBase): remote_checksum = self._remote_checksum(dest, all_vars=task_vars) if remote_checksum != '3': - return dict(failed=True, msg="dest '%s' must be an existing dir" % dest) + result['failed'] = True + result['msg'] = "dest '%s' must be an existing dir" % dest + return result elif remote_checksum == '4': - return dict(failed=True, msg="python isn't present on the system. Unable to compute checksum") + result['failed'] = True + result['msg'] = "python isn't present on the system. Unable to compute checksum" + return result if copy: # transfer the file to a remote tmp location @@ -103,5 +113,5 @@ class ActionModule(ActionBase): ) # execute the unarchive module now, with the updated args - return self._execute_module(module_args=new_module_args, task_vars=task_vars) - + result.update(self._execute_module(module_args=new_module_args, task_vars=task_vars)) + return result diff --git a/lib/ansible/plugins/action/win_copy.py b/lib/ansible/plugins/action/win_copy.py index 54d94e12e66..caaa9927755 100644 --- a/lib/ansible/plugins/action/win_copy.py +++ b/lib/ansible/plugins/action/win_copy.py @@ -22,6 +22,7 @@ __metaclass__ = type from ansible.plugins.action import ActionBase from ansible.plugins.action.copy import ActionModule as CopyActionModule + # Even though CopyActionModule inherits from ActionBase, we still need to # directly inherit from ActionBase to appease the plugin loader. class ActionModule(CopyActionModule, ActionBase): diff --git a/lib/ansible/plugins/action/win_template.py b/lib/ansible/plugins/action/win_template.py index 03091d494f5..17c83858f1d 100644 --- a/lib/ansible/plugins/action/win_template.py +++ b/lib/ansible/plugins/action/win_template.py @@ -22,6 +22,7 @@ __metaclass__ = type from ansible.plugins.action import ActionBase from ansible.plugins.action.template import ActionModule as TemplateActionModule + # Even though TemplateActionModule inherits from ActionBase, we still need to # directly inherit from ActionBase to appease the plugin loader. class ActionModule(TemplateActionModule, ActionBase): diff --git a/lib/ansible/plugins/callback/minimal.py b/lib/ansible/plugins/callback/minimal.py index 74ca7e705e0..fcdc1750534 100644 --- a/lib/ansible/plugins/callback/minimal.py +++ b/lib/ansible/plugins/callback/minimal.py @@ -61,20 +61,26 @@ class CallbackModule(CallbackBase): if result._task.action in C.MODULE_NO_JSON: self._display.display(self._command_generic_msg(result._host.get_name(), result._result,"FAILED"), color='red') else: - self._display.display("%s | FAILED! => %s" % (result._host.get_name(), self._dump_results(result._result, indent=4)), color='red') + abridged_result = result.copy(result._result) + abridged_result.pop('invocation', None) + self._display.display("%s | FAILED! => %s" % (result._host.get_name(), self._dump_results(abridged_result, indent=4)), color='red') def v2_runner_on_ok(self, result): if result._task.action in C.MODULE_NO_JSON: self._display.display(self._command_generic_msg(result._host.get_name(), result._result,"SUCCESS"), color='green') else: - self._display.display("%s | SUCCESS => %s" % (result._host.get_name(), self._dump_results(result._result, indent=4)), color='green') + abridged_result = result.copy(result._result) + abridged_result.pop('invocation', None) + self._display.display("%s | SUCCESS => %s" % (result._host.get_name(), self._dump_results(abridged_result, indent=4)), color='green') self._handle_warnings(result._result) def v2_runner_on_skipped(self, result): self._display.display("%s | SKIPPED" % (result._host.get_name()), color='cyan') def v2_runner_on_unreachable(self, result): - self._display.display("%s | UNREACHABLE! => %s" % (result._host.get_name(), self._dump_results(result._result, indent=4)), color='yellow') + abridged_result = result.copy(result._result) + abridged_result.pop('invocation', None) + self._display.display("%s | UNREACHABLE! => %s" % (result._host.get_name(), self._dump_results(abridged_result, indent=4)), color='yellow') def v2_on_file_diff(self, result): if 'diff' in result._result and result._result['diff']: diff --git a/test/integration/roles/test_good_parsing/tasks/main.yml b/test/integration/roles/test_good_parsing/tasks/main.yml index 2297442b733..ed4a91d5603 100644 --- a/test/integration/roles/test_good_parsing/tasks/main.yml +++ b/test/integration/roles/test_good_parsing/tasks/main.yml @@ -155,11 +155,9 @@ that: - complex_param == "this is a param in a complex arg with double quotes" -#- name: test variable module name -# action: "{{ variable_module_name }} msg='this should be debugged'" -# register: result -# -#- debug: var=result +- name: test variable module name + action: "{{ variable_module_name }} msg='this should be debugged'" + register: result - name: assert the task with variable module name ran assert: diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index acf297a8a88..0e47b6a5381 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -29,9 +29,15 @@ from ansible.plugins.action import ActionBase class TestActionBase(unittest.TestCase): + class DerivedActionBase(ActionBase): + def run(self, tmp=None, task_vars=None): + # We're not testing the plugin run() method, just the helper + # methods ActionBase defines + return dict() + def test_sudo_only_if_user_differs(self): play_context = PlayContext() - action_base = ActionBase(None, None, play_context, None, None, None) + action_base = self.DerivedActionBase(None, None, play_context, None, None, None) action_base._connection = Mock(exec_command=Mock(return_value=(0, '', ''))) play_context.become = True