Fix number of ends at end of file. Move update_pool_size out of repopulate_pool_vms. Remove instances of check_pool_vms within the test.

lib/vmpooler/pool_manager.rb

@@ -822,7 +822,7 @@ module Vmpooler
     end
 
     def repopulate_pool_vms(pool_name, provider, pool_check_response, pool_size)
-      unless pool_mutex(pool_name).locked?
+      return if pool_mutex(pool_name).locked?
       ready = $redis.scard("vmpooler__ready__#{pool_name}")
       total = $redis.scard("vmpooler__pending__#{pool_name}") + ready
 
@@ -836,11 +836,6 @@ module Vmpooler
         $logger.log('s', "[!] [#{pool_name}] is empty")
       end
 
-        # Check to see if a pool size change has been made via the configuration API
-        # Since check_pool runs in a loop it does not
-        # otherwise identify this change when running
-        update_pool_size(pool)
-
       if total < pool_size
         (1..(pool_size - total)).each do |_i|
           if $redis.get('vmpooler__tasks__clone').to_i < $config[:config]['task_limit'].to_i
@@ -857,7 +852,6 @@ module Vmpooler
         end
       end
     end
-    end
 
     def _check_pool(pool, provider)
       pool_check_response = {
@@ -894,6 +888,11 @@ module Vmpooler
       # Additionally, a pool will drain ready and pending instances
       evaluate_template(pool, provider)
 
+      # Check to see if a pool size change has been made via the configuration API
+      # Since check_pool runs in a loop it does not
+      # otherwise identify this change when running
+      update_pool_size(pool)
+
       repopulate_pool_vms(pool['name'], provider, pool_check_response, pool['size'])
 
       # Remove VMs in excess of the configured pool size

spec/unit/pool_manager_spec.rb

@@ -3104,14 +3104,12 @@ EOT
     }
 
     it 'should not call clone_vm when number of VMs is equal to the pool size' do
-        expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
       expect(subject).to receive(:clone_vm).exactly(0).times
 
-        subject.repopulate_pool_vms(pool ,provider, pool_check_response, pool_size)
+      subject.repopulate_pool_vms(pool, provider, pool_check_response, pool_size)
     end
 
     it 'should not call clone_vm when number of VMs is greater than the pool size' do
-        expect(provider).to receive(:vms_in_pool).with(pool).and_return(vm_response)
       create_ready_vm(pool,vm,token)
       expect(subject).to receive(:clone_vm).exactly(0).times
 
@@ -3120,7 +3118,6 @@ EOT
 
     ['ready','pending'].each do |queue_name|
       it "should use VMs in #{queue_name} queue to caculate pool size" do
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return(vm_response)
         expect(subject).to receive(:clone_vm).exactly(0).times
         # Modify the pool size to 1 and add a VM in the queue
         redis.sadd("vmpooler__#{queue_name}__#{pool}",vm)
@@ -3132,7 +3129,6 @@ EOT
 
     ['running','completed','discovered','migrating'].each do |queue_name|
       it "should not use VMs in #{queue_name} queue to caculate pool size" do
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return(vm_response)
         expect(subject).to receive(:clone_vm)
         # Modify the pool size to 1 and add a VM in the queue
         redis.sadd("vmpooler__#{queue_name}__#{pool}",vm)
@@ -3143,31 +3139,24 @@ EOT
     end
 
     it 'should log a message the first time a pool is empty' do
-        expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
       expect(logger).to receive(:log).with('s', "[!] [#{pool}] is empty")
 
       subject._check_pool(pool_object,provider)
     end
 
     context 'when pool is marked as empty' do
-        let(:vm_response) {
-          # Mock response from Base Provider for vms_in_pool
-          [{ 'name' => vm}]
-        }          
 
       before(:each) do
         redis.set("vmpooler__empty__#{pool}", 'true')
       end
 
       it 'should not log a message when the pool remains empty' do
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
         expect(logger).to receive(:log).with('s', "[!] [#{pool}] is empty").exactly(0).times
 
         subject._check_pool(pool_object,provider)
       end
 
       it 'should remove the empty pool mark if it is no longer empty' do
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return(vm_response)
         create_ready_vm(pool,vm,token)
 
         expect(redis.get("vmpooler__empty__#{pool}")).to be_truthy
@@ -3177,9 +3166,6 @@ EOT
     end
 
     context 'when number of VMs is less than the pool size' do
-        before(:each) do
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
-        end
 
       it 'should return the number of cloned VMs' do
         pool_size = 5
@@ -3227,7 +3213,6 @@ EOT
       before(:each) do
         config[:pools][0]['size'] = poolsize
         redis.hset('vmpooler__config__poolsize', pool, newpoolsize)
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
       end
 
       it 'should change the pool size configuration' do
@@ -3241,7 +3226,6 @@ EOT
       let(:poolsize) { 2 }
       before(:each) do
         redis.hset('vmpooler__config__updating', pool, 1)
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
       end
 
       it 'should not call clone_vm to populate the pool' do
@@ -3257,7 +3241,6 @@ EOT
         allow(redis).to receive(:scard)
         expect(redis).to receive(:scard).with("vmpooler__ready__#{pool}").and_return(1)
         expect(redis).to receive(:scard).with("vmpooler__pending__#{pool}").and_return(1)
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
       end
 
       it 'should call remove_excess_vms' do
@@ -3272,7 +3255,6 @@ EOT
         create_ready_vm(pool,'vm1')
         create_ready_vm(pool,'vm2')
         create_ready_vm(pool,'vm3')
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return(multi_vm_response)
 
         expect(metrics).to receive(:gauge).with("ready.#{pool}", 3)
         allow(metrics).to receive(:gauge)
@@ -3284,7 +3266,6 @@ EOT
         create_running_vm(pool,'vm1',token)
         create_running_vm(pool,'vm2',token)
         create_running_vm(pool,'vm3',token)
-          expect(provider).to receive(:vms_in_pool).with(pool).and_return(multi_vm_response)
 
         expect(metrics).to receive(:gauge).with("running.#{pool}", 3)
         allow(metrics).to receive(:gauge)
@@ -3293,7 +3274,6 @@ EOT
       end
 
       it 'increments metrics with 0 when pool empty' do
-        expect(provider).to receive(:vms_in_pool).with(pool).and_return([])
 
         expect(metrics).to receive(:gauge).with("ready.#{pool}", 0)
         expect(metrics).to receive(:gauge).with("running.#{pool}", 0)
@@ -3302,8 +3282,6 @@ EOT
       end
     end
   end
-end
-
 
   describe '#_check_pool' do
     let(:new_vm_response) {
@@ -3568,9 +3546,14 @@ EOT
         subject._check_pool(pool_object,provider)
       end
     end
+    #REPOPULATE
+    context 'when checking if pools need to be repopulated' do
+      it 'should call #repopulate_pool_vms' do
+        expect(subject).to receive(:repopulate_pool_vms).with(pool, provider, pool_check_response, config[:pools][0]['size'])
 
+        subject._check_pool(pool_object, provider)
+      end
+    end
 
-
-    # REPOPULATE
-    context 'Repopulate a pool' 
+  end
 end