From 26ca390b4cdc9433dc32ae976bf342b163f96fec Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Tue, 7 Jun 2016 14:13:39 -0700 Subject: [PATCH 1/8] Improved tests for vmpooler (#152) * Add redis to our travis runs * De-mockistify v1/token specs; use real redis. Open questions: - Do we need to do better cleanup here? - Should we be using a separate database to prevent clobbering other local db's? * Remove mockist tests from main suite. * (MAINT) gitignore some common files * (maint) Clean up some of the /vm/ tests * (maint) Convert specs for /vm/template * (maint) Clean up, reorganize specs * (maint) Move extracted spec helper methods to spec_helper * (maint) rename create_vm -> create_ready_vm * (WIP) add partially-converted /vm/hostname specs * (maint) clean up vm_spec * (WIP) notes for next steps * (maint) Define :config in token tests Miscellaneous whitespace cleanup. * (maint) Lift #redis definition into spec helper library * (maint) drop unneeded clear_pool helper Given the way we're flushing redis (which seems super performant), we don't need to clear pools any more at the beginning of tests. * (maint) Drop clear_pool from vm/template specs * (maint) Update vm/hostname tag and lifetime specs * (maint) Convert vm deletion specs * (maint) Convert specs for vm snapshot operations * (maint) Drop now-obsolete v1_spec.rb * (maint) cosmetic cleanup in spec helper * (maint) begin de-mockistifying api_spec.rb * (maint) repair incorrect test The mockist version of the test allows redis' scard to return nil, which it does not actually do in real life. Verified the behavior in the code via a debugger. Fixed the test. * (maint) finish converting dashboard specs * (maint) rename api_spec to dashboard_spec * (maint) Don't clobber default redis database when running specs --- .gitignore | 3 + .travis.yml | 2 + spec/helpers.rb | 69 +- spec/spec_helper.rb | 1 + spec/vmpooler/api/v1/token_spec.rb | 173 +++++ spec/vmpooler/api/v1/vm_hostname_spec.rb | 317 ++++++++ spec/vmpooler/api/v1/vm_spec.rb | 175 +++++ spec/vmpooler/api/v1/vm_template_spec.rb | 176 +++++ spec/vmpooler/api/v1_spec.rb | 735 ------------------ .../{api_spec.rb => dashboard_spec.rb} | 61 +- 10 files changed, 941 insertions(+), 771 deletions(-) create mode 100644 .gitignore create mode 100644 spec/vmpooler/api/v1/token_spec.rb create mode 100644 spec/vmpooler/api/v1/vm_hostname_spec.rb create mode 100644 spec/vmpooler/api/v1/vm_spec.rb create mode 100644 spec/vmpooler/api/v1/vm_template_spec.rb delete mode 100644 spec/vmpooler/api/v1_spec.rb rename spec/vmpooler/{api_spec.rb => dashboard_spec.rb} (76%) diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..835dcd0 --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ +.ruby-version +Gemfile.lock +vendor diff --git a/.travis.yml b/.travis.yml index e988f97..129267b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,8 @@ cache: bundler sudo: false language: ruby +services: + - redis-server rvm: - 1.9.3 - 2.1.1 diff --git a/spec/helpers.rb b/spec/helpers.rb index a91c987..d28b761 100644 --- a/spec/helpers.rb +++ b/spec/helpers.rb @@ -1,7 +1,12 @@ -def expect_json( - ok = true, - http = 200 -) +def redis + unless @redis + @redis = Redis.new + @redis.select(15) # let's use the highest numbered database available in a default install + end + @redis +end + +def expect_json(ok = true, http = 200) expect(last_response.header['Content-Type']).to eq('application/json') if (ok == true) then @@ -12,3 +17,59 @@ def expect_json( expect(last_response.status).to eq(http) end + +def create_token(token, user, timestamp) + redis.hset("vmpooler__token__#{token}", 'user', user) + redis.hset("vmpooler__token__#{token}", 'created', timestamp) +end + +def get_token_data(token) + redis.hgetall("vmpooler__token__#{token}") +end + +def token_exists?(token) + result = get_token_data + result && !result.empty? +end + +def create_ready_vm(template, name, token = nil) + create_vm(name, token) + redis.sadd("vmpooler__ready__#{template}", name) + redis.hset("vmpooler_vm_#{name}", "template", template) +end + +def create_running_vm(template, name, token = nil) + create_vm(name, token) + redis.sadd("vmpooler__running__#{template}", name) + redis.hset("vmpooler__vm__#{name}", "template", template) +end + +def create_vm(name, token = nil) + redis.hset("vmpooler__vm__#{name}", 'checkout', Time.now) + if token + redis.hset("vmpooler__vm__#{name}", 'token:token', token) + end +end + +def fetch_vm(vm) + redis.hgetall("vmpooler__vm__#{vm}") +end + +def snapshot_vm(vm, snapshot = '12345678901234567890123456789012') + redis.sadd('vmpooler__tasks__snapshot', "#{vm}:#{snapshot}") + redis.hset("vmpooler__vm__#{vm}", "snapshot:#{snapshot}", "1") +end + +def has_vm_snapshot?(vm) + redis.smembers('vmpooler__tasks__snapshot').any? do |snapshot| + instance, sha = snapshot.split(':') + vm == instance + end +end + +def vm_reverted_to_snapshot?(vm, snapshot = nil) + redis.smembers('vmpooler__tasks__snapshot-revert').any? do |action| + instance, sha = action.split(':') + instance == vm and (snapshot ? (sha == snapshot) : true) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2186bdb..7589276 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,3 +2,4 @@ require 'helpers' require 'rbvmomi' require 'rspec' require 'vmpooler' +require 'redis' diff --git a/spec/vmpooler/api/v1/token_spec.rb b/spec/vmpooler/api/v1/token_spec.rb new file mode 100644 index 0000000..d386457 --- /dev/null +++ b/spec/vmpooler/api/v1/token_spec.rb @@ -0,0 +1,173 @@ +require 'spec_helper' +require 'rack/test' + +module Vmpooler + class API + module Helpers + def authenticate(auth, username_str, password_str) + username_str == 'admin' and password_str == 's3cr3t' + end + end + end +end + +describe Vmpooler::API::V1 do + include Rack::Test::Methods + + def app() + Vmpooler::API + end + + describe '/token' do + let(:prefix) { '/api/v1' } + let(:current_time) { Time.now } + let(:config) { { } } + + before do + app.settings.set :config, config + app.settings.set :redis, redis + end + + describe 'GET /token' do + context '(auth not configured)' do + let(:config) { { auth: false } } + + it 'returns a 404' do + get "#{prefix}/token" + expect_json(ok = false, http = 404) + end + end + + context '(auth configured)' do + let(:config) { { auth: true } } + + it 'returns a 401 if not authed' do + get "#{prefix}/token" + expect_json(ok = false, http = 401) + end + + it 'returns a list of tokens if authed' do + create_token "abc", "admin", current_time + + authorize 'admin', 's3cr3t' + get "#{prefix}/token" + expect_json(ok = true, http = 200) + + expect(JSON.parse(last_response.body)['abc']['created']).to eq(current_time.to_s) + end + end + end + + describe 'POST /token' do + context '(auth not configured)' do + let(:config) { { auth: false } } + + it 'returns a 404' do + post "#{prefix}/token" + expect_json(ok = false, http = 404) + end + end + + context '(auth configured)' do + let(:config) { { auth: true } } + + it 'returns a 401 if not authed' do + post "#{prefix}/token" + expect_json(ok = false, http = 401) + end + + it 'returns a newly created token if authed' do + authorize 'admin', 's3cr3t' + post "#{prefix}/token" + expect_json(ok = true, http = 200) + + returned_token = JSON.parse(last_response.body)['token'] + expect(returned_token.length).to be(32) + expect(get_token_data(returned_token)['user']).to eq("admin") + end + end + end + end + + describe '/token/:token' do + let(:prefix) { '/api/v1' } + let(:current_time) { Time.now } + + before do + app.settings.set :config, config + app.settings.set :redis, redis + end + + def create_vm_for_token(token, pool, vm) + redis.sadd("vmpooler__running__#{pool}", vm) + redis.hset("vmpooler__vm__#{vm}", "token:token", token) + end + + describe 'GET /token/:token' do + context '(auth not configured)' do + let(:config) { { auth: false } } + + it 'returns a 404' do + get "#{prefix}/token/this" + expect_json(ok = false, http = 404) + end + end + + context '(auth configured)' do + let(:config) { { + auth: true, + pools: [ + {'name' => 'pool1', 'size' => 5} + ] + } } + + it 'returns a token' do + create_token "mytoken", "admin", current_time + create_vm_for_token "mytoken", "pool1", "vmhostname" + + get "#{prefix}/token/mytoken" + expect_json(ok = true, http = 200) + + expect(JSON.parse(last_response.body)['ok']).to eq(true) + expect(JSON.parse(last_response.body)['mytoken']['user']).to eq('admin') + expect(JSON.parse(last_response.body)['mytoken']['vms']['running']).to include('vmhostname') + end + end + end + + describe 'DELETE /token/:token' do + context '(auth not configured)' do + let(:config) { { auth: false } } + + it 'returns a 404' do + delete "#{prefix}/token/this" + expect_json(ok = false, http = 404) + end + end + + context '(auth configured)' do + let(:config) { { auth: true } } + + it 'returns a 401 if not authed' do + delete "#{prefix}/token/this" + expect_json(ok = false, http = 401) + end + + it 'deletes a token if authed' do + create_token("mytoken", "admin", current_time) + authorize 'admin', 's3cr3t' + + delete "#{prefix}/token/mytoken" + expect_json(ok = true, http = 200) + end + + it 'fails if token does not exist' do + authorize 'admin', 's3cr3t' + + delete "#{prefix}/token/missingtoken" + expect_json(ok = false, http = 401) # TODO: should this be 404? + end + end + end + end +end diff --git a/spec/vmpooler/api/v1/vm_hostname_spec.rb b/spec/vmpooler/api/v1/vm_hostname_spec.rb new file mode 100644 index 0000000..f5dce5b --- /dev/null +++ b/spec/vmpooler/api/v1/vm_hostname_spec.rb @@ -0,0 +1,317 @@ +require 'spec_helper' +require 'rack/test' + +module Vmpooler + class API + module Helpers + def authenticate(auth, username_str, password_str) + username_str == 'admin' and password_str == 's3cr3t' + end + end + end +end + +def has_set_tag?(vm, tag, value) + value == redis.hget("vmpooler__vm__#{vm}", "tag:#{tag}") +end + +describe Vmpooler::API::V1 do + include Rack::Test::Methods + + def app() + Vmpooler::API + end + + describe '/vm/:hostname' do + let(:prefix) { '/api/v1' } + + let(:config) { + { + config: { + 'site_name' => 'test pooler', + 'vm_lifetime_auth' => 2, + }, + pools: [ + {'name' => 'pool1', 'size' => 5}, + {'name' => 'pool2', 'size' => 10} + ], + alias: { 'poolone' => 'pool1' }, + } + } + + let(:current_time) { Time.now } + + before(:each) do + redis.flushdb + + app.settings.set :config, config + app.settings.set :redis, redis + app.settings.set :config, auth: false + create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) + end + + describe 'PUT /vm/:hostname' do + it 'allows tags to be set' do + create_vm('testhost') + put "#{prefix}/vm/testhost", '{"tags":{"tested_by":"rspec"}}' + expect_json(ok = true, http = 200) + + expect has_set_tag?('testhost', 'tested_by', 'rspec') + end + + it 'skips empty tags' do + create_vm('testhost') + put "#{prefix}/vm/testhost", '{"tags":{"tested_by":""}}' + expect_json(ok = true, http = 200) + + expect !has_set_tag?('testhost', 'tested_by', '') + end + + it 'does not set tags if request body format is invalid' do + create_vm('testhost') + put "#{prefix}/vm/testhost", '{"tags":{"tested"}}' + expect_json(ok = false, http = 400) + + expect !has_set_tag?('testhost', 'tested', '') + end + + context '(allowed_tags configured)' do + it 'fails if specified tag is not in allowed_tags array' do + app.settings.set :config, + { :config => { 'allowed_tags' => ['created_by', 'project', 'url'] } } + + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"tags":{"created_by":"rspec","tested_by":"rspec"}}' + expect_json(ok = false, http = 400) + + expect !has_set_tag?('testhost', 'tested_by', 'rspec') + end + end + + context '(tagfilter configured)' do + let(:config) { { + tagfilter: { 'url' => '(.*)\/' }, + } } + + it 'correctly filters tags' do + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"tags":{"url":"foo.com/something.html"}}' + expect_json(ok = true, http = 200) + + expect has_set_tag?('testhost', 'url', 'foo.com') + end + + it "doesn't eat tags not matching filter" do + create_vm('testhost') + put "#{prefix}/vm/testhost", '{"tags":{"url":"foo.com"}}' + expect_json(ok = true, http = 200) + + expect has_set_tag?('testhost', 'url', 'foo.com') + end + end + + context '(auth not configured)' do + let(:config) { { auth: false } } + + it 'allows VM lifetime to be modified without a token' do + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"lifetime":"1"}' + expect_json(ok = true, http = 200) + + vm = fetch_vm('testhost') + expect(vm['lifetime'].to_i).to eq(1) + end + + it 'does not allow a lifetime to be 0' do + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"lifetime":"0"}' + expect_json(ok = false, http = 400) + + vm = fetch_vm('testhost') + expect(vm['lifetime']).to be_nil + end + end + + context '(auth configured)' do + before(:each) do + app.settings.set :config, auth: true + end + + it 'allows VM lifetime to be modified with a token' do + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"lifetime":"1"}', { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 200) + + vm = fetch_vm('testhost') + expect(vm['lifetime'].to_i).to eq(1) + end + + it 'does not allows VM lifetime to be modified without a token' do + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"lifetime":"1"}' + expect_json(ok = false, http = 401) + end + end + end + + describe 'DELETE /vm/:hostname' do + context '(auth not configured)' do + it 'does not delete a non-existant VM' do + delete "#{prefix}/vm/testhost" + expect_json(ok = false, http = 404) + end + + it 'deletes an existing VM' do + create_running_vm('pool1', 'testhost') + expect fetch_vm('testhost') + + delete "#{prefix}/vm/testhost" + expect_json(ok = true, http = 200) + expect !fetch_vm('testhost') + end + end + + context '(auth configured)' do + before(:each) do + app.settings.set :config, auth: true + end + + context '(checked-out without token)' do + it 'deletes a VM without supplying a token' do + create_running_vm('pool1', 'testhost') + expect fetch_vm('testhost') + + delete "#{prefix}/vm/testhost" + expect_json(ok = true, http = 200) + expect !fetch_vm('testhost') + end + end + + context '(checked-out with token)' do + it 'fails to delete a VM without supplying a token' do + create_running_vm('pool1', 'testhost', 'abcdefghijklmnopqrstuvwxyz012345') + expect fetch_vm('testhost') + + delete "#{prefix}/vm/testhost" + expect_json(ok = false, http = 401) + expect fetch_vm('testhost') + end + + it 'deletes a VM when token is supplied' do + create_running_vm('pool1', 'testhost', 'abcdefghijklmnopqrstuvwxyz012345') + expect fetch_vm('testhost') + + delete "#{prefix}/vm/testhost", "", { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 200) + + expect !fetch_vm('testhost') + end + end + end + end + + describe 'POST /vm/:hostname/snapshot' do + context '(auth not configured)' do + it 'creates a snapshot' do + create_vm('testhost') + post "#{prefix}/vm/testhost/snapshot" + expect_json(ok = true, http = 202) + expect(JSON.parse(last_response.body)['testhost']['snapshot'].length).to be(32) + end + end + + context '(auth configured)' do + before(:each) do + app.settings.set :config, auth: true + end + + it 'returns a 401 if not authed' do + post "#{prefix}/vm/testhost/snapshot" + expect_json(ok = false, http = 401) + expect !has_vm_snapshot?('testhost') + end + + it 'creates a snapshot if authed' do + create_vm('testhost') + snapshot_vm('testhost', 'testsnapshot') + + post "#{prefix}/vm/testhost/snapshot", "", { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 202) + expect(JSON.parse(last_response.body)['testhost']['snapshot'].length).to be(32) + expect has_vm_snapshot?('testhost') + end + end + end + + describe 'POST /vm/:hostname/snapshot/:snapshot' do + context '(auth not configured)' do + it 'reverts to a snapshot' do + create_vm('testhost') + snapshot_vm('testhost', 'testsnapshot') + + post "#{prefix}/vm/testhost/snapshot/testsnapshot" + expect_json(ok = true, http = 202) + expect vm_reverted_to_snapshot?('testhost', 'testsnapshot') + end + + it 'fails if the specified snapshot does not exist' do + create_vm('testhost') + + post "#{prefix}/vm/testhost/snapshot/testsnapshot", "", { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = false, http = 404) + expect !vm_reverted_to_snapshot?('testhost', 'testsnapshot') + end + end + + context '(auth configured)' do + before(:each) do + app.settings.set :config, auth: true + end + + it 'returns a 401 if not authed' do + create_vm('testhost') + snapshot_vm('testhost', 'testsnapshot') + + post "#{prefix}/vm/testhost/snapshot/testsnapshot" + expect_json(ok = false, http = 401) + expect !vm_reverted_to_snapshot?('testhost', 'testsnapshot') + end + + it 'fails if authed and the specified snapshot does not exist' do + create_vm('testhost') + + post "#{prefix}/vm/testhost/snapshot/testsnapshot", "", { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = false, http = 404) + expect !vm_reverted_to_snapshot?('testhost', 'testsnapshot') + end + + it 'reverts to a snapshot if authed' do + create_vm('testhost') + snapshot_vm('testhost', 'testsnapshot') + + post "#{prefix}/vm/testhost/snapshot/testsnapshot", "", { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 202) + expect vm_reverted_to_snapshot?('testhost', 'testsnapshot') + end + end + end + end +end diff --git a/spec/vmpooler/api/v1/vm_spec.rb b/spec/vmpooler/api/v1/vm_spec.rb new file mode 100644 index 0000000..6a4a068 --- /dev/null +++ b/spec/vmpooler/api/v1/vm_spec.rb @@ -0,0 +1,175 @@ +require 'spec_helper' +require 'rack/test' + +module Vmpooler + class API + module Helpers + def authenticate(auth, username_str, password_str) + username_str == 'admin' and password_str == 's3cr3t' + end + end + end +end + +describe Vmpooler::API::V1 do + include Rack::Test::Methods + + def app() + Vmpooler::API + end + + describe '/vm' do + let(:prefix) { '/api/v1' } + + let(:config) { + { + config: { + 'site_name' => 'test pooler', + 'vm_lifetime_auth' => 2, + }, + pools: [ + {'name' => 'pool1', 'size' => 5}, + {'name' => 'pool2', 'size' => 10} + ], + alias: { 'poolone' => 'pool1' }, + } + } + + let(:current_time) { Time.now } + + before(:each) do + redis.flushdb + + app.settings.set :config, config + app.settings.set :redis, redis + app.settings.set :config, auth: false + create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) + end + + describe 'POST /vm' do + it 'returns a single VM' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm", '{"pool1":"1"}' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'returns a single VM for an alias' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm", '{"poolone":"1"}' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'fails on nonexistant pools' do + post "#{prefix}/vm", '{"poolpoolpool":"1"}' + expect_json(ok = false, http = 404) + end + + it 'returns multiple VMs' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool2', 'qrstuvwxyz012345' + + post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + }, + pool2: { + hostname: 'qrstuvwxyz012345' + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + context '(auth not configured)' do + it 'does not extend VM lifetime if auth token is provided' do + app.settings.set :config, auth: false + + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm", '{"pool1":"1"}', { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + + vm = fetch_vm('abcdefghijklmnop') + expect(vm['lifetime']).to be_nil + end + end + + context '(auth configured)' do + it 'extends VM lifetime if auth token is provided' do + app.settings.set :config, auth: true + + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm", '{"pool1":"1"}', { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + + vm = fetch_vm('abcdefghijklmnop') + expect(vm['lifetime'].to_i).to eq(2) + end + + it 'does not extend VM lifetime if auth token is not provided' do + app.settings.set :config, auth: true + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm", '{"pool1":"1"}' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + + vm = fetch_vm('abcdefghijklmnop') + expect(vm['lifetime']).to be_nil + end + end + end + end +end diff --git a/spec/vmpooler/api/v1/vm_template_spec.rb b/spec/vmpooler/api/v1/vm_template_spec.rb new file mode 100644 index 0000000..28b85c3 --- /dev/null +++ b/spec/vmpooler/api/v1/vm_template_spec.rb @@ -0,0 +1,176 @@ +require 'spec_helper' +require 'rack/test' + +module Vmpooler + class API + module Helpers + def authenticate(auth, username_str, password_str) + username_str == 'admin' and password_str == 's3cr3t' + end + end + end +end + +describe Vmpooler::API::V1 do + include Rack::Test::Methods + + def app() + Vmpooler::API + end + + describe '/vm/:template' do + let(:prefix) { '/api/v1' } + + let(:config) { + { + config: { + 'site_name' => 'test pooler', + 'vm_lifetime_auth' => 2, + }, + pools: [ + {'name' => 'pool1', 'size' => 5}, + {'name' => 'pool2', 'size' => 10} + ], + alias: { 'poolone' => 'pool1' }, + } + } + + let(:current_time) { Time.now } + + before(:each) do + redis.flushdb + + app.settings.set :config, config + app.settings.set :redis, redis + app.settings.set :config, auth: false + create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) + end + + describe 'POST /vm/:template' do + it 'returns a single VM' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm/pool1", '' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'returns a single VM for an alias' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm/poolone", '' + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + expect_json(ok = true, http = 200) + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'fails on nonexistant pools' do + post "#{prefix}/vm/poolpoolpool", '' + expect_json(ok = false, http = 404) + end + + it 'returns multiple VMs' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool2', 'qrstuvwxyz012345' + + post "#{prefix}/vm/pool1+pool2", '' + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + }, + pool2: { + hostname: 'qrstuvwxyz012345' + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + context '(auth not configured)' do + it 'does not extend VM lifetime if auth token is provided' do + app.settings.set :config, auth: false + + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm/pool1", '', { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + vm = fetch_vm('abcdefghijklmnop') + expect(vm['lifetime']).to be_nil + end + end + + context '(auth configured)' do + it 'extends VM lifetime if auth token is provided' do + app.settings.set :config, auth: true + + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm/pool1", '', { + 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' + } + expect_json(ok = true, http = 200) + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + + vm = fetch_vm('abcdefghijklmnop') + expect(vm['lifetime'].to_i).to eq(2) + end + + it 'does not extend VM lifetime if auth token is not provided' do + app.settings.set :config, auth: true + create_ready_vm 'pool1', 'abcdefghijklmnop' + + post "#{prefix}/vm/pool1", '' + + expected = { + ok: true, + pool1: { + hostname: 'abcdefghijklmnop' + } + } + expect_json(ok = true, http = 200) + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + + vm = fetch_vm('abcdefghijklmnop') + expect(vm['lifetime']).to be_nil + end + end + end + end +end diff --git a/spec/vmpooler/api/v1_spec.rb b/spec/vmpooler/api/v1_spec.rb deleted file mode 100644 index 1b65962..0000000 --- a/spec/vmpooler/api/v1_spec.rb +++ /dev/null @@ -1,735 +0,0 @@ -require 'spec_helper' -require 'rack/test' - -module Vmpooler - class API - module Helpers - def authenticate(auth, username_str, password_str) - username_str == 'admin' and password_str == 's3cr3t' - end - end - end -end - -describe Vmpooler::API::V1 do - include Rack::Test::Methods - - def app() - Vmpooler::API - end - - describe '/token' do - let(:redis) { double('redis') } - let(:prefix) { '/api/v1' } - - before do - app.settings.set :config, config - app.settings.set :redis, redis - end - - describe 'GET /token' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'returns a 404' do - get "#{prefix}/token" - - expect_json(ok = false, http = 404) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - it 'returns a 401 if not authed' do - get "#{prefix}/token" - - expect_json(ok = false, http = 401) - end - - it 'returns a list of tokens if authed' do - expect(redis).to receive(:keys).with('vmpooler__token__*').and_return(["vmpooler__token__abc"]) - expect(redis).to receive(:hgetall).with('vmpooler__token__abc').and_return({"user" => "admin", "created" => "now"}) - - authorize 'admin', 's3cr3t' - - get "#{prefix}/token" - - expect(JSON.parse(last_response.body)['abc']['created']).to eq('now') - - expect_json(ok = true, http = 200) - end - end - end - - describe 'POST /token' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'returns a 404' do - post "#{prefix}/token" - - expect_json(ok = false, http = 404) - end - end - - context '(auth configured)' do - before do - allow(redis).to receive(:hset).and_return '1' - end - - let(:config) { { auth: true } } - - it 'returns a 401 if not authed' do - post "#{prefix}/token" - - expect_json(ok = false, http = 401) - end - - it 'returns a token if authed' do - authorize 'admin', 's3cr3t' - - post "#{prefix}/token" - - expect(JSON.parse(last_response.body)['token'].length).to be(32) - - expect_json(ok = true, http = 200) - end - end - end - end - - describe '/token/:token' do - let(:redis) { double('redis') } - let(:prefix) { '/api/v1' } - - before do - app.settings.set :config, config - app.settings.set :redis, redis - end - - describe 'GET /token/:token' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'returns a 404' do - get "#{prefix}/token/this" - - expect_json(ok = false, http = 404) - end - end - - context '(auth configured)' do - let(:config) { { - auth: true, - pools: [ - {'name' => 'pool1', 'size' => 5} - ] - } } - - it 'returns a token' do - expect(redis).to receive(:hgetall).with('vmpooler__token__this').and_return({'user' => 'admin'}) - expect(redis).to receive(:smembers).with('vmpooler__running__pool1').and_return(['vmhostname']) - expect(redis).to receive(:hget).with('vmpooler__vm__vmhostname', 'token:token').and_return('this') - - get "#{prefix}/token/this" - - expect(JSON.parse(last_response.body)['ok']).to eq(true) - expect(JSON.parse(last_response.body)['this']['user']).to eq('admin') - expect(JSON.parse(last_response.body)['this']['vms']['running']).to include('vmhostname') - - expect_json(ok = true, http = 200) - end - end - end - - describe 'DELETE /token/:token' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'returns a 404' do - delete "#{prefix}/token/this" - - expect_json(ok = false, http = 404) - end - end - - context '(auth configured)' do - before do - allow(redis).to receive(:del).and_return '1' - end - - let(:config) { { auth: true } } - - it 'returns a 401 if not authed' do - delete "#{prefix}/token/this" - - expect_json(ok = false, http = 401) - end - - it 'deletes a token if authed' do - authorize 'admin', 's3cr3t' - - delete "#{prefix}/token/this" - - expect_json(ok = true, http = 200) - end - end - end - end - - describe '/vm' do - let(:redis) { double('redis') } - let(:prefix) { '/api/v1' } - let(:config) { { - config: { - 'site_name' => 'test pooler', - 'vm_lifetime_auth' => 2 - }, - pools: [ - {'name' => 'pool1', 'size' => 5}, - {'name' => 'pool2', 'size' => 10} - ], - alias: { 'poolone' => 'pool1' } - } } - - before do - app.settings.set :config, config - app.settings.set :redis, redis - - allow(redis).to receive(:exists).and_return '1' - allow(redis).to receive(:hget).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345', 'user').and_return 'jdoe' - allow(redis).to receive(:hset).and_return '1' - allow(redis).to receive(:sadd).and_return '1' - allow(redis).to receive(:scard).and_return '5' - allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' - allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return 'qrstuvwxyz012345' - end - - describe 'POST /vm' do - it 'returns a single VM' do - post "#{prefix}/vm", '{"pool1":"1"}' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - it 'returns a single VM for an alias' do - expect(redis).to receive(:exists).with("vmpooler__ready__poolone").and_return(false) - - post "#{prefix}/vm", '{"poolone":"1"}' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - it 'fails on nonexistant pools' do - expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false) - - post "#{prefix}/vm", '{"poolpoolpool":"1"}' - - expect_json(ok = false, http = 404) - end - - it 'returns multiple VMs' do - post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - }, - pool2: { - hostname: 'qrstuvwxyz012345' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'does not extend VM lifetime if auth token is provided' do - expect(redis).not_to receive(:hset).with("vmpooler__vm__abcdefghijklmnop", "lifetime", 2) - - post "#{prefix}/vm", '{"pool1":"1"}', { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - it 'extends VM lifetime if auth token is provided' do - expect(redis).to receive(:hset).with("vmpooler__vm__abcdefghijklmnop", "lifetime", 2).once - - post "#{prefix}/vm", '{"pool1":"1"}', { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - it 'does not extend VM lifetime if auth token is not provided' do - expect(redis).not_to receive(:hset).with("vmpooler__vm__abcdefghijklmnop", "lifetime", 2) - - post "#{prefix}/vm", '{"pool1":"1"}' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - end - end - end - - describe '/vm/:template' do - let(:redis) { double('redis') } - let(:prefix) { '/api/v1' } - let(:config) { { - config: { - 'site_name' => 'test pooler', - 'vm_lifetime_auth' => 2 - }, - pools: [ - {'name' => 'pool1', 'size' => 5}, - {'name' => 'pool2', 'size' => 10} - ], - alias: { 'poolone' => 'pool1' } - } } - - before do - app.settings.set :config, config - app.settings.set :redis, redis - - allow(redis).to receive(:exists).and_return '1' - allow(redis).to receive(:hget).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345', 'user').and_return 'jdoe' - allow(redis).to receive(:hset).and_return '1' - allow(redis).to receive(:sadd).and_return '1' - allow(redis).to receive(:scard).and_return '5' - allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' - allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return 'qrstuvwxyz012345' - end - - describe 'POST /vm/:template' do - it 'returns a single VM' do - post "#{prefix}/vm/pool1", '' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - it 'returns a single VM for an alias' do - expect(redis).to receive(:exists).with("vmpooler__ready__poolone").and_return(false) - - post "#{prefix}/vm/poolone", '' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - it 'fails on nonexistant pools' do - expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false) - - post "#{prefix}/vm/poolpoolpool", '' - - expect_json(ok = false, http = 404) - end - - it 'returns multiple VMs' do - post "#{prefix}/vm/pool1+pool2", '' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - }, - pool2: { - hostname: 'qrstuvwxyz012345' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'does not extend VM lifetime if auth token is provided' do - expect(redis).not_to receive(:hset).with("vmpooler__vm__abcdefghijklmnop", "lifetime", 2) - - post "#{prefix}/vm/pool1", '', { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - it 'extends VM lifetime if auth token is provided' do - expect(redis).to receive(:hset).with("vmpooler__vm__abcdefghijklmnop", "lifetime", 2).once - - post "#{prefix}/vm/pool1", '', { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - - it 'does not extend VM lifetime if auth token is not provided' do - expect(redis).not_to receive(:hset).with("vmpooler__vm__abcdefghijklmnop", "lifetime", 2) - - post "#{prefix}/vm/pool1", '' - - expected = { - ok: true, - pool1: { - hostname: 'abcdefghijklmnop' - } - } - - expect(last_response.body).to eq(JSON.pretty_generate(expected)) - - expect_json(ok = true, http = 200) - end - end - end - end - - describe '/vm/:hostname' do - let(:redis) { double('redis') } - let(:prefix) { '/api/v1' } - let(:config) { { - pools: [ - {'name' => 'pool1', 'size' => 5}, - {'name' => 'pool2', 'size' => 10} - ] - } } - - before do - app.settings.set :config, config - app.settings.set :redis, redis - - allow(redis).to receive(:exists).and_return '1' - allow(redis).to receive(:hset).and_return '1' - end - - describe 'PUT /vm/:hostname' do - it 'allows tags to be set' do - put "#{prefix}/vm/testhost", '{"tags":{"tested_by":"rspec"}}' - - expect_json(ok = true, http = 200) - end - - it 'skips empty tags' do - put "#{prefix}/vm/testhost", '{"tags":{"tested_by":""}}' - - expect_json(ok = true, http = 200) - end - - it 'does not set tags if request body format is invalid' do - put "#{prefix}/vm/testhost", '{"tags":{"tested"}}' - - expect_json(ok = false, http = 400) - end - - context '(allowed_tags configured)' do - let(:config) { { - config: { - 'allowed_tags' => ['created_by', 'project', 'url'] - } - } } - - it 'fails if specified tag is not in allowed_tags array' do - put "#{prefix}/vm/testhost", '{"tags":{"created_by":"rspec","tested_by":"rspec"}}' - - expect_json(ok = false, http = 400) - end - end - - context '(tagfilter configured)' do - let(:config) { { - tagfilter: { 'url' => '(.*)\/' }, - } } - - it 'correctly filters tags' do - expect(redis).to receive(:hset).with("vmpooler__vm__testhost", "tag:url", "foo.com") - - put "#{prefix}/vm/testhost", '{"tags":{"url":"foo.com/something.html"}}' - - expect_json(ok = true, http = 200) - end - - it 'doesn\'t eat tags not matching filter' do - expect(redis).to receive(:hset).with("vmpooler__vm__testhost", "tag:url", "foo.com") - - put "#{prefix}/vm/testhost", '{"tags":{"url":"foo.com"}}' - - expect_json(ok = true, http = 200) - end - end - - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'allows VM lifetime to be modified without a token' do - put "#{prefix}/vm/testhost", '{"lifetime":"1"}' - - expect_json(ok = true, http = 200) - end - - it 'does not allow a lifetime to be 0' do - put "#{prefix}/vm/testhost", '{"lifetime":"0"}' - - expect_json(ok = false, http = 400) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - it 'allows VM lifetime to be modified with a token' do - put "#{prefix}/vm/testhost", '{"lifetime":"1"}', { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expect_json(ok = true, http = 200) - end - - it 'does not allows VM lifetime to be modified without a token' do - put "#{prefix}/vm/testhost", '{"lifetime":"1"}' - - expect_json(ok = false, http = 401) - end - end - end - - describe 'DELETE /vm/:hostname' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'does not delete a non-existant VM' do - expect(redis).to receive(:hgetall).and_return({}) - expect(redis).not_to receive(:sadd) - expect(redis).not_to receive(:srem) - - delete "#{prefix}/vm/testhost" - - expect_json(ok = false, http = 404) - end - - it 'deletes an existing VM' do - 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) - - delete "#{prefix}/vm/testhost" - - expect_json(ok = true, http = 200) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - context '(checked-out without token)' do - it 'deletes a VM without supplying a token' do - 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) - - delete "#{prefix}/vm/testhost" - - expect_json(ok = true, http = 200) - end - end - - context '(checked-out with token)' do - it 'fails to delete a VM without supplying a token' do - 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) - - delete "#{prefix}/vm/testhost" - - expect_json(ok = false, http = 401) - end - - it 'deletes a VM when token is supplied' do - expect(redis).to receive(:hgetall).with('vmpooler__vm__testhost').and_return({"template" => "pool1", "token:token" => "abcdefghijklmnopqrstuvwxyz012345"}) - expect(redis).to receive(:srem).and_return(true) - expect(redis).to receive(:sadd) - - delete "#{prefix}/vm/testhost", "", { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expect_json(ok = true, http = 200) - end - end - end - end - - describe 'POST /vm/:hostname/snapshot' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'creates a snapshot' do - expect(redis).to receive(:sadd) - - post "#{prefix}/vm/testhost/snapshot" - - expect(JSON.parse(last_response.body)['testhost']['snapshot'].length).to be(32) - - expect_json(ok = true, http = 202) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - it 'returns a 401 if not authed' do - post "#{prefix}/vm/testhost/snapshot" - - expect_json(ok = false, http = 401) - end - - it 'creates a snapshot if authed' do - expect(redis).to receive(:sadd) - - post "#{prefix}/vm/testhost/snapshot", "", { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expect(JSON.parse(last_response.body)['testhost']['snapshot'].length).to be(32) - - expect_json(ok = true, http = 202) - end - end - end - - describe 'POST /vm/:hostname/snapshot/:snapshot' do - context '(auth not configured)' do - let(:config) { { auth: false } } - - it 'reverts to a snapshot' do - expect(redis).to receive(:hget).with('vmpooler__vm__testhost', 'snapshot:testsnapshot').and_return(1) - expect(redis).to receive(:sadd) - - post "#{prefix}/vm/testhost/snapshot/testsnapshot" - - expect_json(ok = true, http = 202) - end - end - - context '(auth configured)' do - let(:config) { { auth: true } } - - it 'returns a 401 if not authed' do - post "#{prefix}/vm/testhost/snapshot" - - expect_json(ok = false, http = 401) - end - - it 'reverts to a snapshot if authed' do - expect(redis).to receive(:hget).with('vmpooler__vm__testhost', 'snapshot:testsnapshot').and_return(1) - expect(redis).to receive(:sadd) - - post "#{prefix}/vm/testhost/snapshot/testsnapshot", "", { - 'HTTP_X_AUTH_TOKEN' => 'abcdefghijklmnopqrstuvwxyz012345' - } - - expect_json(ok = true, http = 202) - end - end - - end - end - -end diff --git a/spec/vmpooler/api_spec.rb b/spec/vmpooler/dashboard_spec.rb similarity index 76% rename from spec/vmpooler/api_spec.rb rename to spec/vmpooler/dashboard_spec.rb index 246503f..06d2a86 100644 --- a/spec/vmpooler/api_spec.rb +++ b/spec/vmpooler/dashboard_spec.rb @@ -10,6 +10,10 @@ describe Vmpooler::API do describe 'Dashboard' do + before(:each) do + redis.flushdb + end + context '/' do before { get '/' } @@ -38,7 +42,6 @@ describe Vmpooler::API do it { expect(last_response.status).to eq(404) } it { expect(last_response.header['Content-Type']).to eq('application/json') } it { expect(last_response.body).to eq(JSON.pretty_generate({ok: false})) } - end describe '/dashboard/stats/vmpooler/pool' do @@ -49,7 +52,6 @@ describe Vmpooler::API do ], graphite: {} } } - let(:redis) { double('redis') } before do $config = config @@ -59,13 +61,12 @@ describe Vmpooler::API do end context 'without history param' do - it 'returns basic JSON' do - allow(redis).to receive(:scard) - allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(3) - allow(redis).to receive(:scard).with('vmpooler__ready__pool2').and_return(2) - - expect(redis).to receive(:scard).twice + create_ready_vm('pool1', 'vm1') + create_ready_vm('pool1', 'vm2') + create_ready_vm('pool1', 'vm3') + create_ready_vm('pool2', 'vm4') + create_ready_vm('pool2', 'vm5') get '/dashboard/stats/vmpooler/pool' @@ -78,19 +79,15 @@ describe Vmpooler::API do expect(last_response.body).to eq(JSON.pretty_generate(json_hash)) expect(last_response.header['Content-Type']).to eq('application/json') end - end context 'with history param' do - it 'returns JSON with null history when redis does not has values' do - allow(redis).to receive(:scard) - expect(redis).to receive(:scard).exactly(4).times - + it 'returns JSON with zeroed history when redis does not have values' do get '/dashboard/stats/vmpooler/pool', :history => true json_hash = { - pool1: {size: 5, ready: nil, history: [nil]}, - pool2: {size: 1, ready: nil, history: [nil]} + pool1: {size: 5, ready: 0, history: [0]}, + pool2: {size: 1, ready: 0, history: [0]} } expect(last_response).to be_ok @@ -99,10 +96,11 @@ describe Vmpooler::API do end it 'returns JSON with history when redis has values' do - allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(3) - allow(redis).to receive(:scard).with('vmpooler__ready__pool2').and_return(2) - - expect(redis).to receive(:scard).exactly(4).times + create_ready_vm('pool1', 'vm1') + create_ready_vm('pool1', 'vm2') + create_ready_vm('pool1', 'vm3') + create_ready_vm('pool2', 'vm4') + create_ready_vm('pool2', 'vm5') get '/dashboard/stats/vmpooler/pool', :history => true @@ -115,9 +113,7 @@ describe Vmpooler::API do expect(last_response.body).to eq(JSON.pretty_generate(json_hash)) expect(last_response.header['Content-Type']).to eq('application/json') end - end - end describe '/dashboard/stats/vmpooler/running' do @@ -129,7 +125,6 @@ describe Vmpooler::API do ], graphite: {} } } - let(:redis) { double('redis') } before do $config = config @@ -141,10 +136,6 @@ describe Vmpooler::API do context 'without history param' do it 'returns basic JSON' do - allow(redis).to receive(:scard) - - expect(redis).to receive(:scard).exactly(3).times - get '/dashboard/stats/vmpooler/running' json_hash = {pool: {running: 0}, diffpool: {running: 0}} @@ -155,9 +146,18 @@ describe Vmpooler::API do end it 'adds major correctly' do - allow(redis).to receive(:scard).with('vmpooler__running__pool-1').and_return(3) - allow(redis).to receive(:scard).with('vmpooler__running__pool-2').and_return(5) - allow(redis).to receive(:scard).with('vmpooler__running__diffpool-1').and_return(2) + create_running_vm('pool-1', 'vm1') + create_running_vm('pool-1', 'vm2') + create_running_vm('pool-1', 'vm3') + + create_running_vm('pool-2', 'vm4') + create_running_vm('pool-2', 'vm5') + create_running_vm('pool-2', 'vm6') + create_running_vm('pool-2', 'vm7') + create_running_vm('pool-2', 'vm8') + + create_running_vm('diffpool-1', 'vm9') + create_running_vm('diffpool-1', 'vm10') get '/dashboard/stats/vmpooler/running' @@ -167,10 +167,7 @@ describe Vmpooler::API do expect(last_response.body).to eq(JSON.pretty_generate(json_hash)) expect(last_response.header['Content-Type']).to eq('application/json') end - end end - end - end From 28a664420919a2bdd796116cd5966e5bc5f4535d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 8 Jun 2016 14:18:09 -0700 Subject: [PATCH 2/8] Add info about vmfloaty --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 7cdaed6..febf4bf 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,8 @@ A dashboard is provided to offer real-time statistics and historical graphs. It ## Command-line Utility -The [vmpooler_client.py](https://github.com/puppetlabs/vmpooler-client) CLI utility provides easy access to the vmpooler service. The tool is cross-platform and written in Python. +- The [vmpooler_client.py](https://github.com/puppetlabs/vmpooler-client) CLI utility provides easy access to the vmpooler service. The tool is cross-platform and written in Python. +- [vmfloaty](https://github.com/briancain/vmfloaty) is a ruby based CLI tool and scripting library written in ruby. ## Build status From 4e2a1fb62cf612e48a37bfe16217e9847c710461 Mon Sep 17 00:00:00 2001 From: FOXX Date: Tue, 28 Jun 2016 17:29:06 -0500 Subject: [PATCH 3/8] Added IP lookup functionality for /vm/hostname (#154) --- lib/vmpooler/api/v1.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 1046593..ccd8c9a 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -459,6 +459,15 @@ module Vmpooler result[params[:hostname]]['disk'] = rdata['disk'].split(':') end + # Look up IP address of the hostname + begin + ipAddress = TCPSocket.gethostbyname(params[:hostname])[3] + rescue + ipAddress = "" + end + + result[params[:hostname]]['ip'] = ipAddress + if config['domain'] result[params[:hostname]]['domain'] = config['domain'] end From 0fd6fff934f554706cabecf2354cee0457a03173 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 8 Jul 2016 11:48:48 -0700 Subject: [PATCH 4/8] Revert "Merge pull request #155 from shermdog/RE-7014-cinext" This reverts commit cc03a86f6a8e08ef2d8e10247b4b1850c39f77a5, reversing changes made to 5aaab7c5c2e433959578530060a5c529bda60ebf. --- Gemfile | 2 - lib/vmpooler.rb | 16 ------ lib/vmpooler/api.rb | 3 +- lib/vmpooler/api/v1.rb | 47 +++-------------- lib/vmpooler/pool_manager.rb | 17 ++---- lib/vmpooler/statsd.rb | 9 ---- spec/spec_helper.rb | 4 -- spec/vmpooler/api/v1_spec.rb | 51 +++--------------- spec/vmpooler/pool_manager_spec.rb | 83 +----------------------------- vmpooler | 14 +---- vmpooler.yaml.example | 31 +---------- 11 files changed, 26 insertions(+), 251 deletions(-) delete mode 100644 lib/vmpooler/statsd.rb diff --git a/Gemfile b/Gemfile index c6cecf1..c71be35 100644 --- a/Gemfile +++ b/Gemfile @@ -7,12 +7,10 @@ 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 -gem 'statsd-ruby', '>= 1.3.0' # Test deps group :test do gem 'rack-test', '>= 0.6' gem 'rspec', '>= 3.2' - gem 'simplecov', '>= 0.11.2' gem 'yarjuf', '>= 2.0' end diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 02c8e9f..70bb819 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -7,7 +7,6 @@ module Vmpooler require 'rbvmomi' require 'redis' require 'sinatra/base' - require "statsd-ruby" require 'time' require 'timeout' require 'yaml' @@ -53,13 +52,6 @@ module Vmpooler parsed_config[:graphite]['prefix'] ||= 'vmpooler' end - # statsd is an addition and my not be present in YAML configuration - if parsed_config[:statsd] - if parsed_config[:statsd]['server'] - parsed_config[:statsd]['prefix'] ||= 'vmpooler' - end - end - if parsed_config[:tagfilter] parsed_config[:tagfilter].keys.each do |tag| parsed_config[:tagfilter][tag] = Regexp.new(parsed_config[:tagfilter][tag]) @@ -87,14 +79,6 @@ module Vmpooler end end - def self.new_statsd(server, port) - if server.nil? || server.empty? - nil - else - Statsd.new server, port - end - end - def self.pools(conf) conf[:pools] end diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index 45a9bfa..ee51cbc 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -42,10 +42,9 @@ module Vmpooler use Vmpooler::API::Reroute use Vmpooler::API::V1 - def configure(config, redis, statsd, environment = :production) + def configure(config, redis, environment = :production) self.settings.set :config, config self.settings.set :redis, redis - self.settings.set :statsd, statsd self.settings.set :environment, environment end diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index b762151..140bfb8 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -12,16 +12,6 @@ module Vmpooler Vmpooler::API.settings.redis end - def statsd - Vmpooler::API.settings.statsd - end - - def statsd_prefix - if Vmpooler::API.settings.statsd - Vmpooler::API.settings.config[:statsd]['prefix'] ? Vmpooler::API.settings.config[:statsd]['prefix'] : 'vmpooler' - end - end - def config Vmpooler::API.settings.config[:config] end @@ -42,16 +32,13 @@ module Vmpooler newhash = {} hash.each do |key, val| - if Vmpooler::API.settings.config[:alias][key] - key = Vmpooler::API.settings.config[:alias][key] - end - if backend.exists('vmpooler__ready__' + key) newhash[key] = val - elsif backend.exists('vmpooler__empty__' + key) - newhash['empty'] = (newhash['empty'] || 0) + val.to_i else - newhash['invalid'] = (newhash['invalid'] || 0) + val.to_i + if Vmpooler::API.settings.config[:alias][key] + newkey = Vmpooler::API.settings.config[:alias][key] + newhash[newkey] = val + end end end @@ -107,10 +94,8 @@ module Vmpooler vm = fetch_single_vm(template) if !vm failed = true - statsd.increment(statsd_prefix + '.checkout.fail.' + template, 1) break else - statsd.increment(statsd_prefix + '.checkout.success.' + template, 1) vms << [ template, vm ] end end @@ -390,16 +375,8 @@ module Vmpooler content_type :json result = { 'ok' => false } - if jdata - empty = jdata.delete('empty') - invalid = jdata.delete('invalid') - statsd.increment(statsd_prefix + '.checkout.empty', empty) if empty - statsd.increment(statsd_prefix + '.checkout.invalid', invalid) if invalid - unless jdata.empty? - result = atomically_allocate_vms(jdata) - else - status 404 - end + if jdata and !jdata.empty? + result = atomically_allocate_vms(jdata) else status 404 end @@ -423,16 +400,8 @@ module Vmpooler content_type :json result = { 'ok' => false } - if payload - empty = payload.delete('empty') - invalid = payload.delete('invalid') - statsd.increment(statsd_prefix + '.checkout.empty', empty) if empty - statsd.increment(statsd_prefix + '.checkout.invalid', invalid) if invalid - unless payload.empty? - result = atomically_allocate_vms(payload) - else - status 404 - end + if payload and !payload.empty? + result = atomically_allocate_vms(payload) else status 404 end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 5f54169..1a0454c 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,15 +1,12 @@ module Vmpooler class PoolManager - def initialize(config, logger, redis, graphite = nil, statsd = nil) + def initialize(config, logger, redis, graphite=nil) $config = config # Load logger library $logger = logger - # statsd and graphite are mutex in the context of vmpooler - if statsd - $statsd = statsd - elsif graphite + unless graphite.nil? $graphite = graphite end @@ -261,8 +258,7 @@ module Vmpooler $redis.decr('vmpooler__tasks__clone') begin - $statsd.timing($config[:statsd]['prefix'] + ".clone.#{vm['template']}", finish) if $statsd - $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if $graphite + $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if defined? $graphite rescue end end @@ -298,7 +294,7 @@ module Vmpooler $logger.log('s', "[-] [#{pool}] '#{vm}' destroyed in #{finish} seconds") - $graphite.log($config[:graphite]['prefix'] + ".destroy.#{pool}", finish) if $graphite + $graphite.log($config[:graphite]['prefix'] + ".destroy.#{pool}", finish) if defined? $graphite end end end @@ -569,10 +565,7 @@ module Vmpooler total = $redis.scard('vmpooler__pending__' + pool['name']) + ready begin - if $statsd - $statsd.gauge($config[:statsd]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name'])) - $statsd.gauge($config[:statsd]['prefix'] + '.running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name'])) - elsif $graphite + if defined? $graphite $graphite.log($config[:graphite]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name'])) $graphite.log($config[:graphite]['prefix'] + '.running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name'])) end diff --git a/lib/vmpooler/statsd.rb b/lib/vmpooler/statsd.rb deleted file mode 100644 index b449a59..0000000 --- a/lib/vmpooler/statsd.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'rubygems' unless defined?(Gem) - -module Vmpooler - class Statsd - def initialize(server = 'statsd', port = 8125) - @server = Statsd.new(server, port) - end - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f4fbf55..2186bdb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,3 @@ -require 'simplecov' -SimpleCov.start do - add_filter '/spec/' -end require 'helpers' require 'rbvmomi' require 'rspec' diff --git a/spec/vmpooler/api/v1_spec.rb b/spec/vmpooler/api/v1_spec.rb index a269f60..fac75de 100644 --- a/spec/vmpooler/api/v1_spec.rb +++ b/spec/vmpooler/api/v1_spec.rb @@ -180,7 +180,6 @@ describe Vmpooler::API::V1 do describe '/vm' do let(:redis) { double('redis') } - let(:statsd) { double('stats') } let(:prefix) { '/api/v1' } let(:config) { { config: { @@ -191,27 +190,20 @@ describe Vmpooler::API::V1 do {'name' => 'pool1', 'size' => 5}, {'name' => 'pool2', 'size' => 10} ], - alias: { 'poolone' => 'pool1' }, - statsd: { 'prefix' => 'vmpooler' } + alias: { 'poolone' => 'pool1' } } } before do app.settings.set :config, config app.settings.set :redis, redis - app.settings.set :statsd, statsd - allow(redis).to receive(:exists).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345').and_return '1' - allow(redis).to receive(:exists).with('vmpooler__ready__pool1').and_return '1' - allow(redis).to receive(:exists).with('vmpooler__ready__pool2').and_return '1' + allow(redis).to receive(:exists).and_return '1' allow(redis).to receive(:hget).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345', 'user').and_return 'jdoe' allow(redis).to receive(:hset).and_return '1' allow(redis).to receive(:sadd).and_return '1' allow(redis).to receive(:scard).and_return '5' allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return 'qrstuvwxyz012345' - allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool1', 1) - allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool2', 1) - allow(statsd).to receive(:increment).with('vmpooler.checkout.fail.pool2', 1) end describe 'POST /vm' do @@ -231,7 +223,7 @@ describe Vmpooler::API::V1 do end it 'returns a single VM for an alias' do - expect(redis).to receive(:exists).with("vmpooler__ready__pool1").and_return(1) + expect(redis).to receive(:exists).with("vmpooler__ready__poolone").and_return(false) post "#{prefix}/vm", '{"poolone":"1"}' @@ -249,22 +241,12 @@ describe Vmpooler::API::V1 do it 'fails on nonexistent pools' do expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false) - expect(redis).to receive(:exists).with("vmpooler__empty__poolpoolpool").and_return(false) - expect(statsd).to receive(:increment).with('vmpooler.checkout.invalid', 1) + post "#{prefix}/vm", '{"poolpoolpool":"1"}' expect_json(ok = false, http = 404) end - it 'fails on empty pools' do - expect(redis).to receive(:exists).with("vmpooler__ready__emptypool").and_return(false) - expect(redis).to receive(:exists).with("vmpooler__empty__emptypool").and_return(true) - expect(statsd).to receive(:increment).with('vmpooler.checkout.empty', 1) - post "#{prefix}/vm", '{"emptypool":"1"}' - - expect_json(ok = false, http = 404) - end - it 'returns multiple VMs' do post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' @@ -464,7 +446,6 @@ describe Vmpooler::API::V1 do describe '/vm/:template' do let(:redis) { double('redis') } - let(:statsd) { double('stats') } let(:prefix) { '/api/v1' } let(:config) { { config: { @@ -475,27 +456,20 @@ describe Vmpooler::API::V1 do {'name' => 'pool1', 'size' => 5}, {'name' => 'pool2', 'size' => 10} ], - alias: { 'poolone' => 'pool1' }, - statsd: { 'prefix' => 'vmpooler' } + alias: { 'poolone' => 'pool1' } } } before do app.settings.set :config, config app.settings.set :redis, redis - app.settings.set :statsd, statsd - allow(redis).to receive(:exists).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345').and_return '1' - allow(redis).to receive(:exists).with('vmpooler__ready__pool1').and_return '1' - allow(redis).to receive(:exists).with('vmpooler__ready__pool2').and_return '1' + allow(redis).to receive(:exists).and_return '1' allow(redis).to receive(:hget).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345', 'user').and_return 'jdoe' allow(redis).to receive(:hset).and_return '1' allow(redis).to receive(:sadd).and_return '1' allow(redis).to receive(:scard).and_return '5' allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return 'qrstuvwxyz012345' - allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool1', 1) - allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool2', 1) - allow(statsd).to receive(:increment).with('vmpooler.checkout.fail.pool2', 1) end describe 'POST /vm/:template' do @@ -515,7 +489,7 @@ describe Vmpooler::API::V1 do end it 'returns a single VM for an alias' do - expect(redis).to receive(:exists).with("vmpooler__ready__pool1").and_return(1) + expect(redis).to receive(:exists).with("vmpooler__ready__poolone").and_return(false) post "#{prefix}/vm/poolone", '' @@ -533,23 +507,12 @@ describe Vmpooler::API::V1 do it 'fails on nonexistent pools' do expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false) - expect(redis).to receive(:exists).with("vmpooler__empty__poolpoolpool").and_return(false) - expect(statsd).to receive(:increment).with('vmpooler.checkout.invalid', 1) post "#{prefix}/vm/poolpoolpool", '' expect_json(ok = false, http = 404) end - it 'fails on empty pools' do - expect(redis).to receive(:exists).with("vmpooler__ready__emptypool").and_return(false) - expect(redis).to receive(:exists).with("vmpooler__empty__emptypool").and_return(true) - expect(statsd).to receive(:increment).with('vmpooler.checkout.empty', 1) - post "#{prefix}/vm/emptypool", '' - - expect_json(ok = false, http = 404) - end - it 'returns multiple VMs' do post "#{prefix}/vm/pool1+pool2", '' diff --git a/spec/vmpooler/pool_manager_spec.rb b/spec/vmpooler/pool_manager_spec.rb index 9adeab9..a402bf4 100644 --- a/spec/vmpooler/pool_manager_spec.rb +++ b/spec/vmpooler/pool_manager_spec.rb @@ -5,12 +5,13 @@ describe 'Pool Manager' do let(:logger) { double('logger') } let(:redis) { double('redis') } let(:config) { {} } + let(:graphite) { nil } let(:pool) { 'pool1' } let(:vm) { 'vm1' } let(:timeout) { 5 } let(:host) { double('host') } - subject { Vmpooler::PoolManager.new(config, logger, redis) } + subject { Vmpooler::PoolManager.new(config, logger, redis, graphite) } describe '#_check_pending_vm' do let(:pool_helper) { double('pool') } @@ -251,86 +252,6 @@ describe 'Pool Manager' do end end - describe '#_stats_running_ready' do - let(:pool_helper) { double('pool') } - let(:vsphere) { {pool => pool_helper} } - let(:graphite) { double('graphite') } - let(:config) { { - config: { task_limit: 10 }, - pools: [ {'name' => 'pool1', 'size' => 5} ], - graphite: { 'prefix' => 'vmpooler' } - } } - - before do - expect(subject).not_to be_nil - $vsphere = vsphere - allow(logger).to receive(:log) - allow(pool_helper).to receive(:find_folder) - allow(redis).to receive(:smembers).and_return([]) - allow(redis).to receive(:set) - allow(redis).to receive(:get).with('vmpooler__tasks__clone').and_return(0) - allow(redis).to receive(:get).with('vmpooler__empty__pool1').and_return(nil) - end - - context 'graphite' do - let(:graphite) { double('graphite') } - subject { Vmpooler::PoolManager.new(config, logger, redis, graphite) } - - it 'increments graphite when enabled and statsd disabled' do - allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(1) - allow(redis).to receive(:scard).with('vmpooler__cloning__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__pending__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__running__pool1').and_return(5) - - expect(graphite).to receive(:log).with('vmpooler.ready.pool1', 1) - expect(graphite).to receive(:log).with('vmpooler.running.pool1', 5) - subject._check_pool(config[:pools][0]) - end - - it 'increments graphite when ready with 0 when pool empty and statsd disabled' do - allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__cloning__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__pending__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__running__pool1').and_return(5) - - expect(graphite).to receive(:log).with('vmpooler.ready.pool1', 0) - expect(graphite).to receive(:log).with('vmpooler.running.pool1', 5) - subject._check_pool(config[:pools][0]) - end - end - - context 'statsd' do - let(:statsd) { double('statsd') } - let(:config) { { - config: { task_limit: 10 }, - pools: [ {'name' => 'pool1', 'size' => 5} ], - statsd: { 'prefix' => 'vmpooler' } - } } - subject { Vmpooler::PoolManager.new(config, logger, redis, graphite, statsd) } - - it 'increments statsd when configured' do - allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(1) - allow(redis).to receive(:scard).with('vmpooler__cloning__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__pending__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__running__pool1').and_return(5) - - expect(statsd).to receive(:gauge).with('vmpooler.ready.pool1', 1) - expect(statsd).to receive(:gauge).with('vmpooler.running.pool1', 5) - subject._check_pool(config[:pools][0]) - end - - it 'increments statsd ready with 0 when pool empty' do - allow(redis).to receive(:scard).with('vmpooler__running__pool1').and_return(1) - allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(0) - allow(redis).to receive(:scard).with('vmpooler__pending__pool1').and_return(0) - allow(statsd).to receive(:gauge).with('vmpooler.running.pool1', 1) - - expect(statsd).to receive(:gauge).with('vmpooler.ready.pool1', 0) - subject._check_pool(config[:pools][0]) - end - end - end - describe '#_create_vm_snapshot' do let(:snapshot_manager) { 'snapshot_manager' } let(:pool_helper) { double('snapshot_manager') } diff --git a/vmpooler b/vmpooler index 44ef89c..5d0dd51 100755 --- a/vmpooler +++ b/vmpooler @@ -9,19 +9,10 @@ config = Vmpooler.config redis_host = config[:redis]['server'] logger_file = config[:config]['logfile'] graphite = config[:graphite]['server'] ? config[:graphite]['server'] : nil -# statsd is an addition and my not be present in YAML configuration -if config[:statsd] - statsd = config[:statsd]['server'] ? config[:statsd]['server'] : nil - statsd_port = config[:statsd]['port'] ? config[:statsd]['port'] : 8125 -end api = Thread.new { thr = Vmpooler::API.new - if statsd - thr.helpers.configure(config, Vmpooler.new_redis(redis_host), Vmpooler.new_statsd(statsd, statsd_port)) - else - thr.helpers.configure(config, Vmpooler.new_redis(redis_host), statsd=nil) - end + thr.helpers.configure(config, Vmpooler.new_redis(redis_host)) thr.helpers.execute! } @@ -30,8 +21,7 @@ manager = Thread.new { config, Vmpooler.new_logger(logger_file), Vmpooler.new_redis(redis_host), - Vmpooler.new_graphite(graphite), - Vmpooler.new_statsd(statsd, statsd_port) + Vmpooler.new_graphite(graphite) ).execute! } diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index 5476094..26f2c51 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -53,39 +53,10 @@ :redis: server: 'redis.company.com' - - # :statsd: - # - # This section contains the connection information required to store - # historical data via statsd. This is mutually exclusive with graphite - # and takes precedence. - # - # Available configuration parameters: - # - # - server - # The FQDN hostname of the statsd daemon. - # (optional) - # - # - prefix - # The prefix to use while storing statsd data. - # (optional; default: 'vmpooler') - # - # - port - # The UDP port to communicate with statsd daemon. - # (optional; default: 8125) - - # Example: - - :statsd: - server: 'statsd.company.com' - prefix: 'vmpooler' - port: 8125 - # :graphite: # # This section contains the connection information required to store -# historical data in an external Graphite database. This is mutually exclusive -# with statsd. +# historical data in an external Graphite database. # # Available configuration parameters: # From 97e59974f3cae5422ac2f2ff280b2ecb009d3c40 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 8 Jul 2016 16:27:28 -0700 Subject: [PATCH 5/8] (QENG-4070) Consistently return 503 if valid pool is empty There were several problems with how the pooler checked out vms with respect to empty pools, invalid pools, and aliases: - If the vmpooler config did not contain any aliases and the caller requested a vm from an empty pool or a non-existent one, the vmpooler would error with: NoMethodError - undefined method `[]' for nil:NilClass If the config contained a non-nil alias section, then: - If the caller requested a vm from an empty pool and either the vm didn't have an alias or the aliased pool was empty or non-existent, then the request for that vm would be silently ignored. The vmpooler would return 200 if the caller asked for multiple vms and the vmpooler was able to checkout at least one vm. Otherwise it would return 404. - Similarly, if the caller requested a vm from a non-existent pool, then the request was silently ignored. This commit adds a `pool_names` Set to the config containing all valid pool names including aliases. This is used to determine whether a requested template name is valid or not. This is necessary because redis does not distinguish between empty and non-existent sets, e.g. the following returns false in both cases: backend.exists('vmpooler__ready__' + key) If the caller requests a vm (single or multiple), and any vm references an invalid pool name, we immediately return 404. Otherwise, we know the request is for valid pool names, since the vmpooler requires a restart to change pool names and counts. We then attempt to acquire each vm, trying to match on pool name or failing back to aliased pool name, as was the previous behavior. The resulting behavior is: - If the caller asks for at least one vm from an unknown pool, then don't try to checkout any vms and respond with 404. - If the caller asks for a vm, and at least one pool is empty, then respond with 503, returning checked out vms back to the pool. - Otherwise return 200 with the list of checked out vms. This commit also makes `alias` optional again. This commit also re-enables tests that were merged in from master, but originally commented out due to the bugs described above.. --- lib/vmpooler.rb | 5 + lib/vmpooler/api/v1.rb | 68 +++++----- spec/helpers.rb | 5 + spec/vmpooler/api/v1/vm_spec.rb | 152 +++++++++++++++++++++++ spec/vmpooler/api/v1/vm_template_spec.rb | 135 ++++++++++++++++++++ 5 files changed, 334 insertions(+), 31 deletions(-) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 70bb819..7cfacbb 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -10,6 +10,7 @@ module Vmpooler require 'time' require 'timeout' require 'yaml' + require 'set' %w( api graphite logger pool_manager vsphere_helper ).each do |lib| begin @@ -35,15 +36,19 @@ module Vmpooler parsed_config[:config]['prefix'] ||= '' # Create an index of pool aliases + parsed_config[:pool_names] = Set.new parsed_config[:pools].each do |pool| + parsed_config[:pool_names] << pool['name'] if pool['alias'] if pool['alias'].kind_of?(Array) pool['alias'].each do |a| parsed_config[:alias] ||= {} parsed_config[:alias][a] = pool['name'] + parsed_config[:pool_names] << a end elsif pool['alias'].kind_of?(String) parsed_config[:alias][pool['alias']] = pool['name'] + parsed_config[:pool_names] << pool['alias'] end end end diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index e6c1ea7..f9f29b3 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -20,6 +20,10 @@ module Vmpooler Vmpooler::API.settings.config[:pools] end + def pool_exists?(template) + Vmpooler::API.settings.config[:pool_names].include?(template) + end + def need_auth! validate_auth(backend) end @@ -28,25 +32,19 @@ module Vmpooler validate_token(backend) end - def alias_deref(hash) - newhash = {} + def fetch_single_vm(template) + vm = backend.spop('vmpooler__ready__' + template) - hash.each do |key, val| - if backend.exists('vmpooler__ready__' + key) - newhash[key] = val - else - if Vmpooler::API.settings.config[:alias][key] - newkey = Vmpooler::API.settings.config[:alias][key] - newhash[newkey] = val - end - end + return [vm, template] if vm + + aliases = Vmpooler::API.settings.config[:alias] + if aliases && aliased_template = aliases[template] + vm = backend.spop('vmpooler__ready__' + aliased_template) + + return [vm, aliased_template] if vm end - newhash - end - - def fetch_single_vm(template) - backend.spop('vmpooler__ready__' + template) + [nil, nil] end def return_vm_to_ready_state(template, vm) @@ -83,33 +81,31 @@ module Vmpooler 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) + vm, name = fetch_single_vm(template) if !vm failed = true break else - vms << [ template, vm ] + vms << [ name, vm ] end end end if failed - vms.each do |(template, vm)| - return_vm_to_ready_state(template, vm) - status 503 + vms.each do |(name, vm)| + return_vm_to_ready_state(name, vm) end + status 503 else - vms.each do |(template, vm)| - account_for_starting_vm(template, vm) - update_result_hosts(result, template, vm) + vms.each do |(name, vm)| + account_for_starting_vm(name, vm) + update_result_hosts(result, name, vm) end result['ok'] = true @@ -371,12 +367,13 @@ module Vmpooler end post "#{api_prefix}/vm/?" do - jdata = alias_deref(JSON.parse(request.body.read)) content_type :json result = { 'ok' => false } - if jdata and !jdata.empty? - result = atomically_allocate_vms(jdata) + payload = JSON.parse(request.body.read) + + if all_templates_valid?(payload) + result = atomically_allocate_vms(payload) else status 404 end @@ -395,12 +392,21 @@ module Vmpooler payload end + def all_templates_valid?(payload) + return false unless payload + + payload.keys.all? do |templates| + pool_exists?(templates) + end + end + 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? + payload = extract_templates_from_query_params(params[:template]) + + if all_templates_valid?(payload) result = atomically_allocate_vms(payload) else status 404 diff --git a/spec/helpers.rb b/spec/helpers.rb index d28b761..292b9f4 100644 --- a/spec/helpers.rb +++ b/spec/helpers.rb @@ -35,6 +35,7 @@ end def create_ready_vm(template, name, token = nil) create_vm(name, token) redis.sadd("vmpooler__ready__#{template}", name) + # REMIND: should be __vm__? redis.hset("vmpooler_vm_#{name}", "template", template) end @@ -73,3 +74,7 @@ def vm_reverted_to_snapshot?(vm, snapshot = nil) instance == vm and (snapshot ? (sha == snapshot) : true) end end + +def pool_has_ready_vm?(pool, vm) + !!redis.sismember('vmpooler__ready__' + pool, vm) +end diff --git a/spec/vmpooler/api/v1/vm_spec.rb b/spec/vmpooler/api/v1/vm_spec.rb index 6a4a068..5622468 100644 --- a/spec/vmpooler/api/v1/vm_spec.rb +++ b/spec/vmpooler/api/v1/vm_spec.rb @@ -32,6 +32,7 @@ describe Vmpooler::API::V1 do {'name' => 'pool2', 'size' => 10} ], alias: { 'poolone' => 'pool1' }, + pool_names: [ 'pool1', 'pool2', 'poolone' ] } } @@ -84,6 +85,31 @@ describe Vmpooler::API::V1 do expect_json(ok = false, http = 404) end + it 'returns 503 for empty pool when aliases are not defined' do + Vmpooler::API.settings.config.delete(:alias) + Vmpooler::API.settings.config[:pool_names] = ['pool1', 'pool2'] + + create_ready_vm 'pool1', 'abcdefghijklmnop' + post "#{prefix}/vm/pool1" + post "#{prefix}/vm/pool1" + + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + expect_json(ok = false, http = 503) + end + + it 'returns 503 for empty pool referenced by alias' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + post "#{prefix}/vm/poolone" + post "#{prefix}/vm/poolone" + + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + expect_json(ok = false, http = 503) + end + it 'returns multiple VMs' do create_ready_vm 'pool1', 'abcdefghijklmnop' create_ready_vm 'pool2', 'qrstuvwxyz012345' @@ -104,6 +130,132 @@ describe Vmpooler::API::V1 do expect(last_response.body).to eq(JSON.pretty_generate(expected)) end + it 'returns multiple VMs even when multiple instances from the same pool are requested' do + create_ready_vm 'pool1', '1abcdefghijklmnop' + create_ready_vm 'pool1', '2abcdefghijklmnop' + create_ready_vm 'pool2', 'qrstuvwxyz012345' + + post "#{prefix}/vm", '{"pool1":"2","pool2":"1"}' + + expected = { + ok: true, + pool1: { + hostname: [ '1abcdefghijklmnop', '2abcdefghijklmnop' ] + }, + pool2: { + hostname: 'qrstuvwxyz012345' + } + } + + result = JSON.parse(last_response.body) + expect(result['ok']).to eq(true) + expect(result['pool1']['hostname']).to include('1abcdefghijklmnop', '2abcdefghijklmnop') + expect(result['pool2']['hostname']).to eq('qrstuvwxyz012345') + + expect_json(ok = true, http = 200) + end + + it 'returns multiple VMs even when multiple instances from multiple pools are requested' do + create_ready_vm 'pool1', '1abcdefghijklmnop' + create_ready_vm 'pool1', '2abcdefghijklmnop' + create_ready_vm 'pool2', '1qrstuvwxyz012345' + create_ready_vm 'pool2', '2qrstuvwxyz012345' + create_ready_vm 'pool2', '3qrstuvwxyz012345' + + post "#{prefix}/vm", '{"pool1":"2","pool2":"3"}' + + expected = { + ok: true, + pool1: { + hostname: [ '1abcdefghijklmnop', '2abcdefghijklmnop' ] + }, + pool2: { + hostname: [ '1qrstuvwxyz012345', '2qrstuvwxyz012345', '3qrstuvwxyz012345' ] + } + } + + result = JSON.parse(last_response.body) + expect(result['ok']).to eq(true) + expect(result['pool1']['hostname']).to include('1abcdefghijklmnop', '2abcdefghijklmnop') + expect(result['pool2']['hostname']).to include('1qrstuvwxyz012345', '2qrstuvwxyz012345', '3qrstuvwxyz012345') + + expect_json(ok = true, http = 200) + end + + it 'fails when not all requested vms can be allocated' do + create_ready_vm 'pool1', '1abcdefghijklmnop' + + 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 + create_ready_vm 'pool1', '1abcdefghijklmnop' + + 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) + + expect(pool_has_ready_vm?('pool1', '1abcdefghijklmnop')).to eq(true) + end + + it 'fails when not all requested vms can be allocated, when requesting multiple instances from a pool' do + create_ready_vm 'pool1', '1abcdefghijklmnop' + + 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 + create_ready_vm 'pool1', '1abcdefghijklmnop' + + 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) + + expect(pool_has_ready_vm?('pool1', '1abcdefghijklmnop')).to eq(true) + end + + it 'fails when not all requested vms can be allocated, when requesting multiple instances from multiple pools' do + create_ready_vm 'pool1', '1abcdefghijklmnop' + + 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 + create_ready_vm 'pool1', '1abcdefghijklmnop' + create_ready_vm 'pool1', '2abcdefghijklmnop' + + 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) + + expect(pool_has_ready_vm?('pool1', '1abcdefghijklmnop')).to eq(true) + expect(pool_has_ready_vm?('pool1', '2abcdefghijklmnop')).to eq(true) + end + context '(auth not configured)' do it 'does not extend VM lifetime if auth token is provided' do app.settings.set :config, auth: false diff --git a/spec/vmpooler/api/v1/vm_template_spec.rb b/spec/vmpooler/api/v1/vm_template_spec.rb index 28b85c3..3b7f055 100644 --- a/spec/vmpooler/api/v1/vm_template_spec.rb +++ b/spec/vmpooler/api/v1/vm_template_spec.rb @@ -32,6 +32,7 @@ describe Vmpooler::API::V1 do {'name' => 'pool2', 'size' => 10} ], alias: { 'poolone' => 'pool1' }, + pool_names: [ 'pool1', 'pool2', 'poolone' ] } } @@ -84,6 +85,31 @@ describe Vmpooler::API::V1 do expect_json(ok = false, http = 404) end + it 'returns 503 for empty pool when aliases are not defined' do + Vmpooler::API.settings.config.delete(:alias) + Vmpooler::API.settings.config[:pool_names] = ['pool1', 'pool2'] + + create_ready_vm 'pool1', 'abcdefghijklmnop' + post "#{prefix}/vm/pool1" + post "#{prefix}/vm/pool1" + + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + expect_json(ok = false, http = 503) + end + + it 'returns 503 for empty pool referenced by alias' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + post "#{prefix}/vm/poolone" + post "#{prefix}/vm/poolone" + + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + expect_json(ok = false, http = 503) + end + it 'returns multiple VMs' do create_ready_vm 'pool1', 'abcdefghijklmnop' create_ready_vm 'pool2', 'qrstuvwxyz012345' @@ -104,6 +130,115 @@ describe Vmpooler::API::V1 do expect(last_response.body).to eq(JSON.pretty_generate(expected)) end + it 'returns multiple VMs even when multiple instances from multiple pools are requested' do + create_ready_vm 'pool1', '1abcdefghijklmnop' + create_ready_vm 'pool1', '2abcdefghijklmnop' + + create_ready_vm 'pool2', '1qrstuvwxyz012345' + create_ready_vm 'pool2', '2qrstuvwxyz012345' + create_ready_vm 'pool2', '3qrstuvwxyz012345' + + post "#{prefix}/vm/pool1+pool1+pool2+pool2+pool2", '' + + expected = { + ok: true, + pool1: { + hostname: [ '1abcdefghijklmnop', '2abcdefghijklmnop' ] + }, + pool2: { + hostname: [ '1qrstuvwxyz012345', '2qrstuvwxyz012345', '3qrstuvwxyz012345' ] + } + } + + result = JSON.parse(last_response.body) + expect(result['ok']).to eq(true) + expect(result['pool1']['hostname']).to include('1abcdefghijklmnop', '2abcdefghijklmnop') + expect(result['pool2']['hostname']).to include('1qrstuvwxyz012345', '2qrstuvwxyz012345', '3qrstuvwxyz012345') + expect_json(ok = true, http = 200) + end + + it 'fails when not all requested vms can be allocated' do + create_ready_vm '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 + create_ready_vm '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) + + expect(pool_has_ready_vm?('pool1', 'abcdefghijklmnop')).to eq(true) + end + + it 'fails when not all requested vms can be allocated, when requesting multiple instances from a pool' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', '0123456789012345' + + 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 + create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool1', '0123456789012345' + + 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) + + expect(pool_has_ready_vm?('pool1', 'abcdefghijklmnop')).to eq(true) + expect(pool_has_ready_vm?('pool1', '0123456789012345')).to eq(true) + end + + it 'fails when not all requested vms can be allocated, when requesting multiple instances from multiple pools' do + create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool2', '0123456789012345' + + 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 + create_ready_vm 'pool1', 'abcdefghijklmnop' + create_ready_vm 'pool2', '0123456789012345' + + 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) + + expect(pool_has_ready_vm?('pool1', 'abcdefghijklmnop')).to eq(true) + expect(pool_has_ready_vm?('pool2', '0123456789012345')).to eq(true) + end + context '(auth not configured)' do it 'does not extend VM lifetime if auth token is provided' do app.settings.set :config, auth: false From ad0f0107b871570c4b87118bf5d650e377c991d6 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 8 Jul 2016 23:40:29 -0700 Subject: [PATCH 6/8] (maint) Add json pessimistic pin json 2.0.x was released on July 1 and is not compatible with ruby < 2.0. Since we still support that version, add a pessimistic pin, which is what we were using prior to July 1. --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index c71be35..c38a5e6 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source ENV['GEM_SOURCE'] || 'https://rubygems.org' -gem 'json', '>= 1.8' +gem 'json', '~> 1.8' gem 'rack', '>= 1.6' gem 'rake', '>= 10.4' gem 'rbvmomi', '>= 1.8' From 5993813f50c41f3c34c01bbfeb6f3bd496f00954 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Mon, 11 Jul 2016 12:47:12 -0500 Subject: [PATCH 7/8] [QENG-4070] Make json version conditional on RUBY_VERSION --- Gemfile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index c38a5e6..4ccc870 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,11 @@ source ENV['GEM_SOURCE'] || 'https://rubygems.org' -gem 'json', '~> 1.8' +if RUBY_VERSION =~ /^1\.9\./ + gem 'json', '~> 1.8' +else + gem 'json', '>= 1.8' +end + gem 'rack', '>= 1.6' gem 'rake', '>= 10.4' gem 'rbvmomi', '>= 1.8' From 33ac1450b098ec86e665b313fb2ee256c27fe271 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Mon, 11 Jul 2016 13:28:36 -0500 Subject: [PATCH 8/8] Drop extraneous mocks from updated test --- spec/vmpooler/api/v1/vm_template_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/vmpooler/api/v1/vm_template_spec.rb b/spec/vmpooler/api/v1/vm_template_spec.rb index 3b7f055..3604427 100644 --- a/spec/vmpooler/api/v1/vm_template_spec.rb +++ b/spec/vmpooler/api/v1/vm_template_spec.rb @@ -185,10 +185,6 @@ describe Vmpooler::API::V1 do create_ready_vm 'pool1', 'abcdefghijklmnop' create_ready_vm 'pool1', '0123456789012345' - 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 }