Merge pull request #18880 from djberg96/conversion_host_cache_ssh_val…

…idation

[V2V] Use authentication_check instead of verify credentials

app/models/conversion_host.rb

@@ -46,7 +46,7 @@ class ConversionHost < ApplicationRecord
   # TODO: Use the verify_credentials_ssh method in host.rb? Move that to the
   # AuthenticationMixin?
   #
-  def verify_credentials(auth_type = nil, options = {})
+  def verify_credentials(auth_type = 'v2v', options = {})
     if authentications.empty?
       check_ssh_connection
     else
@@ -68,6 +68,9 @@ def verify_credentials(auth_type = nil, options = {})
         raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}")
       end
 
+      # Don't connect again if the authentication is still valid
+      return true if authentication_status_ok?(auth_type)
+
       # Options from STI subclasses will override the defaults we've set above.
       ssh_options.merge!(options)
 
@@ -91,7 +94,7 @@ def verify_credentials(auth_type = nil, options = {})
   #  - The number of concurrent tasks has not reached the limit.
   #
   def eligible?
-    source_transport_method.present? && verify_credentials && check_concurrent_tasks
+    source_transport_method.present? && authentication_check('v2v').first && check_concurrent_tasks
   end
 
   # Returns a boolean indicating whether or not the current number of active tasks

lib/infra_conversion_throttler.rb

@@ -8,7 +8,7 @@ def self.start_conversions
       _log.debug("-- Number of pending jobs: #{jobs.size}")
       running = ems.conversion_hosts.inject(0) { |sum, ch| sum + ch.active_tasks.count }
       _log.debug("-- Currently running jobs in EMS: #{running}")
-      slots = (ems.miq_custom_get('Max Transformation Runners') || Settings.transformation.limits.max_concurrent_tasks_per_ems).to_i - running
+      slots = (ems.miq_custom_get('MaxTransformationRunners') || Settings.transformation.limits.max_concurrent_tasks_per_ems).to_i - running
 
       if slots <= 0
         _log.debug("-- No available slot in EMS. Stopping.")

spec/lib/infra_conversion_throttler_spec.rb

@@ -1,7 +1,7 @@
-describe InfraConversionThrottler do
-  let(:ems) { FactoryBot.create(:ext_management_system, :zone => FactoryBot.create(:zone)) }
-  let(:host) { FactoryBot.create(:host, :ext_management_system => ems) }
-  let(:vm) { FactoryBot.create(:vm_or_template, :ext_management_system => ems) }
+RSpec.describe InfraConversionThrottler, :v2v do
+  let(:ems) { FactoryBot.create(:ext_management_system, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') }
+  let(:host) { FactoryBot.create(:host_redhat, :ext_management_system => ems) }
+  let(:vm) { FactoryBot.create(:vm_openstack, :ext_management_system => ems) }
   let(:task_waiting) { FactoryBot.create(:service_template_transformation_plan_task, :source => vm) }
   let(:task_running_1) { FactoryBot.create(:service_template_transformation_plan_task, :source => vm) }
   let(:task_running_2) { FactoryBot.create(:service_template_transformation_plan_task, :source => vm) }
@@ -25,18 +25,20 @@
       allow(ems).to receive(:conversion_hosts).and_return([conversion_host1, conversion_host2])
       allow(conversion_host1).to receive(:check_ssh_connection).and_return(true)
       allow(conversion_host2).to receive(:check_ssh_connection).and_return(true)
+      allow(conversion_host1).to receive(:authentication_check).and_return([true, 'passed'])
+      allow(conversion_host2).to receive(:authentication_check).and_return([true, 'passed'])
     end
 
     it 'will not start a job when ems limit hit' do
-      ems.miq_custom_set('Max Transformation Runners', 2)
+      ems.miq_custom_set('MaxTransformationRunners', 2)
       allow(conversion_host1).to receive(:active_tasks).and_return([1])
       allow(conversion_host2).to receive(:active_tasks).and_return([1])
       expect(job_waiting).not_to receive(:queue_signal)
       described_class.start_conversions
     end
 
     it 'will not start a job when conversion_host limit hit' do
-      ems.miq_custom_set('Max Transformation Runners', 100)
+      ems.miq_custom_set('MaxTransformationRunners', 100)
       allow(conversion_host1).to receive(:active_tasks).and_return([1, 2])
       allow(conversion_host2).to receive(:active_tasks).and_return([1, 2])
       expect(job_waiting).not_to receive(:queue_signal)

spec/models/conversion_host_spec.rb

@@ -25,28 +25,28 @@
     context "#eligible?" do
       it "fails when no source transport method is enabled" do
         allow(conversion_host_1).to receive(:source_transport_method).and_return(nil)
-        allow(conversion_host_1).to receive(:verify_credentials).and_return(true)
+        allow(conversion_host_1).to receive(:authentication_check).and_return([true, 'worked'])
         allow(conversion_host_1).to receive(:check_concurrent_tasks).and_return(true)
         expect(conversion_host_1.eligible?).to eq(false)
       end
 
       it "fails when no source transport method is enabled" do
         allow(conversion_host_1).to receive(:source_transport_method).and_return('vddk')
-        allow(conversion_host_1).to receive(:verify_credentials).and_return(false)
+        allow(conversion_host_1).to receive(:authentication_check).and_return([false, 'failed'])
         allow(conversion_host_1).to receive(:check_concurrent_tasks).and_return(true)
         expect(conversion_host_1.eligible?).to eq(false)
       end
 
       it "fails when no source transport method is enabled" do
         allow(conversion_host_1).to receive(:source_transport_method).and_return('vddk')
-        allow(conversion_host_1).to receive(:verify_credentials).and_return(true)
+        allow(conversion_host_1).to receive(:authentication_check).and_return([true, 'worked'])
         allow(conversion_host_1).to receive(:check_concurrent_tasks).and_return(false)
         expect(conversion_host_1.eligible?).to eq(false)
       end
 
       it "succeeds when all criteria are met" do
         allow(conversion_host_1).to receive(:source_transport_method).and_return('vddk')
-        allow(conversion_host_1).to receive(:verify_credentials).and_return(true)
+        allow(conversion_host_1).to receive(:authentication_check).and_return([true, 'worked'])
         allow(conversion_host_1).to receive(:check_concurrent_tasks).and_return(true)
         expect(conversion_host_1.eligible?).to eq(true)
       end
@@ -437,6 +437,27 @@
       expect(conversion_host_vm.verify_credentials).to be_truthy
     end
 
+    it "makes an ssh call if the authentication is not valid" do
+      authentication = FactoryBot.create(:authentication_ssh_keypair, :status => 'Error', :authtype => 'v2v')
+      conversion_host_vm.authentications << authentication
+      expect(Net::SSH).to receive(:start)
+      conversion_host_vm.verify_credentials
+    end
+
+    it "does not make an ssh call if the authentication is valid" do
+      authentication = FactoryBot.create(:authentication_ssh_keypair, :status => 'Valid', :authtype => 'v2v')
+      conversion_host_vm.authentications << authentication
+      expect(Net::SSH).not_to receive(:start)
+      conversion_host_vm.verify_credentials
+    end
+
+    it "makes an ssh call if the authentication status is not set" do
+      authentication = FactoryBot.create(:authentication_ssh_keypair, :status => nil, :authtype => 'v2v')
+      conversion_host_vm.authentications << authentication
+      expect(Net::SSH).to receive(:start)
+      conversion_host_vm.verify_credentials
+    end
+
     it "works as expected if there is an associated validation that is invalid" do
       authentication = FactoryBot.create(:authentication_ssh_keypair)
       conversion_host_vm.authentications << authentication