From acb95d34c8324e6263e5e7bd275592691e465393 Mon Sep 17 00:00:00 2001 From: Colin Date: Tue, 28 Jul 2015 14:40:52 -0700 Subject: [PATCH] (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. --- lib/vmpooler/api/v1.rb | 15 ++++++--------- spec/vmpooler/api/v1_spec.rb | 19 ++++--------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 4060e90..5a6c757 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -385,12 +385,11 @@ module Vmpooler 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 result['ok'] = true - rdata = backend.hgetall('vmpooler__vm__' + params[:hostname]) - result[params[:hostname]] = {} result[params[:hostname]]['template'] = rdata['template'] @@ -438,13 +437,11 @@ module Vmpooler 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'] - if backend.sismember('vmpooler__running__' + rdata['template'], params[:hostname]) - backend.srem('vmpooler__running__' + rdata['template'], params[:hostname]) + if backend.srem('vmpooler__running__' + rdata['template'], params[:hostname]) backend.sadd('vmpooler__completed__' + rdata['template'], params[:hostname]) status 200 @@ -554,7 +551,7 @@ module Vmpooler 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]) status 202 diff --git a/spec/vmpooler/api/v1_spec.rb b/spec/vmpooler/api/v1_spec.rb index bd3c915..c3796f7 100644 --- a/spec/vmpooler/api/v1_spec.rb +++ b/spec/vmpooler/api/v1_spec.rb @@ -438,9 +438,7 @@ describe Vmpooler::API::V1 do let(:config) { { auth: false } } it 'does not delete a non-existant VM' do - expect(redis).to receive(:exists).with('vmpooler__vm__testhost').and_return false - expect(redis).not_to receive(:sismember) - expect(redis).not_to receive(:hgetall) + expect(redis).to receive(:hgetall).and_return({}) expect(redis).not_to receive(:sadd) expect(redis).not_to receive(:srem) @@ -453,11 +451,9 @@ describe Vmpooler::API::V1 do end 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(:srem).and_return(true) expect(redis).to receive(:sadd) - expect(redis).to receive(:srem) delete "#{prefix}/vm/testhost" @@ -473,11 +469,9 @@ describe Vmpooler::API::V1 do context '(checked-out without 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(:srem).and_return(true) expect(redis).to receive(:sadd) - expect(redis).to receive(:srem) delete "#{prefix}/vm/testhost" @@ -490,7 +484,6 @@ describe Vmpooler::API::V1 do context '(checked-out with 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).not_to receive(:sadd) expect(redis).not_to receive(:srem) @@ -504,11 +497,9 @@ describe Vmpooler::API::V1 do end 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(: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(:srem) delete "#{prefix}/vm/testhost", "", { 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' @@ -571,7 +562,6 @@ describe Vmpooler::API::V1 do let(:config) { { auth: false } } 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(:sadd) @@ -596,7 +586,6 @@ describe Vmpooler::API::V1 do end 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(:sadd)