From 26cdddaebf20b8ae0be0f1e75637def26507b98e Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Thu, 11 Sep 2014 12:26:54 -0400 Subject: [PATCH] Tracebacks are now catchable with ignore_errors and have streamlined output. Also removes 'baby-JSON' for bash modules. --- CHANGELOG.md | 4 ++- lib/ansible/callbacks.py | 2 +- lib/ansible/inventory/script.py | 8 +++++- lib/ansible/runner/__init__.py | 2 +- lib/ansible/runner/return_data.py | 2 +- lib/ansible/utils/__init__.py | 26 +++---------------- library/system/ping | 4 +++ test/units/TestUtils.py | 24 +++++------------ .../inventory_test_data/inventory_api.py | 7 +++-- 9 files changed, 32 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba09bc04517..62850280c89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ Ansible Changes By Release ## 1.8 "You Really Got Me" - Active Development -New core features: +Major changes: * fact caching support, pluggable, initially supports Redis (DOCS pending) * 'serial' size in a rolling update can be specified as a percentage @@ -15,6 +15,7 @@ New core features: * ansible-galaxy install -f requirements.yml allows advanced options and installs from non-galaxy SCM sources and tarballs. * command_warnings feature will warn about when usage of the shell/command module can be simplified to use core modules - this can be enabled in ansible.cfg * new omit value can be used to leave off a parameter when not set, like so module_name: a=1 b={{ c | default(omit) }}, would not pass value for b (not even an empty value) if c was not set. +* developers: 'baby JSON' in module responses, originally intended for writing modules in bash, is removed as a feature to simplify logic, script module remains available for running bash scripts. New Modules: @@ -30,6 +31,7 @@ New Modules: Some other notable changes: +* if a module should ever traceback, it will return a standard error, catchable by ignore_errors, versus an 'unreachable' * ec2_lc: added support for multiple new parameters like kernel_id, ramdisk_id and ebs_optimized. * ec2_elb_lb: added support for the connection_draining_timeout and cross_az_load_balancing options. * support for symbolic representations (ie. u+rw) for file permission modes (file/copy/template modules etc.). diff --git a/lib/ansible/callbacks.py b/lib/ansible/callbacks.py index ca78b3adfb4..95269bd145b 100644 --- a/lib/ansible/callbacks.py +++ b/lib/ansible/callbacks.py @@ -493,7 +493,7 @@ class PlaybookRunnerCallbacks(DefaultRunnerCallbacks): if returned_msg: display("msg: %s" % returned_msg, color='red', runner=self.runner) if not parsed and module_msg: - display("invalid output was: %s" % module_msg, color='red', runner=self.runner) + display(module_msg, color='red', runner=self.runner) if ignore_errors: display("...ignoring", color='cyan', runner=self.runner) super(PlaybookRunnerCallbacks, self).on_failed(host, results, ignore_errors=ignore_errors) diff --git a/lib/ansible/inventory/script.py b/lib/ansible/inventory/script.py index e43cf249724..723089db88f 100644 --- a/lib/ansible/inventory/script.py +++ b/lib/ansible/inventory/script.py @@ -138,4 +138,10 @@ class InventoryScript(object): except OSError, e: raise errors.AnsibleError("problem running %s (%s)" % (' '.join(cmd), e)) (out, err) = sp.communicate() - return utils.parse_json(out) + if out.strip() == '': + return dict() + try: + return utils.parse_json(out) + except ValueError: + raise errors.AnsibleError("could not parse post variable response: %s, %s" % (cmd, out)) + diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 16713dfa9ac..54ee93a8d77 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -539,7 +539,7 @@ class Runner(object): cmd2 = conn.shell.remove(tmp, recurse=True) self._low_level_exec_command(conn, cmd2, tmp, sudoable=False) - data = utils.parse_json(res['stdout'], from_remote=True) + data = utils.parse_json(res['stdout'], from_remote=True, no_exceptions=True) if 'parsed' in data and data['parsed'] == False: data['msg'] += res['stderr'] return ReturnData(conn=conn, result=data) diff --git a/lib/ansible/runner/return_data.py b/lib/ansible/runner/return_data.py index 5b99c458985..2656ae1a6f0 100644 --- a/lib/ansible/runner/return_data.py +++ b/lib/ansible/runner/return_data.py @@ -43,7 +43,7 @@ class ReturnData(object): self.diff = diff if type(self.result) in [ str, unicode ]: - self.result = utils.parse_json(self.result, from_remote=True) + self.result = utils.parse_json(self.result, from_remote=True, exceptions=False) if self.host is None: raise Exception("host not set") diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index 98b7f2f36b9..861053da20d 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -506,7 +506,7 @@ def _clean_data_struct(orig_data, from_remote=False, from_inventory=False): data = orig_data return data -def parse_json(raw_data, from_remote=False, from_inventory=False): +def parse_json(raw_data, from_remote=False, from_inventory=False, no_exceptions=False): ''' this version for module return data only ''' orig_data = raw_data @@ -517,28 +517,10 @@ def parse_json(raw_data, from_remote=False, from_inventory=False): try: results = json.loads(data) except: - # not JSON, but try "Baby JSON" which allows many of our modules to not - # require JSON and makes writing modules in bash much simpler - results = {} - try: - tokens = shlex.split(data) - except: - print "failed to parse json: "+ data + if no_exceptions: + return dict(failed=True, parsed=False, msg=raw_data) + else: raise - for t in tokens: - if "=" not in t: - raise errors.AnsibleError("failed to parse: %s" % orig_data) - (key,value) = t.split("=", 1) - if key == 'changed' or 'failed': - if value.lower() in [ 'true', '1' ]: - value = True - elif value.lower() in [ 'false', '0' ]: - value = False - if key == 'rc': - value = int(value) - results[key] = value - if len(results.keys()) == 0: - return { "failed" : True, "parsed" : False, "msg" : orig_data } if from_remote: results = _clean_data_struct(results, from_remote, from_inventory) diff --git a/library/system/ping b/library/system/ping index 4bda3971504..b098d0054cd 100644 --- a/library/system/ping +++ b/library/system/ping @@ -37,6 +37,8 @@ EXAMPLES = ''' ansible webservers -m ping ''' +import exceptions + def main(): module = AnsibleModule( argument_spec = dict( @@ -46,6 +48,8 @@ def main(): ) result = dict(ping='pong') if module.params['data']: + if module.params['data'] == 'crash': + raise exceptions.Exception("boom") result['ping'] = module.params['data'] module.exit_json(**result) diff --git a/test/units/TestUtils.py b/test/units/TestUtils.py index 59688a0acce..58535f0488f 100644 --- a/test/units/TestUtils.py +++ b/test/units/TestUtils.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import traceback import unittest import os import os.path @@ -217,36 +218,23 @@ class TestUtils(unittest.TestCase): # leading junk self.assertEqual(ansible.utils.parse_json('ansible\n{"foo": "bar"}'), dict(foo="bar")) - # "baby" json - self.assertEqual(ansible.utils.parse_json('foo=bar baz=qux'), dict(foo='bar', baz='qux')) - # No closing quotation try: - ansible.utils.parse_json('foo=bar "') + rc = ansible.utils.parse_json('foo=bar "') + print rc except ValueError: pass else: + traceback.print_exc() raise AssertionError('Incorrect exception, expected ValueError') # Failed to parse try: ansible.utils.parse_json('{') - except ansible.errors.AnsibleError: + except ValueError: pass else: - raise AssertionError('Incorrect exception, expected ansible.errors.AnsibleError') - - # boolean changed/failed - self.assertEqual(ansible.utils.parse_json('changed=true'), dict(changed=True)) - self.assertEqual(ansible.utils.parse_json('changed=false'), dict(changed=False)) - self.assertEqual(ansible.utils.parse_json('failed=true'), dict(failed=True)) - self.assertEqual(ansible.utils.parse_json('failed=false'), dict(failed=False)) - - # rc - self.assertEqual(ansible.utils.parse_json('rc=0'), dict(rc=0)) - - # Just a string - self.assertEqual(ansible.utils.parse_json('foo'), dict(failed=True, parsed=False, msg='foo')) + raise AssertionError('Incorrect exception, expected ValueError') def test_parse_yaml(self): #json diff --git a/test/units/inventory_test_data/inventory_api.py b/test/units/inventory_test_data/inventory_api.py index 0510862a10a..9bdca22ed30 100644 --- a/test/units/inventory_test_data/inventory_api.py +++ b/test/units/inventory_test_data/inventory_api.py @@ -34,8 +34,11 @@ if options.host is not None: if options.extra: k,v = options.extra.split("=") variables[options.host][k] = v - print json.dumps(variables[options.host]) + if options.host in variables: + print json.dumps(variables[options.host]) + else: + print "{}" sys.exit(0) parser.print_help() -sys.exit(1) \ No newline at end of file +sys.exit(1)