diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index e39afc2bda6..454a6ae7620 100644 --- a/app/models/conversion_host.rb +++ b/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 diff --git a/lib/infra_conversion_throttler.rb b/lib/infra_conversion_throttler.rb index fda03effb28..f36fd33717a 100644 --- a/lib/infra_conversion_throttler.rb +++ b/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.") diff --git a/spec/lib/infra_conversion_throttler_spec.rb b/spec/lib/infra_conversion_throttler_spec.rb index dacefcfee61..ca45aff164f 100644 --- a/spec/lib/infra_conversion_throttler_spec.rb +++ b/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,10 +25,12 @@ 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) @@ -36,7 +38,7 @@ 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) diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index 4857f56adc1..9d017523656 100644 --- a/spec/models/conversion_host_spec.rb +++ b/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