Revert "Merge pull request #155 from shermdog/RE-7014-cinext"

This reverts commit cc03a86f6a, reversing
changes made to 5aaab7c5c2.

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

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

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
 

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
+      if jdata and !jdata.empty?
-        empty = jdata.delete('empty')
+        result = atomically_allocate_vms(jdata)
-        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
       else
         status 404
       end
@@ -423,16 +400,8 @@ module Vmpooler
       content_type :json
       result = { 'ok' => false }
 
-      if payload
+      if payload and !payload.empty?
-        empty = payload.delete('empty')
+        result = atomically_allocate_vms(payload)
-        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
       else
         status 404
       end

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
+      unless graphite.nil?
-      if statsd
-        $statsd = statsd
-      elsif graphite
         $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 defined? $graphite
-          $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if $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
+        if defined? $graphite
-          $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

lib/vmpooler/statsd.rb

@@ -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

spec/spec_helper.rb

@@ -1,7 +1,3 @@
-require 'simplecov'
-SimpleCov.start do
-  add_filter '/spec/'
-end
 require 'helpers'
 require 'rbvmomi'
 require 'rspec'

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' },
+      alias: { 'poolone' => 'pool1' }
-      statsd: { 'prefix' => 'vmpooler' }
     } }
 
     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).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(: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' },
+      alias: { 'poolone' => 'pool1' }
-      statsd: { 'prefix' => 'vmpooler' }
     } }
 
     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).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(: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", ''
 

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') }

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))
-    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!
 }
 
@@ -30,8 +21,7 @@ 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!
 }
 

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
+# historical data in an external Graphite database.
-# with statsd.
 #
 # Available configuration parameters:
 #