[QENG-3919] Make vmpooler checkouts be all or nothing (#153)

* (QENG-3919) spike for implementation of all-or-nothing checkout

* Fix two botched variable references

* Aggregate API helper methods

* Add specs for failed multi-vm allocation API endpoints

* (QENG-3919) Add tests for multiple vm requests

* (QENG-3919) Add (failing) specs for POST /vm/pool1+pool2 usages

This exposes the old (bad) behavior on this other code path. Will fix this up next.

* (QENG-3919) Bring query params version in line with JSON post version

Not clear to me why these had to be implemented so differently.

* (QENG-3919) extract common method from both methods of VM allocation

* (QENG-3919) Naming fix, cosmetic cleanups

I mean, I presume all these commits are going to get squashed away on merge anyway.

* (QENG-3919) Update API docs

We consider it a bug that the actual behavior was not this behavior, but the
documentation was also silent on this point.

* (QENG-3919) minor readability tweak in refactored method

* (QENG-3919) Clean up interim comments re: status codes

* (QENG-3919) Drop now-orphaned `checkout_vm` method

We kept this up-to-date while we were upgrading and refactoring, but, turns out,
this method is no longer called anywhere.  💀 🔥

* (QENG-3919) Return 503 status on failed allocation

Making sure we go back to the original functionality, which was:

 - status 200 when vms successfully allocated
 - status 404 when a pool name is unknown
 - status 404 when no pool name is specified
 - status 503 when vm allocation failed

* (QENG-3919) add net-ldap to Gemfile

Maybe we shouldn't foil-ball gems onto servers.

* (QENG-3919) Turns out, spush isn't a redis command

And hence we see once again the weakness of mockist tests.

* (QENG-3919) Pin the net-ldap gem to 0.11 for the jrubies, etc.

* (QENG-3919) Correct an old spelling error in spec descriptions

* (QENG-3919) Further tweak net-ldap version

* (QENG-3919) return_single_vm -> return_vm_to_ready_state

cc @shermdog
This commit is contained in:
Rick Bradley 2016-05-27 12:49:57 -05:00 committed by Rick Sherman
parent b59a1f8886
commit 5aaab7c5c2
4 changed files with 310 additions and 79 deletions

4
API.md
View file

@ -117,6 +117,8 @@ $ curl -d '{"debian-7-i386":"2","debian-7-x86_64":"1"}' --url vmpooler.company.c
}
```
**NOTE: Returns either all requested VMs or no VMs.**
##### POST /vm/<pool>
Check-out a VM or VMs.
@ -156,6 +158,8 @@ $ curl -d --url vmpooler.company.com/api/v1/vm/debian-7-i386+debian-7-i386+debia
}
```
**NOTE: Returns either all requested VMs or no VMs.**
##### GET /vm/<hostname>
Query a checked-out VM.

View file

@ -6,6 +6,7 @@ gem 'rake', '>= 10.4'
gem 'rbvmomi', '>= 1.8'
gem 'redis', '>= 3.2'
gem 'sinatra', '>= 1.4'
gem 'net-ldap', '<= 0.12.1' # keep compatibility w/ jruby & mri-1.9.3
# Test deps
group :test do

View file

@ -45,38 +45,75 @@ module Vmpooler
newhash
end
def checkout_vm(template, result)
vm = backend.spop('vmpooler__ready__' + template)
def fetch_single_vm(template)
backend.spop('vmpooler__ready__' + template)
end
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)
def return_vm_to_ready_state(template, vm)
backend.sadd('vmpooler__ready__' + template, vm)
end
if Vmpooler::API.settings.config[:auth] and has_token?
validate_token(backend)
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)
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 Vmpooler::API.settings.config[:auth] and has_token?
validate_token(backend)
if config['vm_lifetime_auth'].to_i > 0
backend.hset('vmpooler__vm__' + vm, 'lifetime', config['vm_lifetime_auth'].to_i)
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 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
def atomically_allocate_vms(payload)
return false unless payload and !payload.empty?
result = { 'ok' => false }
failed = false
vms = []
payload.each do |template, count|
count.to_i.times do |_i|
vm = fetch_single_vm(template)
if !vm
failed = true
break
else
vms << [ template, vm ]
end
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
if failed
vms.each do |(template, vm)|
return_vm_to_ready_state(template, vm)
status 503
end
else
status 503
result['ok'] = false
vms.each do |(template, vm)|
account_for_starting_vm(template, vm)
update_result_hosts(result, template, vm)
end
result['ok'] = true
result['domain'] = config['domain'] if config['domain']
end
result
@ -334,80 +371,41 @@ module Vmpooler
end
post "#{api_prefix}/vm/?" do
jdata = alias_deref(JSON.parse(request.body.read))
content_type :json
result = { 'ok' => false }
jdata = alias_deref(JSON.parse(request.body.read))
if not jdata.nil? and not jdata.empty?
available = 1
if jdata and !jdata.empty?
result = atomically_allocate_vms(jdata)
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
post "#{api_prefix}/vm/:template/?" do
content_type :json
result = { 'ok' => false }
def extract_templates_from_query_params(params)
payload = {}
params[:template].split('+').each do |template|
params.split('+').each do |template|
payload[template] ||= 0
payload[template] = payload[template] + 1
payload[template] += 1
end
payload = alias_deref(payload)
payload
end
if not payload.nil? and not payload.empty?
available = 1
post "#{api_prefix}/vm/:template/?" do
payload = alias_deref(extract_templates_from_query_params(params[:template]))
content_type :json
result = { 'ok' => false }
if payload and !payload.empty?
result = atomically_allocate_vms(payload)
else
status 404
end
payload.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
payload.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

View file

@ -239,7 +239,7 @@ describe Vmpooler::API::V1 do
expect_json(ok = true, http = 200)
end
it 'fails on nonexistant pools' do
it 'fails on nonexistent pools' do
expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false)
post "#{prefix}/vm", '{"poolpoolpool":"1"}'
@ -265,6 +265,120 @@ describe Vmpooler::API::V1 do
expect_json(ok = true, http = 200)
end
it 'returns multiple VMs even when multiple instances from the same pool are requested' do
post "#{prefix}/vm", '{"pool1":"2","pool2":"1"}'
expected = {
ok: true,
pool1: {
hostname: [ 'abcdefghijklmnop', 'abcdefghijklmnop' ]
},
pool2: {
hostname: 'qrstuvwxyz012345'
}
}
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = true, http = 200)
end
it 'returns multiple VMs even when multiple instances from multiple pools are requested' do
post "#{prefix}/vm", '{"pool1":"2","pool2":"3"}'
expected = {
ok: true,
pool1: {
hostname: [ 'abcdefghijklmnop', 'abcdefghijklmnop' ]
},
pool2: {
hostname: [ 'qrstuvwxyz012345', 'qrstuvwxyz012345', 'qrstuvwxyz012345' ]
}
}
expect(last_response.body).to eq(JSON.pretty_generate(expected))
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(:sadd).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 = 503)
end
it 'returns any checked out vms to their pools 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(:sadd).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 = 503)
end
it 'fails when not all requested vms can be allocated, when requesting multiple instances from a pool' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop")
post "#{prefix}/vm", '{"pool1":"2","pool2":"1"}'
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'returns any checked out vms to their pools when not all requested vms can be allocated, when requesting multiple instances from a pool' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop").exactly(2).times
post "#{prefix}/vm", '{"pool1":"2","pool2":"1"}'
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'fails when not all requested vms can be allocated, when requesting multiple instances from multiple pools' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop")
post "#{prefix}/vm", '{"pool1":"2","pool2":"3"}'
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'returns any checked out vms to their pools when not all requested vms can be allocated, when requesting multiple instances from multiple pools' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop").exactly(2).times
post "#{prefix}/vm", '{"pool1":"2","pool2":"3"}'
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
context '(auth not configured)' do
let(:config) { { auth: false } }
@ -391,7 +505,7 @@ describe Vmpooler::API::V1 do
expect_json(ok = true, http = 200)
end
it 'fails on nonexistant pools' do
it 'fails on nonexistent pools' do
expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false)
post "#{prefix}/vm/poolpoolpool", ''
@ -417,6 +531,120 @@ describe Vmpooler::API::V1 do
expect_json(ok = true, http = 200)
end
it 'returns multiple VMs even when multiple instances from the same pool are requested' do
post "#{prefix}/vm/pool1+pool1+pool2", ''
expected = {
ok: true,
pool1: {
hostname: [ 'abcdefghijklmnop', 'abcdefghijklmnop' ]
},
pool2: {
hostname: 'qrstuvwxyz012345'
}
}
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = true, http = 200)
end
it 'returns multiple VMs even when multiple instances from multiple pools are requested' do
post "#{prefix}/vm/pool1+pool1+pool2+pool2+pool2", ''
expected = {
ok: true,
pool1: {
hostname: [ 'abcdefghijklmnop', 'abcdefghijklmnop' ]
},
pool2: {
hostname: [ 'qrstuvwxyz012345', 'qrstuvwxyz012345', 'qrstuvwxyz012345' ]
}
}
expect(last_response.body).to eq(JSON.pretty_generate(expected))
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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop")
post "#{prefix}/vm/pool1+pool2", ''
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'returns any checked out vms to their pools 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop")
post "#{prefix}/vm/pool1+pool2", ''
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'fails when not all requested vms can be allocated, when requesting multiple instances from a pool' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop")
post "#{prefix}/vm/pool1+pool1+pool2", ''
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'returns any checked out vms to their pools when not all requested vms can be allocated, when requesting multiple instances from a pool' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop").exactly(2).times
post "#{prefix}/vm/pool1+pool1+pool2", ''
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'fails when not all requested vms can be allocated, when requesting multiple instances from multiple pools' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop")
post "#{prefix}/vm/pool1+pool1+pool2+pool2+pool2", ''
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
it 'returns any checked out vms to their pools when not all requested vms can be allocated, when requesting multiple instances from multiple pools' 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(:sadd).with("vmpooler__ready__pool1", "abcdefghijklmnop").exactly(2).times
post "#{prefix}/vm/pool1+pool1+pool2+pool2+pool2", ''
expected = { ok: false }
expect(last_response.body).to eq(JSON.pretty_generate(expected))
expect_json(ok = false, http = 503)
end
context '(auth not configured)' do
let(:config) { { auth: false } }