Update pool_manager and vsphere tests to support changes in host selection

lib/vmpooler/pool_manager.rb

@@ -542,7 +542,7 @@ module Vmpooler
     def migrate_vm(vm_name, pool_name, provider)
       Thread.new do
         begin
-          _migrate_vm(vm_name, pool_name, provider, $provider_hosts)
+          _migrate_vm(vm_name, pool_name, provider)
         rescue => err
           $logger.log('s', "[x] [#{pool_name}] '#{vm_name}' migration failed with an error: #{err}")
           remove_vmpooler_migration_vm(pool_name, vm_name)

lib/vmpooler/providers/vsphere.rb

@@ -178,17 +178,12 @@ module Vmpooler
               vm_target_folder = find_folder(target_folder_path, connection, target_datacenter_name)
             rescue => _err
               if _err =~ /Unexpected object type encountered/
-                vm_target_folder = nil
-              else
-                raise(_err)
-              end
-            end
-            if vm_target_folder.nil?
                 if $config[:config]['create_folders'] == true
                   dc = connection.serviceInstance.find_datacenter(target_datacenter_name)
                   vm_target_folder = dc.vmFolder.traverse(target_folder_path, type=RbVmomi::VIM::Folder, create=true)
                 else
-                raise("Unexpected object type encountered while finding folder")
+                  raise(_err)
+                end
               end
             end
 

spec/unit/pool_manager_spec.rb

@@ -1476,6 +1476,383 @@ EOT
   end
 
 
+  describe '#get_provider_name' do
+    context 'with a single provider and no pool specified provider' do
+      let(:config) {
+        YAML.load(<<-EOT
+---
+:providers:
+  :vc1:
+    server: 'server1'
+:pools:
+  - name: #{pool}
+EOT
+        )
+      }
+      it 'returns the name of the configured provider' do
+        expect(subject.get_provider_name(pool, config)).to eq('vc1')
+      end
+    end
+
+    context 'with no providers configured' do
+      let(:config) {
+        YAML.load(<<-EOT
+---
+:pools:
+  - name: #{pool}
+EOT
+        )
+      }
+
+      it 'should return default' do
+        expect(subject.get_provider_name(pool, config)).to eq('default')
+      end
+    end
+
+    context 'with a provider configured for the pool' do
+      let(:config) {
+        YAML.load(<<-EOT
+---
+:providers:
+  :vc1:
+    server: 'server1'
+  :vc2:
+    server: 'server2'
+:pools:
+  - name: #{pool}
+    provider: 'vc2'
+EOT
+        )
+      }
+
+      it 'should return the configured provider name' do
+        expect(subject.get_provider_name(pool, config)).to eq('vc2')
+      end
+    end
+  end
+
+
+  describe '#get_cluster' do
+    let(:cluster) { 'cluster1' }
+    let(:datacenter) { 'dc1' }
+    let(:cluster_dc) { { 'cluster' => cluster, 'datacenter' => datacenter } }
+
+    context 'defaults configured' do
+      let(:config) {
+        YAML.load(<<-EOT
+---
+:config:
+  clone_target: 'cluster1'
+  datacenter: 'dc1'
+:pools:
+  - name: #{pool}
+EOT
+        )
+      }
+
+      it 'should return the default cluster and dc' do
+        expect(subject.get_cluster(pool)).to eq(cluster_dc)
+      end
+    end
+
+    context 'with clone_target specified for pool' do
+      let(:config) {
+        YAML.load(<<-EOT
+---
+:config:
+  datacenter: 'dc1'
+:pools:
+  - name: #{pool}
+    clone_target: 'cluster1'
+EOT
+        )
+      }
+
+      it 'should return the configured cluster and dc' do
+        expect(subject.get_cluster(pool)).to eq(cluster_dc)
+      end
+    end
+
+    context 'with clone_target and datacenter specified for pool' do
+      let(:config) {
+        YAML.load(<<-EOT
+---
+:config:
+  task_limit: 10
+:pools:
+  - name: #{pool}
+    clone_target: 'cluster1'
+    datacenter: 'dc1'
+EOT
+        )
+      }
+      before(:each) do
+        $config = config
+      end
+      it 'should return the configured cluster and dc' do
+        expect(subject.get_cluster(pool)).to eq(cluster_dc)
+      end
+    end
+  end
+
+
+  describe '#select_hosts' do
+
+    let(:config) {
+      YAML.load(<<-EOT
+---
+:config:
+  task_limit: 10
+  clone_target: 'cluster1'
+:pools:
+  - name: #{pool}
+    size: 10
+EOT
+      )
+    }
+    let(:provider_name) { 'default' }
+    let(:datacenter) { 'dc1' }
+    let(:cluster) { 'cluster1' }
+    let(:percentage) { 100 }
+    let(:architecture) { 'v3' }
+    let(:hosts_hash) {
+      {
+        'hosts' => [ 'host1' ],
+        'architectures' => { architecture => ['host1'] }
+      }
+    }
+    let(:provider_hosts) {
+      {
+        provider_name => {
+          datacenter => {
+            cluster => hosts_hash
+          }
+        }
+      }
+    }
+
+    it 'should populate $provider_hosts' do
+      expect(provider).to receive(:select_target_hosts).with(cluster, datacenter, percentage).and_return(hosts_hash)
+      subject.select_hosts(pool, provider, provider_name, cluster, datacenter, percentage)
+      expect($provider_hosts).to eq(provider_hosts)
+    end
+  end
+
+  describe '#run_select_hosts' do
+    let(:config) {
+      YAML.load(<<-EOT
+---
+:config:
+  task_limit: 10
+  clone_target: 'cluster1'
+:pools:
+  - name: #{pool}
+    size: 10
+EOT
+      )
+    }
+    let(:provider_hosts) {
+      {
+        provider_name => {
+          datacenter => {
+            cluster => {
+              'check_time_finished' => Time.now,
+              'hosts' => [ 'host1' ],
+              'architectures' => { 'v3' => ['host1'] },
+            }
+          }
+        }
+      }
+    }
+    let(:provider) { double('provider') }
+    let(:pool_object) { config[:pools][0] }
+    let(:pool_name) { pool_object['name'] }
+    let(:maxage) { 60 }
+    let(:cluster) { 'cluster1' }
+    let(:datacenter) { 'dc1' }
+    let(:provider_name) { 'default' }
+    let(:maxage) { 60 }
+    let(:percentage_of_hosts_below_average) { 100 }
+    # A wrapper to ensure select_hosts is not run more than once, and results are present
+    before(:each) do
+      expect(subject).not_to be_nil
+      $provider_hosts = provider_hosts
+    end
+
+    context '$provider_hosts has key checking' do
+      before(:each) do
+        $provider_hosts[provider_name][datacenter][cluster]['checking'] = true
+      end
+
+      it 'runs wait_for_host_selection' do
+        expect(subject).to receive(:wait_for_host_selection).with(pool_name, provider_name, cluster, datacenter)
+        subject.run_select_hosts(provider, pool_name, provider_name, cluster, datacenter, maxage, percentage_of_hosts_below_average)
+      end
+
+    end
+
+    context '$provider_hosts has check_time_finished key and is 100 seconds old' do
+
+      before(:each) do
+        $provider_hosts[provider_name][datacenter][cluster]['check_time_finished'] = Time.now - 100
+      end
+
+      it 'runs select_hosts' do
+        expect(provider).to receive(:select_target_hosts).with(cluster, datacenter, percentage_of_hosts_below_average).and_return(provider_hosts[provider_name][datacenter][cluster])
+
+        subject.run_select_hosts(provider, pool_name, provider_name, cluster, datacenter, maxage, percentage_of_hosts_below_average)
+      end
+    end
+
+    context '$provider_hosts has check_time_finished key 10 seconds old' do
+      before(:each) do
+        $provider_hosts = provider_hosts
+        $provider_hosts['check_time_finished'] = Time.now - 10
+      end
+
+      it 'does not run select_hosts' do
+        expect(subject).not_to receive(:select_hosts)
+
+        subject.run_select_hosts(provider, pool_name, provider_name, cluster, datacenter, maxage, percentage_of_hosts_below_average)
+      end
+    end
+
+    context '$provider_hosts does not have key check_time_finished' do
+      let(:provider_hosts) { { } }
+
+      before(:each) do
+        $provider_hosts = provider_hosts
+      end
+
+      it 'runs select_hosts' do
+        expect(subject).to receive(:select_hosts).with(provider).with(pool_name, provider, provider_name, cluster, datacenter, percentage_of_hosts_below_average)
+
+        subject.run_select_hosts(provider, pool_name, provider_name, cluster, datacenter, maxage, percentage_of_hosts_below_average)
+      end
+    end
+  end
+
+
+  describe '#wait_for_host_selection' do
+    let(:loop_delay) { 0 }
+    let(:max_age) { 60 }
+    let(:provider_name) { 'default' }
+    let(:cluster) { 'cluster1' }
+    let(:datacenter) { 'dc1' }
+    let(:maxloop) { 1 }
+    let(:provider_hosts) {
+      {
+        provider_name => {
+          datacenter => {
+            cluster => {
+            }
+          }
+        }
+      }
+    }
+
+    before(:each) do
+      expect(subject).not_to be_nil
+      $provider_hosts = provider_hosts
+    end
+
+    context 'when provider_hosts does not have key check_time_finished and maxloop is one' do
+
+      it 'sleeps for loop_delay once' do
+        expect(subject).to receive(:sleep).with(loop_delay).exactly(maxloop).times
+        expect($provider_hosts).to eq(provider_hosts)
+        expect($provider_hosts).to have_key(provider_name)
+        expect(provider_hosts[provider_name]).to have_key(datacenter)
+
+        subject.wait_for_host_selection(pool, provider_name, cluster, datacenter, maxloop, loop_delay, max_age)
+      end
+    end
+
+    context 'when provider_hosts does not have key check_time_finished and maxloop is two' do
+      let(:maxloop) { 2 }
+
+      it 'sleeps for loop_delay two times' do
+        expect(subject).to receive(:sleep).with(loop_delay).exactly(maxloop).times
+
+        subject.wait_for_host_selection(pool, provider_name, cluster, datacenter, maxloop, loop_delay, max_age)
+      end
+    end
+
+    context 'when $provider_hosts has key check_time_finished and age is greater than max_age' do
+      let(:provider_hosts) {
+        {
+          provider_name => {
+            datacenter => {
+              cluster => {
+                'check_time_finished' => Time.now - 100
+              }
+            }
+          }
+        }
+      }
+
+      before(:each) do
+        $provider_hosts = provider_hosts
+      end
+
+      it 'sleeps for loop_delay once' do
+        expect(subject).to receive(:sleep).with(loop_delay).exactly(maxloop).times
+
+        subject.wait_for_host_selection(pool, provider_name, cluster, datacenter, maxloop, loop_delay, max_age)
+      end
+    end
+  end
+
+
+  describe '#select_next_host' do
+
+    let(:hosts_hash) { }
+    let(:cluster) { 'cluster1' }
+    let(:architecture) { 'v3' }
+    let(:target_host) { 'host1' }
+    let(:provider_name) { 'default' }
+    let(:datacenter) { 'dc1' }
+    let(:provider_hosts) {
+      {
+        provider_name => {
+          datacenter => {
+            cluster => {
+              'check_time_finished' => Time.now,
+              'architectures' => { architecture => ['host1', 'host2'] }
+            }
+          }
+        }
+      }
+    }
+    before(:each) do
+      expect(subject).not_to be_nil
+      $provider_hosts = provider_hosts
+    end
+
+    context 'with a list of hosts available' do
+
+      it 'returns the first host from the target cluster and architecture list' do
+        expect(subject.select_next_host(provider_name, datacenter, cluster, architecture)).to eq(target_host)
+      end
+
+      it 'return the second host on the second call to select_next_host' do
+        expect(subject.select_next_host(provider_name, datacenter, cluster, architecture)).to eq(target_host)
+        expect(subject.select_next_host(provider_name, datacenter, cluster, architecture)).to eq('host2')
+      end
+    end
+
+    context 'with no hosts available' do
+      before(:each) do
+        $provider_hosts[provider_name][datacenter][cluster]['architectures'][architecture] = []
+      end
+      it 'returns nil' do
+        expect(subject.select_next_host(provider_name, datacenter, cluster, architecture)).to be_nil
+      end
+    end
+
+  end
+
+
   describe '#migration_limit' do
     # This is a little confusing.  Is this supposed to return a boolean
     # or integer type?
@@ -1510,6 +1887,19 @@ EOT
       subject.migrate_vm(vm, pool, provider)
     end
 
+      let(:provider_hosts) {
+        {
+          'check_time_finished' => Time.now,
+          'clusters' => {
+            'cluster1' => {
+              'hosts' => ['host1'],
+              'architectures' => { 'v3' => ['host1'] },
+              'all_hosts' => ['host1']
+            }
+          }
+        }
+      }
+
     context 'When an error is raised' do
       before(:each) do
         expect(subject).to receive(:_migrate_vm).with(vm, pool, provider).and_raise('MockError')
@@ -1531,24 +1921,53 @@ EOT
   end
 
   describe "#_migrate_vm" do
-    let(:vm_parent_hostname) { 'parent1' }
+    let(:vm_parent_hostname) { 'host1' }
+    let(:cluster_name) { 'cluster1' }
+    let(:host_architecture) { 'v3' }
+    let(:datacenter) { 'dc1' }
+    let(:provider_name) { 'default' }
+    let(:percentage) { 100 }
     let(:config) {
       YAML.load(<<-EOT
 ---
 :config:
   migration_limit: 5
+  clone_target: 'cluster1'
+:pools:
+  - name: #{pool}
 EOT
       )
     }
+    let(:provider_hosts) {
+      {
+        provider_name => {
+          datacenter => {
+            cluster_name => {
+              'check_time_finished' => Time.now,
+              'hosts' => [vm_parent_hostname],
+              'architectures' => { host_architecture => [vm_parent_hostname] }
+            }
+          }
+        }
+      }
+    }
+    let(:vm_data) {
+      {
+       'host' => vm_parent_hostname,
+       'cluster' => cluster_name,
+       'architecture' => host_architecture,
+       'datacenter' => datacenter
+      }
+    }
 
     before(:each) do
       expect(subject).not_to be_nil
-      allow(provider).to receive(:get_vm_host).with(pool, vm).and_return(vm_parent_hostname)
+      allow(provider).to receive(:get_vm_details).with(pool, vm).and_return(vm_data)
     end
 
     context 'when an error occurs trying to retrieve the current host' do
       before(:each) do
-        expect(provider).to receive(:get_vm_host).with(pool, vm).and_raise(RuntimeError,'MockError')
+        expect(provider).to receive(:get_vm_details).with(pool, vm).and_raise(RuntimeError,'MockError')
       end
 
       it 'should raise an error' do
@@ -1557,8 +1976,18 @@ EOT
     end
 
     context 'when the current host can not be determined' do
+      let(:vm_data) {
+        {
+         'host' => nil,
+         'cluster' => cluster_name,
+         'architecture' => host_architecture,
+         'datacenter' => datacenter
+        }
+      }
+
       before(:each) do
-        expect(provider).to receive(:get_vm_host).with(pool, vm).and_return(nil)
+        $provider_hosts = provider_hosts
+        allow(provider).to receive(:get_vm_details).with(pool, vm).and_return(vm_data)
       end
 
       it 'should raise an error' do
@@ -1569,6 +1998,7 @@ EOT
     context 'when VM exists but migration is disabled' do
       before(:each) do
         create_migrating_vm(vm, pool)
+        expect(provider).to receive(:get_vm_details).with(pool, vm).and_return(vm_data)
       end
 
       [-1,-32768,false,0].each do |testvalue|
@@ -1578,8 +2008,7 @@ EOT
           subject._migrate_vm(vm, pool, provider)
         end
 
-        it "should remove the VM from vmpooler__migrating queue in redis if the migration limit is #{testvalue}" do
-          redis.sadd("vmpooler__migrating__#{pool}", vm)
+        it "should not remove the VM from vmpooler__migrating queue in redis if the migration limit is #{testvalue}" do
           config[:config]['migration_limit'] = testvalue
 
           expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_truthy
@@ -1591,6 +2020,9 @@ EOT
 
     context 'when VM exists but migration limit is reached' do
       before(:each) do
+        $provider_hosts = provider_hosts
+        expect(provider).to receive(:select_target_hosts).with(cluster_name, datacenter, percentage).and_return($provider_hosts[provider_name][datacenter][cluster_name])
+        expect(provider).to receive(:get_vm_details).with(pool, vm).and_return(vm_data)
 
         create_migrating_vm(vm, pool)
         redis.sadd('vmpooler__migration', 'fakevm1')
@@ -1613,15 +2045,22 @@ EOT
     end
 
     context 'when VM exists but migration limit is not yet reached' do
+
       before(:each) do
         create_migrating_vm(vm, pool)
+        expect(provider).to receive(:get_vm_details).with(pool, vm).and_return(vm_data)
         redis.sadd('vmpooler__migration', 'fakevm1')
         redis.sadd('vmpooler__migration', 'fakevm2')
       end
 
-      context 'and host to migrate to is the same as the current host' do
+      context 'and current host is within the list of available targets' do
+        let(:target_hosts) { ['host1'] }
+        #
         before(:each) do
-          expect(provider).to receive(:find_least_used_compatible_host).with(pool, vm).and_return(vm_parent_hostname)
+          $provider_hosts = provider_hosts
+          expect(provider).to receive(:select_target_hosts).with(cluster_name, datacenter, percentage).and_return($provider_hosts[provider_name][datacenter][cluster_name])
+
+          #expect(subject).to receive(:host_in_targets?).with(vm_parent_hostname, target_hosts).and_return(true)
         end
 
         it "should not migrate the VM" do
@@ -1641,16 +2080,26 @@ EOT
           expect(redis.scard('vmpooler__migration')).to eq(before_count)
         end
 
-        it "should call remove_vmpooler_migration_vm" do
-          expect(subject).to receive(:remove_vmpooler_migration_vm)
+        it "should not call remove_vmpooler_migration_vm" do
+          expect(subject).not_to receive(:remove_vmpooler_migration_vm)
           subject._migrate_vm(vm, pool, provider)
         end
       end
 
       context 'and host to migrate to different to the current host' do
-        let(:vm_new_hostname) { 'new_hostname' }
+        let(:vm_new_hostname) { 'host1' }
+        let(:vm_parent_hostname) { 'host2' }
+        let(:dc) { 'dc1' }
+        let(:host_architecture) { 'v3' }
+        let(:hosts_hash) {
+          {
+            'hosts' => [ 'host1' ],
+            'architectures' => { 'v3' => ['host1'] },
+          }
+        }
         before(:each) do
-          expect(provider).to receive(:find_least_used_compatible_host).with(pool, vm).and_return(vm_new_hostname)
+          $provider_hosts = provider_hosts
+          expect(provider).to receive(:select_target_hosts).with(cluster_name, dc, percentage).and_return(hosts_hash)
           expect(subject).to receive(:migrate_vm_and_record_timing).with(vm, pool, vm_parent_hostname, vm_new_hostname, provider).and_return('1.00')
         end
 

spec/unit/providers/vsphere_spec.rb

@@ -161,136 +161,6 @@ EOT
     end
   end
 
-  describe '#get_vm_host' do
-    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)
-    end
-
-    context 'when VM does not exist' do
-      let(:vm_object) { nil }
-
-      it 'should get a connection' do
-        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
-
-        subject.get_vm_host(poolname,vmname)
-      end
-
-      it 'should return nil' do
-        expect(subject.get_vm_host(poolname,vmname)).to be_nil
-      end
-    end
-
-    context 'when VM exists but missing runtime information' do
-      # For example if the VM is shutdown
-      let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({
-          :name => vmname,
-        })
-      }
-
-      before(:each) do
-        vm_object.summary.runtime = nil
-      end
-
-      it 'should get a connection' do
-        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
-
-        subject.get_vm_host(poolname,vmname)
-      end
-
-      it 'should return nil' do
-        expect(subject.get_vm_host(poolname,vmname)).to be_nil
-      end
-    end
-
-    context 'when VM exists and is running on a host' do
-      let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({
-          :name => vmname,
-        })
-      }
-      let(:hostname) { 'HOST001' }
-
-      before(:each) do
-        mock_host = mock_RbVmomi_VIM_HostSystem({ :name => hostname })
-        vm_object.summary.runtime.host = mock_host
-      end
-
-      it 'should get a connection' do
-        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
-
-        subject.get_vm_host(poolname,vmname)
-      end
-
-      it 'should return the hostname' do
-        expect(subject.get_vm_host(poolname,vmname)).to eq(hostname)
-      end
-    end
-  end
-
-  describe '#find_least_used_compatible_host' do
-    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)
-    end
-
-    context 'when VM does not exist' do
-      let(:vm_object) { nil }
-
-      it 'should get a connection' do
-        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
-
-        subject.find_least_used_compatible_host(poolname,vmname)
-      end
-
-      it 'should return nil' do
-        expect(subject.find_least_used_compatible_host(poolname,vmname)).to be_nil
-      end
-    end
-
-    context 'when VM exists but no compatible host' do
-      let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname }) }
-      let(:host_list) { nil }
-
-      before(:each) do
-        expect(subject).to receive(:find_least_used_vpshere_compatible_host).with(vm_object).and_return(host_list)
-      end
-
-      it 'should get a connection' do
-        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
-
-        subject.find_least_used_compatible_host(poolname,vmname)
-      end
-
-      it 'should return nil' do
-        expect(subject.find_least_used_compatible_host(poolname,vmname)).to be_nil
-      end
-    end
-
-    context 'when VM exists and a compatible host' do
-      let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname }) }
-      let(:hostname) { 'HOST001' }
-      # As per find_least_used_vpshere_compatible_host, the return value is an array
-      #  [ <HostObject>, <Hostname> ]
-      let(:host_list) { [mock_RbVmomi_VIM_HostSystem({ :name => hostname }), hostname] }
-
-      before(:each) do
-        expect(subject).to receive(:find_least_used_vpshere_compatible_host).with(vm_object).and_return(host_list)
-      end
-
-      it 'should get a connection' do
-        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
-
-        subject.find_least_used_compatible_host(poolname,vmname)
-      end
-
-      it 'should return the hostname' do
-        expect(subject.find_least_used_compatible_host(poolname,vmname)).to eq(hostname)
-      end
-    end
-  end
-
   describe '#migrate_vm_to_host' do
     let(:dest_host_name) { 'HOST002' }
     let(:cluster_name) { 'CLUSTER001' }
@@ -321,55 +191,52 @@ EOT
       end
     end
 
-    context 'Given a missing cluster name in the pool configuration' do
-      let(:cluster_name) { 'missing_cluster' }
+    context 'Given a missing host targeted for migration' do
+      let(:host) { mock_RbVmomi_VIM_HostSystem() }
 
       before(:each) do
         config[:pools][0]['clone_target'] = cluster_name
-        expect(subject).to receive(:find_cluster).with(cluster_name,connection,datacenter_name).and_return(nil)
+        allow(connection.searchIndex).to receive(:FindByDnsName).and_return(nil)
       end
 
       it 'should raise an error' do
-        expect{ subject.migrate_vm_to_host(poolname, vmname, dest_host_name) }.to raise_error(/#{cluster_name} which does not exist/)
+        expect{ subject.migrate_vm_to_host(poolname, vmname, dest_host_name) }.to raise_error(/#{dest_host_name} which can not be found/)
       end
     end
 
     context 'Given a missing cluster name in the global configuration' do
+      let(:host) { mock_RbVmomi_VIM_HostSystem() }
       let(:cluster_name) { 'missing_cluster' }
 
       before(:each) do
         config[:pools][0]['clone_target'] = nil
         config[:config]['clone_target'] = cluster_name
-        expect(subject).to receive(:find_cluster).with(cluster_name,connection,datacenter_name).and_return(nil)
+        allow(connection.searchIndex).to receive(:FindByDnsName).and_return(nil)
       end
 
       it 'should raise an error' do
-        expect{ subject.migrate_vm_to_host(poolname, vmname, dest_host_name) }.to raise_error(/#{cluster_name} which does not exist/)
+        expect{ subject.migrate_vm_to_host(poolname, vmname, dest_host_name) }.to raise_error(/#{dest_host_name} which can not be found/)
       end
     end
 
-    context 'Given a missing hostname in the cluster' do
-      before(:each) do
-        config[:pools][0]['clone_target'] = cluster_name
-        mock_cluster = mock_RbVmomi_VIM_ComputeResource({
-          :hosts => [ { :name => 'HOST001' },{ :name => dest_host_name} ]
-        })
-        expect(subject).to receive(:find_cluster).with(cluster_name,connection,datacenter_name).and_return(mock_cluster)
-        expect(subject).to receive(:migrate_vm_host).exactly(0).times
-      end
-
-      it 'should return true' do
-        expect(subject.migrate_vm_to_host(poolname, vmname, 'missing_host')).to be false
-      end
-    end
+#   context 'Given a missing hostname in the cluster' do
+#     let(:host) { mock_RbVmomi_VIM_HostSystem() }
+#     before(:each) do
+#       config[:pools][0]['clone_target'] = cluster_name
+#       allow(connection.searchIndex).to receive(:FindByDnsName).and_return(host)
+#       expect(subject).to receive(:migrate_vm_host).exactly(0).times
+#     end
+#
+#     it 'should return true' do
+#       expect(subject.migrate_vm_to_host(poolname, vmname, 'missing_host')).to be false
+#     end
+#   end
 
     context 'Given an error during migration' do
+      let(:host) { mock_RbVmomi_VIM_HostSystem() }
       before(:each) do
         config[:pools][0]['clone_target'] = cluster_name
-        mock_cluster = mock_RbVmomi_VIM_ComputeResource({
-          :hosts => [ { :name => 'HOST001' },{ :name => dest_host_name} ]
-        })
-        expect(subject).to receive(:find_cluster).with(cluster_name,connection,datacenter_name).and_return(mock_cluster)
+        allow(connection.searchIndex).to receive(:FindByDnsName).and_return(host)
         expect(subject).to receive(:migrate_vm_host).with(Object,Object).and_raise(RuntimeError,'MockMigrationError')
       end
 
@@ -379,12 +246,10 @@ EOT
     end
 
     context 'Given a successful migration' do
+      let(:host) { mock_RbVmomi_VIM_HostSystem() }
       before(:each) do
         config[:pools][0]['clone_target'] = cluster_name
-        mock_cluster = mock_RbVmomi_VIM_ComputeResource({
-          :hosts => [ { :name => 'HOST001' },{ :name => dest_host_name} ]
-        })
-        expect(subject).to receive(:find_cluster).with(cluster_name,connection,datacenter_name).and_return(mock_cluster)
+        allow(connection.searchIndex).to receive(:FindByDnsName).and_return(host)
         expect(subject).to receive(:migrate_vm_host).with(Object,Object).and_return(nil)
       end
 
@@ -1871,8 +1736,8 @@ EOT
     let(:cpu_model) { 'vendor line type sku v4 speed' }
     let(:model) { 'v4' }
     let(:different_model) { 'different_model' }
-    let(:limit) { 80 }
-    let(:default_limit) { 90 }
+    let(:limit) { 75 }
+    let(:default_limit) { 80 }
 
     context "host with a different model" do
       let(:host) { mock_RbVmomi_VIM_HostSystem() }
@@ -1998,7 +1863,7 @@ EOT
         })
       }
       it 'should return the sum of CPU and Memory utilization' do
-        expect(subject.get_host_utilization(host,model,limit)[0]).to eq(10 + 20)
+        expect(subject.get_host_utilization(host,model,limit)[0]).to eq(10)
       end
 
       it 'should return the host' do
@@ -2103,15 +1968,85 @@ EOT
     end
   end
 
-  describe '#find_least_used_host' do
+# describe '#get_vm_cluster' do
+#   it 'returns the name of a vm_object parent cluster' do
+#
+#   end
+#
+#   it 'returns nil when cluster_name is not found for the vm_object' do
+#
+#   end
+# end
+#
+# describe '#get_vm_cpu_architecture' do
+#   it 'returns the architecture of a vm_object parent host' do
+#
+#   end
+# end
+#
+# describe '#select_target_hosts' do
+#   it 'returns a hash of the least used hosts by cluster and architecture' do
+#
+#   end
+#
+#   it 'raises an error if the target cluster does not exist' do
+#
+#   end
+#
+#   it 'finds a cluster without a specified datacenter' do
+#
+#   end
+#
+#   it 'finds a cluster with a datacenter specified' do
+#
+#   end
+# end
+#
+# describe '#get_average_cluster_utilization' do
+#   it 'returns the average utilization for a given set of host utilizations assuming the first member of the list for each host is the utilization value' do
+#
+#   end
+# end
+#
+# describe '#build_compatible_hosts_lists' do
+#   it 'returns a hash of target host architecture versions containing lists of target hosts' do
+#
+#   end
+# end
+#
+# describe '#select_least_used_hosts' do
+#   it 'returns the percentage specified of the least used hosts in the cluster determined by selecting from less than or equal to average cluster utilization' do
+#
+#   end
+#
+#   it 'raises an error when the provided hosts list is empty' do
+#
+#   end
+# end
+#
+# describe '#find_host_by_dnsname' do
+#   it 'returns a host object when a matching host is found by dnsname in connection.searchIndex' do
+#
+#   end
+#
+#   it 'returns nil when the host object is not found by dnsname in connection.searchIndex' do
+#
+#   end
+# end
+
+  describe '#find_least_used_hosts' do
     let(:cluster_name) { 'cluster' }
     let(:missing_cluster_name) { 'missing_cluster' }
     let(:datacenter_object) { mock_RbVmomi_VIM_Datacenter() }
+    let(:percentage) { 100 }
 
     before(:each) do
       # This mocking is a little fragile but hard to do without a real vCenter instance
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       allow(connection.serviceInstance).to receive(:find_datacenter).and_return(datacenter_object)
       datacenter_object.hostFolder.childEntity = [cluster_object]
+      #allow(connection.serviceInstance).to receive(:find_cluster).and_return(nil)
+      #allow(connection.serviceInstance).to receive(:find_least_used_hosts)
     end
 
     context 'missing cluster' do
@@ -2123,7 +2058,7 @@ EOT
       let(:expected_host) { cluster_object.host[0] }
 #,datacenter_name
       it 'should raise an error' do
-        expect{subject.find_least_used_host(missing_cluster_name,connection,datacenter_name)}.to raise_error(NoMethodError,/undefined method/)
+        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(NoMethodError,/undefined method/)
       end
     end
 
@@ -2133,12 +2068,12 @@ EOT
         :hosts => [{
           :name => cluster_name,
       }]})}
-      let(:expected_host) { cluster_object.host[0] }
+      let(:expected_host) { cluster_object.host[0][:name] }
 
       it 'should return the standalone host' do
-        result = subject.find_least_used_host(cluster_name,connection,datacenter_name)
+        result = subject.find_least_used_hosts(cluster_name,datacenter_name,percentage)
 
-        expect(result).to be(expected_host)
+        expect(result['hosts'][0]).to be(expected_host)
       end
     end
 
@@ -2152,7 +2087,7 @@ EOT
       let(:expected_host) { cluster_object.host[0] }
 
       it 'should raise an error' do
-        expect{subject.find_least_used_host(missing_cluster_name,connection,datacenter_name)}.to raise_error(NoMethodError,/undefined method/)
+        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(NoMethodError,/undefined method/)
       end
     end
 
@@ -2164,12 +2099,12 @@ EOT
           { :overall_cpu_usage => 1, :overall_memory_usage => 1, :cpu_speed => 100, :num_cores_per_cpu => 1, :num_cpu => 1, :memory_size => 100.0 * 1024 * 1024 },
           { :overall_cpu_usage => 21, :overall_memory_usage => 21, :cpu_speed => 100, :num_cores_per_cpu => 1, :num_cpu => 1, :memory_size => 100.0 * 1024 * 1024 },
       ]}) }
-      let(:expected_host) { cluster_object.host[1] }
+      let(:expected_host) { cluster_object.host[1].name }
 
       it 'should return the standalone host' do
-        result = subject.find_least_used_host(cluster_name,connection,datacenter_name)
+        result = subject.find_least_used_hosts(cluster_name,datacenter_name,percentage)
 
-        expect(result).to be(expected_host)
+        expect(result['hosts'][0]).to be(expected_host)
       end
     end
 
@@ -2184,7 +2119,7 @@ EOT
       let(:expected_host) { cluster_object.host[1] }
 
       it 'should raise an error' do
-        expect{subject.find_least_used_host(missing_cluster_name,connection,datacenter_name)}.to raise_error(NoMethodError,/undefined method/)
+        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(NoMethodError,/undefined method/)
       end
     end
 
@@ -2198,12 +2133,12 @@ EOT
           { :overall_cpu_usage => 100, :overall_memory_usage => 100, :cpu_speed => 100, :num_cores_per_cpu => 1, :num_cpu => 1, :memory_size => 100.0 * 1024 * 1024 },
           { :overall_cpu_usage => 21, :overall_memory_usage => 21, :cpu_speed => 100, :num_cores_per_cpu => 1, :num_cpu => 1, :memory_size => 100.0 * 1024 * 1024 },
       ]}) }
-      let(:expected_host) { cluster_object.host[1] }
+      let(:expected_host) { cluster_object.host[1].name }
 
       it 'should return the standalone host' do
-        result = subject.find_least_used_host(cluster_name,connection,datacenter_name)
+        result = subject.find_least_used_hosts(cluster_name,datacenter_name,percentage)
 
-        expect(result).to be(expected_host)
+        expect(result['hosts'][0]).to be(expected_host)
       end
     end
 
@@ -2219,7 +2154,7 @@ EOT
 
       it 'should return a host' do
         pending('https://github.com/puppetlabs/vmpooler/issues/206')
-        result = subject.find_least_used_host(missing_cluster_name,connection,datacenter_name)
+        result = subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)
         expect(result).to_not be_nil
       end
     end