From c71b2cd003c152a7bcc465f6818746b4aac5f1a1 Mon Sep 17 00:00:00 2001 From: Rick Bradley Date: Mon, 11 Jul 2016 15:20:43 -0500 Subject: [PATCH] Revert "Revert "Merge pull request #155 from shermdog/RE-7014-cinext"" This reverts commit 0fd6fff934f554706cabecf2354cee0457a03173. --- Gemfile | 2 + lib/vmpooler.rb | 16 ++++++ lib/vmpooler/api.rb | 3 +- lib/vmpooler/api/v1.rb | 52 ++++++++++++++----- lib/vmpooler/pool_manager.rb | 17 ++++-- lib/vmpooler/statsd.rb | 9 ++++ spec/spec_helper.rb | 4 ++ spec/vmpooler/pool_manager_spec.rb | 83 +++++++++++++++++++++++++++++- vmpooler | 14 ++++- vmpooler.yaml.example | 31 ++++++++++- 10 files changed, 207 insertions(+), 24 deletions(-) create mode 100644 lib/vmpooler/statsd.rb diff --git a/Gemfile b/Gemfile index 4ccc870..fb32937 100644 --- a/Gemfile +++ b/Gemfile @@ -12,10 +12,12 @@ 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 7cfacbb..5d07036 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -7,6 +7,7 @@ module Vmpooler require 'rbvmomi' require 'redis' require 'sinatra/base' + require "statsd-ruby" require 'time' require 'timeout' require 'yaml' @@ -57,6 +58,13 @@ 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]) @@ -84,6 +92,14 @@ 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 ee51cbc..45a9bfa 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -42,9 +42,10 @@ module Vmpooler use Vmpooler::API::Reroute use Vmpooler::API::V1 - def configure(config, redis, environment = :production) + def configure(config, redis, statsd, 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 f9f29b3..2d76970 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -12,6 +12,16 @@ 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 @@ -34,13 +44,11 @@ module Vmpooler def fetch_single_vm(template) vm = backend.spop('vmpooler__ready__' + template) - 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 @@ -85,14 +93,16 @@ module Vmpooler failed = false vms = [] - payload.each do |template, count| + payload.each do |requested, count| count.to_i.times do |_i| - vm, name = fetch_single_vm(template) + vm, name = fetch_single_vm(requested) if !vm failed = true + statsd.increment(statsd_prefix + '.checkout.empty.' + name, 1) break else vms << [ name, vm ] + statsd.increment(statsd_prefix + '.checkout.success.' + name, 1) end end end @@ -372,8 +382,16 @@ module Vmpooler payload = JSON.parse(request.body.read) - if all_templates_valid?(payload) - result = atomically_allocate_vms(payload) + if payload + invalid = invalid_templates(payload) + if invalid.empty? + result = atomically_allocate_vms(payload) + else + invalid.each do |bad_template| + statsd.increment(statsd_prefix + '.checkout.invalid', bad_template) + end + status 404 + end else status 404 end @@ -392,12 +410,12 @@ module Vmpooler payload end - def all_templates_valid?(payload) - return false unless payload - - payload.keys.all? do |templates| - pool_exists?(templates) + def invalid_templates(payload) + invalid = [] + payload.keys.each do |template| + invalid << template unless pool_exists?(template) end + invalid end post "#{api_prefix}/vm/:template/?" do @@ -406,8 +424,16 @@ module Vmpooler payload = extract_templates_from_query_params(params[:template]) - if all_templates_valid?(payload) - result = atomically_allocate_vms(payload) + if payload + invalid = invalid_templates(payload) + if invalid.empty? + result = atomically_allocate_vms(payload) + else + invalid.each do |bad_template| + statsd.increment(statsd_prefix + '.checkout.invalid', bad_template) + end + status 404 + end else status 404 end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 1a0454c..5f54169 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,12 +1,15 @@ module Vmpooler class PoolManager - def initialize(config, logger, redis, graphite=nil) + def initialize(config, logger, redis, graphite = nil, statsd = nil) $config = config # Load logger library $logger = logger - unless graphite.nil? + # statsd and graphite are mutex in the context of vmpooler + if statsd + $statsd = statsd + elsif graphite $graphite = graphite end @@ -258,7 +261,8 @@ module Vmpooler $redis.decr('vmpooler__tasks__clone') begin - $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if defined? $graphite + $statsd.timing($config[:statsd]['prefix'] + ".clone.#{vm['template']}", finish) if $statsd + $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if $graphite rescue end end @@ -294,7 +298,7 @@ module Vmpooler $logger.log('s', "[-] [#{pool}] '#{vm}' destroyed in #{finish} seconds") - $graphite.log($config[:graphite]['prefix'] + ".destroy.#{pool}", finish) if defined? $graphite + $graphite.log($config[:graphite]['prefix'] + ".destroy.#{pool}", finish) if $graphite end end end @@ -565,7 +569,10 @@ module Vmpooler total = $redis.scard('vmpooler__pending__' + pool['name']) + ready begin - if defined? $graphite + 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 $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 new file mode 100644 index 0000000..b449a59 --- /dev/null +++ b/lib/vmpooler/statsd.rb @@ -0,0 +1,9 @@ +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 7589276..c401e32 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,7 @@ +require 'simplecov' +SimpleCov.start do + add_filter '/spec/' +end require 'helpers' require 'rbvmomi' require 'rspec' diff --git a/spec/vmpooler/pool_manager_spec.rb b/spec/vmpooler/pool_manager_spec.rb index a402bf4..9adeab9 100644 --- a/spec/vmpooler/pool_manager_spec.rb +++ b/spec/vmpooler/pool_manager_spec.rb @@ -5,13 +5,12 @@ 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, graphite) } + subject { Vmpooler::PoolManager.new(config, logger, redis) } describe '#_check_pending_vm' do let(:pool_helper) { double('pool') } @@ -252,6 +251,86 @@ 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 5d0dd51..44ef89c 100755 --- a/vmpooler +++ b/vmpooler @@ -9,10 +9,19 @@ 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 - thr.helpers.configure(config, Vmpooler.new_redis(redis_host)) + 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.execute! } @@ -21,7 +30,8 @@ manager = Thread.new { config, Vmpooler.new_logger(logger_file), Vmpooler.new_redis(redis_host), - Vmpooler.new_graphite(graphite) + Vmpooler.new_graphite(graphite), + Vmpooler.new_statsd(statsd, statsd_port) ).execute! } diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index 26f2c51..5476094 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -53,10 +53,39 @@ :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. +# historical data in an external Graphite database. This is mutually exclusive +# with statsd. # # Available configuration parameters: #