Merge pull request #18946 from carbonin/pass_hosts_to_runner_from_ser…

…vices

Pass hosts to runner from services

app/models/manageiq/providers/ansible_playbook_workflow.rb

@@ -1,8 +1,9 @@
 class ManageIQ::Providers::AnsiblePlaybookWorkflow < ManageIQ::Providers::AnsibleRunnerWorkflow
-  def self.job_options(env_vars, extra_vars, playbook_options, timeout, poll_interval)
+  def self.job_options(env_vars, extra_vars, playbook_options, timeout, poll_interval, hosts)
     {
       :env_vars      => env_vars,
       :extra_vars    => extra_vars,
+      :hosts         => hosts,
       :playbook_path => playbook_options[:playbook_path],
       :timeout       => timeout,
       :poll_interval => poll_interval,
@@ -15,9 +16,9 @@ def pre_playbook
   end
 
   def run_playbook
-    env_vars, extra_vars, playbook_path = options.values_at(:env_vars, :extra_vars, :playbook_path)
+    env_vars, extra_vars, hosts, playbook_path = options.values_at(:env_vars, :extra_vars, :hosts, :playbook_path)
 
-    response = Ansible::Runner.run_async(env_vars, extra_vars, playbook_path)
+    response = Ansible::Runner.run_async(env_vars, extra_vars, playbook_path, :hosts => hosts)
     if response.nil?
       queue_signal(:abort, "Failed to run ansible playbook", "error")
     else

app/models/manageiq/providers/ansible_role_workflow.rb

@@ -1,8 +1,9 @@
 class ManageIQ::Providers::AnsibleRoleWorkflow < ManageIQ::Providers::AnsibleRunnerWorkflow
-  def self.job_options(env_vars, extra_vars, role_options, timeout, poll_interval)
+  def self.job_options(env_vars, extra_vars, role_options, timeout, poll_interval, hosts)
     {
       :env_vars        => env_vars,
       :extra_vars      => extra_vars,
+      :hosts           => hosts,
       :role_name       => role_options[:role_name],
       :roles_path      => role_options[:roles_path],
       :role_skip_facts => role_options[:role_skip_facts],

app/models/manageiq/providers/ansible_runner_workflow.rb

@@ -1,6 +1,6 @@
 class ManageIQ::Providers::AnsibleRunnerWorkflow < Job
-  def self.create_job(env_vars, extra_vars, role_or_playbook_options, timeout: 1.hour, poll_interval: 1.second)
-    super(name, job_options(env_vars, extra_vars, role_or_playbook_options, timeout, poll_interval))
+  def self.create_job(env_vars, extra_vars, role_or_playbook_options, hosts = "localhost", timeout: 1.hour, poll_interval: 1.second)
+    super(name, job_options(env_vars, extra_vars, role_or_playbook_options, timeout, poll_interval, hosts))
   end
 
   def current_job_timeout(_timeout_adjustment = 1)

app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script.rb

@@ -33,7 +33,7 @@ def run(vars = {})
     extra_vars    = merge_extra_vars(vars[:extra_vars])
     playbook_vars = { :playbook_path => parent.path }
 
-    workflow.create_job({}, extra_vars, playbook_vars).tap do |job|
+    workflow.create_job({}, extra_vars, playbook_vars, vars[:hosts]).tap do |job|
       job.signal(:start)
     end
   end

app/models/service_ansible_playbook.rb

@@ -21,12 +21,12 @@ def execute(action)
         'manageiq_connection' => manageiq_connection_env(evm_owner)
       }
     )
-    hosts = opts.delete(:hosts)
+    opts[:hosts] = hosts_array(opts.delete(:hosts))
 
     _log.info("Launching Ansible Tower job with options:")
     $log.log_hashes(opts)
     new_job = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Job.create_job(jt, decrypt_options(opts))
-    update_job_for_playbook(action, new_job, hosts)
+    update_job_for_playbook(action, new_job, opts[:hosts])
 
     _log.info("Ansible Tower job with ref #{new_job.ems_ref} was created.")
     add_resource!(new_job, :name => action)
@@ -53,17 +53,11 @@ def job(action)
     service_resources.find_by(:name => action, :resource_type => 'OrchestrationStack').try(:resource)
   end
 
-  def postprocess(action)
-    inventory_raw_id = options.fetch_path(job_option_key(action), :inventory)
-    delete_inventory(action, inventory_raw_id) if inventory_raw_id
-    log_stdout(action)
-  end
-
   def on_error(action)
     _log.info("on_error called for service action: #{action}")
     update_attributes(:retirement_state => 'error') if action == "Retirement"
     job(action).try(:refresh_ems)
-    postprocess(action)
+    log_stdout(action)
   end
 
   def retain_resources_on_retirement?
@@ -102,9 +96,6 @@ def save_job_options(action, overrides)
       job_options[cred] = Authentication.find(credential_id).native_ref if credential_id.present?
     end
 
-    hosts = job_options[:hosts]
-    job_options[:inventory] = create_inventory_with_hosts(action, hosts).id unless use_default_inventory?(hosts)
-
     options[job_option_key(action)] = job_options
     save!
   end
@@ -132,42 +123,20 @@ def extra_vars_from_dialog
     params.blank? ? {} : {:extra_vars => params}
   end
 
-  def create_inventory_with_hosts(action, hosts)
-    tower = manager(action)
-    tower.with_provider_connection do |connection|
-      miq_org = tower.provider.default_organization
-      connection.api.inventories.create!(:name => inventory_name(action), :organization => miq_org).tap do |inventory|
-        hosts.split(',').each do |host|
-          connection.api.hosts.create!(:name => host, :inventory => inventory.id)
-        end
-      end
-    end
-  end
-
-  def delete_inventory(action, inventory_raw_id)
-    manager(action).with_provider_connection do |connection|
-      connection.api.inventories.find(inventory_raw_id).destroy!
-    end
-  end
-
-  def manager(action)
-    job_template(action).manager
+  def use_default_inventory?(hosts)
+    hosts.blank? || hosts == 'localhost'
   end
 
-  def inventory_name(action)
-    "#{job_template(action).name}_#{id}"
-  end
+  def hosts_array(hosts_string)
+    return ["localhost"] if use_default_inventory?(hosts_string)
 
-  def use_default_inventory?(hosts)
-    hosts.blank? || hosts == 'localhost'
+    hosts_string.split(',').map(&:strip).delete_blanks
   end
 
   # update job attributes only available to playbook provisioning
   def update_job_for_playbook(action, job, hosts)
-    hosts = 'localhost' if use_default_inventory?(hosts)
-    host_array = hosts.split(',')
     playbook_id = options.fetch_path(:config_info, action.downcase.to_sym, :playbook_id)
-    job.update_attributes!(:configuration_script_base_id => playbook_id, :hosts => host_array)
+    job.update!(:configuration_script_base_id => playbook_id, :hosts => hosts)
   end
 
   def decrypt_options(opts)

spec/models/manageiq/providers/ansible_playbook_workflow_spec.rb

@@ -1,6 +1,6 @@
 describe ManageIQ::Providers::AnsiblePlaybookWorkflow do
   let(:job)     { described_class.create_job(*options).tap { |job| job.state = state } }
-  let(:options) { [{"ENV" => "VAR"}, %w(arg1 arg2), {:playbook_path => "/path/to/playbook"}] }
+  let(:options) { [{"ENV" => "VAR"}, %w[arg1 arg2], {:playbook_path => "/path/to/playbook"}, %w[192.0.2.0 192.0.2.1]] }
   let(:state)   { "waiting_to_start" }
 
   context ".create_job" do
@@ -151,7 +151,7 @@
     end
 
     context ".deliver_on" do
-      let(:options) { [{"ENV" => "VAR"}, %w(arg1 arg2), {:playbook_path => "/path/to/playbook"}, :poll_interval => 5.minutes] }
+      let(:options) { [{"ENV" => "VAR"}, %w[arg1 arg2], {:playbook_path => "/path/to/playbook"}, %w[192.0.2.0 192.0.2.1], :poll_interval => 5.minutes] }
 
       it "uses the option to queue poll_runner" do
         now = Time.now.utc

spec/models/service_ansible_playbook_spec.rb

@@ -117,24 +117,20 @@
   describe '#preprocess' do
     context 'basic service' do
       it 'prepares job options from service template' do
-        hosts = config_info_options.fetch_path(:config_info, :provision, :hosts)
-        expect(basic_service).to receive(:create_inventory_with_hosts).with(action, hosts).and_return(double(:id => 10))
         basic_service.preprocess(action)
         service.reload
-        expect(basic_service.options[:provision_job_options]).to include(:inventory => 10)
+        expect(basic_service.options[:provision_job_options][:hosts]).to eq("default_host1,default_host2")
       end
     end
 
     context 'with dialog overrides' do
       it 'prepares job options combines from service template and dialog' do
-        hosts = dialog_options[:dialog]['dialog_hosts']
-        expect(service).to receive(:create_inventory_with_hosts).with(action, hosts).and_return(double(:id => 20))
         service.preprocess(action)
         service.reload
-        expect(service.options[:provision_job_options]).to include(
-          :inventory  => 20,
-          :credential => credential_1.native_ref,
-          :extra_vars => {'var1' => 'value1', 'var2' => 'value2', 'var3' => 'default_val3', 'pswd' => encrypted_val}
+        expect(service.options[:provision_job_options][:hosts]).to eq("host1,host2")
+        expect(service.options[:provision_job_options][:credential]).to eq(credential_1.native_ref)
+        expect(service.options[:provision_job_options][:extra_vars]).to eq(
+          'var1' => 'value1', 'var2' => 'value2', 'var3' => 'default_val3', 'pswd' => encrypted_val
         )
       end
 
@@ -148,13 +144,11 @@
         end
 
         it 'ignores dialog options' do
-          hosts = service.options.fetch_path(:config_info, :retirement, :hosts)
-          expect(service).to receive(:create_inventory_with_hosts).with(action, hosts).and_return(double(:id => 20))
           service.preprocess(action)
           service.reload
-          expect(service.options[:retirement_job_options]).to include(
-            :inventory  => 20,
-            :extra_vars => {'var1' => 'default_val1', 'var2' => 'default_val2', 'var3' => 'default_val3'}
+          expect(service.options[:retirement_job_options][:hosts]).to eq("default_host1,default_host2")
+          expect(service.options[:retirement_job_options][:extra_vars]).to eq(
+            'var1' => 'default_val1', 'var2' => 'default_val2', 'var3' => 'default_val3'
           )
           expect(service.options[:retirement_job_options]).not_to have_key(:credential)
         end
@@ -163,14 +157,12 @@
 
     context 'with runtime overrides' do
       it 'prepares job options combined from service template, dialog, and overrides' do
-        hosts = override_options[:hosts]
-        expect(service).to receive(:create_inventory_with_hosts).with(action, hosts).and_return(double(:id => 30))
         service.preprocess(action, override_options)
         service.reload
-        expect(service.options[:provision_job_options]).to include(
-          :inventory  => 30,
-          :credential => credential_2.native_ref,
-          :extra_vars => {'var1' => 'new_val1', 'var2' => 'value2', 'var3' => 'default_val3', 'pswd' => encrypted_val2}
+        expect(service.options[:provision_job_options][:hosts]).to eq("host3")
+        expect(service.options[:provision_job_options][:credential]).to eq(credential_2.native_ref)
+        expect(service.options[:provision_job_options][:extra_vars]).to eq(
+          'var1' => 'new_val1', 'var2' => 'value2', 'var3' => 'default_val3', 'pswd' => encrypted_val2
         )
       end
     end
@@ -243,59 +235,16 @@
     it { expect(executed_service.check_refreshed(action)).to eq([true, nil]) }
   end
 
-  describe '#postprocess' do
-    context 'with user selected hosts' do
-      it 'deletes temporary inventory' do
-        expect(executed_service).to receive(:delete_inventory)
-        expect(executed_service).to receive(:log_stdout)
-        executed_service.postprocess(action)
-      end
-    end
-
-    context 'with default localhost' do
-      let(:provision_options) do
-        {
-          :provision_job_options => {
-            :credential => 1,
-            :extra_vars => {'var1' => 'value1', 'var2' => 'value2', 'pswd' => encrypted_val}
-          }
-        }
-      end
-
-      it 'needs not to delete the inventory' do
-        expect(executed_service).to receive(:log_stdout)
-        expect(executed_service).not_to receive(:delete_inventory)
-        executed_service.postprocess(action)
-      end
-    end
-
-    context 'require log stdout when job failed' do
-      before do
-        task   = MiqTask.new(:state => MiqTask::STATE_FINISHED, :status => MiqTask::STATUS_ERROR)
-        status = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Job::Status.new(task, nil)
-        allow(runner_job).to receive(:raw_status).and_return(status)
-      end
-
-      it 'writes stdout to log' do
-        expect(runner_job).to receive(:raw_stdout).with('txt_download')
-        expect(executed_service).to receive(:delete_inventory)
-        executed_service.postprocess(action)
-      end
-    end
-  end
-
   describe '#on_error' do
     it 'handles retirement error' do
       executed_service.update_attributes(:retirement_state => 'Retiring')
       expect(runner_job).to receive(:refresh_ems)
-      expect(executed_service).to receive(:postprocess)
       executed_service.on_error(ResourceAction::RETIREMENT)
       expect(executed_service.retirement_state).to eq('error')
     end
 
     it 'handles provisioning error' do
       expect(runner_job).to receive(:refresh_ems)
-      expect(executed_service).to receive(:postprocess)
       executed_service.on_error(action)
       expect(executed_service.retirement_state).to be_nil
     end
@@ -312,4 +261,26 @@
       expect(service.job('Retirement')).to be_nil
     end
   end
+
+  describe '#hosts_array (private)' do
+    it "is localhost if the hosts list is empty" do
+      hosts = ""
+      expect(basic_service.send(:hosts_array, hosts)).to eq(["localhost"])
+    end
+
+    it "is localhost if the hosts list is nil" do
+      hosts = nil
+      expect(basic_service.send(:hosts_array, hosts)).to eq(["localhost"])
+    end
+
+    it "handles multiple hosts" do
+      hosts = "192.0.2.0,192.0.2.1,192.0.2.2"
+      expect(basic_service.send(:hosts_array, hosts)).to match_array(%w[192.0.2.0 192.0.2.1 192.0.2.2])
+    end
+
+    it "works with weird commas" do
+      hosts = "192.0.2.0, , 192.0.2.1, 192.0.2.2  ,,,"
+      expect(basic_service.send(:hosts_array, hosts)).to match_array(%w[192.0.2.0 192.0.2.1 192.0.2.2])
+    end
+  end
 end