Merge CI.next into Master (#161)

* [QENG-3919] Make vmpooler checkouts be all or nothing (#153)

* (QENG-3919) spike for implementation of all-or-nothing checkout

* Fix two botched variable references

* Aggregate API helper methods

* Add specs for failed multi-vm allocation API endpoints

* (QENG-3919) Add tests for multiple vm requests

* (QENG-3919) Add (failing) specs for POST /vm/pool1+pool2 usages

This exposes the old (bad) behavior on this other code path. Will fix this up next.

* (QENG-3919) Bring query params version in line with JSON post version

Not clear to me why these had to be implemented so differently.

* (QENG-3919) extract common method from both methods of VM allocation

* (QENG-3919) Naming fix, cosmetic cleanups

I mean, I presume all these commits are going to get squashed away on merge anyway.

* (QENG-3919) Update API docs

We consider it a bug that the actual behavior was not this behavior, but the
documentation was also silent on this point.

* (QENG-3919) minor readability tweak in refactored method

* (QENG-3919) Clean up interim comments re: status codes

* (QENG-3919) Drop now-orphaned `checkout_vm` method

We kept this up-to-date while we were upgrading and refactoring, but, turns out,
this method is no longer called anywhere.  💀 🔥

* (QENG-3919) Return 503 status on failed allocation

Making sure we go back to the original functionality, which was:

 - status 200 when vms successfully allocated
 - status 404 when a pool name is unknown
 - status 404 when no pool name is specified
 - status 503 when vm allocation failed

* (QENG-3919) add net-ldap to Gemfile

Maybe we shouldn't foil-ball gems onto servers.

* (QENG-3919) Turns out, spush isn't a redis command

And hence we see once again the weakness of mockist tests.

* (QENG-3919) Pin the net-ldap gem to 0.11 for the jrubies, etc.

* (QENG-3919) Correct an old spelling error in spec descriptions

* (QENG-3919) Further tweak net-ldap version

* (QENG-3919) return_single_vm -> return_vm_to_ready_state

cc @shermdog

* (RE-7014) Add support for statsd
They way we were using graphite was incorrect for the type of data we were sending it.  statsd is the appropriate mechanism for our needs.
statsd and graphite are mutually exclusive and configuring statsd will take precendence over Graphite.  Example of configuration in vmpooler.yaml.example

* (RE-7014) Add tracking of vm gets via statsd
Add the tracking of successful, failed, invalid, and empty pool vm gets.  It is possible we may want to tweak this, but have validated with spec tests and pcaps.

```
vmpooler-tmp-dev.ready.debian-7-x86_64:1|c
vmpooler-tmp-dev.running.debian-7-x86_64:1|c

vmpooler-tmp-dev.checkout.invalid:1|c
vmpooler-tmp-dev.checkout.success.debian-7-x86_64:1|c
vmpooler-tmp-dev.checkout.empty:1|c

vmpooler-tmp-dev.running.debian-7-x86_64:1|c

vmpooler-tmp-dev.clone.debian-7-x86_64:12.10|ms

vmpooler-tmp-dev.ready.debian-7-x86_64:1|c
```

* (RE-7014) statsd nitpicks and additional rspec
Cleaned up some code review nitpicks and added pool_manager_spec for empty pool.

* (RE-7014) update statsd to use gauge for running/ready
Previously was using increment which was incorrect for that particular application.

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

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

* (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..

* (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.

* [QENG-4070] Make json version conditional on RUBY_VERSION

* Drop extraneous mocks from updated test

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

This reverts commit 0fd6fff934.

* Fix some spec errors

These were caused in part by dropping changes from the original PR when we
dropped the v1_spec.rb master test file (in favor of the updated and separated
versions).

* [QENG-4075] Fix bug with template name on allocation failure

We're returning [nil,nil] in this case, meaning that name will not be set. This
means we'll get an error trying to concatenate the stats string. Use the
requested template name here instead.

* [QENG-4075] Refactor statsd methods / classes

Prior to this we could easily run into situations where `statds_prefix` would
be `nil` (and possibly the `statsd` handle itself). There was some significant
complexity and brittleness in how statsd was set up.

Refactored so that:

 - `statsd_prefix` is no longer exposed to any callers of statsd methods
 - there is now a `Vmpooler::DummyStatsd` class which can be returned when we are not actually going to publish stats, but would like to keep the calling interface consistent
 - setup of the statsd handle is via just passing in `config[:statsd]`, if `nil`, this will result in a dummy handle being return
 - defaulting of `server` values was fixed -- this did not actually work in the previous implementation. `config[:statsd][:server]` is now required.
 - tests use a `DummyStatsd` instance instead of an rspec double.
 - calls to `statsd.increment` were taking incorrect arguments (some our fault, some part of the prior implementation), and were not collecting data on which pools were "invalid" or "empty". Fixed this and are now explicitly tracking the invalid/empty pool names.

* [QENG-4075] Drop now-superfluous :statsd config defaulting

* [QENG-4075] Unify graphite and statsd for the pool manager

Prior to this, the `pool_manager.rb` library could take handles for both
graphite and statsd endpoints (which were considered mutually exclusive),
and then would use one. There was a bevy of conditional logic around sending
metrics to the graphite/statsd handles (and actually at least one bug of
omission).

Here we refactor more, building on earlier work:

 - Our graphite class comes into line with the API of our Statsd and DummyStatsd classes
 - In `pool_manager.rb` we now accept a single "metrics" handle, and we drop all the conditional logic around statsd vs. graphite
 - We move the inconsistent error handling out of the calling classes and into our metrics classes, actually logging to `$stderr` when we can't publish metrics
 - We unify the setup code to use `config` to determine whether statsd, graphite, or a dummy metrics handle should be used, and make that happen.
 - Cleaned up some tests. We could probably stand to do a bit more work in this area.

* [QENG-4075] Clean up pool manager, specs

Prior to this, `pool_manager.rb` allowed the `metrics` argument to be optional,
but at this point it will be an instance of `Vmpooler::Statsd`,
'Vmpooler::Graphite', or `Vmpooler::DummyStatsd`, so making this non-optional.

Cleaned up that file's tests, cosmetically, as well as recognizing that the
behavioral difference between graphite and statsd does not depend on the pool
manager.

* [QENG-4075] update example vmpooler.yaml file

This documents the changes to :server being mandatory for all metrics
endpoints, as well as the graphite endpoint supporting an optional :port
configuration value.

* [QENG-4075] Rename usages of statsd -> metrics

Really, let's just support a generic metrics interface.

* (maint) move statsd-ruby require into Vmpooler::Statsd class

We've managed to move mentions of this out of the calling code, so let's
move the require.

* (maint) metrics.log -> metrics.timing

We missed this during the refactoring. Bringing this up to date.

* [QENG-4075] Allow specifying 'graphs:' for dashboard

Prior to this the dashboard front-end would use the configuration settings
for `graphite[:server]`/`graphite[:prefix]` to locate a graphite server
to use for rendering graphs.

Now that we have multiple possible metrics backends, the front-end graph
host for the dashboard could be entirely different from the back-end metrics
server that we publish to (if any).

This decouples those settings:

 - use `graphs[:server]` / `graphs[:prefix]` for the graphite-compatible web front-end to use for dashboard display graphs
 - fall back to `graphite[:server]`/`graphite[:prefix]` if `graphs` is not specified, in order to support legacy `vmpooler.yaml` configurations.

Note that since `statsd` takes precedence over `graphite`, it's possible to specify both `statsd` (for publishing) and `graphite` (for reading). We still prefer `graphs` over `graphite`.

Updated the example `vmpooler.yaml` config file.

* (maint) fix variable reference in new_metrics

This was referencing config directly, when what we want is for a
hash to be passed in (derived from config).

* (maint) Fix typo in updated graph link call

* (maint) default :graphs prefix to 'vmpooler'

* (maint) Fix parse error in vmpooler script

The things you find through manual QA 🧌

* (maint) use strings instead of symbols in config

Nested hash data comes back with string keys, not symbols. Be consistent.

* [QENG-4075] Factor out Vmpooler::DummyStatsd

This makes it visible to lib/vmpooler.rb, as well as putting this dummy
metrics endpoint in its own file for easier discovery.

* (maint) clean up statsd inclusion and require lines

The library is actually required as 'statsd' and not 'ruby-statsd', best I can tell.

* (maint) construct ::Statsd instead of Statsd

Because it's ambiguous in this scope, and, well, it doesn't
actually work in production.

* [QENG-4075] Also track completely invalid requests

When we don't even get a pool name we still want metrics to be recorded.
This commit is contained in:
Rick Sherman 2016-07-25 10:43:32 -05:00 committed by GitHub
parent 4e2a1fb62c
commit 34b93ca6c3
17 changed files with 706 additions and 166 deletions

4
API.md
View file

@ -117,6 +117,8 @@ $ curl -d '{"debian-7-i386":"2","debian-7-x86_64":"1"}' --url vmpooler.company.c
}
```
**NOTE: Returns either all requested VMs or no VMs.**
##### POST /vm/&lt;pool&gt;
Check-out a VM or VMs.
@ -156,6 +158,8 @@ $ curl -d --url vmpooler.company.com/api/v1/vm/debian-7-i386+debian-7-i386+debia
}
```
**NOTE: Returns either all requested VMs or no VMs.**
##### GET /vm/&lt;hostname&gt;
Query a checked-out VM.

View file

@ -1,15 +1,23 @@
source ENV['GEM_SOURCE'] || 'https://rubygems.org'
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'
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', :require => 'statsd'
# Test deps
group :test do
gem 'rack-test', '>= 0.6'
gem 'rspec', '>= 3.2'
gem 'simplecov', '>= 0.11.2'
gem 'yarjuf', '>= 2.0'
end

View file

@ -10,8 +10,9 @@ module Vmpooler
require 'time'
require 'timeout'
require 'yaml'
require 'set'
%w( api graphite logger pool_manager vsphere_helper ).each do |lib|
%w( api graphite logger pool_manager vsphere_helper statsd dummy_statsd ).each do |lib|
begin
require "vmpooler/#{lib}"
rescue LoadError
@ -35,23 +36,23 @@ 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
if parsed_config[:graphite]['server']
parsed_config[:graphite]['prefix'] ||= 'vmpooler'
end
if parsed_config[:tagfilter]
parsed_config[:tagfilter].keys.each do |tag|
parsed_config[:tagfilter][tag] = Regexp.new(parsed_config[:tagfilter][tag])
@ -59,7 +60,6 @@ module Vmpooler
end
parsed_config[:uptime] = Time.now
parsed_config
end
@ -71,11 +71,13 @@ module Vmpooler
Vmpooler::Logger.new logfile
end
def self.new_graphite(server)
if server.nil? or server.empty? or server.length == 0
nil
def self.new_metrics(params)
if params[:statsd]
Vmpooler::Statsd.new(params[:statsd])
elsif params[:graphite]
Vmpooler::Graphite.new(params[:graphite])
else
Vmpooler::Graphite.new server
Vmpooler::DummyStatsd.new
end
end

View file

@ -42,9 +42,10 @@ module Vmpooler
use Vmpooler::API::Reroute
use Vmpooler::API::V1
def configure(config, redis, environment = :production)
def configure(config, redis, metrics, environment = :production)
self.settings.set :config, config
self.settings.set :redis, redis
self.settings.set :metrics, metrics
self.settings.set :environment, environment
end

View file

@ -1,9 +1,57 @@
module Vmpooler
class API
class Dashboard < Sinatra::Base
# handle to the App's configuration information
def config
@config ||= Vmpooler::API.settings.config
end
# configuration setting for server hosting graph URLs to view
def graph_server
return @graph_server if @graph_server
if config[:graphs]
return false unless config[:graphs]['server']
@graph_server = config[:graphs]['server']
elsif config[:graphite]
return false unless config[:graphite]['server']
@graph_server = config[:graphite]['server']
else
false
end
end
# configuration setting for URL prefix for graphs to view
def graph_prefix
return @graph_prefix if @graph_prefix
if config[:graphs]
return "vmpooler" unless config[:graphs]['prefix']
@graph_prefix = config[:graphs]['prefix']
elsif config[:graphite]
return false unless config[:graphite]['prefix']
@graph_prefix = config[:graphite]['prefix']
else
false
end
end
# what is the base URL for viewable graphs?
def graph_url
return false unless graph_server && graph_prefix
@graph_url ||= "http://#{graph_server}/render?target=#{graph_prefix}"
end
# return a full URL to a viewable graph for a given metrics target (graphite syntax)
def graph_link(target = "")
return "" unless graph_url
graph_url + target
end
get '/dashboard/stats/vmpooler/pool/?' do
content_type :json
result = {}
Vmpooler::API.settings.config[:pools].each do |pool|
@ -13,13 +61,11 @@ module Vmpooler
end
if params[:history]
if Vmpooler::API.settings.config[:graphite]['server']
if graph_url
history ||= {}
begin
buffer = open(
'http://' + Vmpooler::API.settings.config[:graphite]['server'] + '/render?target=' + Vmpooler::API.settings.config[:graphite]['prefix'] + '.ready.*&from=-1hour&format=json'
).read
buffer = open(graph_link('.ready.*&from=-1hour&format=json')).read
history = JSON.parse(buffer)
history.each do |pool|
@ -52,59 +98,47 @@ module Vmpooler
end
end
end
JSON.pretty_generate(result)
end
get '/dashboard/stats/vmpooler/running/?' do
content_type :json
result = {}
Vmpooler::API.settings.config[:pools].each do |pool|
running = Vmpooler::API.settings.redis.scard('vmpooler__running__' + pool['name'])
pool['major'] = Regexp.last_match[1] if pool['name'] =~ /^(\w+)\-/
result[pool['major']] ||= {}
result[pool['major']]['running'] = result[pool['major']]['running'].to_i + running.to_i
end
if params[:history]
if Vmpooler::API.settings.config[:graphite]['server']
if graph_url
begin
buffer = open(
'http://' + Vmpooler::API.settings.config[:graphite]['server'] + '/render?target=' + Vmpooler::API.settings.config[:graphite]['prefix'] + '.running.*&from=-1hour&format=json'
).read
buffer = open(graph_link('.running.*&from=-1hour&format=json')).read
JSON.parse(buffer).each do |pool|
if pool['target'] =~ /.*\.(.*)$/
pool['name'] = Regexp.last_match[1]
pool['major'] = Regexp.last_match[1] if pool['name'] =~ /^(\w+)\-/
result[pool['major']]['history'] ||= Array.new
for i in 0..pool['datapoints'].length
if
pool['datapoints'][i] &&
pool['datapoints'][i][0]
pool['last'] = pool['datapoints'][i][0]
result[pool['major']]['history'][i] ||= 0
result[pool['major']]['history'][i] = result[pool['major']]['history'][i].to_i + pool['datapoints'][i][0].to_i
else
result[pool['major']]['history'][i] = result[pool['major']]['history'][i].to_i + pool['last'].to_i
end
end
end
end
rescue
end
end
end
JSON.pretty_generate(result)
end
end

View file

@ -12,6 +12,10 @@ module Vmpooler
Vmpooler::API.settings.redis
end
def metrics
Vmpooler::API.settings.metrics
end
def config
Vmpooler::API.settings.config[:config]
end
@ -20,6 +24,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,27 +36,24 @@ module Vmpooler
validate_token(backend)
end
def alias_deref(hash)
newhash = {}
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
end
newhash
end
def checkout_vm(template, result)
def fetch_single_vm(template)
vm = backend.spop('vmpooler__ready__' + template)
return [vm, template] if vm
unless vm.nil?
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
[nil, nil]
end
def return_vm_to_ready_state(template, vm)
backend.sadd('vmpooler__ready__' + template, vm)
end
def account_for_starting_vm(template, vm)
backend.sadd('vmpooler__running__' + template, vm)
backend.hset('vmpooler__active__' + template, vm, Time.now)
backend.hset('vmpooler__vm__' + vm, 'checkout', Time.now)
@ -65,18 +70,50 @@ module Vmpooler
backend.hset('vmpooler__vm__' + vm, 'lifetime', config['vm_lifetime_auth'].to_i)
end
end
end
def update_result_hosts(result, template, vm)
result[template] ||= {}
if result[template]['hostname']
result[template]['hostname'] = [result[template]['hostname']] unless result[template]['hostname'].is_a?(Array)
result[template]['hostname'] = Array(result[template]['hostname'])
result[template]['hostname'].push(vm)
else
result[template]['hostname'] = vm
end
end
def atomically_allocate_vms(payload)
result = { 'ok' => false }
failed = false
vms = []
payload.each do |requested, count|
count.to_i.times do |_i|
vm, name = fetch_single_vm(requested)
if !vm
failed = true
metrics.increment('checkout.empty.' + requested)
break
else
vms << [ name, vm ]
metrics.increment('checkout.success.' + name)
end
end
end
if failed
vms.each do |(name, vm)|
return_vm_to_ready_state(name, vm)
end
status 503
result['ok'] = false
else
vms.each do |(name, vm)|
account_for_starting_vm(name, vm)
update_result_hosts(result, name, vm)
end
result['ok'] = true
result['domain'] = config['domain'] if config['domain']
end
result
@ -335,77 +372,66 @@ module Vmpooler
post "#{api_prefix}/vm/?" do
content_type :json
result = { 'ok' => false }
jdata = alias_deref(JSON.parse(request.body.read))
payload = JSON.parse(request.body.read)
if not jdata.nil? and not jdata.empty?
available = 1
if payload
invalid = invalid_templates(payload)
if invalid.empty?
result = atomically_allocate_vms(payload)
else
invalid.each do |bad_template|
metrics.increment('checkout.invalid.' + bad_template)
end
status 404
end
jdata.each do |key, val|
if backend.scard('vmpooler__ready__' + key).to_i < val.to_i
available = 0
end
end
if (available == 1)
result['ok'] = true
jdata.each do |key, val|
val.to_i.times do |_i|
result = checkout_vm(key, result)
end
end
end
if result['ok'] && config['domain']
result['domain'] = config['domain']
else
metrics.increment('checkout.invalid.unknown')
status 404
end
JSON.pretty_generate(result)
end
post "#{api_prefix}/vm/:template/?" do
content_type :json
result = { 'ok' => false }
def extract_templates_from_query_params(params)
payload = {}
params[:template].split('+').each do |template|
params.split('+').each do |template|
payload[template] ||= 0
payload[template] = payload[template] + 1
payload[template] += 1
end
payload = alias_deref(payload)
payload
end
if not payload.nil? and not payload.empty?
available = 1
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
content_type :json
result = { 'ok' => false }
payload = extract_templates_from_query_params(params[:template])
if payload
invalid = invalid_templates(payload)
if invalid.empty?
result = atomically_allocate_vms(payload)
else
invalid.each do |bad_template|
metrics.increment('checkout.invalid.' + bad_template)
end
status 404
end
payload.each do |key, val|
if backend.scard('vmpooler__ready__' + key).to_i < val.to_i
available = 0
end
end
if (available == 1)
result['ok'] = true
payload.each do |key, val|
val.to_i.times do |_i|
result = checkout_vm(key, result)
end
end
end
if result['ok'] && config['domain']
result['domain'] = config['domain']
else
metrics.increment('checkout.invalid.unknown')
status 404
end
JSON.pretty_generate(result)

View file

@ -0,0 +1,20 @@
module Vmpooler
class DummyStatsd
attr_reader :server, :port, :prefix
def initialize(params = {})
end
def increment(label)
true
end
def gauge(label, value)
true
end
def timing(label, duration)
true
end
end
end

View file

@ -2,18 +2,41 @@ require 'rubygems' unless defined?(Gem)
module Vmpooler
class Graphite
def initialize(
s = 'graphite'
)
@server = s
attr_reader :server, :port, :prefix
def initialize(params = {})
if params["server"].nil? || params["server"].empty?
raise ArgumentError, "Graphite server is required. Config: #{params.inspect}"
end
@server = params["server"]
@port = params["port"] || 2003
@prefix = params["prefix"] || "vmpooler"
end
def increment(label)
log label, 1
end
def gauge(label, value)
log label, value
end
def timing(label, duration)
log label, duration
end
def log(path, value)
Thread.new do
socket = TCPSocket.new(@server, 2003)
socket.puts "#{path} #{value} #{Time.now.to_i}"
socket = TCPSocket.new(server, port)
begin
socket.puts "#{prefix}.#{path} #{value} #{Time.now.to_i}"
ensure
socket.close
end
end
rescue => err
$stderr.puts "Failure logging #{path} to graphite server [#{server}:#{port}]: #{err}"
end
end
end

View file

@ -1,14 +1,13 @@
module Vmpooler
class PoolManager
def initialize(config, logger, redis, graphite=nil)
def initialize(config, logger, redis, metrics)
$config = config
# Load logger library
$logger = logger
unless graphite.nil?
$graphite = graphite
end
# metrics logging handle
$metrics = metrics
# Connect to Redis
$redis = redis
@ -63,7 +62,7 @@ module Vmpooler
(host.summary.guest.hostName == vm)
begin
Socket.getaddrinfo(vm, nil)
Socket.getaddrinfo(vm, nil) # WTF?
rescue
end
@ -257,10 +256,7 @@ module Vmpooler
$redis.decr('vmpooler__tasks__clone')
begin
$graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if defined? $graphite
rescue
end
$metrics.timing("clone.#{vm['template']}", finish)
end
end
@ -293,8 +289,7 @@ module Vmpooler
finish = '%.2f' % (Time.now - start)
$logger.log('s', "[-] [#{pool}] '#{vm}' destroyed in #{finish} seconds")
$graphite.log($config[:graphite]['prefix'] + ".destroy.#{pool}", finish) if defined? $graphite
$metrics.timing("destroy.#{pool}", finish)
end
end
end
@ -564,13 +559,8 @@ module Vmpooler
ready = $redis.scard('vmpooler__ready__' + pool['name'])
total = $redis.scard('vmpooler__pending__' + pool['name']) + ready
begin
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
rescue
end
$metrics.gauge('ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name']))
$metrics.gauge('running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name']))
if $redis.get('vmpooler__empty__' + pool['name'])
unless ready == 0

37
lib/vmpooler/statsd.rb Normal file
View file

@ -0,0 +1,37 @@
require 'rubygems' unless defined?(Gem)
require 'statsd'
module Vmpooler
class Statsd
attr_reader :server, :port, :prefix
def initialize(params = {})
if params["server"].nil? || params["server"].empty?
raise ArgumentError, "Statsd server is required. Config: #{params.inspect}"
end
host = params["server"]
@port = params["port"] || 8125
@prefix = params["prefix"] || 'vmpooler'
@server = ::Statsd.new(host, @port)
end
def increment(label)
server.increment(prefix + "." + label)
rescue => err
$stderr.puts "Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
end
def gauge(label, value)
server.gauge(prefix + "." + label, value)
rescue => err
$stderr.puts "Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
end
def timing(label, duration)
server.timing(prefix + "." + label, duration)
rescue => err
$stderr.puts "Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
end
end
end

View file

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

View file

@ -1,5 +1,10 @@
require 'simplecov'
SimpleCov.start do
add_filter '/spec/'
end
require 'helpers'
require 'rbvmomi'
require 'rspec'
require 'vmpooler'
require 'redis'
require 'vmpooler/statsd'

View file

@ -20,7 +20,7 @@ describe Vmpooler::API::V1 do
describe '/vm' do
let(:prefix) { '/api/v1' }
let(:metrics) { Vmpooler::DummyStatsd.new }
let(:config) {
{
config: {
@ -31,7 +31,9 @@ describe Vmpooler::API::V1 do
{'name' => 'pool1', 'size' => 5},
{'name' => 'pool2', 'size' => 10}
],
statsd: { 'prefix' => 'stats_prefix'},
alias: { 'poolone' => 'pool1' },
pool_names: [ 'pool1', 'pool2', 'poolone' ]
}
}
@ -42,6 +44,7 @@ describe Vmpooler::API::V1 do
app.settings.set :config, config
app.settings.set :redis, redis
app.settings.set :metrics, metrics
app.settings.set :config, auth: false
create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time)
end
@ -84,6 +87,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 +132,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

View file

@ -20,7 +20,7 @@ describe Vmpooler::API::V1 do
describe '/vm/:template' do
let(:prefix) { '/api/v1' }
let(:metrics) { Vmpooler::DummyStatsd.new }
let(:config) {
{
config: {
@ -31,7 +31,9 @@ describe Vmpooler::API::V1 do
{'name' => 'pool1', 'size' => 5},
{'name' => 'pool2', 'size' => 10}
],
statsd: { 'prefix' => 'stats_prefix'},
alias: { 'poolone' => 'pool1' },
pool_names: [ 'pool1', 'pool2', 'poolone' ]
}
}
@ -42,6 +44,7 @@ describe Vmpooler::API::V1 do
app.settings.set :config, config
app.settings.set :redis, redis
app.settings.set :metrics, metrics
app.settings.set :config, auth: false
create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time)
end
@ -84,6 +87,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 +132,111 @@ 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'
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

View file

@ -4,14 +4,14 @@ require 'time'
describe 'Pool Manager' do
let(:logger) { double('logger') }
let(:redis) { double('redis') }
let(:metrics) { Vmpooler::DummyStatsd.new }
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, metrics) }
describe '#_check_pending_vm' do
let(:pool_helper) { double('pool') }
@ -23,14 +23,12 @@ describe 'Pool Manager' do
end
context 'host not in pool' do
it 'calls fail_pending_vm' do
allow(pool_helper).to receive(:find_vm).and_return(nil)
allow(redis).to receive(:hget)
expect(redis).to receive(:hget).with(String, 'clone').once
subject._check_pending_vm(vm, pool, timeout)
end
end
context 'host is in pool' do
@ -58,7 +56,6 @@ describe 'Pool Manager' do
end
context 'a host without correct summary' do
it 'does nothing when summary is nil' do
allow(host).to receive(:summary).and_return nil
subject.move_pending_vm_to_ready(vm, pool, host)
@ -114,7 +111,6 @@ describe 'Pool Manager' do
subject.move_pending_vm_to_ready(vm, pool, host)
end
end
end
@ -191,9 +187,7 @@ describe 'Pool Manager' do
subject._check_running_vm(vm, pool, timeout)
end
end
end
describe '#move_running_to_completed' do
@ -240,15 +234,62 @@ describe 'Pool Manager' do
end
context 'logging' do
it 'logs empty pool' do
allow(redis).to receive(:scard).with('vmpooler__pending__pool1').and_return(0)
allow(redis).to receive(:scard).with('vmpooler__ready__pool1').and_return(0)
allow(redis).to receive(:scard).with('vmpooler__running__pool1').and_return(0)
expect(logger).to receive(:log).with('s', "[!] [pool1] is empty")
subject._check_pool(config[:pools][0])
end
end
end
describe '#_stats_running_ready' do
let(:pool_helper) { double('pool') }
let(:vsphere) { {pool => pool_helper} }
let(:metrics) { Vmpooler::DummyStatsd.new }
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 'metrics' do
subject { Vmpooler::PoolManager.new(config, logger, redis, metrics) }
it 'increments metrics' 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(metrics).to receive(:gauge).with('ready.pool1', 1)
expect(metrics).to receive(:gauge).with('running.pool1', 5)
subject._check_pool(config[:pools][0])
end
it 'increments metrics when ready with 0 when pool empty' 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(metrics).to receive(:gauge).with('ready.pool1', 0)
expect(metrics).to receive(:gauge).with('running.pool1', 5)
subject._check_pool(config[:pools][0])
end
end
end
@ -319,5 +360,4 @@ describe 'Pool Manager' do
subject._check_snapshot_queue
end
end
end

View file

@ -8,11 +8,12 @@ require 'lib/vmpooler'
config = Vmpooler.config
redis_host = config[:redis]['server']
logger_file = config[:config]['logfile']
graphite = config[:graphite]['server'] ? config[:graphite]['server'] : nil
metrics = Vmpooler.new_metrics(config)
api = Thread.new {
thr = Vmpooler::API.new
thr.helpers.configure(config, Vmpooler.new_redis(redis_host))
thr.helpers.configure(config, Vmpooler.new_redis(redis_host), metrics)
thr.helpers.execute!
}
@ -21,9 +22,8 @@ manager = Thread.new {
config,
Vmpooler.new_logger(logger_file),
Vmpooler.new_redis(redis_host),
Vmpooler.new_graphite(graphite)
metrics
).execute!
}
[api, manager].each { |t| t.join }

View file

@ -53,20 +53,79 @@
:redis:
server: 'redis.company.com'
# :graphs:
#
# This section contains the server and prefix information for a graphite-
# compatible web front-end where graphs may be viewed. This is used by the
# vmpooler dashboard to retrieve statistics and graphs for a given instance.
#
# NOTE: This is not the endpoint for publishing metrics data. See `graphite:`
# and `statsd:` below.
#
# NOTE: If `graphs:` is not set, for legacy compatibility, `graphite:` will be
# consulted for `server`/`prefix` information to use in locating a
# graph server for our dashboard. `graphs:` is recommended over
# `graphite:`
#
#
# Available configuration parameters:
#
#
# - server
# The FQDN hostname of the statsd daemon.
# (required)
#
# - prefix
# The prefix to use while storing statsd data.
# (optional; default: 'vmpooler')
# :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.
# (required)
#
# - prefix
# The prefix to use while storing statsd data.
# (optional; default: 'vmpooler')
#
# - port
# The UDP port to communicate with the 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:
#
# - server
# The FQDN hostname of the Graphite server.
# (optional)
# (required)
#
# - prefix
# The prefix to use while storing Graphite data.
# (optional; default: 'vmpooler')
#
# - port
# The TCP port to communicate with the graphite server.
# (optional; default: 2003)
# Example:
@ -246,4 +305,3 @@
size: 5
timeout: 15
ready_ttl: 1440