From 2a041d10d26d5746132061da4b874f33cb73ffb0 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 5 Jul 2017 13:50:32 -0400 Subject: [PATCH] better backwards compat handling of status restored 'rc' inspection but only when failed is not specified removed redundant changed from basic.py as task_executor already adds removed redundant filters, they are tests added aliases to tests removed from filters fixed test to new rc handling --- lib/ansible/executor/task_executor.py | 6 +- lib/ansible/module_utils/basic.py | 6 -- lib/ansible/plugins/filter/core.py | 60 ------------------- lib/ansible/plugins/test/core.py | 49 +++++++-------- .../targets/git/tasks/specific-revision.yml | 4 +- .../module_utils/basic/test_exit_json.py | 12 ++-- 6 files changed, 36 insertions(+), 101 deletions(-) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 7f8437cc00c..b828ef5701d 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -572,7 +572,11 @@ class TaskExecutor: # set the failed property if it was missing. if 'failed' not in result: - result['failed'] = False + # rc is here for backwards compatibility and modules that use it instead of 'failed' + if 'rc' in result and result['rc'] not in [0, "0"]: + result['failed'] = True + else: + result['failed'] = False # set the changed property if it was missing. if 'changed' not in result: diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 70a55b3dcca..519f1c20200 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -2118,9 +2118,6 @@ class AnsibleModule(object): def exit_json(self, **kwargs): ''' return from the module, without error ''' - if 'changed' not in kwargs: - kwargs['changed'] = False - self.do_cleanup_files() self._return_formatted(kwargs) sys.exit(0) @@ -2131,9 +2128,6 @@ class AnsibleModule(object): assert 'msg' in kwargs, "implementation error -- msg to explain the error is required" kwargs['failed'] = True - if 'changed' not in kwargs: - kwargs['changed'] = False - # add traceback if debug or high verbosity and it is missing # Note: badly named as exception, it is really always been 'traceback' if 'exception' not in kwargs and sys.exc_info()[2] and (self._debug or self._verbosity >= 3): diff --git a/lib/ansible/plugins/filter/core.py b/lib/ansible/plugins/filter/core.py index 12130444f8b..e89ca89169f 100644 --- a/lib/ansible/plugins/filter/core.py +++ b/lib/ansible/plugins/filter/core.py @@ -432,52 +432,6 @@ def extract(item, container, morekeys=None): return value -def failed(*a, **kw): - ''' Test if task result yields failed ''' - item = a[0] - if not isinstance(item, MutableMapping): - raise errors.AnsibleFilterError("|failed expects a dictionary") - rc = item.get('rc', 0) - failed = item.get('failed', False) - if rc != 0 or failed: - return True - else: - return False - - -def success(*a, **kw): - ''' Test if task result yields success ''' - return not failed(*a, **kw) - - -def changed(*a, **kw): - ''' Test if task result yields changed ''' - item = a[0] - if not isinstance(item, MutableMapping): - raise errors.AnsibleFilterError("|changed expects a dictionary") - if 'changed' not in item: - changed = False - if ( - 'results' in item and # some modules return a 'results' key - isinstance(item['results'], MutableSequence) and - isinstance(item['results'][0], MutableMapping) - ): - for result in item['results']: - changed = changed or result.get('changed', False) - else: - changed = item.get('changed', False) - return changed - - -def skipped(*a, **kw): - ''' Test if task result yields skipped ''' - item = a[0] - if not isinstance(item, MutableMapping): - raise errors.AnsibleFilterError("|skipped expects a dictionary") - skipped = item.get('skipped', False) - return skipped - - @environmentfilter def do_groupby(environment, value, attribute): """Overridden groupby filter for jinja2, to address an issue with @@ -593,20 +547,6 @@ class FilterModule(object): # array and dict lookups 'extract': extract, - # failure testing - 'failed': failed, - 'failure': failed, - 'success': success, - 'succeeded': success, - - # changed testing - 'changed': changed, - 'change': changed, - - # skip testing - 'skipped': skipped, - 'skip': skipped, - # debug 'type_debug': lambda o: o.__class__.__name__, } diff --git a/lib/ansible/plugins/test/core.py b/lib/ansible/plugins/test/core.py index 7440b2197fc..6d869c35d0f 100644 --- a/lib/ansible/plugins/test/core.py +++ b/lib/ansible/plugins/test/core.py @@ -27,50 +27,43 @@ from distutils.version import LooseVersion, StrictVersion from ansible import errors -def failed(*a, **kw): +def failed(result): ''' Test if task result yields failed ''' - item = a[0] - if not isinstance(item, MutableMapping): + if not isinstance(result, MutableMapping): raise errors.AnsibleFilterError("|failed expects a dictionary") - rc = item.get('rc', 0) - failed = item.get('failed', False) - if rc != 0 or failed: - return True - else: - return False + return result.get('failed', False) -def success(*a, **kw): +def success(result): ''' Test if task result yields success ''' - return not failed(*a, **kw) + return not failed(result) -def changed(*a, **kw): +def changed(result): ''' Test if task result yields changed ''' - item = a[0] - if not isinstance(item, MutableMapping): + if not isinstance(result, MutableMapping): raise errors.AnsibleFilterError("|changed expects a dictionary") - if 'changed' not in item: + if 'changed' not in result: changed = False if ( - 'results' in item and # some modules return a 'results' key - isinstance(item['results'], MutableSequence) and - isinstance(item['results'][0], MutableMapping) + 'results' in result and # some modules return a 'results' key + isinstance(result['results'], MutableSequence) and + isinstance(result['results'][0], MutableMapping) ): - for result in item['results']: - changed = changed or result.get('changed', False) + for res in result['results']: + if res.get('changed', False): + changed = True + break else: - changed = item.get('changed', False) + changed = result.get('changed', False) return changed -def skipped(*a, **kw): +def skipped(result): ''' Test if task result yields skipped ''' - item = a[0] - if not isinstance(item, MutableMapping): + if not isinstance(result, MutableMapping): raise errors.AnsibleFilterError("|skipped expects a dictionary") - skipped = item.get('skipped', False) - return skipped + return result.get('skipped', False) def regex(value='', pattern='', ignorecase=False, multiline=False, match_type='search'): @@ -133,13 +126,17 @@ class TestModule(object): return { # failure testing 'failed': failed, + 'failure': failed, 'succeeded': success, + 'success': success, # changed testing 'changed': changed, + 'change': changed, # skip testing 'skipped': skipped, + 'skip': skipped, # regex 'match': match, diff --git a/test/integration/targets/git/tasks/specific-revision.yml b/test/integration/targets/git/tasks/specific-revision.yml index 4daa0b1b249..6cc2c7423fd 100644 --- a/test/integration/targets/git/tasks/specific-revision.yml +++ b/test/integration/targets/git/tasks/specific-revision.yml @@ -108,11 +108,11 @@ args: chdir: '{{ checkout_dir }}' -- name: make sure the old commit was not fetched +- name: "make sure the old commit was not fetched, task is 'forced success'" assert: that: - 'checkout_shallow.rc != 0' - - checkout_shallow|failed + - checkout_shallow|success when: git_version.stdout | version_compare("{{git_version_supporting_depth}}", '>=') - name: clear checkout_dir diff --git a/test/units/module_utils/basic/test_exit_json.py b/test/units/module_utils/basic/test_exit_json.py index 5e4d626c3ab..47b3db0df75 100644 --- a/test/units/module_utils/basic/test_exit_json.py +++ b/test/units/module_utils/basic/test_exit_json.py @@ -59,7 +59,7 @@ class TestAnsibleModuleExitJson(unittest.TestCase): else: self.assertEquals(ctx.exception.code, 0) return_val = json.loads(self.fake_stream.getvalue()) - self.assertEquals(return_val, dict(changed=False, invocation=empty_invocation)) + self.assertEquals(return_val, dict(invocation=empty_invocation)) def test_exit_json_args_exits(self): with self.assertRaises(SystemExit) as ctx: @@ -70,7 +70,7 @@ class TestAnsibleModuleExitJson(unittest.TestCase): else: self.assertEquals(ctx.exception.code, 0) return_val = json.loads(self.fake_stream.getvalue()) - self.assertEquals(return_val, dict(msg="message", changed=False, invocation=empty_invocation)) + self.assertEquals(return_val, dict(msg="message", invocation=empty_invocation)) def test_fail_json_exits(self): with self.assertRaises(SystemExit) as ctx: @@ -81,7 +81,7 @@ class TestAnsibleModuleExitJson(unittest.TestCase): else: self.assertEquals(ctx.exception.code, 1) return_val = json.loads(self.fake_stream.getvalue()) - self.assertEquals(return_val, dict(msg="message", changed=False, failed=True, invocation=empty_invocation)) + self.assertEquals(return_val, dict(msg="message", failed=True, invocation=empty_invocation)) def test_exit_json_proper_changed(self): with self.assertRaises(SystemExit) as ctx: @@ -98,7 +98,7 @@ class TestAnsibleModuleExitValuesRemoved(unittest.TestCase): dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/', not_secret='following the leader', msg='here'), dict(one=1, pwd=OMIT, url='https://username:password12345@foo.com/login/', - not_secret='following the leader', changed=False, msg='here', + not_secret='following the leader', msg='here', invocation=dict(module_args=dict(password=OMIT, token=None, username='person'))), ), ( @@ -106,7 +106,7 @@ class TestAnsibleModuleExitValuesRemoved(unittest.TestCase): dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/', not_secret='following the leader', msg='here'), dict(one=1, pwd='$ecret k3y', url='https://username:********@foo.com/login/', - not_secret='following the leader', changed=False, msg='here', + not_secret='following the leader', msg='here', invocation=dict(module_args=dict(password=OMIT, token=None, username='person'))), ), ( @@ -114,7 +114,7 @@ class TestAnsibleModuleExitValuesRemoved(unittest.TestCase): dict(one=1, pwd='$ecret k3y', url='https://username:$ecret k3y@foo.com/login/', not_secret='following the leader', msg='here'), dict(one=1, pwd=OMIT, url='https://username:********@foo.com/login/', - not_secret='following the leader', changed=False, msg='here', + not_secret='following the leader', msg='here', invocation=dict(module_args=dict(password=OMIT, token=None, username='person'))), ), )