From e323a7fb587ce1ac1b86b845bb08c4201a2fd01a Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Fri, 20 May 2016 13:12:44 -0500 Subject: [PATCH 1/4] (QENG-3919) spike for implementation of all-or-nothing checkout --- lib/vmpooler/api/v1.rb | 116 ++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 48 deletions(-) diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 1046593..14dbc49 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -45,35 +45,39 @@ module Vmpooler newhash end + def fetch_single_vm(template) + backend.spop('vmpooler__ready__' + template) + end + + def return_single_vm(template, vm) + backend.spush('vmpooler__ready__' + template, vm) + end + + def account_for_starting_vm(template, vm) + backend.sadd('vmpooler__running__' + template, vm) + backend.hset('vmpooler__active__' + template, vm, Time.now) + backend.hset('vmpooler__vm__' + vm, 'checkout', Time.now) + + if Vmpooler::API.settings.config[:auth] and has_token? + validate_token(backend) + + backend.hset('vmpooler__vm__' + vm, 'token:token', request.env['HTTP_X_AUTH_TOKEN']) + backend.hset('vmpooler__vm__' + vm, 'token:user', + backend.hget('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN'], 'user') + ) + + if config['vm_lifetime_auth'].to_i > 0 + backend.hset('vmpooler__vm__' + vm, 'lifetime', config['vm_lifetime_auth'].to_i) + end + end + end + def checkout_vm(template, result) - vm = backend.spop('vmpooler__ready__' + template) + vm = fetch_single_vm(template) unless vm.nil? - backend.sadd('vmpooler__running__' + template, vm) - backend.hset('vmpooler__active__' + template, vm, Time.now) - backend.hset('vmpooler__vm__' + vm, 'checkout', Time.now) - - if Vmpooler::API.settings.config[:auth] and has_token? - validate_token(backend) - - backend.hset('vmpooler__vm__' + vm, 'token:token', request.env['HTTP_X_AUTH_TOKEN']) - backend.hset('vmpooler__vm__' + vm, 'token:user', - backend.hget('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN'], 'user') - ) - - if config['vm_lifetime_auth'].to_i > 0 - backend.hset('vmpooler__vm__' + vm, 'lifetime', config['vm_lifetime_auth'].to_i) - end - end - - result[template] ||= {} - - if result[template]['hostname'] - result[template]['hostname'] = [result[template]['hostname']] unless result[template]['hostname'].is_a?(Array) - result[template]['hostname'].push(vm) - else - result[template]['hostname'] = vm - end + account_for_starting_vm(template, vm) + update_result_hosts(result, template, vm) else status 503 result['ok'] = false @@ -335,40 +339,56 @@ module Vmpooler post "#{api_prefix}/vm/?" do content_type :json - result = { 'ok' => false } jdata = alias_deref(JSON.parse(request.body.read)) if not jdata.nil? and not jdata.empty? - available = 1 + failed = false + vms = [] + + jdata.each do |template, count| + val.to_i.times do |_i| + vm = fetch_single_vm(template) + if !vm + failed = true + break + else + vms << [ template, vm ] + end + end + end + + if failed + vms.each do |(template, vm)| + return_single_vm(template, vm) + end + else + vms.each do |(template, vm)| + account_for_starting_vm(template, vm) + update_result_hosts(results, template, vm) + end + + result['ok'] = true + result['domain'] = config['domain'] if config['domain'] + end else status 404 end - jdata.each do |key, val| - if backend.scard('vmpooler__ready__' + key).to_i < val.to_i - available = 0 - end - end - - if (available == 1) - result['ok'] = true - - jdata.each do |key, val| - val.to_i.times do |_i| - result = checkout_vm(key, result) - end - end - end - - if result['ok'] && config['domain'] - result['domain'] = config['domain'] - end - JSON.pretty_generate(result) end + def update_result_hosts(result, template, vm) + result[template] ||= {} + if result[template]['hostname'] + result[template]['hostname'] = Array(result[template]['hostname']) + result[template]['hostname'].push(vm) + else + result[template]['hostname'] = vm + end + end + post "#{api_prefix}/vm/:template/?" do content_type :json From d3c5d3b9be0a91c7c70c3499c8fdb6cabfbae53d Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Fri, 20 May 2016 14:22:00 -0500 Subject: [PATCH 2/4] Fix two botched variable references --- lib/vmpooler/api/v1.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 14dbc49..33d5503 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -348,7 +348,7 @@ module Vmpooler vms = [] jdata.each do |template, count| - val.to_i.times do |_i| + count.to_i.times do |_i| vm = fetch_single_vm(template) if !vm failed = true @@ -366,7 +366,7 @@ module Vmpooler else vms.each do |(template, vm)| account_for_starting_vm(template, vm) - update_result_hosts(results, template, vm) + update_result_hosts(result, template, vm) end result['ok'] = true From 2cd83afeb2722eb532b74aed3892aa7b81d3688f Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Fri, 20 May 2016 14:27:21 -0500 Subject: [PATCH 3/4] Aggregate API helper methods --- lib/vmpooler/api/v1.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 33d5503..0ee0efb 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -86,6 +86,16 @@ module Vmpooler result end + def update_result_hosts(result, template, vm) + result[template] ||= {} + if result[template]['hostname'] + result[template]['hostname'] = Array(result[template]['hostname']) + result[template]['hostname'].push(vm) + else + result[template]['hostname'] = vm + end + end + get "#{api_prefix}/status/?" do content_type :json @@ -379,16 +389,6 @@ module Vmpooler JSON.pretty_generate(result) end - def update_result_hosts(result, template, vm) - result[template] ||= {} - if result[template]['hostname'] - result[template]['hostname'] = Array(result[template]['hostname']) - result[template]['hostname'].push(vm) - else - result[template]['hostname'] = vm - end - end - post "#{api_prefix}/vm/:template/?" do content_type :json From 7a587b8992065b6e63b7573e47bdad0c1d67b460 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Fri, 20 May 2016 14:38:13 -0500 Subject: [PATCH 4/4] Add specs for failed multi-vm allocation API endpoints --- spec/vmpooler/api/v1_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/spec/vmpooler/api/v1_spec.rb b/spec/vmpooler/api/v1_spec.rb index 1b65962..2904a2f 100644 --- a/spec/vmpooler/api/v1_spec.rb +++ b/spec/vmpooler/api/v1_spec.rb @@ -265,6 +265,32 @@ describe Vmpooler::API::V1 do expect_json(ok = true, http = 200) end + it 'fails when not all requested vms can be allocated' do + allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' + allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return nil + allow(redis).to receive(:spush).with("vmpooler__ready__pool1", "abcdefghijklmnop") + + post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' + + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + expect_json(ok = false, http = 200) # which HTTP status code? + end + + it 'returns any checked out vms when not all requested vms can be allocated' do + allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' + allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return nil + expect(redis).to receive(:spush).with("vmpooler__ready__pool1", "abcdefghijklmnop") + + post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' + + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + expect_json(ok = false, http = 200) # which HTTP status code? + end + context '(auth not configured)' do let(:config) { { auth: false } }