Add json parameter to get JSON output from ansible.command*

[cognifloyd]

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).

[arm4b]

Thanks for the PR.

As noted in Slack discussion, while for ansible-playbook there is a native "ANSIBLE_STDOUT_CALLBACK":"json" https://github.com/StackStorm-Exchange/stackstorm-ansible#structured-output.
For ad-hoc executions this doesn't work well because of the bug ansible/ansible#16194 to fix in upstream (5 lines change) https://groups.google.com/forum/#!topic/ansible-project/s5AFTqmm5i0 as well as related to it ansible/ansible#17122 which would be a consistent way to solve this.

This will help us to further support the pack, instead of adding modify/parse layers on top of stdout where the correct result is not guaranteed. So I'm more about addressing this in upstream.

[cognifloyd]

--tree seems to be the 'official' way to get machine readable output from ansible, though that option doesn't exist for ansible-playbook. The callbacks are nice in ansible-playbook, but even with the JSON callback for ansible-playbook, they want to change the callback to write to a file instead of stdout[1]. Since the folks at ansible don't want to cleanup stdout, I think we should find a way to use the more dependable (ie stable api) file output.

So, long term in stackstorm we shouldn't be depending on the ansible callback plugin to continue using stdout. Perhaps that means that we don't mess with the ansible-playbook until the change is actually made. However, I don't want to wait for the same callback plugin with ansible ad-hoc, because it is due for significant behavioral api change. At least for ansible.command and ansible.command_local actions, let's find a nice way to use ansible's --tree argument to get good json output without an entire workflow just to process the ansible output.

[1] ansible/ansible#17122 is linked to PR ansible/ansible#17448, which was closed because they don't want to guarantee clean machine parseable stdout output (quote from @mattclay in ansible/ansible#17122 (comment)):

The real issue here is that this callback plugin shouldn't be using stdout. ...
Fixing this correctly will require either mandating that all stdout output from a playbook run goes through callback plugins, or changing this plugin to write to a file instead. ...
Probably the best fix for this issue will be to add support for writing to a file, defaulting to stdout, and deprecating the use of stdout (which will then be removed in a future release).

[cognifloyd]

Hey, I just cleaned up the flake and lint warnings. What if I bagged/stashed the ansible-playbook changes, and made this PR focus only on ansible.command and ansible.command_local? @armab Do you like the idea of using --tree for those actions (I got a +1 from @lakshmi-kannan in slack)? What do you think of the general approach?

[cognifloyd]

I just trimmed this change so that it is only focused on ansible.command and ansible.command_local.

[cognifloyd]

I've tested this with:

$ st2 run ansible.command_local become=true module_name=setup json=True

Just getting stdout is still available via:

$ st2 --debug run ansible.command_local become=true module_name=setup

I'm going to add something about this to the readme. Other than that, what would it take to get this merged as 0.6.0?

[cognifloyd]

I think this is merge ready.

[cognifloyd]

Both this and #14 specify version 0.6.0. Whichever is merged second should change to 0.7.0

[arm4b]

I appreciate the efforts, but won't merge this PR before we try the right way by providing fixes in Ansible upstream (as said before #11 (comment)).
We need to be more consistent in the official pack. In a meantime you can use your own st2 pack install https://github/your/pack.

As a first step, I submitted PR to Ansible that fixed ANSIBLE_STDOUT_CALLBACK in ansible ad-hoc: ansible/ansible#26098 which was merged really quick yesterday and backported to both 2.3 and 2.2.

The next step would be adding missing functionality in json callback plugin: https://github.com/ansible/ansible/blob/67eae347cd8c80c60c65435be9dbfb4530ea4548/lib/ansible/plugins/callback/json.py#L60 to make it work for ansible adhoc.
@cognifloyd I'd be glad if you can help with that.

Having desired functionality in upstream, from the stackstorm-ansible pack I agree that adding new --json param would be a good idea.

Hope that makes sense.

[cognifloyd]

With ansible/ansible#32280, fixing the json callback plugin for adhoc command output makes sense. I didn't see a point before because @mattclay said that callbacks should be using files not stdout. ansible/ansible#32280, however shows an effort to clean up the stdout so we can actually rely on valid JSON in stdout, not just in files.

@bcoca did warn, however, that:

the only problem are verbose messages before the callback is loaded, but those are guaranteed to only be on the top so they are easily filtered out.

So, we could filter out everything before the first "{" and return that as stdout (or something like that). But that little bit of "filtering", only required when verbose is enabled, should be easy enough to maintain. :)

===========

As far as what has to be done to make the JSON callback work with ansible-adhoc (I'm making notes partially for my own benefit):

This ansible/ansible#26098 (comment) points out at least one method that needs to be adjusted. Changing v2_runner_on_ok also covers v2_runner_on_failed, v2_runner_on_unreachable, and v2_runner_on_skipped, as each of those merely reuse v2_runner_on_ok.

    def v2_runner_on_ok(self, result, **kwargs):
        host = result._host
        self.results[-1]['tasks'][-1]['hosts'][host.name] = result._result

(lines 58-60 of callback/json.py)

The issue here is that it saves the results but never prints them out.

Looking at CallbackBase, it seems like these are the v2 methods that the JSON callback doesn't currently implement/override. Ansible-adhoc uses minimal or oneline by default, but only one method is missing from the json callback that is in minimal (v2_runner_on_file_diff is in bold). Those that are in default, but not json are in italics.

• v2_on_any
• v2_runner_on_no_hosts
• v2_runner_on_async_poll
• v2_runner_on_async_ok
• v2_runner_on_async_failed
• v2_runner_on_file_diff
• v2_playbook_on_start edit: now in ansible.posix.json
• v2_playbook_on_notify
• v2_playbook_on_no_hosts_matched
• v2_playbook_on_no_hosts_remaining
• v2_playbook_on_cleanup_task_start (What is a "cleanup" task?)
• v2_playbook_on_handler_task_start edit: now in ansible.posix.json
• v2_playbook_on_vars_prompt
• v2_playbook_on_setup
• v2_playbook_on_import_for_host
• v2_playbook_on_not_import_for_host
• v2_on_file_diff
• v2_playbook_on_include
• v2_runner_item_on_ok
• v2_runner_item_on_failed
• v2_runner_item_on_skipped
• v2_runner_retry

And it turns out (I had no idea!), that --tree output is actually handled by the tree callback plugin. It only includes these three methods, meaning the tree output didn't include any of the other stuff listed above: v2_runner_on_ok, v2_runner_on_failed, and v2_runner_on_unreachable.

So, though it would be cool to include information in JSON output that is currently only mentioned in the display plugin used by ansible-playbook, really, the bare minimum to get this working is to update the json callback so that it detects when it is running in a playbook. When running in a playbook, it doesn't output anything until playbook end, but with adhoc, it needs to output right away with each task (there is only one task).

In the future, it would be nice to extend it even further so that the json callback, or maybe a json_stream callback, could send the task notifications right away (like how display prints the details right away). That could be part of making ansible feel more like an ST2 action runner instead of an ST2 pack.

[cognifloyd]

OK. So the JSON callback can't do what we want because it would have to print a stream of objects instead of printing one object at the end of the run to work with adhoc commands. To remedy that, I submitted ansible/ansible#32304. Hopefully they accept it and get it in soon. Once that's in, I can follow up with JSON callback changes to support adhoc commands.

[arm4b]

@cognifloyd Cool thing!
Appreciate your efforts and in-depth research 👍

[cognifloyd]

status update:
ansible/ansible#17122 - still open and unresolved
ansible/ansible#26098 - merged (allows using json callback with ad-hoc ansible commands)
ansible/ansible#32304 - merged (allows json callback to treat ad-hoc as a playbook w/ one task and still print its output)
ansible/ansible#32280 - abandoned (supposed to cleanup stdout by delegating display to callbacks)
Also, the json callback has moved to the ansible.posix collection for ansible 2.10+

So, we can use json callback with ansible ad-hoc commands now, but the stdout might not be parseable as other things in ansible also write messages directly to stdout. The upstream response so far is: Use intermediate files. That is what this PR did, use intermediate files.

The ansible-runner might be a viable alternative to running ansible and ansible-playbook but would require rewriting everything in this pack to use it.

[CLAassistant]

All committers have signed the CLA.