From 0fd6fff934f554706cabecf2354cee0457a03173 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 8 Jul 2016 11:48:48 -0700 Subject: [PATCH] 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: #