From df89617fdcb9cd540b26950764219d588d3d7cbe Mon Sep 17 00:00:00 2001
From: "kirby@puppetlabs.com" <mk.kirby@gmail.com>
Date: Thu, 28 Jun 2018 20:32:44 -0700
Subject: [PATCH] Do not run duplicate instances of inventory check for a pool

This commit updates check_pool inventory check to prevent multiple instances from running at once. Without this change the inventory check may run in multiple threads simultaneously.
---
 lib/vmpooler/pool_manager.rb   | 27 +++++++++++++++------------
 spec/unit/pool_manager_spec.rb | 22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb
index a00acb4..907620f 100644
--- a/lib/vmpooler/pool_manager.rb
+++ b/lib/vmpooler/pool_manager.rb
@@ -708,21 +708,24 @@ module Vmpooler
       # INVENTORY
       inventory = {}
       begin
-        provider.vms_in_pool(pool['name']).each do |vm|
-          if !$redis.sismember('vmpooler__running__' + pool['name'], vm['name']) &&
-             !$redis.sismember('vmpooler__ready__' + pool['name'], vm['name']) &&
-             !$redis.sismember('vmpooler__pending__' + pool['name'], vm['name']) &&
-             !$redis.sismember('vmpooler__completed__' + pool['name'], vm['name']) &&
-             !$redis.sismember('vmpooler__discovered__' + pool['name'], vm['name']) &&
-             !$redis.sismember('vmpooler__migrating__' + pool['name'], vm['name'])
+        mutex = pool_mutex(pool['name'])
+        mutex.synchronize do
+          provider.vms_in_pool(pool['name']).each do |vm|
+            if !$redis.sismember('vmpooler__running__' + pool['name'], vm['name']) &&
+               !$redis.sismember('vmpooler__ready__' + pool['name'], vm['name']) &&
+               !$redis.sismember('vmpooler__pending__' + pool['name'], vm['name']) &&
+               !$redis.sismember('vmpooler__completed__' + pool['name'], vm['name']) &&
+               !$redis.sismember('vmpooler__discovered__' + pool['name'], vm['name']) &&
+               !$redis.sismember('vmpooler__migrating__' + pool['name'], vm['name'])
 
-            pool_check_response[:discovered_vms] += 1
-            $redis.sadd('vmpooler__discovered__' + pool['name'], vm['name'])
+              pool_check_response[:discovered_vms] += 1
+              $redis.sadd('vmpooler__discovered__' + pool['name'], vm['name'])
 
-            $logger.log('s', "[?] [#{pool['name']}] '#{vm['name']}' added to 'discovered' queue")
+              $logger.log('s', "[?] [#{pool['name']}] '#{vm['name']}' added to 'discovered' queue")
+            end
+
+            inventory[vm['name']] = 1
           end
-
-          inventory[vm['name']] = 1
         end
       rescue => err
         $logger.log('s', "[!] [#{pool['name']}] _check_pool failed with an error while inspecting inventory: #{err}")
diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb
index be176d6..9f7ffe4 100644
--- a/spec/unit/pool_manager_spec.rb
+++ b/spec/unit/pool_manager_spec.rb
@@ -2685,6 +2685,7 @@ EOT
     let(:pool_object) { config[:pools][0] }
     let(:new_vm) { 'newvm'}
     let(:pool_name) { pool_object['name'] }
+    let(:mutex) { Mutex.new }
 
     before do
       expect(subject).not_to be_nil
@@ -2790,6 +2791,27 @@ EOT
           end
         end
       end
+
+      it 'should get the pool mutex' do
+        expect(subject).to receive(:pool_mutex).and_return(mutex).at_least(:once)
+
+        subject._check_pool(pool_object,provider)
+      end
+
+      it 'should run synchronize' do
+        expect(subject).to receive(:pool_mutex).and_return(mutex).at_least(:once)
+        expect(mutex).to receive(:synchronize).at_least(:once)
+
+        subject._check_pool(pool_object,provider)
+      end
+
+      it 'should yield when locked' do
+        expect(subject).to receive(:pool_mutex).and_return(mutex).at_least(:once)
+        mutex.lock
+        expect(mutex).to receive(:synchronize).and_yield
+
+        subject._check_pool(pool_object,provider)
+      end
     end
 
     # RUNNING