Merge pull request #20738 from djberg96/ansible_playbook_log_stdout

Update playbook_log_stdout to handle possible nil job

app/models/mixins/ansible_playbook_mixin.rb

@@ -32,12 +32,17 @@ def hosts_array(hosts_string)
   end
 
   def playbook_log_stdout(log_option, job)
+    raise ArgumentError, "invalid job object" if job.nil?
     return unless %(on_error always).include?(log_option)
     return if log_option == 'on_error' && job.raw_status.succeeded?
 
     $log.info("Stdout from ansible job #{job.name}: #{job.raw_stdout('txt_download')}")
   rescue StandardError => err
-    $log.error("Failed to get stdout from ansible job #{job.name}")
+    if job.nil?
+      $log.error("Job was nil, must pass a valid job")
+    else
+      $log.error("Failed to get stdout from ansible job #{job.name}")
+    end
     $log.log_backtrace(err)
   end
 end

app/models/service_ansible_playbook.rb

@@ -171,6 +171,11 @@ def decrypt_options(opts)
 
   def log_stdout(action)
     log_option = options.fetch_path(:config_info, action.downcase.to_sym, :log_output) || 'on_error'
-    playbook_log_stdout(log_option, job(action))
+    job = job(action)
+    if job.nil?
+      $log.info("No stdout available due to missing job")
+    else
+      playbook_log_stdout(log_option, job)
+    end
   end
 end

spec/models/mixins/ansible_playbook_mixin_spec.rb

@@ -0,0 +1,37 @@
+RSpec.describe AnsiblePlaybookMixin do
+  let(:test_instance) do
+    Class.new(ActiveRecord::Base) do
+      def self.name; "TestClass"; end
+      self.table_name = "services"
+      include AnsiblePlaybookMixin
+    end.new
+  end
+
+  let(:job)    { FactoryBot.create(:embedded_ansible_job, :name => 'ansible_playbook_mixin_test') }
+  let(:task)   { FactoryBot.create(:miq_task) }
+  let(:status) { ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Job::Status.new(task, MiqTask::STATUS_OK) }
+
+  context "#playbook_log_stdout" do
+    it "returns if the log option is not on_error or always" do
+      expect(test_instance.playbook_log_stdout('test', job)).to be_nil
+    end
+
+    it "returns if the log option is on_error but the job's raw_status is succeeded" do
+      allow(status).to receive(:succeeded?).and_return(true)
+      allow(job).to receive(:raw_status).and_return(status)
+      expect(test_instance.playbook_log_stdout('on_error', job)).to be_nil
+    end
+
+    it "logs the expected message if the log option is not on_error, and the job has succeeded" do
+      allow(job).to receive(:raw_stdout).with('txt_download').and_return("<test ansible text>")
+      expect($log).to receive(:info).with("Stdout from ansible job ansible_playbook_mixin_test: <test ansible text>")
+      test_instance.playbook_log_stdout('always', job)
+    end
+
+    it "raises an error if the job is nil" do
+      expect($log).to receive(:error).with("Job was nil, must pass a valid job")
+      expect($log).to receive(:log_backtrace)
+      test_instance.playbook_log_stdout('always', nil)
+    end
+  end
+end