From 30bf2074fe410d18baaf2927f17e6fd886d0ba4d Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Mon, 21 Oct 2019 14:24:34 -0700 Subject: [PATCH] (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 | 1 + bin/vmpooler | 3 ++- lib/vmpooler/api.rb | 3 ++- lib/vmpooler/api/v1.rb | 30 ++++++++++++--------- spec/integration/api/v1/vm_spec.rb | 2 ++ spec/integration/api/v1/vm_template_spec.rb | 2 ++ 6 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef8064399..3f7f5da49 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/bin/vmpooler b/bin/vmpooler index b84a13916..29a4e4ed5 100755 --- a/bin/vmpooler +++ b/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 diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index 239155b14..0b1fbe66c 100644 --- a/lib/vmpooler/api.rb +++ b/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! diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 4b84e3eed..1f63fc786 100644 --- a/lib/vmpooler/api/v1.rb +++ b/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) diff --git a/spec/integration/api/v1/vm_spec.rb b/spec/integration/api/v1/vm_spec.rb index 9bbe08a7d..78be43278 100644 --- a/spec/integration/api/v1/vm_spec.rb +++ b/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 diff --git a/spec/integration/api/v1/vm_template_spec.rb b/spec/integration/api/v1/vm_template_spec.rb index d94b537af..9620f1f4c 100644 --- a/spec/integration/api/v1/vm_template_spec.rb +++ b/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