diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index cfd087c..6e7721a 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -161,11 +161,11 @@ module Vmpooler return false end - def get_vm(_pool_name, vm_name) + def get_vm(pool_name, vm_name) vm_hash = nil @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) - vm_object = find_vm(vm_name, connection) + vm_object = find_vm(pool_name, vm_name, connection) return vm_hash if vm_object.nil? vm_folder_path = get_vm_folder_path(vm_object) @@ -283,7 +283,7 @@ module Vmpooler @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) - vm_object = find_vm(vm_name, connection) + vm_object = find_vm(pool_name, vm_name, connection) raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil? add_disk(vm_object, disk_size, datastore_name, connection, get_target_datacenter_from_config(pool_name)) @@ -294,7 +294,7 @@ module Vmpooler def create_snapshot(pool_name, vm_name, new_snapshot_name) @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) - vm_object = find_vm(vm_name, connection) + vm_object = find_vm(pool_name, vm_name, connection) raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil? old_snap = find_snapshot(vm_object, new_snapshot_name) @@ -313,7 +313,7 @@ module Vmpooler def revert_snapshot(pool_name, vm_name, snapshot_name) @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) - vm_object = find_vm(vm_name, connection) + vm_object = find_vm(pool_name, vm_name, connection) raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil? snapshot_object = find_snapshot(vm_object, snapshot_name) @@ -324,10 +324,10 @@ module Vmpooler true end - def destroy_vm(_pool_name, vm_name) + def destroy_vm(pool_name, vm_name) @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) - vm_object = find_vm(vm_name, connection) + vm_object = find_vm(pool_name, vm_name, connection) # If a VM doesn't exist then it is effectively deleted return true if vm_object.nil? @@ -784,60 +784,29 @@ module Vmpooler get_snapshot_list(vm.snapshot.rootSnapshotList, snapshotname) if vm.snapshot end - def find_vm(vmname, connection) - find_vm_light(vmname, connection) || find_vm_heavy(vmname, connection)[vmname] + def build_propSpecs(datacenter, folder, vmname) + propSpecs = { + entity => self, + :inventoryPath => "#{datacenter}/vm/#{folder}/#{vmname}" + } + propSpecs end - def find_vm_light(vmname, connection) - connection.searchIndex.FindByDnsName(vmSearch: true, dnsName: vmname) - end + def find_vm(pool_name, vmname, connection) + # Find a VM by its inventory path and return the VM object + # Returns nil when a VM, or pool configuration, cannot be found + pool_configuration = pool_config(pool_name) + return nil if pool_configuration.nil? + folder = pool_configuration['folder'] + datacenter = get_target_datacenter_from_config(pool_name) + return nil if datacenter.nil? - def find_vm_heavy(vmname, connection) - vmname = vmname.is_a?(Array) ? vmname : [vmname] - container_view = get_base_vm_container_from(connection) - property_collector = connection.propertyCollector + propSpecs = { + :entity => self, + :inventoryPath => "#{datacenter}/vm/#{folder}/#{vmname}" + } - object_set = [{ - obj: container_view, - skip: true, - selectSet: [RbVmomi::VIM::TraversalSpec.new( - name: 'gettingTheVMs', - path: 'view', - skip: false, - type: 'ContainerView' - )] - }] - - prop_set = [{ - pathSet: ['name'], - type: 'VirtualMachine' - }] - - results = property_collector.RetrievePropertiesEx( - specSet: [{ - objectSet: object_set, - propSet: prop_set - }], - options: { maxObjects: nil } - ) - - vms = {} - results.objects.each do |result| - name = result.propSet.first.val - next unless vmname.include? name - vms[name] = result.obj - end - - while results.token - results = property_collector.ContinueRetrievePropertiesEx(token: results.token) - results.objects.each do |result| - name = result.propSet.first.val - next unless vmname.include? name - vms[name] = result.obj - end - end - - vms + connection.searchIndex.FindByInventoryPath(propSpecs) end def find_vmdks(vmname, datastore, connection, datacentername) @@ -880,8 +849,8 @@ module Vmpooler snapshot end - def get_vm_details(vm_name, connection) - vm_object = find_vm(vm_name, connection) + def get_vm_details(pool_name, vm_name, connection) + vm_object = find_vm(pool_name, vm_name, connection) return nil if vm_object.nil? parent_host_object = vm_object.summary.runtime.host if vm_object.summary && vm_object.summary.runtime && vm_object.summary.runtime.host raise('Unable to determine which host the VM is running on') if parent_host_object.nil? @@ -905,7 +874,7 @@ module Vmpooler @connection_pool.with_metrics do |pool_object| begin connection = ensured_vsphere_connection(pool_object) - vm_hash = get_vm_details(vm_name, connection) + vm_hash = get_vm_details(pool_name, vm_name, connection) migration_limit = @config[:config]['migration_limit'] if @config[:config].key?('migration_limit') migration_count = $redis.scard('vmpooler__migration') if migration_enabled? @config diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb index 39c891e..1757da9 100644 --- a/spec/unit/providers/vsphere_spec.rb +++ b/spec/unit/providers/vsphere_spec.rb @@ -165,7 +165,7 @@ EOT let(:vm_object) { nil } before(:each) do allow(subject).to receive(:connect_to_vsphere).and_return(connection) - expect(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object) + expect(subject).to receive(:find_vm).with(poolname,vmname,connection).and_return(vm_object) end context 'when VM does not exist' do @@ -361,7 +361,7 @@ EOT let(:disk_size) { 10 } before(:each) do allow(subject).to receive(:connect_to_vsphere).and_return(connection) - allow(subject).to receive(:find_vm).with(vmname, connection).and_return(vm_object) + allow(subject).to receive(:find_vm).with(poolname, vmname, connection).and_return(vm_object) end context 'Given an invalid pool name' do @@ -382,7 +382,7 @@ EOT context 'when VM does not exist' do before(:each) do - expect(subject).to receive(:find_vm).with(vmname, connection).and_return(nil) + expect(subject).to receive(:find_vm).with(poolname, vmname, connection).and_return(nil) end it 'should raise an error' do @@ -419,12 +419,12 @@ EOT before(:each) do allow(subject).to receive(:connect_to_vsphere).and_return(connection) - allow(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object) + allow(subject).to receive(:find_vm).with(poolname, vmname,connection).and_return(vm_object) end context 'when VM does not exist' do before(:each) do - expect(subject).to receive(:find_vm).with(vmname, connection).and_return(nil) + expect(subject).to receive(:find_vm).with(poolname, vmname, connection).and_return(nil) end it 'should raise an error' do @@ -474,12 +474,12 @@ EOT before(:each) do allow(subject).to receive(:connect_to_vsphere).and_return(connection) - allow(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object) + allow(subject).to receive(:find_vm).with(poolname,vmname,connection).and_return(vm_object) end context 'when VM does not exist' do before(:each) do - expect(subject).to receive(:find_vm).with(vmname,connection).and_return(nil) + expect(subject).to receive(:find_vm).with(poolname,vmname,connection).and_return(nil) end it 'should raise an error' do @@ -2303,14 +2303,14 @@ EOT end it 'returns nil when vm_object is not found' do - expect(subject).to receive(:find_vm).with(vmname, connection).and_return(nil) - result = subject.get_vm_details(vmname, connection) + expect(subject).to receive(:find_vm).with(poolname, vmname, connection).and_return(nil) + result = subject.get_vm_details(poolname, vmname, connection) expect(result).to be nil end it 'raises an error when unable to determine parent_host' do - expect(subject).to receive(:find_vm).with(vmname, connection).and_return(vm_object) - expect{subject.get_vm_details(vmname, connection)}.to raise_error('Unable to determine which host the VM is running on') + expect(subject).to receive(:find_vm).with(poolname, vmname, connection).and_return(vm_object) + expect{subject.get_vm_details(poolname, vmname, connection)}.to raise_error('Unable to determine which host the VM is running on') end context 'when it can find parent host' do @@ -2320,8 +2320,8 @@ EOT end it 'returns vm details hash' do - expect(subject).to receive(:find_vm).with(vmname, connection).and_return(vm_object) - result = subject.get_vm_details(vmname, connection) + expect(subject).to receive(:find_vm).with(poolname, vmname, connection).and_return(vm_object) + result = subject.get_vm_details(poolname, vmname, connection) expect(result).to eq(vm_details) end end @@ -3291,147 +3291,30 @@ EOT end describe '#find_vm' do - before(:each) do - allow(subject).to receive(:find_vm_light).and_return('vmlight') - allow(subject).to receive(:find_vm_heavy).and_return( { vmname => 'vmheavy' }) - end - - it 'should call find_vm_light' do - expect(subject).to receive(:find_vm_light).and_return('vmlight') - - expect(subject.find_vm(vmname,connection)).to eq('vmlight') - end - - it 'should not call find_vm_heavy if find_vm_light finds the VM' do - expect(subject).to receive(:find_vm_light).and_return('vmlight') - expect(subject).to receive(:find_vm_heavy).exactly(0).times - - expect(subject.find_vm(vmname,connection)).to eq('vmlight') - end - - it 'should call find_vm_heavy when find_vm_light returns nil' do - expect(subject).to receive(:find_vm_light).and_return(nil) - expect(subject).to receive(:find_vm_heavy).and_return( { vmname => 'vmheavy' }) - - expect(subject.find_vm(vmname,connection)).to eq('vmheavy') - end - end - - describe '#find_vm_light' do let(:missing_vm) { 'missing_vm' } + let(:folder) { 'Pooler/pool1' } + let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine() } before(:each) do - allow(connection.searchIndex).to receive(:FindByDnsName).and_return(nil) + allow(connection.searchIndex).to receive(:FindByInventoryPath) end - it 'should call FindByDnsName with the correct parameters' do - expect(connection.searchIndex).to receive(:FindByDnsName).with({ - :vmSearch => true, - dnsName: vmname, - }) + it 'should call FindByInventoryPath with the correct parameters' do + expect(connection.searchIndex).to receive(:FindByInventoryPath) - subject.find_vm_light(vmname,connection) + subject.find_vm(poolname,vmname,connection) end it 'should return the VM object when found' do - vm_object = mock_RbVmomi_VIM_VirtualMachine() - expect(connection.searchIndex).to receive(:FindByDnsName).with({ - :vmSearch => true, - dnsName: vmname, - }).and_return(vm_object) + expect(connection.searchIndex).to receive(:FindByInventoryPath).and_return(vm_object) - expect(subject.find_vm_light(vmname,connection)).to be(vm_object) + expect(subject.find_vm(poolname,vmname,connection)).to be(vm_object) end it 'should return nil if the VM is not found' do - expect(connection.searchIndex).to receive(:FindByDnsName).with({ - :vmSearch => true, - dnsName: missing_vm, - }).and_return(nil) + expect(connection.searchIndex).to receive(:FindByInventoryPath).and_return(nil) - expect(subject.find_vm_light(missing_vm,connection)).to be_nil - end - end - - describe '#find_vm_heavy' do - let(:missing_vm) { 'missing_vm' } - # Return an empty result by default - let(:retrieve_result) {{}} - - before(:each) do - allow(connection.propertyCollector).to receive(:RetrievePropertiesEx).and_return(mock_RbVmomi_VIM_RetrieveResult(retrieve_result)) - end - - context 'Search result is empty' do - it 'should return empty hash' do - expect(subject.find_vm_heavy(vmname,connection)).to eq({}) - end - end - - context 'Search result contains VMs but no matches' do - let(:retrieve_result) { - { :response => [ - { 'name' => 'no_match001'}, - { 'name' => 'no_match002'}, - { 'name' => 'no_match003'}, - { 'name' => 'no_match004'}, - ] - } - } - - it 'should return empty hash' do - expect(subject.find_vm_heavy(vmname,connection)).to eq({}) - end - end - - context 'Search contains a single match' do - let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname })} - let(:retrieve_result) { - { :response => [ - { 'name' => 'no_match001'}, - { 'name' => 'no_match002'}, - { 'name' => vmname, :object => vm_object }, - { 'name' => 'no_match003'}, - { 'name' => 'no_match004'}, - ] - } - } - - it 'should return single result' do - result = subject.find_vm_heavy(vmname,connection) - expect(result.keys.count).to eq(1) - end - - it 'should return the matching VM Object' do - result = subject.find_vm_heavy(vmname,connection) - expect(result[vmname]).to be(vm_object) - end - end - - context 'Search contains a two matches' do - let(:vm_object1) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname })} - let(:vm_object2) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname })} - let(:retrieve_result) { - { :response => [ - { 'name' => 'no_match001'}, - { 'name' => 'no_match002'}, - { 'name' => vmname, :object => vm_object1 }, - { 'name' => 'no_match003'}, - { 'name' => 'no_match004'}, - { 'name' => vmname, :object => vm_object2 }, - ] - } - } - - it 'should return one result' do - result = subject.find_vm_heavy(vmname,connection) - expect(result.keys.count).to eq(1) - end - - it 'should return the last matching VM Object' do - result = subject.find_vm_heavy(vmname,connection) - expect(result[vmname]).to be(vm_object2) - end + expect(subject.find_vm(poolname,missing_vm,connection)).to be_nil end end