(QENG-5305) Improve vmpooler host selection

lib/vmpooler/pool_manager.rb

@@ -224,10 +224,9 @@ def _clone_vm(pool, provider)
         $logger.log('s', "[+] [#{pool_name}] '#{new_vmname}' cloned in #{finish} seconds")
 
         $metrics.timing("clone.#{pool_name}", finish)
-      rescue => err
-        $logger.log('s', "[!] [#{pool_name}] '#{new_vmname}' clone failed with an error: #{err}")
+      rescue => _err
         $redis.srem('vmpooler__pending__' + pool_name, new_vmname)
-        raise
+        raise _err
       ensure
         $redis.decr('vmpooler__tasks__clone')
       end
@@ -459,69 +458,17 @@ def _check_snapshot_queue
       end
     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)
-
-      parent_host_name = provider.get_vm_host(pool_name, vm_name)
-      raise('Unable to determine which host the VM is running on') if parent_host_name.nil?
-      migration_limit = migration_limit $config[:config]['migration_limit']
-      migration_count = $redis.scard('vmpooler__migration')
-
-      if !migration_limit
-        $logger.log('s', "[ ] [#{pool_name}] '#{vm_name}' is running on #{parent_host_name}")
-        return
-      elsif migration_count >= migration_limit
-        $logger.log('s', "[ ] [#{pool_name}] '#{vm_name}' is running on #{parent_host_name}. No migration will be evaluated since the migration_limit has been reached")
-        return
-      else
-        $redis.sadd('vmpooler__migration', vm_name)
-        host_name = provider.find_least_used_compatible_host(pool_name, vm_name)
-        if host_name == parent_host_name
-          $logger.log('s', "[ ] [#{pool_name}] No migration required for '#{vm_name}' running on #{parent_host_name}")
-        else
-          finish = migrate_vm_and_record_timing(vm_name, pool_name, parent_host_name, host_name, provider)
-          $logger.log('s', "[>] [#{pool_name}] '#{vm_name}' migrated from #{parent_host_name} to #{host_name} in #{finish} seconds")
-        end
-        remove_vmpooler_migration_vm(pool_name, vm_name)
-      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
@@ -764,9 +711,6 @@ def _check_pool(pool, provider)
       end
 
       pool_check_response
-    rescue => err
-      $logger.log('d', "[!] [#{pool['name']}] _check_pool failed with an error: #{err}")
-      raise
     end
 
     # Create a provider object, usually based on the providers/*.rb class, that implements providers/base.rb

lib/vmpooler/providers/base.rb

@@ -119,6 +119,14 @@ def migrate_vm_to_host(_pool_name, _vm_name, _dest_host_name)
           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