From 9d3eea2fb3bfb9a7a25b75256cec4fc647889bd6 Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Tue, 13 Oct 2020 13:59:13 -0700
Subject: [PATCH] (POOLER-191) Add checking for running instances that are not
 in active

This change adds detection of running instances that are in a running
queue, but have no data in a active queue for the same pool. When this
happens a machine will live forever, impacting the running count, and
preventing the machine from being killed. Without this change running
instances that are not marked as active will live forever.
---
 lib/vmpooler/pool_manager.rb   | 16 ++++++----------
 spec/unit/pool_manager_spec.rb | 12 ++++++++++--
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb
index 95927d470..4f11e98b0 100644
--- a/lib/vmpooler/pool_manager.rb
+++ b/lib/vmpooler/pool_manager.rb
@@ -289,19 +289,15 @@ def _check_running_vm(vm, pool, ttl, provider)
                 move_vm_queue(pool, vm, 'running', 'completed', redis, "reached end of TTL after #{ttl} hours")
                 throw :stop_checking
               end
+            else
+              move_vm_queue(pool, vm, 'running', 'completed', redis, "is listed as running, but has no checkouttime data. Removing from running")
             end
 
-            if provider.vm_ready?(pool, vm)
-              throw :stop_checking
-            else
-              host = provider.get_vm(pool, vm)
+            throw :stop_checking if provider.vm_ready?(pool, vm)
 
-              if host
-                throw :stop_checking
-              else
-                move_vm_queue(pool, vm, 'running', 'completed', redis, 'is no longer in inventory, removing from running')
-              end
-            end
+            throw :stop_checking if provider.get_vm(pool, vm)
+
+            move_vm_queue(pool, vm, 'running', 'completed', redis, 'is no longer in inventory, removing from running')
           end
         end
       end
diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb
index e8a17a14a..3fb44da90 100644
--- a/spec/unit/pool_manager_spec.rb
+++ b/spec/unit/pool_manager_spec.rb
@@ -647,12 +647,20 @@
     end
 
     context 'valid host' do
-      it 'should not move VM if it has no checkout time' do
+      it 'should kill a VM if it has no checkout time' do
         redis_connection_pool.with do |redis|
           expect(provider).to receive(:vm_ready?).and_return(true)
           expect(redis.sismember("vmpooler__running__#{pool}", vm)).to be(true)
           subject._check_running_vm(vm, pool, 0, provider)
-          expect(redis.sismember("vmpooler__running__#{pool}", vm)).to be(true)
+          expect(redis.sismember("vmpooler__running__#{pool}", vm)).to be(false)
+        end
+      end
+
+      it 'should log a message when the machine is removed due to no active data' do
+        redis_connection_pool.with do |redis|
+          expect(provider).to receive(:vm_ready?).and_return(true)
+          expect(logger).to receive(:log).with('d',"[!] [#{pool}] '#{vm}' is listed as running, but has no checkouttime data. Removing from running")
+          subject._check_running_vm(vm, pool, 0, provider)
         end
       end