From cd979fc24da70ba3b9fa4f75264cb38887569f74 Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Fri, 3 Nov 2017 12:50:05 -0700
Subject: [PATCH] Move migrate_vm logic to vsphere provider

This commit moves the migrate_vm logic to the vsphere provider. Without
this change migrate_vm has lots of vsphere specific logic in
pool_manager migrate_vm method.
---
 lib/vmpooler/pool_manager.rb        | 133 +-----
 lib/vmpooler/providers/base.rb      |  12 +
 lib/vmpooler/providers/vsphere.rb   | 246 ++++++++--
 spec/unit/pool_manager_spec.rb      | 691 +---------------------------
 spec/unit/providers/vsphere_spec.rb | 334 +++++++-------
 5 files changed, 382 insertions(+), 1034 deletions(-)

diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb
index 68ba157..f9ff39f 100644
--- a/lib/vmpooler/pool_manager.rb
+++ b/lib/vmpooler/pool_manager.rb
@@ -21,9 +21,6 @@ module Vmpooler
 
       # Our thread-tracker object
       $threads = {}
-
-      # Host tracking object
-      $provider_hosts = {}
     end
 
     def config
@@ -462,143 +459,17 @@ module Vmpooler
       end
     end
 
-    def get_provider_name(pool_name, config = $config)
-      pool = config[:pools].select { |p| p['name'] == pool_name }[0]
-      provider_name = pool['provider'] if pool.key?('provider')
-      provider_name = config[:providers].first[0].to_s if provider_name.nil? and config.key?(:providers)
-      provider_name = 'default' if provider_name.nil?
-      provider_name
-    end
-
-    def get_cluster(pool_name)
-      default_cluster = $config[:config]['clone_target'] if $config[:config].key?('clone_target')
-      default_datacenter = $config[:config]['datacenter'] if $config[:config].key?('datacenter')
-      pool = $config[:pools].select { |p| p['name'] == pool_name }[0]
-      cluster = pool['clone_target'] if pool.key?('clone_target')
-      cluster = default_cluster if cluster.nil?
-      datacenter = pool['datacenter'] if pool.key?('datacenter')
-      datacenter = default_datacenter if datacenter.nil?
-      return if cluster.nil?
-      return if datacenter.nil?
-      { 'cluster' => cluster, 'datacenter' => datacenter }
-    end
-
-    def select_hosts(pool_name, provider, provider_name, cluster, datacenter, percentage)
-      $provider_hosts[provider_name] = {} unless $provider_hosts.key?(provider_name)
-      $provider_hosts[provider_name][datacenter] = {} unless $provider_hosts[provider_name].key?(datacenter)
-      $provider_hosts[provider_name][datacenter][cluster] = {} unless $provider_hosts[provider_name][datacenter].key?(cluster)
-      $provider_hosts[provider_name][datacenter][cluster]['checking'] = true
-      hosts_hash = provider.select_target_hosts(cluster, datacenter, percentage)
-      $provider_hosts[provider_name][datacenter][cluster] = hosts_hash
-      $provider_hosts[provider_name][datacenter][cluster]['check_time_finished'] = Time.now
-    end
-
-    def run_select_hosts(provider, pool_name, provider_name, cluster, datacenter, max_age, percentage)
-      now = Time.now
-      if $provider_hosts.key?(provider_name) and $provider_hosts[provider_name].key?(datacenter) and $provider_hosts[provider_name][datacenter].key?(cluster) and $provider_hosts[provider_name][datacenter][cluster].key?('checking')
-        wait_for_host_selection(pool_name, provider_name, cluster, datacenter)
-      elsif $provider_hosts.key?(provider_name) and $provider_hosts[provider_name].key?(datacenter) and $provider_hosts[provider_name][datacenter].key?(cluster) and $provider_hosts[provider_name][datacenter][cluster].key?('check_time_finished')
-        select_hosts(pool_name, provider, provider_name, cluster, datacenter, percentage) if now - $provider_hosts[provider_name][datacenter][cluster]['check_time_finished'] > max_age
-      else
-        select_hosts(pool_name, provider, provider_name, cluster, datacenter, percentage)
-      end
-    end
-
-    def wait_for_host_selection(pool_name, provider_name, cluster, datacenter, maxloop = 0, loop_delay = 5, max_age = 60)
-      loop_count = 1
-      until $provider_hosts[provider_name][datacenter][cluster].key?('check_time_finished')
-        sleep(loop_delay)
-        unless maxloop.zero?
-          break if loop_count >= maxloop
-          loop_count += 1
-        end
-      end
-      return unless $provider_hosts[provider_name][datacenter][cluster].key?('check_time_finished')
-      loop_count = 1
-      while Time.now - $provider_hosts[provider_name][datacenter][cluster]['check_time_finished'] > max_age
-        sleep(loop_delay)
-        unless maxloop.zero?
-          break if loop_count >= maxloop
-          loop_count += 1
-        end
-      end
-    end
-
-    def select_next_host(provider_name, datacenter, cluster, architecture)
-      provider_hosts = $provider_hosts
-      host = provider_hosts[provider_name][datacenter][cluster]['architectures'][architecture][0]
-      return if host.nil?
-      provider_hosts[provider_name][datacenter][cluster]['architectures'][architecture].delete(host)
-      provider_hosts[provider_name][datacenter][cluster]['architectures'][architecture] << host
-      host
-    end
-
-    def migration_limit(migration_limit)
-      # Returns migration_limit setting when enabled
-      return false if migration_limit == 0 || !migration_limit # rubocop:disable Style/NumericPredicate
-      migration_limit if migration_limit >= 1
-    end
-
     def migrate_vm(vm_name, pool_name, provider)
       Thread.new do
         begin
-          _migrate_vm(vm_name, pool_name, provider)
+          $redis.srem('vmpooler__migrating__' + pool_name, vm_name)
+          provider.migrate_vm(pool_name, vm_name)
         rescue => err
           $logger.log('s', "[x] [#{pool_name}] '#{vm_name}' migration failed with an error: #{err}")
-          remove_vmpooler_migration_vm(pool_name, vm_name)
         end
       end
     end
 
-    def _migrate_vm(vm_name, pool_name, provider)
-      $redis.srem("vmpooler__migrating__#{pool_name}", vm_name)
-
-      provider_name = get_provider_name(pool_name)
-      vm = provider.get_vm_details(pool_name, vm_name)
-      raise('Unable to determine which host the VM is running on') if vm['host'].nil?
-      migration_limit = migration_limit $config[:config]['migration_limit']
-      migration_count = $redis.scard('vmpooler__migration')
-
-      if migration_limit
-        max_age = 60
-        percentage_of_hosts_below_average = 100
-        run_select_hosts(provider, pool_name, provider_name, vm['cluster'], vm['datacenter'], max_age, percentage_of_hosts_below_average)
-        if migration_count >= migration_limit
-          $logger.log('s', "[ ] [#{pool_name}] '#{vm_name}' is running on #{vm['host']}. No migration will be evaluated since the migration_limit has been reached")
-        elsif $provider_hosts[provider_name][vm['datacenter']][vm['cluster']]['architectures'][vm['architecture']].include?(vm['host'])
-          $logger.log('s', "[ ] [#{pool_name}] No migration required for '#{vm_name}' running on #{vm['host']}")
-        else
-          $redis.sadd('vmpooler__migration', vm_name)
-          target_host_name = select_next_host(provider_name, vm['datacenter'], vm['cluster'], vm['architecture'])
-          finish = migrate_vm_and_record_timing(vm_name, pool_name, vm['host'], target_host_name, provider)
-          $logger.log('s', "[>] [#{pool_name}] '#{vm_name}' migrated from #{vm['host']} to #{target_host_name} in #{finish} seconds")
-          remove_vmpooler_migration_vm(pool_name, vm_name)
-        end
-          return
-      else
-        $logger.log('s', "[ ] [#{pool_name}] '#{vm_name}' is running on #{vm['host']}")
-      end
-    end
-
-    def remove_vmpooler_migration_vm(pool, vm)
-      $redis.srem('vmpooler__migration', vm)
-    rescue => err
-      $logger.log('s', "[x] [#{pool}] '#{vm}' removal from vmpooler__migration failed with an error: #{err}")
-    end
-
-    def migrate_vm_and_record_timing(vm_name, pool_name, source_host_name, dest_host_name, provider)
-      start = Time.now
-      provider.migrate_vm_to_host(pool_name, vm_name, dest_host_name)
-      finish = format('%.2f', Time.now - start)
-      $metrics.timing("migrate.#{pool_name}", finish)
-      $metrics.increment("migrate_from.#{source_host_name}")
-      $metrics.increment("migrate_to.#{dest_host_name}")
-      checkout_to_migration = format('%.2f', Time.now - Time.parse($redis.hget("vmpooler__vm__#{vm_name}", 'checkout')))
-      $redis.hset("vmpooler__vm__#{vm_name}", 'migration_time', finish)
-      $redis.hset("vmpooler__vm__#{vm_name}", 'checkout_to_migration', checkout_to_migration)
-      finish
-    end
-
     # Helper method mainly used for unit testing
     def time_passed?(_event, time)
       Time.now > time
diff --git a/lib/vmpooler/providers/base.rb b/lib/vmpooler/providers/base.rb
index 23b0bfb..7ad1a6d 100644
--- a/lib/vmpooler/providers/base.rb
+++ b/lib/vmpooler/providers/base.rb
@@ -11,12 +11,16 @@ module Vmpooler
         attr_reader :metrics
         # Provider options passed in during initialization
         attr_reader :provider_options
+        # Hash for tracking hosts for deployment
+        attr_reader :provider_hosts
 
         def initialize(config, logger, metrics, name, options)
           @config = config
           @logger = logger
           @metrics = metrics
           @provider_name = name
+          @provider_hosts = {}
+          @provider_hosts_lock = Mutex.new
 
           # Ensure that there is not a nil provider configuration
           @config[:providers] = {} if @config[:providers].nil?
@@ -119,6 +123,14 @@ module Vmpooler
           raise("#{self.class.name} does not implement migrate_vm_to_host")
         end
 
+        # inputs
+        #   [String] pool_name      : Name of the pool
+        #   [String] vm_name        : Name of the VM to migrate
+        #   [Class] redis           : Redis object
+        def migrate_vm(_pool_name, _vm_name, _redis)
+          raise("#{self.class.name} does not implement migrate_vm")
+        end
+
         # inputs
         #   [String] pool_name : Name of the pool
         #   [String] vm_name   : Name of the VM to find
diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb
index 2971678..df3f205 100644
--- a/lib/vmpooler/providers/vsphere.rb
+++ b/lib/vmpooler/providers/vsphere.rb
@@ -56,42 +56,93 @@ module Vmpooler
           vms
         end
 
-        def get_vm_details(_pool_name, vm_name)
-          vm_hash = {}
-
-          @connection_pool.with_metrics do |pool_object|
-            connection = ensured_vsphere_connection(pool_object)
-            vm_object = find_vm(vm_name, connection)
-            return nil if vm_object.nil?
-            parent_host = vm_object.summary.runtime.host if vm_object.summary && vm_object.summary.runtime && vm_object.summary.runtime.host
-            vm_hash['host'] = parent_host.name
-            vm_hash['architecture'] = get_host_cpu_arch_version(parent_host)
-            vm_hash['cluster'] = parent_host.parent.name
-            vm_hash['datacenter'] = parent_host.parent.parent.parent.name
+        def select_target_hosts(target, cluster, datacenter)
+          percentage = 100
+          dc = "#{datacenter}_#{cluster}"
+          @provider_hosts_lock.synchronize do
+            target[dc] = {} unless target.key?(dc)
+            target[dc]['checking'] = true
+            hosts_hash = find_least_used_hosts(cluster, datacenter, percentage)
+            target[dc] = hosts_hash
+            target[dc]['check_time_finished'] = Time.now
           end
-          vm_hash
         end
 
-        def select_target_hosts(cluster, datacenter, percentage)
-          hosts_hash = find_least_used_hosts(cluster, datacenter, percentage)
-          hosts_hash
+        def run_select_hosts(pool_name, target)
+          now = Time.now
+          max_age = 60
+          datacenter = get_target_datacenter_from_config(pool_name)
+          cluster = get_target_cluster_from_config(pool_name)
+          raise("cluster for pool #{pool_name} cannot be identified") if cluster.nil?
+          raise("datacenter for pool #{pool_name} cannot be identified") if datacenter.nil?
+          dc = "#{datacenter}_#{cluster}"
+          if target.key?(dc) and target[dc].key?('checking')
+            wait_for_host_selection(dc, target)
+          elsif target.key?(dc) and target[dc].key?('check_time_finished')
+            select_target_hosts(target, cluster, datacenter) if now - target[dc]['check_time_finished'] > max_age
+          else
+            select_target_hosts(target, cluster, datacenter)
+          end
         end
 
-        def migrate_vm_to_host(pool_name, vm_name, dest_host_name)
-          pool = pool_config(pool_name)
-          raise("Pool #{pool_name} does not exist for the provider #{name}") if pool.nil?
-
-          @connection_pool.with_metrics do |pool_object|
-            connection = ensured_vsphere_connection(pool_object)
-            vm_object = find_vm(vm_name, connection)
-            raise("VM #{vm_name} does not exist in Pool #{pool_name} for the provider #{name}") if vm_object.nil?
-
-            target_host_object = find_host_by_dnsname(connection, dest_host_name)
-            raise("Pool #{pool_name} specifies host #{dest_host_name} which can not be found by the provider #{name}") if target_host_object.nil?
-            migrate_vm_host(vm_object, target_host_object)
-            return true
+        def wait_for_host_selection(dc, target, maxloop = 0, loop_delay = 5, max_age = 60)
+          loop_count = 1
+          until target.key?(dc) and target[dc].key?('check_time_finished')
+            sleep(loop_delay)
+            unless maxloop.zero?
+              break if loop_count >= maxloop
+              loop_count += 1
+            end
           end
-          false
+          return unless target[dc].key?('check_time_finished')
+          loop_count = 1
+          while Time.now - target[dc]['check_time_finished'] > max_age
+            sleep(loop_delay)
+            unless maxloop.zero?
+              break if loop_count >= maxloop
+              loop_count += 1
+            end
+          end
+        end
+
+        def select_next_host(pool_name, target, architecture = nil)
+          datacenter = get_target_datacenter_from_config(pool_name)
+          cluster = get_target_cluster_from_config(pool_name)
+          raise("cluster for pool #{pool_name} cannot be identified") if cluster.nil?
+          raise("datacenter for pool #{pool_name} cannot be identified") if datacenter.nil?
+          dc = "#{datacenter}_#{cluster}"
+          @provider_hosts_lock.synchronize do
+            if architecture
+              raise("no target hosts are available for #{pool_name} configured with datacenter #{datacenter} and cluster #{cluster}") if target[dc]['architectures'][architecture].size == 0
+              host = target[dc]['architectures'][architecture].shift
+              target[dc]['architectures'][architecture] << host
+              if target[dc]['hosts'].include?(host)
+                target[dc]['hosts'].delete(host)
+                target[dc]['hosts'] << host
+              end
+              return host
+            else
+              raise("no target hosts are available for #{pool_name} configured with datacenter #{datacenter} and cluster #{cluster}") if target[dc]['hosts'].size == 0
+              host = target[dc]['hosts'].shift
+              target[dc]['hosts'] << host
+              target[dc]['architectures'].each do |arch|
+                if arch.include?(host)
+                  target[dc]['architectures'][arch] = arch.partition { |v| v != host }.flatten
+                end
+              end
+              return host
+            end
+          end
+        end
+
+        def vm_in_target?(pool_name, parent_host, architecture, target)
+          datacenter = get_target_datacenter_from_config(pool_name)
+          cluster = get_target_cluster_from_config(pool_name)
+          raise("cluster for pool #{pool_name} cannot be identified") if cluster.nil?
+          raise("datacenter for pool #{pool_name} cannot be identified") if datacenter.nil?
+          dc = "#{datacenter}_#{cluster}"
+          return true if target[dc]['architectures'][architecture].include?(parent_host)
+          return false
         end
 
         def get_vm(_pool_name, vm_name)
@@ -156,16 +207,24 @@ module Vmpooler
               ]
             )
 
-            # Choose a cluster/host to place the new VM on
-            target_cluster_object = find_cluster(target_cluster_name, connection, target_datacenter_name)
-
             # Put the VM in the specified folder and resource pool
             relocate_spec = RbVmomi::VIM.VirtualMachineRelocateSpec(
               datastore: find_datastore(target_datastore, connection, target_datacenter_name),
-              pool: target_cluster_object.resourcePool,
               diskMoveType: :moveChildMostDiskBacking
             )
 
+            manage_host_selection = @config[:config]['manage_host_selection'] if @config[:config].key?('manage_host_selection')
+            if manage_host_selection
+              run_select_hosts(pool_name, @provider_hosts)
+              target_host = select_next_host(pool_name, @provider_hosts)
+              host_object = find_host_by_dnsname(connection, target_host)
+              relocate_spec.host = host_object
+            else
+            # Choose a cluster/host to place the new VM on
+              target_cluster_object = find_cluster(target_cluster_name, connection, target_datacenter_name)
+              relocate_spec.pool = target_cluster_object.resourcePool
+            end
+
             # Create a clone spec
             clone_spec = RbVmomi::VIM.VirtualMachineCloneSpec(
               location: relocate_spec,
@@ -176,14 +235,14 @@ module Vmpooler
 
             begin
               vm_target_folder = find_folder(target_folder_path, connection, target_datacenter_name)
+              if vm_target_folder.nil? and @config[:config].key?('create_folders') and @config[:config]['create_folders'] == true
+                vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name)
+              end
             rescue => _err
-              if _err =~ /Unexpected object type encountered/
-                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(_err)
-                end
+              if @config[:config].key?('create_folders') and @config[:config]['create_folders'] == true
+                vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name)
+              else
+                raise(_err)
               end
             end
 
@@ -586,15 +645,21 @@ module Vmpooler
         end
 
         def build_compatible_hosts_lists(hosts, percentage)
-          hosts_with_arch_versions = hosts.map { |host| [host[0], host[1], get_host_cpu_arch_version(host[1])] }
-          versions = hosts_with_arch_versions.map { |host| host[2] }.uniq
+          hosts_with_arch_versions = hosts.map { |h|
+            {
+            'utilization' => h[0],
+            'host_object' => h[1],
+            'architecture' => get_host_cpu_arch_version(h[1])
+            }
+          }
+          versions = hosts_with_arch_versions.map { |host| host['architecture'] }.uniq
           architectures = {}
           versions.each do |version|
             architectures[version] = []
           end
 
-          hosts_with_arch_versions.each do |host|
-            architectures[host[2]] << [host[0], host[1], host[2]]
+          hosts_with_arch_versions.each do |h|
+            architectures[h['architecture']] << [h['utilization'], h['host_object'], h['architecture']]
           end
 
           versions.each do |version|
@@ -612,8 +677,8 @@ module Vmpooler
           hosts.each do |host|
             least_used_hosts << host if host[0] <= average_utilization
           end
-          hosts_to_select = hosts.count - 1 if percentage == 100
           hosts_to_select = (hosts.count * (percentage  / 100.0)).to_int
+          hosts_to_select = hosts.count - 1 if percentage == 100
           least_used_hosts.sort[0..hosts_to_select].map { |host| host[1].name }
         end
 
@@ -621,15 +686,15 @@ module Vmpooler
           @connection_pool.with_metrics do |pool_object|
             connection = ensured_vsphere_connection(pool_object)
             cluster_object = find_cluster(cluster, connection, datacentername)
+            raise("Cluster #{cluster} cannot be found") if cluster_object.nil?
             target_hosts = get_cluster_host_utilization(cluster_object)
             raise("there is no candidate in vcenter that meets all the required conditions, that that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory'") if target_hosts.nil?
             architectures = build_compatible_hosts_lists(target_hosts, percentage)
             least_used_hosts = select_least_used_hosts(target_hosts, percentage)
-            least_used_hosts_list = {
+            {
               'hosts' => least_used_hosts,
-              'architectures' => architectures,
+              'architectures' => architectures
             }
-            least_used_hosts_list
           end
         end
 
@@ -793,9 +858,88 @@ module Vmpooler
           snapshot
         end
 
-        def migrate_vm_host(vm, host)
+        def get_vm_details(vm_name, connection)
+          vm_object = find_vm(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
+          parent_host = parent_host_object.name
+          raise('Unable to determine which host the VM is running on') if parent_host.nil?
+          architecture = get_host_cpu_arch_version(parent_host_object)
+          {
+            'host_name' => parent_host,
+            'object' => vm_object,
+            'architecture' => architecture
+          }
+        end
+
+        def migration_enabled?(config)
+          migration_limit = config[:config]['migration_limit']
+          return false unless migration_limit.is_a? Integer
+          return true if migration_limit > 0
+          false
+        end
+
+        def migrate_vm(pool_name, vm_name, redis)
+          redis.srem("vmpooler__migrating__#{pool_name}", vm_name)
+          @connection_pool.with_metrics do |pool_object|
+            connection = ensured_vsphere_connection(pool_object)
+            vm_hash = get_vm_details(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
+              if migration_count >= migration_limit
+                logger.log('s', "[ ] [#{pool_name}] '#{vm_name}' is running on #{vm_hash['host_name']}. No migration will be evaluated since the migration_limit has been reached")
+                return
+              end
+              run_select_hosts(pool_name, @provider_hosts)
+              if vm_in_target?(pool_name, vm_hash['host_name'], vm_hash['architecture'], @provider_hosts)
+                logger.log('s', "[ ] [#{pool_name}] No migration required for '#{vm_name}' running on #{vm_hash['host_name']}")
+              else
+                migrate_vm_to_new_host(pool_name, vm_name, vm_hash, connection, redis)
+              end
+            end
+          end
+        end
+
+        def migrate_vm_to_new_host(pool_name, vm_name, vm_hash, connection, redis)
+          redis.sadd('vmpooler__migration', vm_name)
+          target_host_name = select_next_host(pool_name, @provider_hosts, vm_hash['architecture'])
+          target_host_object = find_host_by_dnsname(connection, target_host_name)
+          finish = migrate_vm_and_record_timing(pool_name, vm_name, vm_hash, target_host_object, target_host_name, redis)
+          #logger.log('s', "Provider_hosts is: #{provider.provider_hosts}")
+          logger.log('s', "[>] [#{pool_name}] '#{vm_name}' migrated from #{vm_hash['host_name']} to #{target_host_name} in #{finish} seconds")
+          remove_vmpooler_migration_vm(pool_name, vm_name, redis)
+        end
+
+        def migrate_vm_and_record_timing(pool_name, vm_name, vm_hash, target_host_object, dest_host_name, redis)
+          start = Time.now
+          migrate_vm_host(vm_hash['object'], target_host_object)
+          finish = format('%.2f', Time.now - start)
+          metrics.timing("migrate.#{pool_name}", finish)
+          metrics.increment("migrate_from.#{vm_hash['host_name']}")
+          metrics.increment("migrate_to.#{dest_host_name}")
+          checkout_to_migration = format('%.2f', Time.now - Time.parse(redis.hget("vmpooler__vm__#{vm_name}", 'checkout')))
+          redis.hset("vmpooler__vm__#{vm_name}", 'migration_time', finish)
+          redis.hset("vmpooler__vm__#{vm_name}", 'checkout_to_migration', checkout_to_migration)
+          finish
+        end
+
+        def remove_vmpooler_migration_vm(pool_name, vm_name, redis)
+          redis.srem('vmpooler__migration', vm_name)
+        rescue => err
+          logger.log('s', "[x] [#{pool_name}] '#{vm_name}' removal from vmpooler__migration failed with an error: #{err}")
+        end
+
+        def migrate_vm_host(vm_object, host)
           relospec = RbVmomi::VIM.VirtualMachineRelocateSpec(host: host)
-          vm.RelocateVM_Task(spec: relospec).wait_for_completion
+          vm_object.RelocateVM_Task(spec: relospec).wait_for_completion
+        end
+
+        def create_folder(connection, new_folder, datacenter)
+          dc = connection.serviceInstance.find_datacenter(datacenter)
+          folder_object = dc.vmFolder.traverse(new_folder, type=RbVmomi::VIM::Folder, create=true)
+          raise("Cannot create folder #{new_folder}") if folder_object.nil?
+          folder_object
         end
       end
     end
diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb
index 67e87dc..fe2ba6b 100644
--- a/spec/unit/pool_manager_spec.rb
+++ b/spec/unit/pool_manager_spec.rb
@@ -322,9 +322,7 @@ EOT
 
       context 'is turned off' do
         before(:each) do
-          host['boottime'] = nil
           host['powerstate'] = 'PoweredOff'
-          ttl = 1440
         end
 
         it 'should move the VM to the completed queue' do
@@ -1475,659 +1473,39 @@ EOT
     end
   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?
-    [false,0].each do |testvalue|
-      it "should return false for an input of #{testvalue}" do
-        expect(subject.migration_limit(testvalue)).to eq(false)
-      end
-    end
-
-    [1,32768].each do |testvalue|
-      it "should return #{testvalue} for an input of #{testvalue}" do
-        expect(subject.migration_limit(testvalue)).to eq(testvalue)
-      end
-    end
-
-    [-1,-32768].each do |testvalue|
-      it "should return nil for an input of #{testvalue}" do
-        expect(subject.migration_limit(testvalue)).to be_nil
-      end
-    end
-  end
-
   describe '#migrate_vm' do
     before(:each) do
       expect(subject).not_to be_nil
       expect(Thread).to receive(:new).and_yield
     end
 
-    it 'calls _migrate_vm' do
-      expect(subject).to receive(:_migrate_vm).with(vm, pool, provider)
+    it 'calls migrate_vm' do
+      expect(provider).to receive(:migrate_vm).with(pool, vm, redis)
 
       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')
+        expect(provider).to receive(:migrate_vm).with(pool, vm, redis).and_raise('MockError')
       end
 
       it 'logs a message' do
         allow(logger).to receive(:log)
         expect(logger).to receive(:log).with('s', "[x] [#{pool}] '#{vm}' migration failed with an error: MockError")
+        expect(provider).to receive(:remove_vmpooler_migration_vm).with(pool, vm, redis)
 
         subject.migrate_vm(vm, pool, provider)
       end
 
       it 'should attempt to remove from vmpooler_migration queue' do
-        expect(subject).to receive(:remove_vmpooler_migration_vm).with(pool, vm)
+        expect(provider).to receive(:remove_vmpooler_migration_vm).with(pool, vm, redis)
 
         subject.migrate_vm(vm, pool, provider)
       end
     end
   end
 
-  describe "#_migrate_vm" do
-    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_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_details).with(pool, vm).and_raise(RuntimeError,'MockError')
-      end
-
-      it 'should raise an error' do
-        expect{ subject._migrate_vm(vm, pool, provider) }.to raise_error('MockError')
-      end
-    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
-        $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
-        expect{ subject._migrate_vm(vm, pool, provider) }.to raise_error(/Unable to determine which host the VM is running on/)
-      end
-    end
-
-    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|
-        it "should not migrate a VM if the migration limit is #{testvalue}" do
-          config[:config]['migration_limit'] = testvalue
-          expect(logger).to receive(:log).with('s', "[ ] [#{pool}] '#{vm}' is running on #{vm_parent_hostname}")
-          subject._migrate_vm(vm, pool, provider)
-        end
-
-        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
-          subject._migrate_vm(vm, pool, provider)
-          expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_falsey
-        end
-      end
-    end
-
-    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')
-        redis.sadd('vmpooler__migration', 'fakevm2')
-        redis.sadd('vmpooler__migration', 'fakevm3')
-        redis.sadd('vmpooler__migration', 'fakevm4')
-        redis.sadd('vmpooler__migration', 'fakevm5')
-      end
-
-      it "should not migrate a VM if the migration limit is reached" do
-        expect(logger).to receive(:log).with('s',"[ ] [#{pool}] '#{vm}' is running on #{vm_parent_hostname}. No migration will be evaluated since the migration_limit has been reached")
-        subject._migrate_vm(vm, pool, provider)
-      end
-
-      it "should remove the VM from vmpooler__migrating queue in redis if the migration limit is reached" do
-        expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_truthy
-        subject._migrate_vm(vm, pool, provider)
-        expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_falsey
-      end
-    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 current host is within the list of available targets' do
-        let(:target_hosts) { ['host1'] }
-        #
-        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(subject).to receive(:host_in_targets?).with(vm_parent_hostname, target_hosts).and_return(true)
-        end
-
-        it "should not migrate the VM" do
-          expect(logger).to receive(:log).with('s', "[ ] [#{pool}] No migration required for '#{vm}' running on #{vm_parent_hostname}")
-          subject._migrate_vm(vm, pool, provider)
-        end
-
-        it "should remove the VM from vmpooler__migrating queue in redis" do
-          expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_truthy
-          subject._migrate_vm(vm, pool, provider)
-          expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_falsey
-        end
-
-        it "should not change the vmpooler_migration queue count" do
-          before_count = redis.scard('vmpooler__migration')
-          subject._migrate_vm(vm, pool, provider)
-          expect(redis.scard('vmpooler__migration')).to eq(before_count)
-        end
-
-        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) { 'host1' }
-        let(:vm_parent_hostname) { 'host2' }
-        let(:dc) { 'dc1' }
-        let(:host_architecture) { 'v3' }
-        let(:hosts_hash) {
-          {
-            'hosts' => [ 'host1' ],
-            'architectures' => { 'v3' => ['host1'] },
-          }
-        }
-        before(:each) do
-          $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
-
-        it "should migrate the VM" do
-          expect(logger).to receive(:log).with('s', "[>] [#{pool}] '#{vm}' migrated from #{vm_parent_hostname} to #{vm_new_hostname} in 1.00 seconds")
-          subject._migrate_vm(vm, pool, provider)
-        end
-
-        it "should remove the VM from vmpooler__migrating queue in redis" do
-          expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_truthy
-          subject._migrate_vm(vm, pool, provider)
-          expect(redis.sismember("vmpooler__migrating__#{pool}",vm)).to be_falsey
-        end
-
-        it "should not change the vmpooler_migration queue count" do
-          before_count = redis.scard('vmpooler__migration')
-          subject._migrate_vm(vm, pool, provider)
-          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)
-          subject._migrate_vm(vm, pool, provider)
-        end
-      end
-    end
-  end
-
   describe "#execute!" do
     let(:config) {
       YAML.load(<<-EOT
@@ -2778,65 +2156,6 @@ EOT
     end
   end
 
-  describe '#remove_vmpooler_migration_vm' do
-    before do
-      expect(subject).not_to be_nil
-    end
-
-    it 'should remove the migration from redis' do
-      redis.sadd('vmpooler__migration', vm)
-      expect(redis.sismember('vmpooler__migration',vm)).to be(true)
-      subject.remove_vmpooler_migration_vm(pool, vm)
-      expect(redis.sismember('vmpooler__migration',vm)).to be(false)
-    end
-
-    it 'should log a message and swallow an error if one occurs' do
-      expect(redis).to receive(:srem).and_raise(RuntimeError,'MockError')
-      expect(logger).to receive(:log).with('s', "[x] [#{pool}] '#{vm}' removal from vmpooler__migration failed with an error: MockError")
-      subject.remove_vmpooler_migration_vm(pool, vm)
-    end
-  end
-
-  describe '#migrate_vm_and_record_timing' do
-    let(:source_host_name) { 'source_host' }
-    let(:dest_host_name) { 'dest_host' }
-
-    before(:each) do
-      create_vm(vm,token)
-      expect(subject).not_to be_nil
-
-      expect(provider).to receive(:migrate_vm_to_host).with(pool, vm, dest_host_name)
-    end
-
-    it 'should return the elapsed time for the migration' do
-      result = subject.migrate_vm_and_record_timing(vm, pool, source_host_name, dest_host_name, provider)
-      expect(result).to match(/0\.[\d]+/)
-    end
-
-    it 'should add timing metric' do
-      expect(metrics).to receive(:timing).with("migrate.#{pool}",String)
-      subject.migrate_vm_and_record_timing(vm, pool, source_host_name, dest_host_name, provider)
-    end
-
-    it 'should increment from_host and to_host metric' do
-      expect(metrics).to receive(:increment).with("migrate_from.#{source_host_name}")
-      expect(metrics).to receive(:increment).with("migrate_to.#{dest_host_name}")
-      subject.migrate_vm_and_record_timing(vm, pool, source_host_name, dest_host_name, provider)
-    end
-
-    it 'should set migration_time metric in redis' do
-      expect(redis.hget("vmpooler__vm__#{vm}", 'migration_time')).to be_nil
-      subject.migrate_vm_and_record_timing(vm, pool, source_host_name, dest_host_name, provider)
-      expect(redis.hget("vmpooler__vm__#{vm}", 'migration_time')).to match(/0\.[\d]+/)
-    end
-
-    it 'should set checkout_to_migration metric in redis' do
-      expect(redis.hget("vmpooler__vm__#{vm}", 'checkout_to_migration')).to be_nil
-      subject.migrate_vm_and_record_timing(vm, pool, source_host_name, dest_host_name, provider)
-      expect(redis.hget("vmpooler__vm__#{vm}", 'checkout_to_migration')).to match(/[01]\.[\d]+/)
-    end
-  end
-
   describe '#_check_pool' do
     let(:new_vm_response) {
       # Mock response from Base Provider for vms_in_pool
diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb
index 657c597..2fa5a26 100644
--- a/spec/unit/providers/vsphere_spec.rb
+++ b/spec/unit/providers/vsphere_spec.rb
@@ -161,104 +161,6 @@ EOT
     end
   end
 
-  describe '#migrate_vm_to_host' do
-    let(:dest_host_name) { 'HOST002' }
-    let(:cluster_name) { 'CLUSTER001' }
-    let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({
-        :name => vmname,
-      })
-    }
-
-    before(:each) do
-      config[:pools][0]['clone_target'] = cluster_name
-      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
-      allow(subject).to receive(:find_vm).and_return(vm_object)
-    end
-
-    context 'Given an invalid pool name' do
-      it 'should raise an error' do
-        expect{ subject.migrate_vm_to_host('missing_pool', vmname, dest_host_name) }.to raise_error(/missing_pool does not exist/)
-      end
-    end
-
-    context 'Given a missing VM name' do
-      before(:each) do
-        expect(subject).to receive(:find_vm).and_return(nil)
-      end
-
-      it 'should raise an error' do
-        expect{ subject.migrate_vm_to_host(poolname, 'missing_vm', dest_host_name) }.to raise_error(/missing_vm does not exist/)
-      end
-    end
-
-    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
-        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(/#{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
-        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(/#{dest_host_name} which can not be found/)
-      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
-        allow(connection.searchIndex).to receive(:FindByDnsName).and_return(host)
-        expect(subject).to receive(:migrate_vm_host).with(Object,Object).and_raise(RuntimeError,'MockMigrationError')
-      end
-
-      it 'should raise an error' do
-        expect{ subject.migrate_vm_to_host(poolname, vmname, dest_host_name) }.to raise_error('MockMigrationError')
-      end
-    end
-
-    context 'Given a successful migration' 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).with(Object,Object).and_return(nil)
-      end
-
-      it 'should return true' do
-        expect(subject.migrate_vm_to_host(poolname, vmname, dest_host_name)).to be true
-      end
-    end
-  end
-
   describe '#get_vm' do
     let(:vm_object) { nil }
     before(:each) do
@@ -1968,71 +1870,169 @@ EOT
     end
   end
 
-# 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 '#select_target_hosts' do
+    let(:target) { {} }
+    let(:cluster) { 'cluster1' }
+    let(:missing_cluster_name) { 'missing_cluster' }
+    let(:datacenter) { 'dc1' }
+    let(:architecture) { 'v3' }
+    let(:host) { 'host1' }
+    let(:hosts_hash) {
+      {
+        'hosts' => [host],
+        'architectures' => {
+          architecture => [host]
+        }
+      }
+    }
+
+    it 'returns a hash of the least used hosts by cluster and architecture' do
+      expect(subject).to receive(:find_least_used_hosts).and_return(hosts_hash)
+
+      subject.select_target_hosts(target, cluster, datacenter)
+      expect(target["#{datacenter}_#{cluster}"]).to eq(hosts_hash)
+    end
+
+    context 'with a cluster specified that does not exist' do
+      it 'raises an error' do
+        expect(subject).to receive(:find_least_used_hosts).with(missing_cluster_name, datacenter, 100).and_raise("Cluster #{cluster} cannot be found")
+        expect{subject.select_target_hosts(target, missing_cluster_name, datacenter)}.to raise_error(RuntimeError,/Cluster #{cluster} cannot be found/)
+      end
+    end
+  end
+
+  describe '#get_average_cluster_utilization' do
+    let(:hosts) {
+      [
+        [60, 'host1'],
+        [100, 'host2'],
+        [200, 'host3']
+      ]
+    }
+    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
+      expect(subject.get_average_cluster_utilization(hosts)).to eq(120)
+    end
+  end
+
+  describe '#build_compatible_hosts_lists' do
+    let(:host1) { mock_RbVmomi_VIM_HostSystem({ :name => 'HOST1' })}
+    let(:host2) { mock_RbVmomi_VIM_HostSystem({ :name => 'HOST2' })}
+    let(:host3) { mock_RbVmomi_VIM_HostSystem({ :name => 'HOST3' })}
+    let(:architecture) { 'v4' }
+    let(:percentage) { 100 }
+    let(:hosts) {
+      [
+        [60, host1],
+        [100, host2],
+        [200, host3]
+      ]
+    }
+    let(:result) {
+      {
+          architecture => ['HOST1','HOST2']
+      }
+    }
+
+    it 'returns a hash of target host architecture versions containing lists of target hosts' do
+
+      expect(subject.build_compatible_hosts_lists(hosts, percentage)).to eq(result)
+    end
+  end
+
+  describe '#select_least_used_hosts' do
+    let(:percentage) { 100 }
+    let(:host1) { mock_RbVmomi_VIM_HostSystem({ :name => 'HOST1' })}
+    let(:host2) { mock_RbVmomi_VIM_HostSystem({ :name => 'HOST2' })}
+    let(:host3) { mock_RbVmomi_VIM_HostSystem({ :name => 'HOST3' })}
+    let(:hosts) {
+      [
+        [60, host1],
+        [100, host2],
+        [200, host3]
+      ]
+    }
+    let(:result) { ['HOST1','HOST2'] }
+    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
+      expect(subject.select_least_used_hosts(hosts, percentage)).to eq(result)
+    end
+
+    context 'when selecting 20 percent of hosts below average' do
+      let(:percentage) { 20 }
+      let(:result) { ['HOST1'] }
+
+      it 'should return the result' do
+        expect(subject.select_least_used_hosts(hosts, percentage)).to eq(result)
+      end
+    end
+
+    it 'should raise' do
+      expect{subject.select_least_used_hosts([], percentage)}.to raise_error(RuntimeError,/Provided hosts list to select_least_used_hosts is empty/)
+    end
+  end
+
+  describe '#run_select_hosts' do
+    it 'should raise an error when cluster cannot be identified' do
+    end
+    it 'should raise an error when datacenter for pool_name cannot be identified' do
+    end
+    it 'should run wait_for_host_selection if the specified target has the key checking' do
+    end
+    it 'should run select_target_hosts if the specified target has the key check_time_finished and the difference between max_age and check_time_finished is greater than max_age' do
+    end
+    context 'when neither checking or check_time_finished key are present in target' do
+      it 'should run select_target_hosts' do
+      end
+      it 'should populate the target with hosts' do
+      end
+    end
+  end
+
+  describe '#wait_for_host_selection' do
+    it 'does things' do
+    end
+  end
+
+  describe '#select_next_host' do
+    it 'does things' do
+    end
+  end
+
+  describe '#vm_in_target?' do
+    it 'checks if vm is in target' do
+    end
+  end
+
+  describe '#get_vm_details' do
+    it 'gets vm details' do
+    end
+  end
+
+  describe '#migrate_vm' do
+    it 'migrates a vm' do
+    end
+  end
+
+  describe '#migrate_vm_to_new_host' do
+    it' migrates a vm' do
+    end
+  end
+
+  describe '#remove_vmpooler_migration_vm' do
+    it 'removes vm from migrating' do
+    end
+  end
+
+  describe '#create_folder' do
+    it 'creates a folder' do
+    end
+  end
+
+  describe '#migration_enabled?' do
+    it 'checks if migration is enabled' do
+    end
+  end
+
+
 
   describe '#find_least_used_hosts' do
     let(:cluster_name) { 'cluster' }
@@ -2058,7 +2058,7 @@ EOT
       let(:expected_host) { cluster_object.host[0] }
 #,datacenter_name
       it 'should raise an error' do
-        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(NoMethodError,/undefined method/)
+        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(RuntimeError,/Cluster #{missing_cluster_name} cannot be found/)
       end
     end
 
@@ -2087,7 +2087,7 @@ EOT
       let(:expected_host) { cluster_object.host[0] }
 
       it 'should raise an error' do
-        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(NoMethodError,/undefined method/)
+        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(RuntimeError,/Cluster #{missing_cluster_name} cannot be found/)
       end
     end
 
@@ -2119,7 +2119,7 @@ EOT
       let(:expected_host) { cluster_object.host[1] }
 
       it 'should raise an error' do
-        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(NoMethodError,/undefined method/)
+        expect{subject.find_least_used_hosts(missing_cluster_name,datacenter_name,percentage)}.to raise_error(RuntimeError,/Cluster #{missing_cluster_name} cannot be found/)
       end
     end
 
@@ -3053,4 +3053,6 @@ EOT
       expect(subject.migrate_vm_host(vm_object,host_object)).to eq('RELOCATE_RESULT')
     end
   end
+
+
 end