From 2881ec258164d3a9e30677a11eb7713853754cd6 Mon Sep 17 00:00:00 2001 From: Tomas Jelinek Date: Thu, 10 Aug 2017 02:58:43 -0400 Subject: [PATCH] Backport of: fixed cases causing waiting on timeout in vm_import This is a backport of ManageIQ/manageiq-providers-ovirt/pull/73/ It tries to be a minimal backport needed but still includes all the relevant tests. 2 problems fixed: - allow to import only using 4.1.5+ provider. The reason is that the required fix (https://bugzilla.redhat.com/1477375) is present in 4.1.5+ and without this fix manageiq will wait until timeout reached. - check if submit to the provider succeeded. The reason is that if the result is not checked, this state will always succeed and the import will again wait until timeout. --- .../providers/redhat/infra_manager.rb | 7 +++ .../redhat/infra_manager/vm_import.rb | 13 +++++- .../redhat/infra_manager/vm_import_spec.rb | 9 +++- .../providers/redhat/infra_manager_spec.rb | 45 +++++++++++++++++++ 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/app/models/manageiq/providers/redhat/infra_manager.rb b/app/models/manageiq/providers/redhat/infra_manager.rb index 76b5a0ee56b..a68ae8e84d3 100644 --- a/app/models/manageiq/providers/redhat/infra_manager.rb +++ b/app/models/manageiq/providers/redhat/infra_manager.rb @@ -146,4 +146,11 @@ def unsupported_migration_options def supports_migrate_for_all?(vms) vms.map(&:ems_cluster).uniq.compact.size == 1 end + + def version_higher_than?(version) + return false if api_version.nil? + + ems_version = api_version[/\d+\.\d+\.?\d*/x] + Gem::Version.new(ems_version) >= Gem::Version.new(version) + end end diff --git a/app/models/manageiq/providers/redhat/infra_manager/vm_import.rb b/app/models/manageiq/providers/redhat/infra_manager/vm_import.rb index 29b54a81dac..e5204f584db 100644 --- a/app/models/manageiq/providers/redhat/infra_manager/vm_import.rb +++ b/app/models/manageiq/providers/redhat/infra_manager/vm_import.rb @@ -45,21 +45,30 @@ def configure_imported_vm_networks(vm_id) end end + def check_task!(t, msg) + raise msg if t.nil? || MiqTask.status_error?(t.status) || MiqTask.status_timeout?(t.status) + end + def submit_import_vm(userid, source_vm_id, target_params) task_id = queue_self_method_call(userid, 'Import VM', 'import_vm', source_vm_id, target_params) task = MiqTask.wait_for_taskid(task_id) + check_task!(task, _('Error while importing the VM.')) + task.task_results end def validate_import_vm - highest_supported_api_version && highest_supported_api_version >= '4' + # The version of the RHV needs to be at least 4.1.5 due to https://bugzilla.redhat.com/1477375 + version_higher_than?('4.1.5') end def submit_configure_imported_vm_networks(userid, vm_id) task_id = queue_self_method_call(userid, "Configure imported VM's networks", 'configure_imported_vm_networks', vm_id) task = MiqTask.wait_for_taskid(task_id) + check_task!(task, _('Error while configuring VM network.')) + task.task_results end @@ -68,7 +77,7 @@ def submit_configure_imported_vm_networks(userid, vm_id) def check_import_supported!(source_provider) raise _('Cannot import archived VMs') if source_provider.nil? - raise _('Cannot import to a RHEV provider of version < 4.0') unless api_version >= '4.0' + raise _('Cannot import to a RHEV provider of version < 4.1.5') unless validate_import_vm unless source_provider.type == ManageIQ::Providers::Vmware::InfraManager.name raise _('Source provider must be of type Vmware') end diff --git a/spec/models/manageiq/providers/redhat/infra_manager/vm_import_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager/vm_import_spec.rb index 5755ac95c4a..ccb5a68f6b7 100644 --- a/spec/models/manageiq/providers/redhat/infra_manager/vm_import_spec.rb +++ b/spec/models/manageiq/providers/redhat/infra_manager/vm_import_spec.rb @@ -105,12 +105,17 @@ def expect_import(params, expected_request) let(:ems) { FactoryGirl.create(:ems_redhat) } it 'validates successfully' do - allow(ems).to receive(:highest_supported_api_version).and_return('4') + allow(ems).to receive(:api_version).and_return('4.1.5') expect(ems.validate_import_vm).to be_truthy end + it 'fails validation on old api version' do + allow(ems).to receive(:api_version).and_return('4.1.4') + expect(ems.validate_import_vm).to be_falsey + end + it 'validates before connecting' do - allow(ems).to receive(:highest_supported_api_version).and_return(nil) + allow(ems).to receive(:api_version).and_return(nil) expect(ems.validate_import_vm).to be_falsey end end diff --git a/spec/models/manageiq/providers/redhat/infra_manager_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager_spec.rb index 11fb6e1ba22..1426f762a39 100644 --- a/spec/models/manageiq/providers/redhat/infra_manager_spec.rb +++ b/spec/models/manageiq/providers/redhat/infra_manager_spec.rb @@ -288,4 +288,49 @@ end end end + + context "#version_higher_than?" do + let(:api_version) { "4.2" } + let(:ems) { FactoryGirl.create(:ems_redhat, :api_version => api_version) } + + context "api version is higher or equal than checked version" do + it 'supports the right features' do + expect(ems.version_higher_than?("4.1")).to be_truthy + + ems.api_version = "4.1.3.2-0.1.el7" + expect(ems.version_higher_than?("4.1")).to be_truthy + + ems.api_version = "4.2.0_master" + expect(ems.version_higher_than?("4.1")).to be_truthy + + ems.api_version = "4.2.1_master" + expect(ems.version_higher_than?("4.2.0")).to be_truthy + end + end + + context "api version is lowergit than checked version" do + let(:api_version) { "4.0" } + it 'supports the right features' do + expect(ems.version_higher_than?("4.1")).to be_falsey + + ems.api_version = "4.0.3.2-0.1.el7" + expect(ems.version_higher_than?("4.1")).to be_falsey + + ems.api_version = "4.0.0_master" + expect(ems.version_higher_than?("4.1")).to be_falsey + + ems.api_version = "4.0.1_master" + expect(ems.version_higher_than?("4.0.2")).to be_falsey + end + end + + context "api version not set" do + let(:api_version) { nil } + it 'always return false' do + expect(ems.version_higher_than?("10.1")).to be_falsey + + expect(ems.version_higher_than?("0")).to be_falsey + end + end + end end