Skip to content

Commit

Permalink
better backwards compat handling of status
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bcoca committed Jul 6, 2017
1 parent fba7644 commit 2a041d1
Showing 6 changed files with 36 additions and 101 deletions.
6 changes: 5 additions & 1 deletion lib/ansible/executor/task_executor.py
Original file line number Diff line number Diff line change
@@ -572,7 +572,11 @@ def _evaluate_failed_when_result(result):

# 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:
6 changes: 0 additions & 6 deletions lib/ansible/module_utils/basic.py
Original file line number Diff line number Diff line change
@@ -2118,9 +2118,6 @@ def _return_formatted(self, kwargs):
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 @@ def fail_json(self, **kwargs):
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):
60 changes: 0 additions & 60 deletions lib/ansible/plugins/filter/core.py
Original file line number Diff line number Diff line change
@@ -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 @@ def filters(self):
# 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__,
}
49 changes: 23 additions & 26 deletions lib/ansible/plugins/test/core.py
Original file line number Diff line number Diff line change
@@ -27,50 +27,43 @@
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 @@ def tests(self):
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,
4 changes: 2 additions & 2 deletions test/integration/targets/git/tasks/specific-revision.yml
Original file line number Diff line number Diff line change
@@ -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
12 changes: 6 additions & 6 deletions test/units/module_utils/basic/test_exit_json.py
Original file line number Diff line number Diff line change
@@ -59,7 +59,7 @@ def test_exit_json_no_args_exits(self):
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 @@ def test_exit_json_args_exits(self):
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 @@ def test_fail_json_exits(self):
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,23 +98,23 @@ class TestAnsibleModuleExitValuesRemoved(unittest.TestCase):
dict(one=1, pwd='$ecret k3y', url='https://username:[email protected]/login/',
not_secret='following the leader', msg='here'),
dict(one=1, pwd=OMIT, url='https://username:[email protected]/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'))),
),
(
dict(username='person', password='password12345'),
dict(one=1, pwd='$ecret k3y', url='https://username:[email protected]/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'))),
),
(
dict(username='person', password='$ecret k3y'),
dict(one=1, pwd='$ecret k3y', url='https://username:$ecret [email protected]/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'))),
),
)

0 comments on commit 2a041d1

Please sign in to comment.