From f45efb1b86af3d36def581108d5dd6abd15b78a6 Mon Sep 17 00:00:00 2001
From: Spencer McElmurry <mcelmurs@gmail.com>
Date: Tue, 24 Jul 2018 10:06:47 -0700
Subject: [PATCH] Moved repopulate pool vms method, broke loads of spec tests.
 Good luck

---
 lib/vmpooler/pool_manager.rb   |  77 +++---
 spec/unit/pool_manager_spec.rb | 456 ++++++++++++++++++---------------
 2 files changed, 283 insertions(+), 250 deletions(-)

diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb
index 789d79a..d580f40 100644
--- a/lib/vmpooler/pool_manager.rb
+++ b/lib/vmpooler/pool_manager.rb
@@ -821,6 +821,44 @@ module Vmpooler
       end
     end
 
+    def repopulate_pool_vms(pool_name, provider, pool_check_response, pool_size)
+      unless pool_mutex(pool_name).locked?
+        ready = $redis.scard("vmpooler__ready__#{pool_name}")
+        total = $redis.scard("vmpooler__pending__#{pool_name}") + ready
+
+        $metrics.gauge("ready.#{pool_name}", $redis.scard("vmpooler__ready__#{pool_name}"))
+        $metrics.gauge("running.#{pool_name}", $redis.scard("vmpooler__running__#{pool_name}"))
+
+        if $redis.get("vmpooler__empty__#{pool_name}")
+          $redis.del("vmpooler__empty__#{pool_name}") unless ready.zero?
+        elsif ready.zero?
+          $redis.set("vmpooler__empty__#{pool_name}", 'true')
+          $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
+              begin
+                $redis.incr('vmpooler__tasks__clone')
+                pool_check_response[:cloned_vms] += 1
+                clone_vm(pool, provider)
+              rescue => err
+                $logger.log('s', "[!] [#{pool_name}] clone failed during check_pool with an error: #{err}")
+                $redis.decr('vmpooler__tasks__clone')
+                raise
+              end
+            end
+          end
+        end
+      end
+    end
+
     def _check_pool(pool, provider)
       pool_check_response = {
         discovered_vms: 0,
@@ -850,50 +888,13 @@ module Vmpooler
 
       check_migrating_pool_vms(pool['name'], provider, pool_check_response, inventory)
 
-
       # UPDATE TEMPLATE
       # Evaluates a pool template to ensure templates are prepared adequately for the configured provider
       # If a pool template configuration change is detected then template preparation is repeated for the new template
       # Additionally, a pool will drain ready and pending instances
       evaluate_template(pool, provider)
 
-      # REPOPULATE
-      # Do not attempt to repopulate a pool while a template is updating
-      unless pool_mutex(pool['name']).locked?
-        ready = $redis.scard("vmpooler__ready__#{pool['name']}")
-        total = $redis.scard("vmpooler__pending__#{pool['name']}") + ready
-
-        $metrics.gauge("ready.#{pool['name']}", $redis.scard("vmpooler__ready__#{pool['name']}"))
-        $metrics.gauge("running.#{pool['name']}", $redis.scard("vmpooler__running__#{pool['name']}"))
-
-        if $redis.get("vmpooler__empty__#{pool['name']}")
-          $redis.del("vmpooler__empty__#{pool['name']}") unless ready.zero?
-        elsif ready.zero?
-          $redis.set("vmpooler__empty__#{pool['name']}", 'true')
-          $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
-              begin
-                $redis.incr('vmpooler__tasks__clone')
-                pool_check_response[:cloned_vms] += 1
-                clone_vm(pool, provider)
-              rescue => err
-                $logger.log('s', "[!] [#{pool['name']}] clone failed during check_pool with an error: #{err}")
-                $redis.decr('vmpooler__tasks__clone')
-                raise
-              end
-            end
-          end
-        end
-      end
+      repopulate_pool_vms(pool['name'], provider, pool_check_response, pool['size'])
 
       # Remove VMs in excess of the configured pool size
       remove_excess_vms(pool, provider, ready, total)
diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb
index 5e33a5a..ef69b25 100644
--- a/spec/unit/pool_manager_spec.rb
+++ b/spec/unit/pool_manager_spec.rb
@@ -3083,6 +3083,228 @@ EOT
     end
   end
 
+  describe '#repopulate_pool_vms' do
+    let(:pool_size) { 0 }
+    let(:config) {
+      YAML.load(<<-EOT
+---
+:config:
+  task_limit: 10
+:pools:
+  - name: #{pool}
+    size: #{pool_size}
+EOT
+      )
+    }
+    let(:provider) { double('provider') }
+    let(:pool_check_response) {
+      {
+        :cloned_vms => 0
+      }
+    }
+
+      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)
+      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
+
+        subject._check_pool(pool_object,provider)
+      end
+
+      ['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)
+          config[:pools][0]['size'] = 1
+   
+          subject._check_pool(pool_object,provider)
+        end
+      end
+
+      ['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)
+          config[:pools][0]['size'] = 1
+
+          subject._check_pool(pool_object,provider)
+        end
+      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
+          subject._check_pool(pool_object,provider)
+          expect(redis.get("vmpooler__empty__#{pool}")).to be_falsey
+        end
+      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
+          config[:pools][0]['size'] = pool_size
+
+          result = subject._check_pool(pool_object,provider)
+
+          expect(result[:cloned_vms]).to be(pool_size)
+        end
+
+        it 'should call clone_vm to populate the pool' do
+          pool_size = 5
+          config[:pools][0]['size'] = pool_size
+
+          expect(subject).to receive(:clone_vm).exactly(pool_size).times
+          
+          subject._check_pool(pool_object,provider)
+        end
+
+        it 'should call clone_vm until task_limit is hit' do
+          task_limit = 2
+          pool_size = 5
+          config[:pools][0]['size'] = pool_size
+          config[:config]['task_limit'] = task_limit
+
+          expect(subject).to receive(:clone_vm).exactly(task_limit).times
+          
+          subject._check_pool(pool_object,provider)
+        end
+
+        it 'log a message if a cloning error occurs' do
+          pool_size = 1
+          config[:pools][0]['size'] = pool_size
+
+          expect(subject).to receive(:clone_vm).and_raise(RuntimeError,"MockError")
+          expect(logger).to receive(:log).with("s", "[!] [#{pool}] clone failed during check_pool with an error: MockError")
+          
+          expect{ subject._check_pool(pool_object,provider) }.to raise_error(RuntimeError,'MockError')
+        end
+      end
+
+      context 'when a pool size configuration change is detected' do
+        let(:poolsize) { 2 }
+        let(:newpoolsize) { 3 }
+        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
+          subject._check_pool(config[:pools][0],provider)
+
+          expect(config[:pools][0]['size']).to be(newpoolsize)
+        end
+      end
+
+      context 'when a pool template is updating' do
+        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
+          expect(subject).to_not receive(:clone_vm)
+
+          subject._check_pool(config[:pools][0],provider)
+        end
+      end
+
+      context 'when an excess number of ready vms exist' do
+
+        before(:each) do
+          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
+          expect(subject).to receive(:remove_excess_vms).with(config[:pools][0], provider, 1, 2)
+
+          subject._check_pool(config[:pools][0],provider)
+        end
+      end
+
+      context 'export metrics' do
+        it 'increments metrics for ready queue' do
+          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)
+
+          subject._check_pool(pool_object,provider)
+        end
+
+        it 'increments metrics for running queue' do
+          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)
+
+          subject._check_pool(pool_object,provider)
+        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)
+
+          subject._check_pool(pool_object,provider)
+        end
+      end
+    end
+end
+
+
   describe '#_check_pool' do
     let(:new_vm_response) {
       # Mock response from Base Provider for vms_in_pool
@@ -3326,219 +3548,29 @@ EOT
     end
 
     # MIGRATIONS
+    context 'when checking migrating VMs' do
+      let(:pool_check_response) {
+        {
+          discovered_vms: 0,
+          checked_running_vms: 0,
+          checked_ready_vms: 0,
+          checked_pending_vms: 0,
+          destroyed_vms: 0,
+          migrated_vms: 0,
+          cloned_vms: 0
+        }
+      }
+
+      it 'should call #check_migrating_pool_vms' do
+        allow(subject).to receive(:create_inventory).and_return({})
+        expect(subject).to receive(:check_migrating_pool_vms).with(pool, provider, pool_check_response, {})
+
+        subject._check_pool(pool_object,provider)
+      end
+    end
+
 
 
     # REPOPULATE
-    context 'Repopulate a pool' do
-    let(:config) {
-      YAML.load(<<-EOT
----
-:config:
-  task_limit: 10
-:pools:
-  - name: #{pool}
-    size: 0
-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._check_pool(pool_object,provider)
-      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
-
-        subject._check_pool(pool_object,provider)
-      end
-
-      ['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)
-          config[:pools][0]['size'] = 1
-          
-          subject._check_pool(pool_object,provider)
-        end
-      end
-
-      ['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)
-          config[:pools][0]['size'] = 1
-
-          subject._check_pool(pool_object,provider)
-        end
-      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
-          subject._check_pool(pool_object,provider)
-          expect(redis.get("vmpooler__empty__#{pool}")).to be_falsey
-        end
-      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
-          config[:pools][0]['size'] = pool_size
-
-          result = subject._check_pool(pool_object,provider)
-
-          expect(result[:cloned_vms]).to be(pool_size)
-        end
-
-        it 'should call clone_vm to populate the pool' do
-          pool_size = 5
-          config[:pools][0]['size'] = pool_size
-
-          expect(subject).to receive(:clone_vm).exactly(pool_size).times
-          
-          subject._check_pool(pool_object,provider)
-        end
-
-        it 'should call clone_vm until task_limit is hit' do
-          task_limit = 2
-          pool_size = 5
-          config[:pools][0]['size'] = pool_size
-          config[:config]['task_limit'] = task_limit
-
-          expect(subject).to receive(:clone_vm).exactly(task_limit).times
-          
-          subject._check_pool(pool_object,provider)
-        end
-
-        it 'log a message if a cloning error occurs' do
-          pool_size = 1
-          config[:pools][0]['size'] = pool_size
-
-          expect(subject).to receive(:clone_vm).and_raise(RuntimeError,"MockError")
-          expect(logger).to receive(:log).with("s", "[!] [#{pool}] clone failed during check_pool with an error: MockError")
-          
-          expect{ subject._check_pool(pool_object,provider) }.to raise_error(RuntimeError,'MockError')
-        end
-      end
-
-      context 'when a pool size configuration change is detected' do
-        let(:poolsize) { 2 }
-        let(:newpoolsize) { 3 }
-        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
-          subject._check_pool(config[:pools][0],provider)
-
-          expect(config[:pools][0]['size']).to be(newpoolsize)
-        end
-      end
-
-      context 'when a pool template is updating' do
-        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
-          expect(subject).to_not receive(:clone_vm)
-
-          subject._check_pool(config[:pools][0],provider)
-        end
-      end
-
-      context 'when an excess number of ready vms exist' do
-
-        before(:each) do
-          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
-          expect(subject).to receive(:remove_excess_vms).with(config[:pools][0], provider, 1, 2)
-
-          subject._check_pool(config[:pools][0],provider)
-        end
-      end
-
-      context 'export metrics' do
-        it 'increments metrics for ready queue' do
-          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)
-
-          subject._check_pool(pool_object,provider)
-        end
-
-        it 'increments metrics for running queue' do
-          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)
-
-          subject._check_pool(pool_object,provider)
-        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)
-
-          subject._check_pool(pool_object,provider)
-        end
-      end
-    end
-  end
+    context 'Repopulate a pool' 
 end