Merge pull request #339 from mattkirby/pooler_150

(POOLER-150) Synchronize checkout operations for API

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 @@ def initialize
     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 @@ def need_token!
       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 @@ def fetch_single_vm(template)
         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 @@ def app()
     }
     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 @@ def app()
 
     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