From c1c93cd7ee0cc81ca1147786d637bd6fe68dc908 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Oct 2018 09:27:19 -0400 Subject: [PATCH 1/4] Conversion Host - Return hostname as IP address --- app/models/conversion_host.rb | 1 + spec/models/conversion_host_spec.rb | 46 +++++++++++++++++++---------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 1c1cb4fdd88..73194085e09 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -34,6 +34,7 @@ def source_transport_method end def ipaddress(family = 'ipv4') + return resource.hostname if resource.hostname return address if address && IPAddr.new(address).send("#{family}?") resource.ipaddresses.detect { |ip| IPAddr.new(ip).send("#{family}?") } end diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index a5e98a95042..18c23d8888c 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -88,28 +88,42 @@ end context "#ipaddress" do - it "returns first IP address if 'address' is nil" do - expect(conversion_host_1.ipaddress).to eq('10.0.0.1') - expect(conversion_host_2.ipaddress).to eq('10.0.1.1') - expect(conversion_host_1.ipaddress('ipv4')).to eq('10.0.0.1') - expect(conversion_host_2.ipaddress('ipv4')).to eq('10.0.1.1') - expect(conversion_host_1.ipaddress('ipv6')).to eq('FE80:0000:0000:0000:0202:B3FF:FE1E:8329') - expect(conversion_host_2.ipaddress('ipv6')).to eq('FE80::0202:B3FF:FE1E:3267') - end - - context "when address is set" do + context "when hostname is not set" do before do - allow(conversion_host_1).to receive(:address).and_return('172.16.0.1') - allow(conversion_host_2).to receive(:address).and_return('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') + allow(host).to receive(:hostname).and_return(nil) + allow(vm).to receive(:hostname).and_return(nil) end - it "returns 'address' if family matches, is invalid or is nil" do - expect(conversion_host_1.ipaddress).to eq('172.16.0.1') + it "returns first IP address if 'address' is nil" do + expect(conversion_host_1.ipaddress).to eq('10.0.0.1') expect(conversion_host_2.ipaddress).to eq('10.0.1.1') - expect(conversion_host_1.ipaddress('ipv4')).to eq('172.16.0.1') + expect(conversion_host_1.ipaddress('ipv4')).to eq('10.0.0.1') expect(conversion_host_2.ipaddress('ipv4')).to eq('10.0.1.1') expect(conversion_host_1.ipaddress('ipv6')).to eq('FE80:0000:0000:0000:0202:B3FF:FE1E:8329') - expect(conversion_host_2.ipaddress('ipv6')).to eq('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') + expect(conversion_host_2.ipaddress('ipv6')).to eq('FE80::0202:B3FF:FE1E:3267') + end + + context "when address is set" do + before do + allow(conversion_host_1).to receive(:address).and_return('172.16.0.1') + allow(conversion_host_2).to receive(:address).and_return('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') + end + + it "returns 'address' if family matches, is invalid or is nil" do + expect(conversion_host_1.ipaddress).to eq('172.16.0.1') + expect(conversion_host_2.ipaddress).to eq('10.0.1.1') + expect(conversion_host_1.ipaddress('ipv4')).to eq('172.16.0.1') + expect(conversion_host_2.ipaddress('ipv4')).to eq('10.0.1.1') + expect(conversion_host_1.ipaddress('ipv6')).to eq('FE80:0000:0000:0000:0202:B3FF:FE1E:8329') + expect(conversion_host_2.ipaddress('ipv6')).to eq('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') + end + end + + end + + context "when hostname is set" do + it "returns hostname" do + expect(conversion_host_1.ipaddress).to eq(host.hostname) end end end From d824cb767c92de376f8d1840dc0380b5f985558f Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Oct 2018 09:49:42 -0400 Subject: [PATCH 2/4] Fix call to shell_with_su --- app/models/conversion_host.rb | 2 +- spec/models/conversion_host_spec.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 73194085e09..fd0b95cbe6c 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -122,7 +122,7 @@ def check_resource_credentials_openstack def connect_ssh require 'MiqSshUtil' - MiqSshUtil.shell_with_su(miq_ssh_util_args) do |ssu, _shell| + MiqSshUtil.shell_with_su(*miq_ssh_util_args) do |ssu, _shell| yield(ssu) end rescue Exception => e diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index 18c23d8888c..f8fc040b44b 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -118,7 +118,6 @@ expect(conversion_host_2.ipaddress('ipv6')).to eq('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') end end - end context "when hostname is set" do From 9ffbe891c793e54baa739bb9d690491a86508f04 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Oct 2018 15:48:53 -0400 Subject: [PATCH 3/4] Use delegate for hostname and use it in miq_ssh_util_args --- app/models/conversion_host.rb | 9 +++--- spec/models/conversion_host_spec.rb | 45 ++++++++++------------------- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index fd0b95cbe6c..bf2a773b3cd 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -7,6 +7,7 @@ class ConversionHost < ApplicationRecord has_many :service_template_transformation_plan_tasks, :dependent => :nullify has_many :active_tasks, -> { where(:state => 'active') }, :class_name => ServiceTemplateTransformationPlanTask, :inverse_of => :conversion_host delegate :ext_management_system, :to => :resource, :allow_nil => true + delegate :hostname, :to => :resource, :allow_nil => true # To be eligible, a conversion host must have the following properties # - A transport mechanism is configured for source (set by 3rd party) @@ -34,13 +35,13 @@ def source_transport_method end def ipaddress(family = 'ipv4') - return resource.hostname if resource.hostname return address if address && IPAddr.new(address).send("#{family}?") resource.ipaddresses.detect { |ip| IPAddr.new(ip).send("#{family}?") } end def run_conversion(conversion_options) - connect_ssh { |ssu| ssu.shell_exec('/usr/bin/virt-v2v-wrapper.py', nil, nil, conversion_options.to_json) } + result = connect_ssh { |ssu| ssu.shell_exec('/usr/bin/virt-v2v-wrapper.py', nil, nil, conversion_options.to_json) } + JSON.parse(result) rescue => e raise "Starting conversion failed on '#{resource.name}' with [#{e.class}: #{e}]" end @@ -135,7 +136,7 @@ def miq_ssh_util_args end def miq_ssh_util_args_manageiq_providers_redhat_inframanager_host - [ ipaddress, resource.authentication_userid, resource.authentication_password, nil, nil ] + [ hostname || ipaddress, resource.authentication_userid, resource.authentication_password, nil, nil ] end def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm @@ -143,7 +144,7 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm .where(:authtype => 'ssh_keypair') .where.not(:userid => nil, :auth_key => nil) .first - [ ipaddress, authentication.userid, nil, nil, nil, { :key_data => authentication.auth_key, :passwordless_sudo => true }] + [ hostname || ipaddress, authentication.userid, nil, nil, nil, { :key_data => authentication.auth_key, :passwordless_sudo => true }] end def ansible_playbook(playbook, extra_vars, connection) diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index f8fc040b44b..a5e98a95042 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -88,41 +88,28 @@ end context "#ipaddress" do - context "when hostname is not set" do + it "returns first IP address if 'address' is nil" do + expect(conversion_host_1.ipaddress).to eq('10.0.0.1') + expect(conversion_host_2.ipaddress).to eq('10.0.1.1') + expect(conversion_host_1.ipaddress('ipv4')).to eq('10.0.0.1') + expect(conversion_host_2.ipaddress('ipv4')).to eq('10.0.1.1') + expect(conversion_host_1.ipaddress('ipv6')).to eq('FE80:0000:0000:0000:0202:B3FF:FE1E:8329') + expect(conversion_host_2.ipaddress('ipv6')).to eq('FE80::0202:B3FF:FE1E:3267') + end + + context "when address is set" do before do - allow(host).to receive(:hostname).and_return(nil) - allow(vm).to receive(:hostname).and_return(nil) + allow(conversion_host_1).to receive(:address).and_return('172.16.0.1') + allow(conversion_host_2).to receive(:address).and_return('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') end - it "returns first IP address if 'address' is nil" do - expect(conversion_host_1.ipaddress).to eq('10.0.0.1') + it "returns 'address' if family matches, is invalid or is nil" do + expect(conversion_host_1.ipaddress).to eq('172.16.0.1') expect(conversion_host_2.ipaddress).to eq('10.0.1.1') - expect(conversion_host_1.ipaddress('ipv4')).to eq('10.0.0.1') + expect(conversion_host_1.ipaddress('ipv4')).to eq('172.16.0.1') expect(conversion_host_2.ipaddress('ipv4')).to eq('10.0.1.1') expect(conversion_host_1.ipaddress('ipv6')).to eq('FE80:0000:0000:0000:0202:B3FF:FE1E:8329') - expect(conversion_host_2.ipaddress('ipv6')).to eq('FE80::0202:B3FF:FE1E:3267') - end - - context "when address is set" do - before do - allow(conversion_host_1).to receive(:address).and_return('172.16.0.1') - allow(conversion_host_2).to receive(:address).and_return('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') - end - - it "returns 'address' if family matches, is invalid or is nil" do - expect(conversion_host_1.ipaddress).to eq('172.16.0.1') - expect(conversion_host_2.ipaddress).to eq('10.0.1.1') - expect(conversion_host_1.ipaddress('ipv4')).to eq('172.16.0.1') - expect(conversion_host_2.ipaddress('ipv4')).to eq('10.0.1.1') - expect(conversion_host_1.ipaddress('ipv6')).to eq('FE80:0000:0000:0000:0202:B3FF:FE1E:8329') - expect(conversion_host_2.ipaddress('ipv6')).to eq('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') - end - end - end - - context "when hostname is set" do - it "returns hostname" do - expect(conversion_host_1.ipaddress).to eq(host.hostname) + expect(conversion_host_2.ipaddress('ipv6')).to eq('2001:0DB8:85A3:0000:0000:8A2E:0370:7334') end end end From 1de3f2ded0ae9fcd0fa6932d97cad3c4af76ed72 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Oct 2018 16:02:47 -0400 Subject: [PATCH 4/4] Fix rubocop --- app/models/conversion_host.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index bf2a773b3cd..a92c735f0de 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -136,7 +136,7 @@ def miq_ssh_util_args end def miq_ssh_util_args_manageiq_providers_redhat_inframanager_host - [ hostname || ipaddress, resource.authentication_userid, resource.authentication_password, nil, nil ] + [hostname || ipaddress, resource.authentication_userid, resource.authentication_password, nil, nil] end def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm @@ -144,7 +144,7 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm .where(:authtype => 'ssh_keypair') .where.not(:userid => nil, :auth_key => nil) .first - [ hostname || ipaddress, authentication.userid, nil, nil, nil, { :key_data => authentication.auth_key, :passwordless_sudo => true }] + [hostname || ipaddress, authentication.userid, nil, nil, nil, { :key_data => authentication.auth_key, :passwordless_sudo => true }] end def ansible_playbook(playbook, extra_vars, connection)