Directly run a playbook #16161
Directly run a playbook #16161
Conversation
|
@tinaafitz please test. |
|
@miq-bot add_label wip |
bzwei
force-pushed
the
ansible_runner
branch
2 times, most recently
from
98f6e89 to
bba528e
Compare
|
|
||
| private | ||
|
|
||
| def build_parameter_list(options) |
There was a problem hiding this comment.
This method is largely identical to the same method in ServiceTemplateAnsiblePlaybook. That code should be shared.
There was a problem hiding this comment.
Yes. I planned to have another PR to reuse code introduced in this PR.
|
|
||
| if options[:extra_vars] | ||
| params[:extra_vars] = options[:extra_vars].transform_values do |val| | ||
| val.kind_of?(String) ? val : val[:default] # TODO: support Hash only |
There was a problem hiding this comment.
The call to transform_values nils out the manageiq key in :extra_vars. We found options[:extra_vars].deep_stringify_keys!.to_json off the raw :extra_vars hash solves the issue.
There was a problem hiding this comment.
This is old code. I am thinking of simplifying or refactoring this part. Thanks for the input.
| if tower_job.raw_status.completed? | ||
| queue_signal(:finish) | ||
| else | ||
| queue_signal(:poll_ansible_tower_job_status, :deliver_on => Time.now.utc + 1.minute) |
There was a problem hiding this comment.
At a minimal, if no inventory is passed in - each Runner 'Run' will take 3 minutes. Is the 1 minute time here to guarantee the synchronous saves complete?
There was a problem hiding this comment.
No. This is to requeue itself if the tower job is not completed. And yes, if the playbook runs quick enough, the 1 minute waiting time seems too long. But what's the right interval for best user experience?
| if hosts == 'localhost' | ||
| tower.provider.default_inventory | ||
| else | ||
| inventory_name = "#{playbook.name}_#{Time.zone.now.to_i}" |
There was a problem hiding this comment.
Building out a shared naming method for Inventory and JobTemplate would make sense.
bzwei
force-pushed
the
ansible_runner
branch
5 times, most recently
from
9a73d87 to
fa6a616
Compare
| def self.create_inventory_raw(tower, inventory_name, hosts) | ||
| miq_org = tower.provider.default_organization | ||
| tower.with_provider_connection do |connection| | ||
| connection.api.inventories.create!(:name => inventory_name, :organization => miq_org).tap do |inventory| |
There was a problem hiding this comment.
@bzwei Is this a quick process?
What happens if the name already exists, would we have to report that?
| options[:playbook_id] = id | ||
| options[:userid] = userid || 'system' | ||
| options[:name] = "Playbook: #{name}" | ||
| miq_job = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::PlaybookRunner.create_job(options) |
There was a problem hiding this comment.
@bzwei
Do we need try to make sure we have a miq_job?
There was a problem hiding this comment.
I don't think so. create_job is a straight forward method.
|
|
||
| # return provider raw object | ||
| def create_job_template_raw(options) | ||
| job_template_klass = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ConfigurationScript |
There was a problem hiding this comment.
@bzwei can we make this a constant?
| def create_inventory | ||
| set_status('creating inventory') | ||
| tower = playbook.manager | ||
| hosts = options[:hosts] || options.fetch_path(:extra_vars, :hosts) |
There was a problem hiding this comment.
@bzwei Should this default to localhost if its not available in either the config options or in the method input parameters (coming via extra vars). So if someone wants to run it on localhost they dont have to add a hosts method input parameter.
There was a problem hiding this comment.
It has been taken cared of. Yes, if the user does not provide any hosts, default (localhost) is used in https://github.com/ManageIQ/manageiq/pull/16161/files#diff-713a4162657cdeb3c36e2194e0181ab7R36
| queue_signal(:post_ansible_run, err.message, 'error') | ||
| end | ||
|
|
||
| def poll_ansible_tower_job_status(interval) |
There was a problem hiding this comment.
@bzwei Minor, but can we swap the order the methods are listed in the file to match the "happy path" the methods would take. This would mean swapping the order that launch_ansible_tower_job and poll_ansible_tower_job_status appear in this file.
Also, fix the rubocop warning on line 57 with the extra space.
| if tower_job_status.succeeded? | ||
| queue_signal(:post_ansible_run, 'Playbook ran successfully', 'ok') | ||
| else | ||
| queue_signal(:post_ansible_run, 'Playbook failed at Ansible engine' , 'error') |
There was a problem hiding this comment.
@bzwei This message seems incorrect. Could it say "Ansible Job failed"
|
|
||
| tower_job_status = tower_job.raw_status | ||
| if tower_job_status.completed? | ||
| tower_job.refresh_ems |
There was a problem hiding this comment.
@bzwei is this a synchronous refresh. Should it be a different state in the state machine
There was a problem hiding this comment.
No need. It is very quick, just updates the job record in our DB.
| if tower_job_status.succeeded? | ||
| queue_signal(:post_ansible_run, 'Playbook ran successfully', 'ok') | ||
| else | ||
| queue_signal(:post_ansible_run, 'Ansible engine returns an error for the job', 'error') |
There was a problem hiding this comment.
@bzwei returned instead of returns.
A playbook can be run directly from the Playbook instance. It calls an instance of PlaybooRunner which is a state machine.
|
Checked commit bzwei@fdc32bd with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 app/models/manageiq/providers/embedded_ansible/automation_manager/playbook_runner.rb
|
| :ask_credential_on_launch => true, | ||
| :limit => options[:limit], | ||
| :inventory => options[:inventory] || manager.provider.default_inventory, | ||
| :extra_vars => options[:extra_vars].try(:to_json) |
There was a problem hiding this comment.
@bzwei - have you been able to verify that this call will deep stringify nested hashes? We had issues with testing where the :manageiq key would be removed when we called to_json on the :extra_vars hash. -cc @mkanoor - To clarify - to_json works if all the keys are strings, it fails if there is a nested key that is passed as a symbol.
There was a problem hiding this comment.
I cannot reproduce what you observed. For example:
irb(main):007:0> {:test => {:manageiq => {'a' => 'x'}}}.to_json
=> "{\"test\":{\"manageiq\":{\"a\":\"x\"}}}"
irb(main):008:0> JSON.parse(_)
=> {"test"=>{"manageiq"=>{"a"=>"x"}}}
Replaces #16060
Usage:
ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Playbook#run(options, userid)The return is a task id that you can use to track the playbook execution progress.
Has dependency on ManageIQ/manageiq-providers-ansible_tower#25