From 8c2d8457cdfb72c92824e54fcfad2d054172b6d4 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 23 Jun 2017 23:11:16 -0500 Subject: [PATCH 1/2] Fix JSON quoting bug Quotes around the extra-vars JSON only make sense in a shell. Shells strip those quotes off and pass the whole thing in to the application. Ansible couldn't parse or understand the JSON with the quotes in place, so remove them and fix the tests accordingly. --- actions/lib/ansible_base.py | 2 +- tests/test_actions_lib_ansiblebaserunner.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/lib/ansible_base.py b/actions/lib/ansible_base.py index 9b33539..4fee018 100755 --- a/actions/lib/ansible_base.py +++ b/actions/lib/ansible_base.py @@ -64,7 +64,7 @@ def _parse_extra_vars(self): # Add --extra-vars for each json object elif t == 'json': - self.args.append("--extra-vars='{0}'".format(json.dumps(v))) + self.args.append("--extra-vars={0}".format(json.dumps(v))) # Combine contiguous kwarg vars into a single space-separated --extra-vars kwarg elif t == 'kwarg' and last != t: diff --git a/tests/test_actions_lib_ansiblebaserunner.py b/tests/test_actions_lib_ansiblebaserunner.py index 924fc16..1839c7e 100644 --- a/tests/test_actions_lib_ansiblebaserunner.py +++ b/tests/test_actions_lib_ansiblebaserunner.py @@ -86,7 +86,7 @@ def extra_vars_json_yaml_fixture(self, test_name): test_yaml = self.load_yaml('extra_vars_json.yaml') test = next(t for t in test_yaml if t['name'] == test_name) case = test['test'] - expected = ['--extra-vars=\'{}\''.format(json.dumps(e)) for e in case] + expected = ['--extra-vars={}'.format(json.dumps(e)) for e in case] self.check_arg_parse(arg, case, expected) def test_parse_extra_vars_json_yaml_dict(self): @@ -115,7 +115,7 @@ def extra_vars_complex_yaml_fixture(self, test_name): # this does not preserve the order exactly, but it shows that elements are correctly parsed expected = ['--extra-vars={}'.format(e) for e in test['expected'] if isinstance(e, six.string_types)] - expected.extend(['--extra-vars=\'{}\''.format(json.dumps(e)) + expected.extend(['--extra-vars={}'.format(json.dumps(e)) for e in test['expected'] if isinstance(e, dict)]) self.check_arg_parse(arg, case, expected) From 20e652733847baa6740449d1d0e17e1f4b1b57a9 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 21 Jun 2017 05:56:58 -0500 Subject: [PATCH 2/2] v0.6.0: json parameter to get JSON for ad-hoc cmds This adds a json parameter to ansible.command and ansible.command_local. Using this parameter ensures that the stdout from a command is valid JSON. This is especially helpful to make variables/facts from the setup module (for example) available in an st2 workflow. To make this work, we depend on ansible's --tree parameter to save the output for each node in the 'tree' directory. We ignore all of ansible's stdout, and instead feed the contents of all of the tree/node files in one big json object (key is node name, and value is the output). Bump to 0.6.0 Signed-off-by: Jacob Floyd --- CHANGES.md | 28 ++++++++++++++++++++ README.md | 46 ++++++++++++++++++++++++++++++++ actions/ansible.py | 53 ++++++++++++++++++++++++++++++++++++- actions/command.yaml | 3 +++ actions/command_local.yaml | 3 +++ actions/lib/ansible_base.py | 30 ++++++++++++++++++++- pack.yaml | 2 +- 7 files changed, 162 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8c967a9..afb08b5 100755 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,33 @@ # Changelog +## v0.6.0 + +* Add a json parameter to `ansible.command` and `ansible.command_local` actions. When False (the default), stdout is not changed. When True, this replaces ansible's stdout with a valid JSON object. We do this by using ansible's `--tree` argument to save the output to a temporary directory, and then sending a json object where the node name is the key, and the ansible output is the value. + +``` +$ st2 run ansible.command_local become=true module_name=setup json=True +. +id: 594d6657c4da5f08e9ec7c51 +status: succeeded +parameters: + become: true + json: true + module_name: setup +result: + failed: false + return_code: 0 + stderr: '' + stdout: + 127.0.0.1: + ansible_facts: + ansible_all_ipv4_addresses: + ... + changed: false + succeeded: true + +``` + + ## v0.5.0 * Added ability to use yaml structures to pass arbitrarily complex values through extra_vars. key=value and @file syntax is still supported. Example usage: diff --git a/README.md b/README.md index 75e537e..45cce90 100755 --- a/README.md +++ b/README.md @@ -34,6 +34,52 @@ ansible all -c local -i '127.0.0.1,' -a 'echo $TERM' ansible all --connection=local --inventory-file='127.0.0.1,' --args='echo $TERM' ``` +##### structured output from `ansible.command` and `ansible.command_local`: + +To get a JSON object back from `ansible.command*`, set the json parameter to True. This uses ansible's `--tree` output to generate a JSON object with one element per node: `{"node-name": node-ouput}` + + +``` +$ st2 run ansible.command_local become=true module_name=setup json=True +. +id: 594d6657c4da5f08e9ec7c51 +status: succeeded +parameters: + become: true + json: true + module_name: setup +result: + failed: false + return_code: 0 + stderr: '' + stdout: + 127.0.0.1: + ansible_facts: + ansible_all_ipv4_addresses: + ... + changed: false + succeeded: true +``` + +With this structured output, you could use the setup module as the first step of a workflow, and then base additional workflow steps on variables from ansible's setup. It should work similarly for other modules. + +For instance, if you needed the default ipv4 address of a node, you could publish the appropriate ansible_fact like this (in an action-chain workflow): + +```yaml +chain: + - + name: ansible_setup + ref: ansible.command_local + parameters: + become: True + json: True + module_name: setup + publish: + default_ip: "{{ ansible_setup.stdout['127.0.0.1'].ansible_facts.ansible_default_ipv4.address }}" +``` + + + #### `ansible.playbook` examples ```sh # run some simple playbook diff --git a/actions/ansible.py b/actions/ansible.py index d9fd2ba..2dac61a 100755 --- a/actions/ansible.py +++ b/actions/ansible.py @@ -1,7 +1,13 @@ #!/usr/bin/env python +from __future__ import print_function + +import json +import os +import shutil import sys -from lib.ansible_base import AnsibleBaseRunner +import tempfile +from lib.ansible_base import AnsibleBaseRunner, ParamaterConflict __all__ = [ 'AnsibleRunner' @@ -32,8 +38,53 @@ class AnsibleRunner(AnsibleBaseRunner): } def __init__(self, *args, **kwargs): + self.tree_dir = None + self.one_line = False + if '--one_line' in args: + self.one_line = True super(AnsibleRunner, self).__init__(*args, **kwargs) + def handle_json_arg(self): + if next((True for arg in self.args if arg.startswith('--tree')), False): + msg = "--json uses --tree internally. Setting both --tree and --json is not supported." + raise ParamaterConflict(msg) + execution_id = os.environ.get('ST2_ACTION_EXECUTION_ID', 'EXECUTION_ID_NA') + self.tree_dir = tempfile.mkdtemp(prefix='{}.'.format(execution_id)) + + tree_arg = '--tree={}'.format(self.tree_dir) + self.args.append(tree_arg) + + # This sends all ansible stdout to /dev/null - if there's anything in there that's not in + # the --tree output, then it will be lost. Hopefully ansible doesn't print anything truly + # important... If something breaks, I guess we'll just have to run it without --json + # to see what is going on. + self.stdout = open(os.devnull, 'w') + + def output_json(self): + output = {} + for host in os.listdir(self.tree_dir): + # one file per host in tree dir; name of host is name of file + with open(os.path.join(self.tree_dir, host), 'r') as host_output: + try: + output[host] = json.load(host_output) + except ValueError: + # something is messed up in the json, so include it as a string. + host_output.seek(0) + output[host] = host_output.read() + if self.one_line: + print(json.dumps(output)) + else: + print(json.dumps(output, indent=2)) + + def cleanup(self): + shutil.rmtree(self.tree_dir) + self.stdout.close() + + def post_execute(self): + if self.json_output: + self.output_json() + self.cleanup() + if __name__ == '__main__': AnsibleRunner(sys.argv).execute() diff --git a/actions/command.yaml b/actions/command.yaml index 23f13e2..07da139 100755 --- a/actions/command.yaml +++ b/actions/command.yaml @@ -105,3 +105,6 @@ parameters: version: description: "Show ansible version number and exit" type: boolean + json: + description: "Clean up Ansible's output to ensure it is valid jSON" + type: boolean diff --git a/actions/command_local.yaml b/actions/command_local.yaml index 2bdd22e..ea89002 100755 --- a/actions/command_local.yaml +++ b/actions/command_local.yaml @@ -105,3 +105,6 @@ parameters: version: description: "Show ansible version number and exit" type: boolean + json: + description: "Clean up Ansible's output to ensure it is valid jSON" + type: boolean diff --git a/actions/lib/ansible_base.py b/actions/lib/ansible_base.py index 4fee018..755a042 100755 --- a/actions/lib/ansible_base.py +++ b/actions/lib/ansible_base.py @@ -23,7 +23,11 @@ def __init__(self, args): :param args: Input command line arguments :type args: ``list`` """ + self.json_output = False + self.stdout = None + self.args = args[1:] + self._json_output_arg() self._parse_extra_vars() # handle multiple entries in --extra_vars arg self._prepend_venv_path() @@ -96,17 +100,37 @@ def _prepend_venv_path(): os.environ['PATH'] = ':'.join(new_path) + def _json_output_arg(self): + for i, arg in enumerate(self.args): + if '--json' in arg: + self.json_output = True + self.handle_json_arg() + # if ansible-playbook, add env arg + del self.args[i] # The json arg is a ST2 specific addition & should not pass on. + break + elif '--one_line' in arg: + self.one_line = True + + def handle_json_arg(self): + pass + def execute(self): """ Execute the command and stream stdout and stderr output from child process as it appears without delay. Terminate with child's exit code. """ - exit_code = subprocess.call(self.cmd, env=os.environ.copy()) + exit_code = subprocess.call(self.cmd, env=os.environ.copy(), stdout=self.stdout) if exit_code is not 0: sys.stderr.write('Executed command "%s"\n' % ' '.join(self.cmd)) + + self.post_execute() + sys.exit(exit_code) + def post_execute(self): + pass + @property @shell.replace_args('REPLACEMENT_RULES') def cmd(self): @@ -139,3 +163,7 @@ def binary(self): sys.exit(1) return binary_path + + +class ParamaterConflict(Exception): + pass diff --git a/pack.yaml b/pack.yaml index b50e5a6..fa43501 100644 --- a/pack.yaml +++ b/pack.yaml @@ -6,6 +6,6 @@ keywords: - ansible - cfg management - configuration management -version : 0.5.1 +version : 0.6.0 author : StackStorm, Inc. email : info@stackstorm.com