From cf83a9b59f2e5e0cae76cfdf16923ede9ad3f54f Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Thu, 31 Jan 2019 10:11:41 -0500 Subject: [PATCH 1/2] Add nil IP address check to preflight check --- app/models/service_template_transformation_plan_task.rb | 2 +- spec/models/service_template_transformation_plan_task_spec.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/service_template_transformation_plan_task.rb b/app/models/service_template_transformation_plan_task.rb index 3b3bc58b00c..6d2cefdaeda 100644 --- a/app/models/service_template_transformation_plan_task.rb +++ b/app/models/service_template_transformation_plan_task.rb @@ -54,7 +54,7 @@ def task_active def preflight_check destination_cluster virtv2v_disks - network_mappings + network_mappings.each { |m| raise if m.ip_address.nil? } raise if destination_ems.emstype == 'openstack' && source.power_state == 'off' true rescue diff --git a/spec/models/service_template_transformation_plan_task_spec.rb b/spec/models/service_template_transformation_plan_task_spec.rb index deec3bf1b57..7034c25834d 100644 --- a/spec/models/service_template_transformation_plan_task_spec.rb +++ b/spec/models/service_template_transformation_plan_task_spec.rb @@ -453,6 +453,8 @@ ) end + it { expect(task_1.preflight_check). to be false } + context "transport method is vddk" do before do conversion_host.vddk_transport_supported = true @@ -542,6 +544,8 @@ ) end + it { expect(task_1.preflight_check). to be false } + context "transport method is vddk" do before do conversion_host.vddk_transport_supported = true From 3eb16817cb8df17265cda612d9955c14529d0bd2 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Fri, 15 Feb 2019 16:54:24 +0100 Subject: [PATCH 2/2] Move check into network_mappings method --- ...rvice_template_transformation_plan_task.rb | 5 +- ..._template_transformation_plan_task_spec.rb | 56 ++++++++++++------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/app/models/service_template_transformation_plan_task.rb b/app/models/service_template_transformation_plan_task.rb index 6d2cefdaeda..54edea0db47 100644 --- a/app/models/service_template_transformation_plan_task.rb +++ b/app/models/service_template_transformation_plan_task.rb @@ -54,7 +54,7 @@ def task_active def preflight_check destination_cluster virtv2v_disks - network_mappings.each { |m| raise if m.ip_address.nil? } + network_mappings raise if destination_ems.emstype == 'openstack' && source.power_state == 'off' true rescue @@ -256,11 +256,12 @@ def calculate_network_mappings source_network = nic.lan destination_network = transformation_destination(source_network) raise "[#{source.name}] NIC #{nic.device_name} [#{source_network.name}] has no mapping." if destination_network.nil? + raise "[#{source.name}] NIC #{nic.device_name} [#{source_network.name}] has an empty IP address." if nic.network.try(:ipaddress).nil? { :source => source_network.name, :destination => destination_network_ref(destination_network), :mac_address => nic.address, - :ip_address => nic.network.try(:ipaddress) + :ip_address => nic.network.ipaddress } end end diff --git a/spec/models/service_template_transformation_plan_task_spec.rb b/spec/models/service_template_transformation_plan_task_spec.rb index 7034c25834d..b5e557ef22c 100644 --- a/spec/models/service_template_transformation_plan_task_spec.rb +++ b/spec/models/service_template_transformation_plan_task_spec.rb @@ -304,7 +304,9 @@ let(:src_vm_1) { FactoryBot.create(:vm_vmware, :ext_management_system => src_ems, :ems_cluster => src_cluster, :host => src_host, :hardware => src_hardware) } let(:src_vm_2) { FactoryBot.create(:vm_vmware, :ext_management_system => src_ems, :ems_cluster => src_cluster, :host => src_host) } - let(:src_network) { FactoryBot.create(:network, :ipaddress => '10.0.0.1') } + let(:src_network_1) { FactoryBot.create(:network, :ipaddress => '10.0.1.1') } + let(:src_network_2a) { FactoryBot.create(:network, :ipaddress => '10.0.1.2') } + let(:src_network_2b) { FactoryBot.create(:network, :ipaddress => nil) } # Disks have to be stubbed because there's no factory for Disk class before do @@ -312,7 +314,7 @@ allow(src_disk_1).to receive(:storage).and_return(src_storage) allow(src_disk_2).to receive(:storage).and_return(src_storage) allow(src_vm_1).to receive(:allocated_disk_storage).and_return(34_359_738_368) - allow(src_nic_1).to receive(:network).and_return(src_network) + allow(src_nic_1).to receive(:network).and_return(src_network_1) allow(src_host).to receive(:thumbprint_sha1).and_return('01:23:45:67:89:ab:cd:ef:01:23:45:67:89:ab:cd:ef:01:23:45:67') allow(src_host).to receive(:authentication_userid).and_return('esx_user') allow(src_host).to receive(:authentication_password).and_return('esx_passwd') @@ -438,22 +440,28 @@ before do task_1.conversion_host = conversion_host + allow(src_nic_2).to receive(:network).and_return(src_network_2a) end it { expect(task_1.destination_cluster).to eq(dst_cluster) } it_behaves_like "#virtv2v_disks" - it "checks network mappings and generates network_mappings hash" do - expect(task_1.network_mappings).to eq( - [ - { :source => src_lan_1.name, :destination => dst_lan_1.name, :mac_address => src_nic_1.address, :ip_address => '10.0.0.1' }, - { :source => src_lan_2.name, :destination => dst_lan_2.name, :mac_address => src_nic_2.address, :ip_address => nil } - ] - ) - end + context "#network_mappings" do + it "generates network_mappings hash" do + expect(task_1.network_mappings).to eq( + [ + { :source => src_lan_1.name, :destination => dst_lan_1.name, :mac_address => src_nic_1.address, :ip_address => '10.0.1.1' }, + { :source => src_lan_2.name, :destination => dst_lan_2.name, :mac_address => src_nic_2.address, :ip_address => '10.0.1.2' } + ] + ) + end - it { expect(task_1.preflight_check). to be false } + it "fails if network IP address is nil" do + allow(src_nic_2).to receive(:network).and_return(src_network_2b) + expect { task_1.network_mappings }.to raise_error("[#{src_vm_1.name}] NIC #{src_nic_2.device_name} [#{src_lan_2.name}] has an empty IP address.") + end + end context "transport method is vddk" do before do @@ -510,6 +518,8 @@ let(:dst_cloud_volume_type) { FactoryBot.create(:cloud_volume_type) } let(:dst_cloud_network_1) { FactoryBot.create(:cloud_network) } let(:dst_cloud_network_2) { FactoryBot.create(:cloud_network) } + let(:dst_net_1) { dst_cloud_network_1 } + let(:dst_net_2) { dst_cloud_network_2 } let(:dst_flavor) { FactoryBot.create(:flavor) } let(:dst_security_group) { FactoryBot.create(:security_group) } let(:conversion_host_vm) { FactoryBot.create(:vm_openstack, :ext_management_system => dst_ems, :cloud_tenant => dst_cloud_tenant) } @@ -529,22 +539,28 @@ before do task_1.conversion_host = conversion_host + allow(src_nic_2).to receive(:network).and_return(src_network_2a) end it { expect(task_1.destination_cluster).to eq(dst_cloud_tenant) } it_behaves_like "#virtv2v_disks" - it "checks network mappings and generates network_mappings hash" do - expect(task_1.network_mappings).to eq( - [ - { :source => src_lan_1.name, :destination => dst_cloud_network_1.ems_ref, :mac_address => src_nic_1.address, :ip_address => '10.0.0.1' }, - { :source => src_lan_2.name, :destination => dst_cloud_network_2.ems_ref, :mac_address => src_nic_2.address, :ip_address => nil } - ] - ) - end + context "#network_mappings" do + it "generates network_mappings hash" do + expect(task_1.network_mappings).to eq( + [ + { :source => src_lan_1.name, :destination => dst_cloud_network_1.ems_ref, :mac_address => src_nic_1.address, :ip_address => '10.0.1.1' }, + { :source => src_lan_2.name, :destination => dst_cloud_network_2.ems_ref, :mac_address => src_nic_2.address, :ip_address => '10.0.1.2' } + ] + ) + end - it { expect(task_1.preflight_check). to be false } + it "fails if network IP address is nil" do + allow(src_nic_2).to receive(:network).and_return(src_network_2b) + expect { task_1.network_mappings }.to raise_error("[#{src_vm_1.name}] NIC #{src_nic_2.device_name} [#{src_lan_2.name}] has an empty IP address.") + end + end context "transport method is vddk" do before do