Replace connection_pool with vmpooler generic_connection_pool. Fix tests for changes.

spec/fixtures/vmpooler.yaml

@@ -36,6 +36,8 @@
   - name: 'pool01'
     size: 5
     provider: dummy
+    ready_ttl: 5
   - name: 'pool02'
     size: 5
     provider: dummy
+    ready_ttl: 5

spec/fixtures/vmpooler2.yaml

@@ -36,6 +36,8 @@
   - name: 'pool03'
     size: 5
     provider: dummy
+    ready_ttl: 5
   - name: 'pool04'
     size: 5
     provider: dummy
+    ready_ttl: 5

spec/helpers.rb

@@ -1,12 +1,5 @@
 require 'mock_redis'
 
-def redis
-  unless @redis
-    @redis = MockRedis.new
-  end
-  @redis
-end
-
 # Mock an object which represents a Logger.  This stops the proliferation
 # of allow(logger).to .... expectations in tests.
 class MockLogger
@@ -40,59 +33,54 @@ def token_exists?(token)
   result && !result.empty?
 end
 
-def create_ready_vm(template, name, token = nil)
-  create_vm(name, token)
+def create_ready_vm(template, name, redis, token = nil)
+  create_vm(name, redis, token)
   redis.sadd("vmpooler__ready__#{template}", name)
   redis.hset("vmpooler__vm__#{name}", "template", template)
 end
 
-def create_running_vm(template, name, token = nil, user = nil)
-  create_vm(name, token, nil, user)
+def create_running_vm(template, name, redis, token = nil, user = nil)
+  create_vm(name, redis, token, user)
   redis.sadd("vmpooler__running__#{template}", name)
   redis.hset("vmpooler__vm__#{name}", 'template', template)
   redis.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
   redis.hset("vmpooler__vm__#{name}", 'host', 'host1')
 end
 
-def create_pending_vm(template, name, token = nil)
-  create_vm(name, token)
+def create_pending_vm(template, name, redis, token = nil)
+  create_vm(name, redis, token)
   redis.sadd("vmpooler__pending__#{template}", name)
   redis.hset("vmpooler__vm__#{name}", "template", template)
 end
 
-def create_vm(name, token = nil, redis_handle = nil, user = nil)
-  redis_db = redis_handle ? redis_handle : redis
-  redis_db.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
-  redis_db.hset("vmpooler__vm__#{name}", 'token:token', token) if token
-  redis_db.hset("vmpooler__vm__#{name}", 'token:user', user) if user
+def create_vm(name, redis, token = nil, user = nil)
+  redis.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
+  redis.hset("vmpooler__vm__#{name}", 'clone', Time.now)
+  redis.hset("vmpooler__vm__#{name}", 'token:token', token) if token
+  redis.hset("vmpooler__vm__#{name}", 'token:user', user) if user
 end
 
-def create_completed_vm(name, pool, active = false, redis_handle = nil)
-  redis_db = redis_handle ? redis_handle : redis
-  redis_db.sadd("vmpooler__completed__#{pool}", name)
-  redis_db.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
-  redis_db.hset("vmpooler__active__#{pool}", name, Time.now) if active
+def create_completed_vm(name, pool, redis, active = false)
+  redis.sadd("vmpooler__completed__#{pool}", name)
+  redis.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
+  redis.hset("vmpooler__active__#{pool}", name, Time.now) if active
 end
 
-def create_discovered_vm(name, pool, redis_handle = nil)
-  redis_db = redis_handle ? redis_handle : redis
-  redis_db.sadd("vmpooler__discovered__#{pool}", name)
+def create_discovered_vm(name, pool, redis)
+  redis.sadd("vmpooler__discovered__#{pool}", name)
 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)
+def create_migrating_vm(name, pool, redis)
+  redis.hset("vmpooler__vm__#{name}", 'checkout', Time.now)
+  redis.sadd("vmpooler__migrating__#{pool}", name)
 end
 
-def create_tag(vm, tag_name, tag_value, redis_handle = nil)
-  redis_db = redis_handle ? redis-handle : redis
-  redis_db.hset("vmpooler__vm__#{vm}", "tag:#{tag_name}", tag_value)
+def create_tag(vm, tag_name, tag_value, redis)
+  redis.hset("vmpooler__vm__#{vm}", "tag:#{tag_name}", tag_value)
 end
 
-def add_vm_to_migration_set(name, redis_handle = nil)
-  redis_db = redis_handle ? redis_handle : redis
-  redis_db.sadd('vmpooler__migration', name)
+def add_vm_to_migration_set(name, redis)
+  redis.sadd('vmpooler__migration', name)
 end
 
 def fetch_vm(vm)

spec/unit/providers/base_spec.rb

@@ -22,7 +22,9 @@ describe 'Vmpooler::PoolManager::Provider::Base' do
     fake_vm
   }
 
-  subject { Vmpooler::PoolManager::Provider::Base.new(config, logger, metrics, provider_name, provider_options) }
+  let(:redis_connection_pool) { ConnectionPool.new(size: 1) { MockRedis.new } }
+
+  subject { Vmpooler::PoolManager::Provider::Base.new(config, logger, metrics, redis_connection_pool, provider_name, provider_options) }
 
   # Helper attr_reader methods
   describe '#logger' do

spec/unit/providers/dummy_spec.rb

@@ -91,7 +91,9 @@ EOT
     )
   }
 
-  subject { Vmpooler::PoolManager::Provider::Dummy.new(config, logger, metrics, 'dummy', provider_options) }
+  let(:redis_connection_pool) { ConnectionPool.new(size: 1) { MockRedis.new } }
+
+  subject { Vmpooler::PoolManager::Provider::Dummy.new(config, logger, metrics, redis_connection_pool, 'dummy', provider_options) }
 
   describe '#name' do
     it 'should be dummy' do

spec/unit/providers/vsphere_spec.rb

@@ -77,9 +77,15 @@ EOT
   let(:connection_options) {{}}
   let(:connection) { mock_RbVmomi_VIM_Connection(connection_options) }
   let(:vmname) { 'vm1' }
-  let(:redis) { MockRedis.new }
+  let(:redis_connection_pool) { Vmpooler::PoolManager::GenericConnectionPool.new(
+    metrics: metrics,
+    metric_prefix: 'redis_connection_pool',
+    size: 1,
+    timeout: 5
+  ) { MockRedis.new }
+  }
 
-  subject { Vmpooler::PoolManager::Provider::VSphere.new(config, logger, metrics, 'vsphere', provider_options) }
+  subject { Vmpooler::PoolManager::Provider::VSphere.new(config, logger, metrics, redis_connection_pool, 'vsphere', provider_options) }
 
   before(:each) do
     allow(subject).to receive(:vsphere_connection_ok?).and_return(true)
@@ -149,17 +155,6 @@ EOT
         allow(destroy_task).to receive(:wait_for_completion)
         allow(vm_object).to receive(:PowerOffVM_Task).and_return(power_off_task)
         allow(vm_object).to receive(:Destroy_Task).and_return(destroy_task)
-        $redis = redis
-      end
-
-      it 'should remove redis data and expire the vm key' do
-        allow(Time).to receive(:now).and_return(now)
-        expect(redis).to receive(:srem).with("vmpooler__completed__#{pool}", vmname)
-        expect(redis).to receive(:hdel).with("vmpooler__active__#{pool}", vmname)
-        expect(redis).to receive(:hset).with("vmpooler__vm__#{vmname}", 'destroy', now)
-        expect(redis).to receive(:expire).with("vmpooler__vm__#{vmname}", data_ttl * 60 * 60)
-
-        subject.destroy_vm_and_log(vmname, vm_object, pool, data_ttl)
       end
 
       it 'should log a message that the vm is destroyed' do
@@ -169,6 +164,7 @@ EOT
       end
 
       it 'should record metrics' do
+        expect(metrics).to receive(:timing).with('redis_connection_pool.waited', 0)
         expect(metrics).to receive(:timing).with("destroy.#{pool}", finish)
 
         subject.destroy_vm_and_log(vmname, vm_object, pool, data_ttl)
@@ -2573,10 +2569,12 @@ EOT
         subject.connection_pool.with_metrics do |pool_object|
           expect(subject).to receive(:ensured_vsphere_connection).with(pool_object).and_return(connection)
           expect(subject).to receive(:get_vm_details).and_return(vm_details)
-          allow($redis).to receive(:hset)
           expect(subject).to receive(:run_select_hosts).with(poolname, {})
           expect(subject).to receive(:vm_in_target?).and_return false
           expect(subject).to receive(:migration_enabled?).and_return true
+          redis_connection_pool.with do |redis|
+            redis.hset("vmpooler__vm__#{vmname}", 'checkout', Time.now)
+          end
         end
         vm_object.summary.runtime.host = host_object
       end
@@ -2585,7 +2583,6 @@ EOT
         expect(subject).to receive(:select_next_host).and_return(new_host)
         expect(subject).to receive(:find_host_by_dnsname).and_return(new_host_object)
         expect(subject).to receive(:migrate_vm_host).with(vm_object, new_host_object)
-        expect($redis).to receive(:hget).with("vmpooler__vm__#{vmname}", 'checkout').and_return((Time.now - 1).to_s)
         expect(logger).to receive(:log).with('s', "[>] [#{poolname}] '#{vmname}' migrated from #{parent_host} to #{new_host} in 0.00 seconds")
 
         subject.migrate_vm(poolname, vmname)
@@ -2603,7 +2600,6 @@ EOT
         subject.connection_pool.with_metrics do |pool_object|
           expect(subject).to receive(:ensured_vsphere_connection).with(pool_object).and_return(connection)
           expect(subject).to receive(:get_vm_details).and_return(vm_details)
-          expect($redis).to receive(:hset).with("vmpooler__vm__#{vmname}", 'host', parent_host)
           expect(subject).to receive(:run_select_hosts).with(poolname, {})
           expect(subject).to receive(:vm_in_target?).and_return true
           expect(subject).to receive(:migration_enabled?).and_return true
@@ -2622,11 +2618,13 @@ EOT
       before(:each) do
         subject.connection_pool.with_metrics do |pool_object|
           expect(subject).to receive(:ensured_vsphere_connection).with(pool_object).and_return(connection)
-          expect($redis).to receive(:scard).with('vmpooler__migration').and_return(5)
           expect(subject).to receive(:get_vm_details).and_return(vm_details)
           expect(subject).to_not receive(:run_select_hosts)
           expect(subject).to_not receive(:vm_in_target?)
           expect(subject).to receive(:migration_enabled?).and_return true
+          redis_connection_pool.with do |redis|
+            expect(redis).to receive(:scard).with('vmpooler__migration').and_return(5)
+          end
         end
         vm_object.summary.runtime.host = host_object
       end
@@ -2691,14 +2689,10 @@ EOT
     end
 
     it' migrates a vm' do
-      expect($redis).to receive(:sadd).with('vmpooler__migration', vmname)
       expect(subject).to receive(:select_next_host).and_return(new_host)
-      expect($redis).to receive(:hset).with("vmpooler__vm__#{vmname}", 'host', new_host)
-      expect($redis).to receive(:hset).with("vmpooler__vm__#{vmname}", 'migrated', true)
       expect(subject).to receive(:find_host_by_dnsname).and_return(new_host_object)
       expect(subject).to receive(:migrate_vm_and_record_timing).and_return(format('%.2f', (Time.now - (Time.now - 15))))
       expect(logger).to receive(:log).with('s', "[>] [#{poolname}] '#{vmname}' migrated from host1 to host2 in 15.00 seconds")
-      expect($redis).to receive(:srem).with('vmpooler__migration', vmname)
       subject.migrate_vm_to_new_host(poolname, vmname, vm_details, connection)
     end
   end