d0ugal / mistral-ansible-actions

Mistral Actions for calling Ansible
https://pypi.python.org/pypi/mistral-ansible-actions
4 stars 8 forks source link

Support all args of ansible and ansible-playbook via automation #4

Open fultonj opened 7 years ago

fultonj commented 7 years ago

In the short term it's easy to add extra arguments to the ansible or ansible-playbook tasks to pass to their respective commands, but it should be feasible to support all arguments if automation is used.

Attempts to do this may reference this issue.

fultonj commented 7 years ago

Proposal:

The question then becomes how to represent the stored options. Can we import the python library for ansible and ansible-playbook and make a call to their argument parsers?

d0ugal commented 7 years ago

The question then becomes how to represent the stored options. Can we import the python library for ansible and ansible-playbook and make a call to their argument parsers?

I had originally looked into using the Ansible Python API (rather than shelling out like we do now). However, the official Ansible stance on this API is:

Please note that while we make this API available it is not intended for direct consumption, it is here for the support of the Ansible command line tools. We try not to make breaking changes but we reserve the right to do so at any time if it makes sense for the Ansible toolset.

That put me off a bit :smile: Diving in and taking their argument parser would likely be possible, but would be more likely to break and comes with even less promises.

At the moment our usage looks like this:

action: ansible-playbook
input:
  playbook: /home/stack/ansible/my_playbook.yaml
  ...

I would be a bit sad if we had to then nest that further with something like...

input:
  args:
    playbook: /home/stack/ansible/my_playbook.yaml

I have two further ideas.

  1. We parse the output from ansible --help, this could be a little tricky but if it works with docopt then it might be actually really easy. While Ansible offers backwards compatibility fore the CLI, does that include the help output? Perhaps not, but it is unlikely to change unless they switch to a new CLI arg parser.
  2. We continue down the current path now, but maybe abstract it a bit more to avoid all the replication - we could perhaps use argparser and duplicate the interface. We could then add a an input named additional_args (or some better name) that is a dictionary and/or list of additional input arguements, this could then we used for any special cases or more advanced inputs that we don't directly support - I don't think we would even attempt to validate this, the user should be shown errors from the action output if the option is invalid.
d0ugal commented 7 years ago

hah, the docopt idea worked. I'd never used that library before. It is a slight abuse of it, but it works. Note I had to provide "test" as a CLI arg because a host name is required. However, the response is a dict, and if we take the keys then we know all the valid CLI options and it seems to correctly detect if they are booleans or not.

>>> import subprocess, docopt
>>> doc = subprocess.check_output("ansible --help", shell=True)
>>> docopt.docopt(doc, 'test')
{'--args': None,
 '--ask-become-pass': False,
 '--ask-pass': False,
 '--ask-su-pass': False,
 '--ask-sudo-pass': False,
 '--ask-vault-pass': False,
 '--background': None,
 '--become': False,
 '--become-method': None,
 '--become-user': None,
 '--check': False,
 '--connection': None,
 '--diff': False,
 '--extra-vars': None,
 '--forks': None,
 '--help': False,
 '--inventory-file': None,
 '--key-file': None,
 '--limit': None,
 '--list-hosts': False,
 '--module-name': None,
 '--module-path': None,
 '--new-vault-password-file': None,
 '--one-line': False,
 '--output': None,
 '--poll': None,
 '--scp-extra-args': None,
 '--sftp-extra-args': None,
 '--ssh-common-args': None,
 '--ssh-extra-args': None,
 '--su': False,
 '--su-user': None,
 '--sudo': False,
 '--sudo-user': None,
 '--syntax-check': False,
 '--timeout': None,
 '--tree': None,
 '--user': None,
 '--vault-password-file': None,
 '--verbose': False,
 '--version': False,
 '<host-pattern>': 'test'}

With this output we would have to dynamically generate the class - it all becomes quite very magical, so I am not sure this is a good idea... but it does sound like fun :smiling_imp:

fultonj commented 7 years ago

Replying to this one got me thinking. TL;DR version: Sounds awesome but is dynamically generated code something used in production Python?

Details: That docopt idea was clever. I had not seen that library before but I'd be happy to see it again. So, the question is if it should be used for this feature. It's supposed to be a reliable library for parsing the output of --help messages and that's how your using it. It's being used backwards, but it's still doing what it's supposed to be doing from an input to output perspective. So if there was a problem with it that affected mistral-ansible-actions you could still file a bug with docopt by providing input and bad output.

From there dynamically generating the class makes me think of Lisp. Sounds like lots of fun. Would you generate strings from the dict and then exec them as code? The O'Reilly Python Cookbook has an example of that. Or were you thinking of doing something with the AST? AFAICT Lispers are cool with that type of thing (not that I'm a big Lisper but I see this style promoted as a strength in Lisp books) but what about Pythonistas? Are people using Hy or do they just respect it as an experiment?

Do you see this as something defensible enough at the early stage of this project to cleaned up later or something solid enough to stand on long term? Overall, my thoughts are that it would be cool and I don't see the problem but is it something someone would hold against the project for a good reason (or just conservative paranoia)?

d0ugal commented 7 years ago

hah, so, that is an interesting question. I don't think there really is a good way to generate the full action. I didn't fully think that through.

I did notice that some of the OpenStack actions in Mistral accept kwargs - for some reason I didn't think that would be allowed. We should try that.

class AnsibleAction(base.Action):
    def __init__(self, hosts, **kwargs):
        ...

so, basically, your dict idea but without the extra nesting required in the workflow.