From 8e2b911c0ac41635c70d26a432f9a0b53e10957c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Ple=C5=A1ko?= Date: Tue, 22 May 2018 14:49:29 +0200 Subject: [PATCH] Reconfigure VM network connectivity aka. NICs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this commit we support reconfiguration of VM's network adapters - since UI only supports add and remove operations, we focus on these as well. Turns out vCloud's cloud_networks (vApp networks) were not hooked to corresponding orchestration_stack (vApp) yet so we needed to update the parser. Also, we modify parser to omit `subnet-` prefix from cloud_subnet names because it looks strange on the reconfigure UI. Fog version is shifted to 0.2.2 since only this version supports NIC reconfiguration. BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1572086 Signed-off-by: Miha Pleško --- .../providers/vmware/cloud_manager/vm.rb | 1 + .../vmware/cloud_manager/vm/reconfigure.rb | 29 +++++++++- .../vmware/network_manager/refresh_parser.rb | 20 +++---- manageiq-providers-vmware.gemspec | 2 +- .../cloud_manager/vm/reconfigure_spec.rb | 56 +++++++++++++++++++ .../providers/vmware/cloud_manager_spec.rb | 4 ++ .../vmware/network_manager/refresher_spec.rb | 4 +- 7 files changed, 101 insertions(+), 15 deletions(-) diff --git a/app/models/manageiq/providers/vmware/cloud_manager/vm.rb b/app/models/manageiq/providers/vmware/cloud_manager/vm.rb index 8a1627e7c..813b45caa 100644 --- a/app/models/manageiq/providers/vmware/cloud_manager/vm.rb +++ b/app/models/manageiq/providers/vmware/cloud_manager/vm.rb @@ -12,6 +12,7 @@ class ManageIQ::Providers::Vmware::CloudManager::Vm < ManageIQ::Providers::Cloud supports :reconfigure_disksize do unsupported_reason_add(:reconfigure_disksize, 'Cannot resize disks of a VM with snapshots') unless snapshots.empty? end + supports :reconfigure_network_adapters def provider_object(connection = nil) connection ||= ext_management_system.connect diff --git a/app/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure.rb b/app/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure.rb index 78d544832..f74b402b0 100644 --- a/app/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure.rb +++ b/app/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure.rb @@ -28,6 +28,15 @@ def disk_default_type 'LSI Logic Parallel SCSI' end + def available_networks + ['None'] + orchestration_stack.cloud_networks.map(&:name) + end + + def available_adapter_names + available = (0..4).to_a - network_ports.map { |nic| nic_idx(nic.name) }.map(&:to_i) + available.map { |idx| "#{name}#NIC##{idx}" } + end + def validate_config_spec(options) if vm_powered_on? if options[:number_of_cpus] @@ -62,7 +71,15 @@ def build_config_spec(options) Array(options[:disk_remove]).each_with_object(new_hw[:disk]) { |d, res| res << { :id => disk_id(d[:disk_name]), :capacity_mb => -1 } } end - new_hw.empty? ? {} : { :hardware => new_hw } + # Network connection modifications. + nics = [] + Array(options[:network_adapter_add]).each { |a| nics << { :new_idx => nic_idx(a[:name]), :name => vapp_net_name(a[:network]) } } + Array(options[:network_adapter_remove]).each { |a| nics << { :idx => nic_idx(a[:network][:name]), :new_idx => -1 } } + + res = {} + res[:hardware] = new_hw unless new_hw.empty? + res[:networks] = nics unless nics.empty? + res end def disk_id(disk_name) @@ -70,4 +87,14 @@ def disk_id(disk_name) # Disk location is stored as "{addr}/{parent_addr}/{disk_id}" e.g. "0/3/2000" disk.location.to_s.split('/').last end + + def nic_idx(nic_name) + # NIC name is stored as "{vm_name}#NIC#{nic_index}" + nic_name.to_s.split('#').last + end + + def vapp_net_name(name) + # vApp network name is stored as "{name} ({vapp_name})" + name.to_s.chomp(" (#{orchestration_stack.name})") + end end diff --git a/app/models/manageiq/providers/vmware/network_manager/refresh_parser.rb b/app/models/manageiq/providers/vmware/network_manager/refresh_parser.rb index 98b304495..ceee6dfc7 100644 --- a/app/models/manageiq/providers/vmware/network_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/vmware/network_manager/refresh_parser.rb @@ -1,7 +1,7 @@ module ManageIQ::Providers class Vmware::NetworkManager::RefreshParser include ManageIQ::Providers::Vmware::RefreshHelperMethods - VappNetwork = Struct.new(:id, :name, :type, :is_shared, :gateway, :dns1, :dns2, :netmask, :enabled, :dhcp_enabled) + VappNetwork = Struct.new(:id, :name, :type, :is_shared, :gateway, :dns1, :dns2, :netmask, :enabled, :dhcp_enabled, :vapp) def initialize(ems, options = nil) @ems = ems @@ -113,11 +113,12 @@ def parse_network(network) self.class.cloud_network_vdc_type : self.class.cloud_network_vapp_type new_result = { - :name => network.name, - :ems_ref => uid, - :shared => network.is_shared, - :type => network_type, - :cloud_subnets => [] + :name => network.name, + :ems_ref => uid, + :shared => network.is_shared, + :type => network_type, + :cloud_subnets => [], + :orchestration_stack_id => network.try(:vapp).try(:id) } new_result[:cidr] = to_cidr(network.gateway, network.netmask) new_result[:enabled] = network.enabled if network.respond_to?(:enabled) @@ -128,7 +129,7 @@ def parse_network(network) def parse_network_subnet(network) uid = subnet_id(network) new_result = { - :name => subnet_name(network), + :name => network.name, :ems_ref => uid, :gateway => network.gateway, :dns_nameservers => [network.dns1, network.dns2].compact, @@ -215,6 +216,7 @@ def parse_floating_ip(nic_data) def build_vapp_network(vapp, network_id, net_conf) n = VappNetwork.new(network_id) n.name = vapp_network_name(net_conf[:networkName], vapp) + n.vapp = vapp n.is_shared = false n.type = 'application/vnd.vmware.vcloud.vAppNetwork+xml' Array.wrap(net_conf.dig(:Configuration, :IpScopes)).each do |ip_scope| @@ -234,10 +236,6 @@ def subnet_id(network) "subnet-#{network.id}" end - def subnet_name(network) - "subnet-#{network.name}" - end - def vapp_network_name(name, vapp) "#{name} (#{vapp.name})" end diff --git a/manageiq-providers-vmware.gemspec b/manageiq-providers-vmware.gemspec index 75e4f251f..61e51773a 100644 --- a/manageiq-providers-vmware.gemspec +++ b/manageiq-providers-vmware.gemspec @@ -13,7 +13,7 @@ Gem::Specification.new do |s| s.files = Dir["{app,config,lib}/**/*"] - s.add_dependency("fog-vcloud-director", ["~> 0.2.1"]) + s.add_dependency("fog-vcloud-director", ["~> 0.2.2"]) s.add_dependency "fog-core", "~>1.40" s.add_dependency "vmware_web_service", "~>0.2.10" s.add_dependency "rbvmomi", "~>1.11.3" diff --git a/spec/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure_spec.rb b/spec/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure_spec.rb index e0020c546..77059029a 100644 --- a/spec/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure_spec.rb +++ b/spec/models/manageiq/providers/vmware/cloud_manager/vm/reconfigure_spec.rb @@ -8,6 +8,8 @@ :cpu_hot_add_enabled => true, :cpu_hot_remove_enabled => true, :memory_hot_add_enabled => true, + :orchestration_stack => orchestration_stack, + :network_ports => [FactoryGirl.create(:network_port, :name => 'test_vm#NIC#0')], :hardware => FactoryGirl.create( :hardware, :cpu4x2, @@ -19,11 +21,48 @@ ) ) end + let(:orchestration_stack) do + stack = FactoryGirl.create(:orchestration_stack, :name => 'vapp name') + FactoryGirl.create(:cloud_network, :name => 'vApp network name (vapp name)', :orchestration_stack => stack) + stack + end it '#reconfigurable?' do expect(vm.reconfigurable?).to be_truthy end + it '.max_cpu_cores_per_socket' do + expect(vm.max_cpu_cores_per_socket).to eq(128) + end + + it '.max_total_vcpus' do + expect(vm.max_total_vcpus).to eq(128) + end + + it '.max_vcpus' do + expect(vm.max_vcpus).to eq(128) + end + + it '.max_memory_mb' do + expect(vm.max_memory_mb).to eq(4_194_304) + end + + it '.disk_types' do + expect(vm.disk_types).to eq(['LSI Logic Parallel SCSI']) + end + + it '.disk_default_type' do + expect(vm.disk_default_type).to eq('LSI Logic Parallel SCSI') + end + + it '.available_networks' do + expect(vm.available_networks).to eq(['None', 'vApp network name (vapp name)']) + end + + it '.available_adapter_names' do + expect(vm.available_adapter_names).to eq(['test_vm#NIC#1', 'test_vm#NIC#2', 'test_vm#NIC#3', 'test_vm#NIC#4']) + end + describe '#build_config_spec' do let(:fog_options) { vm.build_config_spec(options) } let(:options) do @@ -139,5 +178,22 @@ end end end + + describe 'network adapters' do + let(:options) do + { + :network_adapter_add => [{ :network => 'vApp Network Name (vapp name)', :name => 'VM Name#NIC#2' }], + :network_adapter_remove => [{ :network => { :name => 'VM Name#NIC#0' } }] + } + end + + it 'add' do + expect(fog_options[:networks]).to include(:new_idx => '2', :name => 'vApp Network Name') + end + + it 'remove' do + expect(fog_options[:networks]).to include(:idx => '0', :new_idx => -1) + end + end end end diff --git a/spec/models/manageiq/providers/vmware/cloud_manager_spec.rb b/spec/models/manageiq/providers/vmware/cloud_manager_spec.rb index fd18928ee..3bfdc52d4 100644 --- a/spec/models/manageiq/providers/vmware/cloud_manager_spec.rb +++ b/spec/models/manageiq/providers/vmware/cloud_manager_spec.rb @@ -191,6 +191,10 @@ end end + it 'supports reconfigure_network_adapters' do + expect(vm.supports_reconfigure_network_adapters?).to be_truthy + end + it '.vm_reconfigure' do expect(connection).to receive(:get_vapp).with(vm.ems_ref, :parser => 'xml').and_return(double(:body => vm_xml)) expect(connection).to receive(:post_reconfigure_vm).with(vm.ems_ref, vm_xml, 'fog-options').and_return(double(:body => nil)) diff --git a/spec/models/manageiq/providers/vmware/network_manager/refresher_spec.rb b/spec/models/manageiq/providers/vmware/network_manager/refresher_spec.rb index 21acbaad7..af0ca6589 100644 --- a/spec/models/manageiq/providers/vmware/network_manager/refresher_spec.rb +++ b/spec/models/manageiq/providers/vmware/network_manager/refresher_spec.rb @@ -63,7 +63,7 @@ def assert_specific_network def assert_specific_subnet expect(vdc_subnet).to have_attributes( - :name => 'subnet-RedHat external network', + :name => 'RedHat external network', :cidr => '10.12.0.1/16', :status => nil, :dhcp_enabled => nil, @@ -151,7 +151,7 @@ def assert_specific_network def assert_specific_subnet expect(vapp_subnet).to have_attributes( - :name => 'subnet-vApp network test (RHEL7-web-002)', + :name => 'vApp network test (RHEL7-web-002)', :cidr => '192.168.2.1/24', :dhcp_enabled => true, :gateway => '192.168.2.1',