Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make embedded ansible verbosity and execution_ttl work #18989

Merged
merged 5 commits into from
Jul 18, 2019

Conversation

carbonin
Copy link
Member

This PR adds what is needed to Ansible::Runner to get verbosity working correctly and also passes the option down from services and automate.

The PR also configures the AnsiblePlaybookWorkflow timeout using the execution_ttl from the service.

This seems to have been completed for playbook automate methods (albeit slightly differently) in #17476

cc @NickLaMuro @Fryguy @bzwei @tinaafitz

Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few questions, but I think this looks fine.

lib/ansible/runner.rb Outdated Show resolved Hide resolved
lib/ansible/runner.rb Outdated Show resolved Hide resolved
@carbonin carbonin force-pushed the add_more_options_to_ansible_stuff branch 2 times, most recently from 5b9bb52 to 7ff2557 Compare July 17, 2019 17:32
@carbonin carbonin changed the title [WIP] Make embedded ansible verbosity and execution_ttl work Make embedded ansible verbosity and execution_ttl work Jul 17, 2019
@carbonin carbonin removed the wip label Jul 17, 2019
@carbonin carbonin force-pushed the add_more_options_to_ansible_stuff branch from f410e3c to 6473b88 Compare July 17, 2019 22:40

response = Ansible::Runner.run_async(env_vars, extra_vars, playbook_path, :hosts => hosts, :credentials => credentials)
response = Ansible::Runner.run_async(env_vars, extra_vars, playbook_path, :hosts => hosts, :credentials => credentials, :verbosity => verbosity)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to get slightly tighter code here with something like

env_vars, extra_vars, playbook_path = options.values_at(:env_vars, :extra_vars, :playbook_path)
kw_args = options.slice(:credentials, :hosts, :verbosity)

response = Ansible::Runner.run_async(env_vars, extra_vars, playbook_path, kw_args)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/ansible/runner.rb Outdated Show resolved Hide resolved
Previously this option was being ignored and the job timeout
was always the default of 1.hour
@carbonin carbonin force-pushed the add_more_options_to_ansible_stuff branch from 6473b88 to 6d4d78c Compare July 18, 2019 20:47
@carbonin carbonin force-pushed the add_more_options_to_ansible_stuff branch from 6d4d78c to 40f33b0 Compare July 18, 2019 21:32
@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2019

Checked commits carbonin/manageiq@409c322~...40f33b0 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
13 files checked, 1 offense detected

lib/ansible/runner.rb

network_credential_id
cloud_credential_id
vault_credential_id
verbosity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabet Dance! 🕺

@Fryguy Fryguy merged commit ec14de1 into ManageIQ:master Jul 18, 2019
@Fryguy Fryguy added this to the Sprint 116 Ending Jul 22, 2019 milestone Jul 18, 2019
@Fryguy Fryguy self-assigned this Jul 18, 2019
@carbonin carbonin deleted the add_more_options_to_ansible_stuff branch August 16, 2019 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants