(POOLER-150) Synchronize checkout operations for API

This commit adds a shared mutex to vmpooler API so that checkout requests can be synchronized across threads. Without this change it is possible in some scenarios for vmpooler to allocate the same SUT to different API requests for a VM.

CHANGELOG.md

@@ -14,6 +14,7 @@ git logs & PR history.
 
 ### Fixed
 - Correctly detect create\_linked\_clone on a pool level (POOLER-147)
+- Ensure that checkout operations are synchronized
 
 # [0.7.0](https://github.com/puppetlabs/vmpooler/compare/0.6.3...0.7.0)
 

bin/vmpooler

@@ -24,7 +24,8 @@ if torun.include? 'api'
   api = Thread.new do
     thr = Vmpooler::API.new
     redis = Vmpooler.new_redis(redis_host, redis_port, redis_password)
-    thr.helpers.configure(config, redis, metrics)
+    checkoutlock = Mutex.new
+    thr.helpers.configure(config, redis, metrics, checkoutlock)
     thr.helpers.execute!
   end
   torun_threads << api

lib/vmpooler/api.rb

@@ -36,10 +36,11 @@ module Vmpooler
     use Vmpooler::API::Reroute
     use Vmpooler::API::V1
 
-    def configure(config, redis, metrics)
+    def configure(config, redis, metrics, checkoutlock)
       self.settings.set :config, config
       self.settings.set :redis, redis
       self.settings.set :metrics, metrics
+      self.settings.set :checkoutlock, checkoutlock
     end
 
     def execute!

lib/vmpooler/api/v1.rb

@@ -36,6 +36,10 @@ module Vmpooler
       validate_token(backend)
     end
 
+    def checkoutlock
+      Vmpooler::API::settings.checkoutlock
+    end
+
     def fetch_single_vm(template)
       template_backends = [template]
       aliases = Vmpooler::API.settings.config[:alias]
@@ -67,21 +71,23 @@ module Vmpooler
         end
       end
 
-      template_backends.each do |template_backend|
-        vms = backend.smembers("vmpooler__ready__#{template_backend}")
-        next if vms.empty?
-        vms.reverse.each do |vm|
-          ready = vm_ready?(vm, config['domain'])
-          if ready
-            backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm)
-            return [vm, template_backend, template]
-          else
-            backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__completed__#{template_backend}", vm)
-            metrics.increment("checkout.nonresponsive.#{template_backend}")
+      checkoutlock.synchronize do
+        template_backends.each do |template_backend|
+          vms = backend.smembers("vmpooler__ready__#{template_backend}")
+          next if vms.empty?
+          vms.reverse.each do |vm|
+            ready = vm_ready?(vm, config['domain'])
+            if ready
+              backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm)
+              return [vm, template_backend, template]
+            else
+              backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__completed__#{template_backend}", vm)
+              metrics.increment("checkout.nonresponsive.#{template_backend}")
+            end
           end
         end
+        [nil, nil, nil]
       end
-      [nil, nil, nil]
     end
 
     def return_vm_to_ready_state(template, vm)

spec/integration/api/v1/vm_spec.rb

@@ -29,12 +29,14 @@ describe Vmpooler::API::V1 do
     }
     let(:current_time) { Time.now }
     let(:vmname) { 'abcdefghijkl' }
+    let(:checkoutlock) { Mutex.new }
 
     before(:each) do
       app.settings.set :config, config
       app.settings.set :redis, redis
       app.settings.set :metrics, metrics
       app.settings.set :config, auth: false
+      app.settings.set :checkoutlock, checkoutlock
       create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time)
     end
 

spec/integration/api/v1/vm_template_spec.rb

@@ -29,12 +29,14 @@ describe Vmpooler::API::V1 do
 
     let(:current_time) { Time.now }
     let(:socket) { double('socket') }
+    let(:checkoutlock) { Mutex.new }
 
     before(:each) do
       app.settings.set :config, config
       app.settings.set :redis, redis
       app.settings.set :metrics, metrics
       app.settings.set :config, auth: false
+      app.settings.set :checkoutlock, checkoutlock
       create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time)
     end