Add simple wrapping code for running ansible-playbook #17564

Ladas · 2018-06-11T11:26:39Z

Add simple wrapping code for running ansible-playbook

TODOs:

requires ansible core (e.g. dnf install ansible), will require ansible-runner installed
requires boto library (e.g. dnf install python-boto, I haven't tried the rpm, I am using pip),
stdout_callback = json in .ansible.cfg, to get json output out of the runs
make path to ansible-playbook/ansible-runner bin configurable, e.g. for dev setup I am pointing it to my virtual-env (I am not able to run it other way, due to having python3 and other libs installed, the ec2 modules throw conflicts)

Implements:
https://bugzilla.redhat.com/show_bug.cgi?id=1588189

Add simple wrapping code for running ansible-playbook

Ladas · 2018-06-11T11:38:23Z

@agrare @Fryguy @martinpovolny @gtanzillo so with this simple example, I was able to replace AWS power actions ManageIQ/manageiq-providers-amazon#453

Now we need to make sure the packages are installed and we get the stdout_callback = json. Who can help with this?

And I need to add configurable ansible bin path, since at least for dev, the rpms don't work for me.

agrare · 2018-06-11T13:22:06Z

app/models/ansible/runner.rb

@@ -0,0 +1,27 @@
+module Ansible
+  class Runner


Hm my only problem with this naming is that this is the pattern we use for workers and could be confusing, @Fryguy thoughts?

also, I think I should move this under lib/

agrare · 2018-06-11T13:27:56Z

And I need to add configurable ansible bin path, since at least for dev, the rpms don't work for me.

👍 a setting for the exact path would be good for the appliance but for dev I would have expected to just be able to pull this from PATH

Ladas · 2018-06-12T07:10:05Z

@agrare I've added the brakeman issue to ignore, only possible user facing values are the extra_vars, and those are being JSON.dumped, so it should be safe, right? I did add shellescape to all other values, just for sure. (Is there better way to get rid of the brakeman false positive?)

Ladas · 2018-06-12T07:13:32Z

lib/ansible/runner.rb

+  class Runner
+    class << self
+      def run(env_vars, extra_vars, playbook_path)
+        run_via_cli(env_vars, extra_vars, playbook_path)


@agrare I wonder if allowing any playbook_path is too much? Maybe we should pass ems and limit it to ems/ansible/ dir?

@Ladas you mean from a security standpoint or usability? I think we'll want to keep this generic, in the provider we might want to have a helper based on the engine root path which we won't know here.

Actually what you have here is exactly what I was thinking

ok, sounds good

Fryguy · 2018-06-13T16:44:22Z

lib/ansible/runner.rb

+      end
+
+      def format_extra_vars(extra_vars)
+        "--extra-vars '#{JSON.dump(extra_vars)}'"


I think you need to shell_escape the JSON, right? what if it has a single quote in it?

Fryguy · 2018-06-13T16:48:22Z

lib/ansible/runner.rb

+      private
+
+      def run_via_cli(env_vars, extra_vars, playbook_path)
+        result = %x(#{format_env_vars(env_vars)} #{ansible_command.shellescape} #{format_extra_vars(extra_vars)} #{playbook_path.to_s.shellescape})


I think it would be better to use AwesomeSpawn here for building the CLI and executing. This way, you don't have to do any of the shell escaping or env handling yourself as AwesomeSpawn already handles it, and this can focus on just using normal hashes. AwesomeSpawn can also handle non-zero exit codes.

def run_via_cli(env_vars, extra_vars, playbook_path) result = AwesomeSpawn.run!(ansible_command, :env => env_vars, :params => [{:extra_vars => JSON.dump(extra_vars)}, playbook_path]) JSON.parse(result.output) end # no need for all of the format methods here

the AwesomeSpawn is awesome :-)

Fryguy · 2018-06-13T16:51:52Z

This is looking really nice @Ladas . For build stuff, @simaishi is your best bet. For ansible-runner needed features open issues on https://github.com/ansible/ansible-runner and ping @matburt

Ladas · 2018-06-14T06:01:31Z

@Fryguy thank you, I am already talking to @simaishi about the needed dependencies. I will track needed features from ansible-runner here #17583 (and I will fill the individual issues to ansible-runner)

miq-bot · 2018-06-14T06:01:59Z

Checked commits Ladas/manageiq@b12dbe8~...82a70c5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

lib/ansible/runner.rb

❗ - Line 16, Col 11 - Style/CommentAnnotation - Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.

Ladas · 2018-06-15T08:51:32Z

@martinpovolny ok, so I've created few API actions, so you can test the UI.

follow this PR requirements, i.e. installed RPMs ansible and boto, and the config under ~/.ansible.cfg
fetch this PR plus [WIP] Use ansible power actions manageiq-providers-amazon#453 and [WIP] Expose security_group_create ansible action manageiq-api#398
Try to run rails c examples from [WIP] Use ansible power actions manageiq-providers-amazon#453 (you will need AWS provider with us-east N.Virginia region)
If you have issues running ansible-playbook, create a virtual env and provide path here https://github.com/ManageIQ/manageiq/pull/17564/files#diff-259e0b014fb9528a44a195b1ba3025a4R17, e.g. I have there "/home/Ladas/Projects/ansible/providers_amazon/ansible/bin/ansible-playbook" and install the packages via pip in that virtual env (e.g. my local issue is that it prints boto package is missing, if I use RPMs)
Use API examples from [WIP] Use ansible power actions manageiq-providers-amazon#453, for your UI development

Notes

You can use the create_security_group API for both static dialog and dynamic. For static, there is vpc_id as string. For dynamic, do http://localhost:3001/api/providers/4/cloud_networks?expand=resources and pass :ems_ref into vpc_id.
For more complex example, first do a query for listing all AWS network providers, then use the id (4 in this case) for filtering the cloud_networks (api/providers/4/cloud_networks). So this will demo 2 connected select boxes.
The API actions are using existing product_feature identifier, once you have those pluggable, we can try to create new for special_start and special_stop of a vm

martinpovolny · 2018-06-15T10:58:22Z

The API actions are using existing product_feature identifier, once you have those pluggable, we can try to create new for special_start and special_stop of a vm

This is a missing feature in the core that I am asking for for some time.

Ladas · 2018-06-15T12:31:16Z

@martinpovolny ok, so right now, the pluggable product features are in the UI BZ https://bugzilla.redhat.com/show_bug.cgi?id=1589261, let's talk to @gtanzillo, if you want that separated to core task

agrare

LGTM this is a good building block to allow parallel teams to get started

Ladas · 2018-06-15T15:25:52Z

lib/ansible/runner.rb

+
+      def ansible_command
+        # TODO add possibility to use custom path, e.g. from virtualenv
+        "ansible-playbook"


@agrare where do you think we should expose this configuration?

E.g. I can't use this in my env, I have weird conflicts with all the data science stuff in python probably. So I have to use "ansible-playbook" from my virtualenv. The conflict is that the ansible modules complain I don't have boto and boto3 pkgs, while I have them. There are few upstream issues for this.

Hm I think this should be an appliance level setting not something that we would override per provider or per invocation. Something like Settings.ansible.runner_command?

Appliance level Settings.ansible.runner_command sounds reasonable

Per provider would make sense if we would have virtualenv per provider type, but I think we are not going that way

ManageIQ/manageiq#17564

This ENV var is not necessary and causes problems with output with newer ansible-runner versions such as 2.2.1. This env var was added in ManageIQ#21382, in order to remove the need for the ansible.cfg. However, walking further back to when the ansible.cfg was added leads us to ManageIQ#17564, which was added in order to support running ansible-playbook (_not ansible-runner_). This variable allows ansible core to return JSON, which was honored by ansible-playbook. ansible-runner takes care of wrapping the output properly for us, so we don't need it, and setting it causes us to lose the "normal" STDOUT we'd otherwise get from ansible-runner.

This ENV var is not necessary and causes problems with output with newer ansible-runner versions such as 2.2.1. This env var was added in ManageIQ#21382, in order to remove the need for the ansible.cfg. However, walking further back to when the ansible.cfg was added leads us to ManageIQ#17564, which was added in order to support running ansible-playbook (_not ansible-runner_). This variable allows ansible core to return JSON, which was honored by ansible-playbook. ansible-runner takes care of wrapping the output properly for us, so we don't need it, and setting it causes us to lose the "normal" STDOUT we'd otherwise get from ansible-runner. ansible-runner 1.4.7 seemingly ignores this value (or perhaps overrides it for its own purposes), so we don't need to set it. For the future versions of ansible-runner, it causes the problems above, so this commit removes it altogether.

Add simple wrapping code for running ansible-playbook

b12dbe8

Add simple wrapping code for running ansible-playbook

Ladas mentioned this pull request Jun 11, 2018

[WIP] Use ansible power actions ManageIQ/manageiq-providers-amazon#453

Closed

3 tasks

Ladas changed the title ~~Add simple wrapping code for running ansible-playbook~~ [WIP] Add simple wrapping code for running ansible-playbook Jun 11, 2018

miq-bot added the wip label Jun 11, 2018

agrare reviewed Jun 11, 2018

View reviewed changes

Ladas added 3 commits June 11, 2018 16:47

Move everything under class << self

a6cdf57

Move Ansible::Runner under the lib/

2463480

Shellescape the params for ansible run

2c07b2c

Ladas commented Jun 12, 2018

View reviewed changes

Fryguy reviewed Jun 13, 2018

View reviewed changes

Ladas force-pushed the ansible_operations branch from 6dc389c to b743114 Compare June 14, 2018 05:44

Use AwesomeSpawn to call the ansible command

82a70c5

Ladas force-pushed the ansible_operations branch from b743114 to 82a70c5 Compare June 14, 2018 05:47

agrare self-assigned this Jun 14, 2018

agrare added the enhancement label Jun 14, 2018

agrare added providers and removed wip labels Jun 15, 2018

agrare changed the title ~~[WIP] Add simple wrapping code for running ansible-playbook~~ Add simple wrapping code for running ansible-playbook Jun 15, 2018

agrare approved these changes Jun 15, 2018

View reviewed changes

agrare merged commit bf8ee30 into ManageIQ:master Jun 15, 2018

agrare added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 15, 2018

Ladas commented Jun 15, 2018

View reviewed changes

simaishi added a commit to simaishi/manageiq-appliance-build that referenced this pull request Jun 15, 2018

Install ansible and python-boto, needed for running ansible-playbook

9554809

ManageIQ/manageiq#17564

simaishi added a commit to simaishi/manageiq-appliance that referenced this pull request Jun 15, 2018

Add .ansible.cfg for ansible-playbook

6bc8816

ManageIQ/manageiq#17564

This was referenced Jun 15, 2018

Install ansible and python-boto for running ansible-playbook ManageIQ/manageiq-appliance-build#270

Closed

Add .ansible.cfg for ansible-playbook ManageIQ/manageiq-appliance#196

Merged

Fryguy mentioned this pull request Oct 5, 2022

Remove ANSIBLE_STDOUT_CALLBACK env var #22151

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple wrapping code for running ansible-playbook #17564

Add simple wrapping code for running ansible-playbook #17564

Ladas commented Jun 11, 2018 •

edited by carbonin

Loading

Ladas commented Jun 11, 2018

agrare Jun 11, 2018

Ladas Jun 11, 2018

agrare commented Jun 11, 2018

Ladas commented Jun 12, 2018

Ladas Jun 12, 2018 •

edited

Loading

agrare Jun 12, 2018

agrare Jun 12, 2018

Ladas Jun 12, 2018

Fryguy Jun 13, 2018

Fryguy Jun 13, 2018

Ladas Jun 14, 2018

agrare Jun 14, 2018

Fryguy commented Jun 13, 2018

Ladas commented Jun 14, 2018

miq-bot commented Jun 14, 2018

Ladas commented Jun 15, 2018 •

edited

Loading

martinpovolny commented Jun 15, 2018

Ladas commented Jun 15, 2018

agrare left a comment

Ladas Jun 15, 2018

agrare Jun 20, 2018

Ladas Jul 2, 2018

Add simple wrapping code for running ansible-playbook #17564

Add simple wrapping code for running ansible-playbook #17564

Conversation

Ladas commented Jun 11, 2018 • edited by carbonin Loading

Ladas commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrare commented Jun 11, 2018

Ladas commented Jun 12, 2018

Ladas Jun 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fryguy commented Jun 13, 2018

Ladas commented Jun 14, 2018

miq-bot commented Jun 14, 2018

Ladas commented Jun 15, 2018 • edited Loading

Notes

martinpovolny commented Jun 15, 2018

Ladas commented Jun 15, 2018

agrare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ladas commented Jun 11, 2018 •

edited by carbonin

Loading

Ladas Jun 12, 2018 •

edited

Loading

Ladas commented Jun 15, 2018 •

edited

Loading