Update migration spec to call pool manager and validate results

Use mock_redis instead of redis.

Make passing of mock redis to helper calls more clear

Update pool_manager_spec to specify vsphere argument where applicable. Update pool_helper calls to vsphere where needed for tests to pass. Without this change rspec tests for pool_manager_spec exhibit 12 failures.

Update pool_manager_spec test with open_socket

Pool_manager_spec stubs a tcpsocket connection to simulate this happening directly within _check_pending_vm. This commit updates this to look more like its usage with open_socket, which allows the test to pass.

Gemfile

@@ -16,6 +16,7 @@ gem 'statsd-ruby', '>= 1.3.0', :require => 'statsd'
 
 # Test deps
 group :test do
+  gem 'mock_redis', '>= 0.17.0'
   gem 'rack-test', '>= 0.6'
   gem 'rspec', '>= 3.2'
   gem 'simplecov', '>= 0.11.2'

spec/helpers.rb

@@ -50,11 +50,21 @@ def create_pending_vm(template, name, token = nil)
   redis.hset("vmpooler__vm__#{name}", "template", template)
 end
 
-def create_vm(name, token = nil)
+def create_vm(name, token = nil, redis_handle = nil)
-  redis.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
+  redis_db = redis_handle ? redis_handle : redis
-  if token
+  redis_db.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
-    redis.hset("vmpooler__vm__#{name}", 'token:token', token)
+  redis_db.hset("vmpooler__vm__#{name}", 'token:token', token) if token
-  end
+end
+
+def create_migrating_vm(name, pool, redis_handle = nil)
+  redis_db = redis_handle ? redis_handle : redis
+  redis_db.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
+  redis_db.sadd("vmpooler__migrating__#{pool}", name)
+end
+
+def add_vm_to_migration_set(name, redis_handle = nil)
+  redis_db = redis_handle ? redis_handle : redis
+  redis_db.sadd('vmpooler__migration', name)
 end
 
 def fetch_vm(vm)

spec/vmpooler/pool_manager_migration_spec.rb

@@ -0,0 +1,88 @@
+require 'spec_helper'
+require 'mock_redis'
+require 'time'
+
+describe 'Pool Manager' do
+  let(:logger) { double('logger') }
+  let(:redis) { MockRedis.new }
+  let(:metrics) { Vmpooler::DummyStatsd.new }
+  let(:config) {
+    {
+      config: {
+        'site_name' => 'test pooler',
+        'migration_limit' => 2,
+        vsphere: {
+          'server' => 'vsphere.puppet.com',
+          'username' => 'vmpooler@vsphere.local',
+          'password' => '',
+          'insecure' => true
+        },
+        pools: [ {'name' => 'pool1', 'size' => 5, 'folder' => 'pool1_folder'} ],
+        statsd: { 'prefix' => 'stats_prefix'},
+        pool_names: [ 'pool1' ]
+      }
+    }
+  }
+  let(:pool) { config[:config][:pools][0]['name'] }
+  let(:vm) {
+    {
+      'name' => 'vm1',
+      'host' => 'host1',
+      'template' => pool,
+    }
+  }
+
+  describe '#_migrate_vm' do
+    let(:vsphere) { double(pool) }
+    let(:pooler) { Vmpooler::PoolManager.new(config, logger, redis, metrics) }
+    context 'evaluates VM for migration and logs host' do
+      before do
+        create_migrating_vm vm['name'], pool, redis
+        allow(vsphere).to receive(:find_vm).and_return(vm)
+        allow(pooler).to receive(:get_vm_host_info).and_return([{'name' => 'host1'}, 'host1'])
+        expect(vsphere).to receive(:find_vm).with(vm['name'])
+      end
+
+      it 'logs VM host when migration is disabled' do
+        config[:config]['migration_limit'] = nil
+
+        expect(redis.sismember("vmpooler__migrating__#{pool}", vm['name'])).to be true
+        expect(logger).to receive(:log).with('s', "[ ] [#{pool}] '#{vm['name']}' is running on #{vm['host']}")
+
+        pooler._migrate_vm(vm['name'], pool, vsphere)
+
+        expect(redis.sismember("vmpooler__migrating__#{pool}", vm['name'])).to be false
+      end
+
+      it 'verifies that migration_limit greater than or equal to migrations in progress and logs host' do
+        add_vm_to_migration_set vm['name'], redis
+        add_vm_to_migration_set 'vm2', redis
+
+        expect(logger).to receive(:log).with('s', "[ ] [#{pool}] '#{vm['name']}' is running on #{vm['host']}. No migration will be evaluated since the migration_limit has been reached")
+
+        pooler._migrate_vm(vm['name'], pool, vsphere)
+      end
+
+      it 'verifies that migration_limit is less than migrations in progress and logs old host, new host and migration time' do
+        allow(vsphere).to receive(:find_least_used_compatible_host).and_return([{'name' => 'host2'}, 'host2'])
+        allow(vsphere).to receive(:migrate_vm_host)
+
+        expect(redis.hget("vmpooler__vm__#{vm['name']}", 'migration_time'))
+        expect(redis.hget("vmpooler__vm__#{vm['name']}", 'checkout_to_migration'))
+        expect(logger).to receive(:log).with('s', "[>] [#{pool}] '#{vm['name']}' migrated from #{vm['host']} to host2 in 0.00 seconds")
+
+        pooler._migrate_vm(vm['name'], pool, vsphere)
+      end
+
+      it 'fails when no suitable host can be found' do
+        error = 'ArgumentError: No target host found'
+        allow(vsphere).to receive(:find_least_used_compatible_host)
+        allow(vsphere).to receive(:migrate_vm_host).and_raise(error)
+
+        expect(logger).to receive(:log).with('s', "[x] [#{pool}] '#{vm['name']}' migration failed with an error: #{error}")
+
+        pooler._migrate_vm(vm['name'], pool, vsphere)
+      end
+    end
+  end
+end

spec/vmpooler/pool_manager_spec.rb

@@ -24,10 +24,10 @@ describe 'Pool Manager' do
 
     context 'host not in pool' do
       it 'calls fail_pending_vm' do
-        allow(pool_helper).to receive(:find_vm).and_return(nil)
+        allow(vsphere).to receive(:find_vm).and_return(nil)
         allow(redis).to receive(:hget)
         expect(redis).to receive(:hget).with(String, 'clone').once
-        subject._check_pending_vm(vm, pool, timeout)
+        subject._check_pending_vm(vm, pool, timeout, vsphere)
       end
     end
 
@@ -36,16 +36,14 @@ describe 'Pool Manager' do
       let(:tcpsocket) { double('TCPSocket') }
 
       it 'calls move_pending_vm_to_ready' do
-        stub_const("TCPSocket", tcpsocket)
+        allow(subject).to receive(:open_socket).and_return(true)
-
+        allow(vsphere).to receive(:find_vm).and_return(vm_finder)
-        allow(pool_helper).to receive(:find_vm).and_return(vm_finder)
         allow(vm_finder).to receive(:summary).and_return(nil)
-        allow(tcpsocket).to receive(:new).and_return(true)
 
         expect(vm_finder).to receive(:summary).once
         expect(redis).not_to receive(:hget).with(String, 'clone')
 
-        subject._check_pending_vm(vm, pool, timeout)
+        subject._check_pending_vm(vm, pool, timeout, vsphere)
       end
     end
   end
@@ -156,16 +154,16 @@ describe 'Pool Manager' do
     end
 
     it 'does nothing with nil host' do
-      allow(pool_helper).to receive(:find_vm).and_return(nil)
+      allow(vsphere).to receive(:find_vm).and_return(nil)
       expect(redis).not_to receive(:smove)
-      subject._check_running_vm(vm, pool, timeout)
+      subject._check_running_vm(vm, pool, timeout, vsphere)
     end
 
     context 'valid host' do
       let(:vm_host) { double('vmhost') }
 
       it 'does not move vm when not poweredOn' do
-        allow(pool_helper).to receive(:find_vm).and_return vm_host
+        allow(vsphere).to receive(:find_vm).and_return vm_host
         allow(vm_host).to receive(:runtime).and_return true
         allow(vm_host).to receive_message_chain(:runtime, :powerState).and_return 'poweredOff'
 
@@ -173,11 +171,11 @@ describe 'Pool Manager' do
         expect(redis).not_to receive(:smove)
         expect(logger).not_to receive(:log).with('d', "[!] [#{pool}] '#{vm}' appears to be powered off or dead")
 
-        subject._check_running_vm(vm, pool, timeout)
+        subject._check_running_vm(vm, pool, timeout, vsphere)
       end
 
       it 'moves vm when poweredOn, but past TTL' do
-        allow(pool_helper).to receive(:find_vm).and_return vm_host
+        allow(vsphere).to receive(:find_vm).and_return vm_host
         allow(vm_host).to receive(:runtime).and_return true
         allow(vm_host).to receive_message_chain(:runtime, :powerState).and_return 'poweredOn'
 
@@ -185,7 +183,7 @@ describe 'Pool Manager' do
         expect(redis).to receive(:smove)
         expect(logger).to receive(:log).with('d', "[!] [#{pool}] '#{vm}' reached end of TTL after #{timeout} hours")
 
-        subject._check_running_vm(vm, pool, timeout)
+        subject._check_running_vm(vm, pool, timeout, vsphere)
       end
     end
   end
@@ -228,6 +226,7 @@ describe 'Pool Manager' do
       allow(redis).to receive(:smembers).with('vmpooler__running__pool1').and_return([])
       allow(redis).to receive(:smembers).with('vmpooler__completed__pool1').and_return([])
       allow(redis).to receive(:smembers).with('vmpooler__discovered__pool1').and_return([])
+      allow(redis).to receive(:smembers).with('vmpooler__migrating__pool1').and_return([])
       allow(redis).to receive(:set)
       allow(redis).to receive(:get).with('vmpooler__tasks__clone').and_return(0)
       allow(redis).to receive(:get).with('vmpooler__empty__pool1').and_return(nil)
@@ -240,7 +239,7 @@ describe 'Pool Manager' do
         allow(redis).to receive(:scard).with('vmpooler__running__pool1').and_return(0)
 
         expect(logger).to receive(:log).with('s', "[!] [pool1] is empty")
-        subject._check_pool(config[:pools][0])
+        subject._check_pool(config[:pools][0], vsphere)
       end
     end
   end
@@ -277,7 +276,7 @@ describe 'Pool Manager' do
 
         expect(metrics).to receive(:gauge).with('ready.pool1', 1)
         expect(metrics).to receive(:gauge).with('running.pool1', 5)
-        subject._check_pool(config[:pools][0])
+        subject._check_pool(config[:pools][0], vsphere)
       end
 
       it 'increments metrics when ready with 0 when pool empty' do
@@ -288,7 +287,7 @@ describe 'Pool Manager' do
 
         expect(metrics).to receive(:gauge).with('ready.pool1', 0)
         expect(metrics).to receive(:gauge).with('running.pool1', 5)
-        subject._check_pool(config[:pools][0])
+        subject._check_pool(config[:pools][0], vsphere)
       end
     end
   end
@@ -307,13 +306,13 @@ describe 'Pool Manager' do
       let(:vm_host) { double('vmhost') }
 
       it 'creates a snapshot' do
-        expect(pool_helper).to receive(:find_vm).and_return vm_host
+        expect(vsphere).to receive(:find_vm).and_return vm_host
         expect(logger).to receive(:log)
         expect(vm_host).to receive_message_chain(:CreateSnapshot_Task, :wait_for_completion)
         expect(redis).to receive(:hset).with('vmpooler__vm__testvm', 'snapshot:testsnapshot', Time.now.to_s)
         expect(logger).to receive(:log)
 
-        subject._create_vm_snapshot('testvm', 'testsnapshot')
+        subject._create_vm_snapshot('testvm', 'testsnapshot', vsphere)
       end
     end
   end
@@ -333,13 +332,13 @@ describe 'Pool Manager' do
       let(:vm_snapshot) { double('vmsnapshot') }
 
       it 'reverts a snapshot' do
-        expect(pool_helper).to receive(:find_vm).and_return vm_host
+        expect(vsphere).to receive(:find_vm).and_return vm_host
-        expect(pool_helper).to receive(:find_snapshot).and_return vm_snapshot
+        expect(vsphere).to receive(:find_snapshot).and_return vm_snapshot
         expect(logger).to receive(:log)
         expect(vm_snapshot).to receive_message_chain(:RevertToSnapshot_Task, :wait_for_completion)
         expect(logger).to receive(:log)
 
-        subject._revert_vm_snapshot('testvm', 'testsnapshot')
+        subject._revert_vm_snapshot('testvm', 'testsnapshot', vsphere)
       end
     end
   end
@@ -357,7 +356,7 @@ describe 'Pool Manager' do
       expect(redis).to receive(:spop).with('vmpooler__tasks__snapshot')
       expect(redis).to receive(:spop).with('vmpooler__tasks__snapshot-revert')
 
-      subject._check_snapshot_queue
+      subject._check_snapshot_queue(vsphere)
     end
   end
 end