mirror of
https://github.com/puppetlabs/vmpooler.git
synced 2026-01-26 10:08:40 -05:00
(MAINT) Reduce redis Calls in API
The return values from most redis calls inform the caller of whether a key or hash value exists. Several exists() calls can be removed in favor of this approach. Updated spec tests to account for a removal of exists() and ismember() calls in API tests.
This commit is contained in:
parent
5789d2c708
commit
acb95d34c8
2 changed files with 10 additions and 24 deletions
|
|
@ -385,12 +385,11 @@ module Vmpooler
|
||||||
|
|
||||||
params[:hostname] = hostname_shorten(params[:hostname], config['domain'])
|
params[:hostname] = hostname_shorten(params[:hostname], config['domain'])
|
||||||
|
|
||||||
if backend.exists('vmpooler__vm__' + params[:hostname])
|
rdata = backend.hgetall('vmpooler__vm__' + params[:hostname])
|
||||||
|
unless rdata.empty?
|
||||||
status 200
|
status 200
|
||||||
result['ok'] = true
|
result['ok'] = true
|
||||||
|
|
||||||
rdata = backend.hgetall('vmpooler__vm__' + params[:hostname])
|
|
||||||
|
|
||||||
result[params[:hostname]] = {}
|
result[params[:hostname]] = {}
|
||||||
|
|
||||||
result[params[:hostname]]['template'] = rdata['template']
|
result[params[:hostname]]['template'] = rdata['template']
|
||||||
|
|
@ -438,13 +437,11 @@ module Vmpooler
|
||||||
|
|
||||||
params[:hostname] = hostname_shorten(params[:hostname], config['domain'])
|
params[:hostname] = hostname_shorten(params[:hostname], config['domain'])
|
||||||
|
|
||||||
if backend.exists('vmpooler__vm__' + params[:hostname])
|
|
||||||
rdata = backend.hgetall('vmpooler__vm__' + params[:hostname])
|
rdata = backend.hgetall('vmpooler__vm__' + params[:hostname])
|
||||||
|
unless rdata.empty?
|
||||||
need_token! if rdata['token:token']
|
need_token! if rdata['token:token']
|
||||||
|
|
||||||
if backend.sismember('vmpooler__running__' + rdata['template'], params[:hostname])
|
if backend.srem('vmpooler__running__' + rdata['template'], params[:hostname])
|
||||||
backend.srem('vmpooler__running__' + rdata['template'], params[:hostname])
|
|
||||||
backend.sadd('vmpooler__completed__' + rdata['template'], params[:hostname])
|
backend.sadd('vmpooler__completed__' + rdata['template'], params[:hostname])
|
||||||
|
|
||||||
status 200
|
status 200
|
||||||
|
|
@ -554,7 +551,7 @@ module Vmpooler
|
||||||
|
|
||||||
params[:hostname] = hostname_shorten(params[:hostname], config['domain'])
|
params[:hostname] = hostname_shorten(params[:hostname], config['domain'])
|
||||||
|
|
||||||
if backend.exists('vmpooler__vm__' + params[:hostname]) and backend.hget('vmpooler__vm__' + params[:hostname], 'snapshot:' + params[:snapshot])
|
unless backend.hget('vmpooler__vm__' + params[:hostname], 'snapshot:' + params[:snapshot]).to_i.zero?
|
||||||
backend.sadd('vmpooler__tasks__snapshot-revert', params[:hostname] + ':' + params[:snapshot])
|
backend.sadd('vmpooler__tasks__snapshot-revert', params[:hostname] + ':' + params[:snapshot])
|
||||||
|
|
||||||
status 202
|
status 202
|
||||||
|
|
|
||||||
|
|
@ -438,9 +438,7 @@ describe Vmpooler::API::V1 do
|
||||||
let(:config) { { auth: false } }
|
let(:config) { { auth: false } }
|
||||||
|
|
||||||
it 'does not delete a non-existant VM' do
|
it 'does not delete a non-existant VM' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return false
|
expect(redis).to receive(:hgetall).and_return({})
|
||||||
expect(redis).not_to receive(:sismember)
|
|
||||||
expect(redis).not_to receive(:hgetall)
|
|
||||||
expect(redis).not_to receive(:sadd)
|
expect(redis).not_to receive(:sadd)
|
||||||
expect(redis).not_to receive(:srem)
|
expect(redis).not_to receive(:srem)
|
||||||
|
|
||||||
|
|
@ -453,11 +451,9 @@ describe Vmpooler::API::V1 do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'deletes an existing VM' do
|
it 'deletes an existing VM' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return '1'
|
|
||||||
expect(redis).to receive(:sismember).with('vmpooler__running__pool1', 'testhost').and_return '1'
|
|
||||||
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1"})
|
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1"})
|
||||||
|
expect(redis).to receive(:srem).and_return(true)
|
||||||
expect(redis).to receive(:sadd)
|
expect(redis).to receive(:sadd)
|
||||||
expect(redis).to receive(:srem)
|
|
||||||
|
|
||||||
delete "#{prefix}/vm/testhost"
|
delete "#{prefix}/vm/testhost"
|
||||||
|
|
||||||
|
|
@ -473,11 +469,9 @@ describe Vmpooler::API::V1 do
|
||||||
|
|
||||||
context '(checked-out without token)' do
|
context '(checked-out without token)' do
|
||||||
it 'deletes a VM without supplying a token' do
|
it 'deletes a VM without supplying a token' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return '1'
|
|
||||||
expect(redis).to receive(:sismember).with('vmpooler__running__pool1', 'testhost').and_return '1'
|
|
||||||
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1"})
|
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1"})
|
||||||
|
expect(redis).to receive(:srem).and_return(true)
|
||||||
expect(redis).to receive(:sadd)
|
expect(redis).to receive(:sadd)
|
||||||
expect(redis).to receive(:srem)
|
|
||||||
|
|
||||||
delete "#{prefix}/vm/testhost"
|
delete "#{prefix}/vm/testhost"
|
||||||
|
|
||||||
|
|
@ -490,7 +484,6 @@ describe Vmpooler::API::V1 do
|
||||||
|
|
||||||
context '(checked-out with token)' do
|
context '(checked-out with token)' do
|
||||||
it 'fails to delete a VM without supplying a token' do
|
it 'fails to delete a VM without supplying a token' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return '1'
|
|
||||||
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1", "token:token" => "abcdefghijklmnopqrstuvwxyz012345"})
|
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1", "token:token" => "abcdefghijklmnopqrstuvwxyz012345"})
|
||||||
expect(redis).not_to receive(:sadd)
|
expect(redis).not_to receive(:sadd)
|
||||||
expect(redis).not_to receive(:srem)
|
expect(redis).not_to receive(:srem)
|
||||||
|
|
@ -504,11 +497,9 @@ describe Vmpooler::API::V1 do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'deletes a VM when token is supplied' do
|
it 'deletes a VM when token is supplied' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return '1'
|
|
||||||
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1", "token:token" => "abcdefghijklmnopqrstuvwxyz012345"})
|
expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1", "token:token" => "abcdefghijklmnopqrstuvwxyz012345"})
|
||||||
expect(redis).to receive(:sismember).with('vmpooler__running__pool1', 'testhost').and_return '1'
|
expect(redis).to receive(:srem).and_return(true)
|
||||||
expect(redis).to receive(:sadd)
|
expect(redis).to receive(:sadd)
|
||||||
expect(redis).to receive(:srem)
|
|
||||||
|
|
||||||
delete "#{prefix}/vm/testhost", "", {
|
delete "#{prefix}/vm/testhost", "", {
|
||||||
'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345'
|
'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345'
|
||||||
|
|
@ -571,7 +562,6 @@ describe Vmpooler::API::V1 do
|
||||||
let(:config) { { auth: false } }
|
let(:config) { { auth: false } }
|
||||||
|
|
||||||
it 'reverts to a snapshot' do
|
it 'reverts to a snapshot' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return(1)
|
|
||||||
expect(redis).to receive(:hget).with('vmpooler__vm__testhost', 'snapshot:testsnapshot').and_return(1)
|
expect(redis).to receive(:hget).with('vmpooler__vm__testhost', 'snapshot:testsnapshot').and_return(1)
|
||||||
expect(redis).to receive(:sadd)
|
expect(redis).to receive(:sadd)
|
||||||
|
|
||||||
|
|
@ -596,7 +586,6 @@ describe Vmpooler::API::V1 do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'reverts to a snapshot if authed' do
|
it 'reverts to a snapshot if authed' do
|
||||||
expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return(1)
|
|
||||||
expect(redis).to receive(:hget).with('vmpooler__vm__testhost', 'snapshot:testsnapshot').and_return(1)
|
expect(redis).to receive(:hget).with('vmpooler__vm__testhost', 'snapshot:testsnapshot').and_return(1)
|
||||||
expect(redis).to receive(:sadd)
|
expect(redis).to receive(:sadd)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue