From 93cfe73a7663cf5e5b2c97614c6c829a7ed0aaa9 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Sun, 18 Dec 2016 19:55:21 +0100 Subject: [PATCH] Report detailed error when internal remote functions fail This is a redesign in how plugins call _remote_checksum(). - _remote_stat() has been modified to report the real error as AnsiblError - Action plugin **unarchive** calls _remote_stat() directly instead of _remote_checksum() - Action plugin **unarchive** also handles the exceptions directly - Ensure get_exception() returns native text Two other action plugins, **template** and **fetch**, also do a remote checksum. In **template** we already call _remote_stat(), just like we now do for unarchive, in **fetch** we do call _remote_checksum() and we make the exact same mistake as the unarchive plugin. So that one could use a redesign as well. This fixes #19494 Before: ``` [dag@moria ansible.testing]$ ansible-playbook -v test137.yml Using /home/dag/home-made/ansible.testing/ansible.cfg as config file PLAY [localhost] ****************************************************************************************************** TASK [unarchive] ****************************************************************************************************** fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "msg": "python isn't present on the system. Unable to compute checksum"} PLAY RECAP ****************************************************************************************************** localhost : ok=0 changed=0 unreachable=0 failed=1 ``` After: ``` [dag@moria ansible.testing]$ ansible-playbook -v test137.yml Using /home/dag/home-made/ansible.testing/ansible.cfg as config file PLAY [localhost] ************************************************************************************************************* TASK [unarchive] ************************************************************************************************************* fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "msg": "Failed to get information on remote file (/tmp/): sudo: unknown user: foobar\nsudo: unable to initialize policy plugin\n"} PLAY RECAP ******************************************************************************************************************* localhost : ok=0 changed=0 unreachable=0 failed=1 ``` --- lib/ansible/plugins/action/__init__.py | 9 +++++++-- lib/ansible/plugins/action/template.py | 9 +++++---- lib/ansible/plugins/action/unarchive.py | 15 +++++++++------ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 7fa65ed6ea..d9e5b095ef 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -443,8 +443,13 @@ class ActionBase(with_metaclass(ABCMeta, object)): ) mystat = self._execute_module(module_name='stat', module_args=module_args, task_vars=all_vars, tmp=tmp, delete_remote_tmp=(tmp is None)) - if 'failed' in mystat and mystat['failed']: - raise AnsibleError('Failed to get information on remote file (%s): %s' % (path, mystat['msg'])) + if mystat.get('failed'): + msg = mystat.get('module_stderr') + if not msg: + msg = mystat.get('module_stdout') + if not msg: + msg = mystat.get('msg') + raise AnsibleError('Failed to get information on remote file (%s): %s' % (path, msg)) if not mystat['stat']['exists']: # empty might be matched, 1 should never match, also backwards compatible diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 7b1d26893d..c9a241ddd8 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -25,6 +25,7 @@ import time from ansible import constants as C from ansible.errors import AnsibleError from ansible.module_utils._text import to_bytes, to_native, to_text +from ansible.module_utils.pycompat24 import get_exception from ansible.plugins.action import ActionBase from ansible.utils.hashing import checksum_s @@ -43,8 +44,8 @@ class ActionModule(ActionBase): dest = os.path.join(dest, base) dest_stat = self._execute_remote_stat(dest, all_vars=all_vars, follow=False, tmp=tmp) - except Exception as e: - return dict(failed=True, msg=to_bytes(e)) + except AnsibleError: + return dict(failed=True, msg=to_native(get_exception())) return dest_stat['checksum'] @@ -69,9 +70,9 @@ class ActionModule(ActionBase): else: try: source = self._find_needle('templates', source) - except AnsibleError as e: + except AnsibleError: result['failed'] = True - result['msg'] = to_native(e) + result['msg'] = to_native(get_exception()) if 'failed' in result: return result diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 818d7d884c..38a7faf1a5 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -22,6 +22,7 @@ import os from ansible.errors import AnsibleError from ansible.module_utils._text import to_native +from ansible.module_utils.pycompat24 import get_exception from ansible.plugins.action import ActionBase from ansible.constants import mk_boolean as boolean @@ -79,19 +80,21 @@ class ActionModule(ActionBase): if not remote_src: try: source = self._loader.get_real_file(self._find_needle('files', source)) - except AnsibleError as e: + except AnsibleError: result['failed'] = True - result['msg'] = to_native(e) + result['msg'] = to_native(get_exception()) self._remove_tmp_path(tmp) return result - remote_checksum = self._remote_checksum(dest, all_vars=task_vars, follow=True) - if remote_checksum == '4': + try: + remote_stat = self._execute_remote_stat(dest, all_vars=task_vars, follow=True) + except AnsibleError: result['failed'] = True - result['msg'] = "python isn't present on the system. Unable to compute checksum" + result['msg'] = to_native(get_exception()) self._remove_tmp_path(tmp) return result - elif remote_checksum != '3': + + if not remote_stat['exists'] or not remote_stat['isdir']: result['failed'] = True result['msg'] = "dest '%s' must be an existing dir" % dest self._remove_tmp_path(tmp)