From 830acd705d7616343789204b8ceecf57b1d4b591 Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Wed, 12 Sep 2018 12:28:12 -0700
Subject: [PATCH] Remove debug statements. Return when get_vm returns nil

---
 lib/vmpooler/pool_manager.rb        | 42 ++++++++++++-----------------
 spec/unit/pool_manager_spec.rb      | 14 +++-------
 spec/unit/providers/vsphere_spec.rb |  5 ++--
 3 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb
index 66ac8b6..561d642 100644
--- a/lib/vmpooler/pool_manager.rb
+++ b/lib/vmpooler/pool_manager.rb
@@ -149,34 +149,25 @@ module Vmpooler
       # Periodically check that the VM is available
       mutex = vm_mutex(vm)
       return if mutex.locked?
-      begin
-        mutex.synchronize do
-          stage = 'stamp check'
-          check_stamp = $redis.hget('vmpooler__vm__' + vm, 'check')
-          return if check_stamp && (((Time.now - Time.parse(check_stamp)) / 60) <= $config[:config]['vm_checktime'])
+      mutex.synchronize do
+        check_stamp = $redis.hget('vmpooler__vm__' + vm, 'check')
+        return if check_stamp && (((Time.now - Time.parse(check_stamp)) / 60) <= $config[:config]['vm_checktime'])
 
-          $redis.hset('vmpooler__vm__' + vm, 'check', Time.now)
-          # Check if the hosts TTL has expired
-          stage = 'ttl'
-          if ttl > 0
-            # host['boottime'] may be nil if host is not powered on
-            if ((Time.now - host['boottime']) / 60).to_s[/^\d+\.\d{1}/].to_f > ttl
-              $redis.smove('vmpooler__ready__' + pool['name'], 'vmpooler__completed__' + pool['name'], vm)
+        $redis.hset('vmpooler__vm__' + vm, 'check', Time.now)
+        # Check if the hosts TTL has expired
+        if ttl > 0
+          # host['boottime'] may be nil if host is not powered on
+          if ((Time.now - host['boottime']) / 60).to_s[/^\d+\.\d{1}/].to_f > ttl
+            $redis.smove('vmpooler__ready__' + pool['name'], 'vmpooler__completed__' + pool['name'], vm)
 
-              $logger.log('d', "[!] [#{pool['name']}] '#{vm}' reached end of TTL after #{ttl} minutes, removed from 'ready' queue")
-              return
-            end
+            $logger.log('d', "[!] [#{pool['name']}] '#{vm}' reached end of TTL after #{ttl} minutes, removed from 'ready' queue")
+            return
           end
-
-          stage = 'hostname mismatch'
-          return if has_mismatched_hostname?(vm, pool, provider)
-
-          stage = 'still ready'
-          vm_still_ready?(pool['name'], vm, provider)
         end
-      rescue => err
-        $logger.log('s', "Failed at stage #{stage} for #{vm}")
-        raise
+
+        return if has_mismatched_hostname?(vm, pool, provider)
+
+        vm_still_ready?(pool['name'], vm, provider)
       end
     end
 
@@ -197,6 +188,7 @@ module Vmpooler
 
       # Check if the hostname has magically changed from underneath Pooler
       vm_hash = provider.get_vm(pool['name'], vm)
+      return unless vm_hash.is_a? Hash
       hostname = vm_hash['hostname']
 
       return if hostname.nil?
@@ -875,7 +867,7 @@ module Vmpooler
       end
     end
 
-    def check_ready_pool_vms(pool_name, provider, pool_check_response, inventory, pool_ttl)
+    def check_ready_pool_vms(pool_name, provider, pool_check_response, inventory, pool_ttl = 0)
       $redis.smembers("vmpooler__ready__#{pool_name}").each do |vm|
         if inventory[vm]
           begin
diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb
index b8f144a..4e4ff96 100644
--- a/spec/unit/pool_manager_spec.rb
+++ b/spec/unit/pool_manager_spec.rb
@@ -2974,9 +2974,7 @@ EOT
         # mock response from create_inventory
         {vm => 1}
       }
-      let(:big_lifetime) {
-        2000
-      }
+      let(:big_lifetime) { 2000 }
       before(:each) do
         allow(subject).to receive(:check_ready_vm)
         create_ready_vm(pool,vm,token)
@@ -2992,7 +2990,7 @@ EOT
         expect(subject).to receive(:check_ready_vm).and_raise(RuntimeError,'MockError')
         expect(logger).to receive(:log).with('d', "[!] [#{pool}] _check_pool failed with an error while evaluating ready VMs: MockError")
 
-        subject.check_ready_pool_vms(pool, provider, pool_check_response, inventory)
+        subject.check_ready_pool_vms(pool, provider, pool_check_response, inventory, big_lifetime)
       end
 
       it 'should use the pool TTL if set' do
@@ -3511,12 +3509,8 @@ EOT
       end
 
       it 'captures #create_inventory errors correctly' do
-        allow(subject).to receive(:create_inventory).and_raise(
-          RuntimeError,'Mock Error'
-        )
-        expect {
-          subject._check_pool(pool_object, provider)
-        }.to_not raise_error(RuntimeError, /Mock Error/)
+        allow(subject).to receive(:create_inventory).and_raise(RuntimeError,'Mock Error')
+        subject._check_pool(pool_object, provider)
       end
 
       it 'should return early if an error occurs' do
diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb
index d09f7f4..69134cd 100644
--- a/spec/unit/providers/vsphere_spec.rb
+++ b/spec/unit/providers/vsphere_spec.rb
@@ -925,9 +925,10 @@ EOT
   end
 
   describe '#vm_ready?' do
+    let(:domain) { nil }
     context 'When a VM is ready' do
       before(:each) do
-        expect(subject).to receive(:open_socket).with(vmname)
+        expect(subject).to receive(:open_socket).with(vmname, domain)
       end
 
       it 'should return true' do
@@ -939,7 +940,7 @@ EOT
       # TODO not sure how to handle a VM that is passed in but
       # not located in the pool.  Is that ready or not?
       before(:each) do
-        expect(subject).to receive(:open_socket).with(vmname)
+        expect(subject).to receive(:open_socket).with(vmname, domain)
       end
 
       it 'should return true' do