From 8f3039e3217177660980fd04ba0143a6658fac03 Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Thu, 17 Sep 2020 15:35:21 -0400 Subject: [PATCH 001/249] Add distributed tracing (#399) This change utilizes OpenTelemetry's automatic instrumentation to add distributed tracing capabilities to VMPooler. This is a non-breaking change as traces are processed in noop mode by default. --- Gemfile | 7 ++++++ Vagrantfile | 21 ++++++++++++---- bin/vmpooler | 19 +++++++++++---- docker/Dockerfile_local | 13 ++++++++-- docker/docker-compose.yml | 45 ++++++++++++++++++++++++++++++++--- lib/vmpooler.rb | 50 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 140 insertions(+), 15 deletions(-) diff --git a/Gemfile b/Gemfile index 26eef16..f5c81be 100644 --- a/Gemfile +++ b/Gemfile @@ -16,6 +16,13 @@ gem 'nokogiri', '~> 1.10' gem 'spicy-proton', '~> 2.1' gem 'concurrent-ruby', '~> 1.1' +gem 'opentelemetry-api', '~> 0.6.0' +gem 'opentelemetry-exporter-jaeger', '~> 0.6.0' +gem 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.6.0' +gem 'opentelemetry-instrumentation-redis', '~> 0.6.0' +gem 'opentelemetry-instrumentation-sinatra', '~> 0.6.0' +gem 'opentelemetry-sdk', '~> 0.6.0' + group :development do gem 'pry' end diff --git a/Vagrantfile b/Vagrantfile index e0a9e66..aa667a6 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -1,8 +1,10 @@ # vim: autoindent tabstop=2 shiftwidth=2 expandtab softtabstop=2 filetype=ruby Vagrant.configure("2") do |config| config.vm.box = "genebean/centos-7-rvm-multi" - config.vm.network "forwarded_port", guest: 4567, host: 4567 - config.vm.network "forwarded_port", guest: 8080, host: 8080 + config.vm.network "forwarded_port", guest: 4567, host: 4567 # for when not running docker-compose + config.vm.network "forwarded_port", guest: 8080, host: 8080 # VMPooler api in docker-compose + config.vm.network "forwarded_port", guest: 8081, host: 8081 # VMPooler manager in docker-compose + config.vm.network "forwarded_port", guest: 8082, host: 8082 # Jaeger in docker-compose config.vm.provision "shell", inline: <<-SCRIPT mkdir /var/log/vmpooler chown vagrant:vagrant /var/log/vmpooler @@ -11,9 +13,18 @@ Vagrant.configure("2") do |config| usermod -aG docker vagrant systemctl enable docker systemctl start docker - docker build -t vmpooler /vagrant + curl -L "https://github.com/docker/compose/releases/download/1.26.2/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose + chmod +x /usr/local/bin/docker-compose + ln -s /usr/local/bin/docker-compose /usr/bin/docker-compose + docker-compose --version + cd /vagrant + docker-compose -f docker/docker-compose.yml build docker images - echo 'To use the container with the dummy provider do this after "vagrant ssh":' - echo "docker run -e VMPOOLER_DEBUG=true -p 8080:4567 -v /vagrant/vmpooler.yaml.dummy-example:/var/lib/vmpooler/vmpooler.yaml -e VMPOOLER_LOG='/var/log/vmpooler/vmpooler.log' -it --rm --name pooler vmpooler" SCRIPT + + # config.vm.provider "virtualbox" do |v| + # v.memory = 2048 + # v.cpus = 2 + # end + end diff --git a/bin/vmpooler b/bin/vmpooler index 4d8af45..3483349 100755 --- a/bin/vmpooler +++ b/bin/vmpooler @@ -2,30 +2,39 @@ # frozen_string_literal: true require 'vmpooler' +require 'vmpooler/version' config = Vmpooler.config +logger_file = config[:config]['logfile'] +prefix = config[:config]['prefix'] redis_host = config[:redis]['server'] redis_port = config[:redis]['port'] redis_password = config[:redis]['password'] redis_connection_pool_size = config[:redis]['connection_pool_size'] redis_connection_pool_timeout = config[:redis]['connection_pool_timeout'] redis_reconnect_attempts = config[:redis]['reconnect_attempts'] -logger_file = config[:config]['logfile'] +tracing_enabled = config[:tracing]['enabled'] +tracing_jaeger_host = config[:tracing]['jaeger_host'] logger = Vmpooler::Logger.new logger_file metrics = Vmpooler::Metrics.init(logger, config) +version = Vmpooler::VERSION + +startup_args = ARGV +Vmpooler.configure_tracing(startup_args, prefix, tracing_enabled, tracing_jaeger_host, version) + torun_threads = [] if ARGV.count == 0 torun = %i[api manager] else torun = [] - torun << :api if ARGV.include? 'api' - torun << :manager if ARGV.include? 'manager' + torun << :api if ARGV.include?('api') + torun << :manager if ARGV.include?('manager') exit(2) if torun.empty? end -if torun.include? :api +if torun.include?(:api) api = Thread.new do redis = Vmpooler.new_redis(redis_host, redis_port, redis_password) Vmpooler::API.execute(torun, config, redis, metrics, logger) @@ -39,7 +48,7 @@ elsif metrics.respond_to?(:setup_prometheus_metrics) torun_threads << prometheus_only_api end -if torun.include? :manager +if torun.include?(:manager) manager = Thread.new do Vmpooler::PoolManager.new( config, diff --git a/docker/Dockerfile_local b/docker/Dockerfile_local index 7a47516..c1b3f4e 100644 --- a/docker/Dockerfile_local +++ b/docker/Dockerfile_local @@ -15,7 +15,16 @@ COPY ./ ./ ENV RACK_ENV=production -RUN gem install bundler && bundle install && gem build vmpooler.gemspec && gem install vmpooler*.gem && \ - chmod +x /usr/local/bin/docker-entrypoint.sh +RUN apt-get update -qq && \ + apt-get install -y --no-install-recommends make && \ + apt-get clean autoclean && \ + apt-get autoremove -y && \ + rm -rf /var/lib/apt/lists/* + +RUN gem install bundler && \ + bundle install && \ + gem build vmpooler.gemspec && \ + gem install vmpooler*.gem && \ + chmod +x /usr/local/bin/docker-entrypoint.sh ENTRYPOINT ["docker-entrypoint.sh"] diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index dd00a99..21b179d 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -1,7 +1,7 @@ # For local development run with a dummy provider -version: '3.2' +version: '3.8' services: - vmpooler: + vmpooler-api: build: context: ../ dockerfile: docker/Dockerfile_local @@ -10,7 +10,7 @@ services: source: ${PWD}/vmpooler.yaml target: /etc/vmpooler/vmpooler.yaml ports: - - "4567:4567" + - "8080:4567" networks: - redis-net environment: @@ -18,7 +18,35 @@ services: - VMPOOLER_CONFIG_FILE=/etc/vmpooler/vmpooler.yaml - REDIS_SERVER=redislocal - LOGFILE=/dev/null + - JRUBY_OPTS=-Xinvokedynamic.yield=false + - VMPOOLER_TRACING_ENABLED=true + - VMPOOLER_TRACING_JAEGER_HOST=http://jaeger-aio:14268/api/traces image: vmpooler-local + command: api + depends_on: + - redislocal + vmpooler-manager: + build: + context: ../ + dockerfile: docker/Dockerfile_local + volumes: + - type: bind + source: ${PWD}/vmpooler.yaml + target: /etc/vmpooler/vmpooler.yaml + ports: + - "8081:4567" + networks: + - redis-net + environment: + - VMPOOLER_DEBUG=true # for use of dummy auth + - VMPOOLER_CONFIG_FILE=/etc/vmpooler/vmpooler.yaml + - REDIS_SERVER=redislocal + - LOGFILE=/dev/null + - JRUBY_OPTS=-Xinvokedynamic.yield=false + - VMPOOLER_TRACING_ENABLED=true + - VMPOOLER_TRACING_JAEGER_HOST=http://jaeger-aio:14268/api/traces + image: vmpooler-local + command: manager depends_on: - redislocal redislocal: @@ -29,6 +57,17 @@ services: - "6379:6379" networks: - redis-net + jaeger-aio: + image: jaegertracing/all-in-one:1.18 + ports: + - "14250:14250" + - "8082:16686" + networks: + - redis-net + user: '1001' + read_only: true + cap_drop: + - ALL networks: redis-net: diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index d6e5522..7f5ddf4 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -15,6 +15,14 @@ module Vmpooler require 'timeout' require 'yaml' + # Dependencies for tracing + require 'opentelemetry-api' + require 'opentelemetry/exporter/jaeger' + require 'opentelemetry-instrumentation-concurrent_ruby' + require 'opentelemetry-instrumentation-redis' + require 'opentelemetry-instrumentation-sinatra' + require 'opentelemetry-sdk' + %w[api metrics logger pool_manager generic_connection_pool].each do |lib| require "vmpooler/#{lib}" end @@ -103,6 +111,10 @@ module Vmpooler parsed_config[:graphite]['prefix'] = ENV['GRAPHITE_PREFIX'] if ENV['GRAPHITE_PREFIX'] parsed_config[:graphite]['port'] = string_to_int(ENV['GRAPHITE_PORT']) if ENV['GRAPHITE_PORT'] + parsed_config[:tracing] = parsed_config[:tracing] || {} + parsed_config[:tracing]['enabled'] = ENV['VMPOOLER_TRACING_ENABLED'] || parsed_config[:tracing]['enabled'] || 'false' + parsed_config[:tracing]['jaeger_host'] = ENV['VMPOOLER_TRACING_JAEGER_HOST'] || parsed_config[:tracing]['jaeger_host'] || 'http://localhost:14268/api/traces' + parsed_config[:auth] = parsed_config[:auth] || {} if ENV['AUTH_PROVIDER'] if parsed_config.key? :auth parsed_config[:auth]['provider'] = ENV['AUTH_PROVIDER'] if ENV['AUTH_PROVIDER'] @@ -213,4 +225,42 @@ module Vmpooler parsed_config[:config]['create_linked_clones'] = ENV['CREATE_LINKED_CLONES'] if ENV['CREATE_LINKED_CLONES'] =~ /true|false/ parsed_config[:config]['create_linked_clones'] = true?(parsed_config[:config]['create_linked_clones']) if parsed_config[:config]['create_linked_clones'] end + + def self.configure_tracing(startup_args, prefix, tracing_enabled, tracing_jaeger_host, version) + if startup_args.length == 1 && startup_args.include?('api') + service_name = 'vmpooler-api' + elsif startup_args.length == 1 && startup_args.include?('manager') + service_name = 'vmpooler-manager' + else + service_name = 'vmpooler' + end + + service_name += "-#{prefix}" unless prefix.empty? + + if tracing_enabled.eql?('false') + puts "Exporting of traces has been disabled so the span processor has been se to a 'NoopSpanExporter'" + span_processor = OpenTelemetry::SDK::Trace::Export::BatchSpanProcessor.new( + exporter: OpenTelemetry::SDK::Trace::Export::NoopSpanExporter.new + ) + else + puts "Exporting of traces will be done over HTTP in binary Thrift format to #{tracing_jaeger_host}" + span_processor = OpenTelemetry::SDK::Trace::Export::BatchSpanProcessor.new( + exporter: OpenTelemetry::Exporter::Jaeger::CollectorExporter.new(endpoint: tracing_jaeger_host) + ) + end + + OpenTelemetry::SDK.configure do |c| + c.use 'OpenTelemetry::Instrumentation::Sinatra' + c.use 'OpenTelemetry::Instrumentation::ConcurrentRuby' + c.use 'OpenTelemetry::Instrumentation::Redis' + + c.add_span_processor(span_processor) + c.resource = OpenTelemetry::SDK::Resources::Resource.create( + { + OpenTelemetry::SDK::Resources::Constants::SERVICE_RESOURCE[:name] => service_name, + OpenTelemetry::SDK::Resources::Constants::SERVICE_RESOURCE[:version] => version + } + ) + end + end end From 214f01c5018b7e2121ff467ce5141c2990b983f8 Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Thu, 17 Sep 2020 21:04:41 -0400 Subject: [PATCH 002/249] Add OTel resource detecors Adding this should allow OpenTelemetry to detect information about our Kubernetes environment. --- Gemfile | 1 + lib/vmpooler.rb | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index f5c81be..8c8fff3 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ gem 'opentelemetry-exporter-jaeger', '~> 0.6.0' gem 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.6.0' gem 'opentelemetry-instrumentation-redis', '~> 0.6.0' gem 'opentelemetry-instrumentation-sinatra', '~> 0.6.0' +gem 'opentelemetry-resource_detectors', '~> 0.6.0' gem 'opentelemetry-sdk', '~> 0.6.0' group :development do diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 7f5ddf4..b401a8d 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -17,11 +17,12 @@ module Vmpooler # Dependencies for tracing require 'opentelemetry-api' - require 'opentelemetry/exporter/jaeger' require 'opentelemetry-instrumentation-concurrent_ruby' require 'opentelemetry-instrumentation-redis' require 'opentelemetry-instrumentation-sinatra' require 'opentelemetry-sdk' + require 'opentelemetry/exporter/jaeger' + require 'opentelemetry/resource/detectors' %w[api metrics logger pool_manager generic_connection_pool].each do |lib| require "vmpooler/#{lib}" @@ -255,6 +256,8 @@ module Vmpooler c.use 'OpenTelemetry::Instrumentation::Redis' c.add_span_processor(span_processor) + + c.resource = OpenTelemetry::Resource::Detectors::AutoDetector.detect c.resource = OpenTelemetry::SDK::Resources::Resource.create( { OpenTelemetry::SDK::Resources::Constants::SERVICE_RESOURCE[:name] => service_name, From c49a94a819a02e5a976f283f8114b01d7d3838a6 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Fri, 18 Sep 2020 16:07:03 +0000 Subject: [PATCH 003/249] (GEM) update vmpooler version to 0.14.7 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 098c60c..18b2f6a 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.14.6' + VERSION = '0.14.7' end From 1acc70c89761465f802c4246a8d49b373b384832 Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Fri, 18 Sep 2020 13:53:56 -0400 Subject: [PATCH 004/249] Fix mixup of gem placement. (#404) It seems I should have put these in vmpooler.gemspec instead of the Gemfile. Now I know :) --- Gemfile | 8 -------- vmpooler.gemspec | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index 8c8fff3..26eef16 100644 --- a/Gemfile +++ b/Gemfile @@ -16,14 +16,6 @@ gem 'nokogiri', '~> 1.10' gem 'spicy-proton', '~> 2.1' gem 'concurrent-ruby', '~> 1.1' -gem 'opentelemetry-api', '~> 0.6.0' -gem 'opentelemetry-exporter-jaeger', '~> 0.6.0' -gem 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.6.0' -gem 'opentelemetry-instrumentation-redis', '~> 0.6.0' -gem 'opentelemetry-instrumentation-sinatra', '~> 0.6.0' -gem 'opentelemetry-resource_detectors', '~> 0.6.0' -gem 'opentelemetry-sdk', '~> 0.6.0' - group :development do gem 'pry' end diff --git a/vmpooler.gemspec b/vmpooler.gemspec index 820a8f6..29089b3 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -31,4 +31,12 @@ Gem::Specification.new do |s| s.add_dependency 'concurrent-ruby', '~> 1.1' s.add_dependency 'nokogiri', '~> 1.10' s.add_dependency 'spicy-proton', '~> 2.1' + + s.add_dependency 'opentelemetry-api', '~> 0.6.0' + s.add_dependency 'opentelemetry-exporter-jaeger', '~> 0.6.0' + s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.6.0' + s.add_dependency 'opentelemetry-instrumentation-redis', '~> 0.6.0' + s.add_dependency 'opentelemetry-instrumentation-sinatra', '~> 0.6.0' + s.add_dependency 'opentelemetry-resource_detectors', '~> 0.6.0' + s.add_dependency 'opentelemetry-sdk', '~> 0.6.0' end From b484cc599293d246e36b9753e0f48c107fbe1b98 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Fri, 18 Sep 2020 19:05:23 +0000 Subject: [PATCH 005/249] (GEM) update vmpooler version to 0.14.8 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 18b2f6a..d9c70e1 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.14.7' + VERSION = '0.14.8' end From 87abab272c1a6cd0402f905b56f43a565c382454 Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Fri, 18 Sep 2020 15:27:42 -0400 Subject: [PATCH 006/249] Adding make to the other two Dockerfiles --- docker/Dockerfile | 6 ++++++ docker/Dockerfile-aio | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index e879f1f..9f1d15b 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -17,6 +17,12 @@ COPY docker/docker-entrypoint.sh /usr/local/bin/ ENV LOGFILE=/dev/stdout \ RACK_ENV=production +RUN apt-get update -qq && \ + apt-get install -y --no-install-recommends make && \ + apt-get clean autoclean && \ + apt-get autoremove -y && \ + rm -rf /var/lib/apt/lists/* + RUN gem install vmpooler -v ${vmpooler_version} && \ chmod +x /usr/local/bin/docker-entrypoint.sh diff --git a/docker/Dockerfile-aio b/docker/Dockerfile-aio index 40f2318..48a62d6 100644 --- a/docker/Dockerfile-aio +++ b/docker/Dockerfile-aio @@ -14,17 +14,24 @@ RUN mkdir -p /var/lib/vmpooler WORKDIR /var/lib/vmpooler +RUN echo "deb http://httpredir.debian.org/debian jessie main" >/etc/apt/sources.list.d/jessie-main.list + +RUN apt-get update -qq && \ + apt-get install -y --no-install-recommends make redis-server && \ + apt-get clean autoclean && \ + apt-get autoremove -y && \ + rm -rf /var/lib/apt/lists/* + ADD Gemfile* /var/lib/vmpooler/ + RUN bundle install --system RUN ln -s /opt/jruby/bin/jruby /usr/bin/jruby -RUN echo "deb http://httpredir.debian.org/debian jessie main" >/etc/apt/sources.list.d/jessie-main.list -RUN apt-get update && apt-get install -y redis-server && rm -rf /var/lib/apt/lists/* - COPY . /var/lib/vmpooler ENV VMPOOLER_LOG /var/log/vmpooler.log + CMD \ /etc/init.d/redis-server start \ && /var/lib/vmpooler/scripts/vmpooler_init.sh start \ From 4d06c01d219f84b5b7def5d16c065a4fd998c792 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Mon, 21 Sep 2020 17:45:43 +0000 Subject: [PATCH 007/249] (GEM) update vmpooler version to 0.14.9 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index d9c70e1..29884b8 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.14.8' + VERSION = '0.14.9' end From 08132f75bd17faf9d504bc3a676f48510dbce148 Mon Sep 17 00:00:00 2001 From: Belen Bustamante Date: Tue, 25 Aug 2020 10:45:50 -0700 Subject: [PATCH 008/249] Add healthcheck endpoint, spec testing --- Gemfile | 2 +- lib/vmpooler/api.rb | 6 ++- lib/vmpooler/api/healthcheck.rb | 14 ++++++ spec/integration/api/healthcheck_spec.rb | 56 ++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 lib/vmpooler/api/healthcheck.rb create mode 100644 spec/integration/api/healthcheck_spec.rb diff --git a/Gemfile b/Gemfile index 26eef16..8e9f4b9 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ gem 'puma', '~> 4.3' gem 'rack', '~> 2.2' gem 'rake', '~> 13.0' gem 'redis', '~> 4.1' -gem 'rbvmomi', '~> 2.1' +gem 'rbvmomi', '~> 2.4', '>= 2.4.1' gem 'sinatra', '~> 2.0' gem 'prometheus-client', '~> 2.0' gem 'net-ldap', '~> 0.16' diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index 7d72ec6..eb856fc 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -3,7 +3,7 @@ module Vmpooler class API < Sinatra::Base # Load API components - %w[helpers dashboard reroute v1 request_logger].each do |lib| + %w[helpers dashboard reroute v1 request_logger healthcheck].each do |lib| require "vmpooler/api/#{lib}" end # Load dashboard components @@ -40,6 +40,10 @@ module Vmpooler require 'prometheus/middleware/exporter' use Vmpooler::Metrics::Promstats::CollectorMiddleware, metrics_prefix: "#{metrics.prometheus_prefix}_http" use Prometheus::Middleware::Exporter, path: metrics.prometheus_endpoint + # Note that a user may want to use this check without prometheus + # However, prometheus setup includes the web server which is required for this check + # At this time prometheus is a requirement of using the health check on manager + use Vmpooler::API::Healthcheck end if torun.include? :api diff --git a/lib/vmpooler/api/healthcheck.rb b/lib/vmpooler/api/healthcheck.rb new file mode 100644 index 0000000..50da734 --- /dev/null +++ b/lib/vmpooler/api/healthcheck.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Vmpooler + class API + class Healthcheck < Sinatra::Base + get '/healthcheck/?' do + content_type :json + + status 200 + JSON.pretty_generate({ 'ok' => true }) + end + end + end +end diff --git a/spec/integration/api/healthcheck_spec.rb b/spec/integration/api/healthcheck_spec.rb new file mode 100644 index 0000000..ecce07e --- /dev/null +++ b/spec/integration/api/healthcheck_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' +require 'rack/test' + +describe Vmpooler::API::Healthcheck do + include Rack::Test::Methods + + def app() + Vmpooler::API + end + + # Added to ensure no leakage in rack state from previous tests. + # Removes all routes, filters, middleware and extension hooks from the current class + # https://rubydoc.info/gems/sinatra/Sinatra/Base#reset!-class_method + before(:each) do + app.reset! + end + + let(:config) { + { + config: { + 'site_name' => 'test pooler', + 'vm_lifetime_auth' => 2, + }, + pools: [ + {'name' => 'pool1', 'size' => 5, 'alias' => ['poolone', 'poolun']}, + {'name' => 'pool2', 'size' => 10}, + {'name' => 'pool3', 'size' => 10, 'alias' => 'NotArray'} + ] + } + } + + let(:current_time) { Time.now } + + let(:metrics) { + double("metrics") + } + + before(:each) do + expect(app).to receive(:run!).once + expect(metrics).to receive(:setup_prometheus_metrics) + expect(metrics).to receive(:prometheus_prefix) + expect(metrics).to receive(:prometheus_endpoint) + app.execute([:api], config, redis, metrics, nil) + app.settings.set :config, auth: false + end + + describe '/healthcheck' do + it 'returns OK' do + get "/healthcheck" + expect(last_response.header['Content-Type']).to eq('application/json') + expect(last_response.status).to eq(200) + result = JSON.parse(last_response.body) + expect(result).to eq({'ok' => true}) + end + end +end From 338c97b33fbff17188ced92466e1ae95f2a4f3e4 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Wed, 23 Sep 2020 14:37:27 -0500 Subject: [PATCH 009/249] (maint) Centralize dependency management in the gemspec As per best practices, removed duplication between the Gemfile and gemspec. There was also some seemingly superfluous install in the Gemfile. Ordered the dependency by alphabetical order. Tested locally by deleting the .bundle and recreating successfully --- Gemfile | 35 +---------------------------------- vmpooler.gemspec | 35 ++++++++++++++++++++++------------- 2 files changed, 23 insertions(+), 47 deletions(-) diff --git a/Gemfile b/Gemfile index 26eef16..122d6b5 100644 --- a/Gemfile +++ b/Gemfile @@ -1,39 +1,6 @@ source ENV['GEM_SOURCE'] || 'https://rubygems.org' -gem 'json', '>= 1.8' -gem 'pickup', '~> 0.0.11' -gem 'puma', '~> 4.3' -gem 'rack', '~> 2.2' -gem 'rake', '~> 13.0' -gem 'redis', '~> 4.1' -gem 'rbvmomi', '~> 2.1' -gem 'sinatra', '~> 2.0' -gem 'prometheus-client', '~> 2.0' -gem 'net-ldap', '~> 0.16' -gem 'statsd-ruby', '~> 1.4.0', :require => 'statsd' -gem 'connection_pool', '~> 2.2' -gem 'nokogiri', '~> 1.10' -gem 'spicy-proton', '~> 2.1' -gem 'concurrent-ruby', '~> 1.1' - -group :development do - gem 'pry' -end - -# Test deps -group :test do - # required in order for the providers auto detect mechanism to work - gem 'vmpooler', path: './' - gem 'mock_redis', '>= 0.17.0' - gem 'rack-test', '>= 0.6' - gem 'rspec', '>= 3.2' - gem 'simplecov', '>= 0.11.2' - gem 'yarjuf', '>= 2.0' - gem 'climate_control', '>= 0.2.0' - # Rubocop would be ok jruby but for now we only use it on - # MRI or Windows platforms - gem "rubocop", :platforms => [:ruby, :x64_mingw] -end +gemspec # Evaluate Gemfile.local if it exists if File.exists? "#{__FILE__}.local" diff --git a/vmpooler.gemspec b/vmpooler.gemspec index 29089b3..a4d759f 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -17,21 +17,10 @@ Gem::Specification.new do |s| s.bindir = 'bin' s.executables = 'vmpooler' s.require_paths = ["lib"] - s.add_dependency 'pickup', '~> 0.0.11' - s.add_dependency 'puma', '~> 4.3' - s.add_dependency 'rack', '~> 2.2' - s.add_dependency 'rake', '~> 13.0' - s.add_dependency 'redis', '~> 4.1' - s.add_dependency 'rbvmomi', '>= 2.1', '< 4.0' - s.add_dependency 'sinatra', '~> 2.0' - s.add_dependency 'prometheus-client', '~> 2.0' - s.add_dependency 'net-ldap', '~> 0.16' - s.add_dependency 'statsd-ruby', '~> 1.4' - s.add_dependency 'connection_pool', '~> 2.2' s.add_dependency 'concurrent-ruby', '~> 1.1' + s.add_dependency 'connection_pool', '~> 2.2' + s.add_dependency 'net-ldap', '~> 0.16' s.add_dependency 'nokogiri', '~> 1.10' - s.add_dependency 'spicy-proton', '~> 2.1' - s.add_dependency 'opentelemetry-api', '~> 0.6.0' s.add_dependency 'opentelemetry-exporter-jaeger', '~> 0.6.0' s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.6.0' @@ -39,4 +28,24 @@ Gem::Specification.new do |s| s.add_dependency 'opentelemetry-instrumentation-sinatra', '~> 0.6.0' s.add_dependency 'opentelemetry-resource_detectors', '~> 0.6.0' s.add_dependency 'opentelemetry-sdk', '~> 0.6.0' + s.add_dependency 'pickup', '~> 0.0.11' + s.add_dependency 'prometheus-client', '~> 2.0' + s.add_dependency 'puma', '~> 4.3' + s.add_dependency 'rack', '~> 2.2' + s.add_dependency 'rake', '~> 13.0' + s.add_dependency 'rbvmomi', '>= 2.1', '< 4.0' + s.add_dependency 'redis', '~> 4.1' + s.add_dependency 'sinatra', '~> 2.0' + s.add_dependency 'spicy-proton', '~> 2.1' + s.add_dependency 'statsd-ruby', '~> 1.4' + + # Testing dependencies + s.add_development_dependency 'climate_control', '>= 0.2.0' + s.add_development_dependency 'mock_redis', '>= 0.17.0' + s.add_development_dependency 'pry' + s.add_development_dependency 'rack-test', '>= 0.6' + s.add_development_dependency 'rspec', '>= 3.2' + s.add_development_dependency 'rubocop' + s.add_development_dependency 'simplecov', '>= 0.11.2' + s.add_development_dependency 'yarjuf', '>= 2.0' end From 4e7e16e00158807b85fe23a0dabfcd1d215f0b65 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Wed, 30 Sep 2020 20:12:39 +0000 Subject: [PATCH 010/249] (GEM) update vmpooler version to 0.15.0 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 29884b8..c1f802e 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.14.9' + VERSION = '0.15.0' end From f5698d49fc63b6ed50a5bf5a6e593def513cb54f Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Thu, 8 Oct 2020 09:49:45 -0400 Subject: [PATCH 011/249] Update to OTel 0.7.0 This update includes two key benefits: 1. Spans will be named based on their route instead of the full path info thanks to https://github.com/open-telemetry/opentelemetry-ruby/pull/415 2. Helper methods were added to the configurator to simplify setting service.name and service.version --- lib/vmpooler.rb | 9 +++------ vmpooler.gemspec | 14 +++++++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index b401a8d..bbc68f8 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -257,13 +257,10 @@ module Vmpooler c.add_span_processor(span_processor) + c.service_name = service_name + c.service_version = version + c.resource = OpenTelemetry::Resource::Detectors::AutoDetector.detect - c.resource = OpenTelemetry::SDK::Resources::Resource.create( - { - OpenTelemetry::SDK::Resources::Constants::SERVICE_RESOURCE[:name] => service_name, - OpenTelemetry::SDK::Resources::Constants::SERVICE_RESOURCE[:version] => version - } - ) end end end diff --git a/vmpooler.gemspec b/vmpooler.gemspec index a4d759f..9234528 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -21,13 +21,13 @@ Gem::Specification.new do |s| s.add_dependency 'connection_pool', '~> 2.2' s.add_dependency 'net-ldap', '~> 0.16' s.add_dependency 'nokogiri', '~> 1.10' - s.add_dependency 'opentelemetry-api', '~> 0.6.0' - s.add_dependency 'opentelemetry-exporter-jaeger', '~> 0.6.0' - s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.6.0' - s.add_dependency 'opentelemetry-instrumentation-redis', '~> 0.6.0' - s.add_dependency 'opentelemetry-instrumentation-sinatra', '~> 0.6.0' - s.add_dependency 'opentelemetry-resource_detectors', '~> 0.6.0' - s.add_dependency 'opentelemetry-sdk', '~> 0.6.0' + s.add_dependency 'opentelemetry-api', '~> 0.7.0' + s.add_dependency 'opentelemetry-exporter-jaeger', '~> 0.7.0' + s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.7.0' + s.add_dependency 'opentelemetry-instrumentation-redis', '~> 0.7.0' + s.add_dependency 'opentelemetry-instrumentation-sinatra', '~> 0.7.0' + s.add_dependency 'opentelemetry-resource_detectors', '~> 0.7.0' + s.add_dependency 'opentelemetry-sdk', '~> 0.7.0' s.add_dependency 'pickup', '~> 0.0.11' s.add_dependency 'prometheus-client', '~> 2.0' s.add_dependency 'puma', '~> 4.3' From ccdf8a4ccbbff85cc81f40a3d40d885545fa088f Mon Sep 17 00:00:00 2001 From: Jenkins Date: Thu, 8 Oct 2020 19:02:28 +0000 Subject: [PATCH 012/249] (GEM) update vmpooler version to 0.16.0 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index c1f802e..86e961d 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.15.0' + VERSION = '0.16.0' end From a8bdfc16472ed96d3d090051818c5dde9bba87bd Mon Sep 17 00:00:00 2001 From: Jenkins Date: Thu, 8 Oct 2020 19:06:50 +0000 Subject: [PATCH 013/249] (GEM) update vmpooler version to 0.16.1 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 86e961d..caa4857 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.16.0' + VERSION = '0.16.1' end From cb1f19ad1f4ed33106b7cacea90830778b9d853e Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Thu, 8 Oct 2020 18:13:33 -0400 Subject: [PATCH 014/249] Bump OTel Sinatra to 0.7.1 This is to pull in the bug fix in https://github.com/open-telemetry/opentelemetry-ruby/pull/434 so that the new feature of naming spans based on their Sinatra route actually works. --- vmpooler.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vmpooler.gemspec b/vmpooler.gemspec index 9234528..f6fedeb 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -25,7 +25,7 @@ Gem::Specification.new do |s| s.add_dependency 'opentelemetry-exporter-jaeger', '~> 0.7.0' s.add_dependency 'opentelemetry-instrumentation-concurrent_ruby', '~> 0.7.0' s.add_dependency 'opentelemetry-instrumentation-redis', '~> 0.7.0' - s.add_dependency 'opentelemetry-instrumentation-sinatra', '~> 0.7.0' + s.add_dependency 'opentelemetry-instrumentation-sinatra', '~> 0.7.1' s.add_dependency 'opentelemetry-resource_detectors', '~> 0.7.0' s.add_dependency 'opentelemetry-sdk', '~> 0.7.0' s.add_dependency 'pickup', '~> 0.0.11' From 35104a75bbc6579d49f5cb8f7147149e307b4bd1 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Thu, 8 Oct 2020 22:44:29 +0000 Subject: [PATCH 015/249] (GEM) update vmpooler version to 0.16.2 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index caa4857..b629599 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.16.1' + VERSION = '0.16.2' end From 0ad069b958494afbf8cb7ace197b9dd7413a62dc Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Tue, 13 Oct 2020 13:59:13 -0700 Subject: [PATCH 016/249] (POOLER-191) Add checking for running instances that are not in active This change adds detection of running instances that are in a running queue, but have no data in a active queue for the same pool. When this happens a machine will live forever, impacting the running count, and preventing the machine from being killed. Without this change running instances that are not marked as active will live forever. --- lib/vmpooler/pool_manager.rb | 18 +++++++----------- spec/unit/pool_manager_spec.rb | 12 ++++++++++-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 95927d4..a7773f1 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -289,19 +289,15 @@ module Vmpooler move_vm_queue(pool, vm, 'running', 'completed', redis, "reached end of TTL after #{ttl} hours") throw :stop_checking end - end - - if provider.vm_ready?(pool, vm) - throw :stop_checking else - host = provider.get_vm(pool, vm) - - if host - throw :stop_checking - else - move_vm_queue(pool, vm, 'running', 'completed', redis, 'is no longer in inventory, removing from running') - end + move_vm_queue(pool, vm, 'running', 'completed', redis, 'is listed as running, but has no checkouttime data. Removing from running') end + + throw :stop_checking if provider.vm_ready?(pool, vm) + + throw :stop_checking if provider.get_vm(pool, vm) + + move_vm_queue(pool, vm, 'running', 'completed', redis, 'is no longer in inventory, removing from running') end end end diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index e8a17a1..3fb44da 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -647,12 +647,20 @@ EOT end context 'valid host' do - it 'should not move VM if it has no checkout time' do + it 'should kill a VM if it has no checkout time' do redis_connection_pool.with do |redis| expect(provider).to receive(:vm_ready?).and_return(true) expect(redis.sismember("vmpooler__running__#{pool}", vm)).to be(true) subject._check_running_vm(vm, pool, 0, provider) - expect(redis.sismember("vmpooler__running__#{pool}", vm)).to be(true) + expect(redis.sismember("vmpooler__running__#{pool}", vm)).to be(false) + end + end + + it 'should log a message when the machine is removed due to no active data' do + redis_connection_pool.with do |redis| + expect(provider).to receive(:vm_ready?).and_return(true) + expect(logger).to receive(:log).with('d',"[!] [#{pool}] '#{vm}' is listed as running, but has no checkouttime data. Removing from running") + subject._check_running_vm(vm, pool, 0, provider) end end From a42ed6cb5d7f21a370644060fe5d6b30e5a8d265 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Wed, 14 Oct 2020 19:26:09 +0000 Subject: [PATCH 017/249] (GEM) update vmpooler version to 0.16.3 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index b629599..ca4b1be 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.16.2' + VERSION = '0.16.3' end From b2ac53fa764f97919903cb804e332fb5e91209fd Mon Sep 17 00:00:00 2001 From: suckatrash Date: Tue, 13 Oct 2020 13:16:59 -0700 Subject: [PATCH 018/249] (DIO-1059) Optionally add snapshot tuning params at clone time --- lib/vmpooler/providers/vsphere.rb | 41 +++++++++++++++++++++-------- spec/unit/providers/vsphere_spec.rb | 31 ++++++++++++++++++++++ vmpooler.yaml.example | 11 ++++++++ 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index 63373ec..467a6b2 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -306,19 +306,26 @@ module Vmpooler template_vm_object = find_template_vm(pool, connection) + extra_config = [ + { key: 'guestinfo.hostname', value: new_vmname } + ] + + if pool.key?('snapshot_mainMem_ioBlockPages') + ioblockpages = pool['snapshot_mainMem_ioBlockPages'] + extra_config.push( + { key: 'mainMem.ioBlockPages', value: ioblockpages } + ) + end + if pool.key?('snapshot_mainMem_iowait') + iowait = pool['snapshot_mainMem_iowait'] + extra_config.push( + { key: 'mainMem.iowait', value: iowait } + ) + end + # Annotate with creation time, origin template, etc. # Add extraconfig options that can be queried by vmtools - config_spec = RbVmomi::VIM.VirtualMachineConfigSpec( - annotation: JSON.pretty_generate( - name: new_vmname, - created_by: provider_config['username'], - base_template: template_path, - creation_timestamp: Time.now.utc - ), - extraConfig: [ - { key: 'guestinfo.hostname', value: new_vmname } - ] - ) + config_spec = create_config_spec(new_vmname, template_path, extra_config) # Check if alternate network configuration is specified and add configuration if pool.key?('network') @@ -358,6 +365,18 @@ module Vmpooler vm_hash end + def create_config_spec(vm_name, template_name, extra_config) + RbVmomi::VIM.VirtualMachineConfigSpec( + annotation: JSON.pretty_generate( + name: vm_name, + created_by: provider_config['username'], + base_template: template_name, + creation_timestamp: Time.now.utc + ), + extraConfig: extra_config + ) + end + def create_relocate_spec(target_datastore, target_datacenter_name, pool_name, connection) pool = pool_config(pool_name) target_cluster_name = get_target_cluster_from_config(pool_name) diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb index a2970ac..751a7ec 100644 --- a/spec/unit/providers/vsphere_spec.rb +++ b/spec/unit/providers/vsphere_spec.rb @@ -689,6 +689,37 @@ EOT expect(result['name']).to eq(vmname) end end + + context 'Given valid snapshot tuning settings' do + let(:folder_object) { mock_RbVmomi_VIM_Folder({ :name => 'pool1'}) } + let(:now) { Time.now } + let(:config_spec) { + [ + { key: 'guestinfo.hostname', value: vmname }, + { key: 'mainMem.ioBlockPages', value: '300' }, + { key: 'mainMem.iowait', value: '2' } + ] + } + + before(:each) do + template_vm = new_template_object + allow(subject).to receive(:find_template_vm).and_return(new_template_object) + allow(template_vm).to receive(:CloneVM_Task).and_return(clone_vm_task) + allow(clone_vm_task).to receive(:wait_for_completion).and_return(new_vm_object) + allow(subject).to receive(:find_vm_folder).and_return(folder_object) + allow(Time).to receive(:now).and_return(Time.now) + config[:pools][0]['snapshot_mainMem_ioBlockPages'] = '300' + config[:pools][0]['snapshot_mainMem_iowait'] = '2' + end + + it 'should apply the appropriate extraConfig settings' do + result = subject.create_config_spec(vmname, "template1", config_spec) + expect(result.extraConfig).to include( + { :key => 'mainMem.ioBlockPages', :value => '300' }, + { :key => 'mainMem.iowait', :value => '2'} + ) + end + end end describe '#set_network_device' do diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index 6194167..db50188 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -654,6 +654,15 @@ # The datacenter within vCenter to manage VMs. # (optional: default is the first datacenter in vSphere) # +# - snapshot_mainMem_ioBlockPages +# Provisions VMs with the option "mainMem.ioBlockPages". This setting can be useful +# (paired with mainMem.iowait below) for tuning the performance of snapshot management actions. +# See: https://kb.vmware.com/s/article/76687 +# +# - snapshot_mainMem_iowait +# Provisions VMs with the option "mainMem.iowait". This setting can be useful +# for tuning the performance of snapshot management actions +# # Example: :pools: @@ -677,3 +686,5 @@ ready_ttl: 1440 provider: vsphere create_linked_clone: false + snapshot_mainMem_ioBlockPages: '2048' + snapshot_mainMem_iowait: '0' From 981d110bbfc66b8e921d4e50252cab8e9964bebc Mon Sep 17 00:00:00 2001 From: Jenkins Date: Tue, 20 Oct 2020 15:27:00 +0000 Subject: [PATCH 019/249] (GEM) update vmpooler version to 0.17.0 --- lib/vmpooler/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index ca4b1be..36d1b0b 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '0.16.3' + VERSION = '0.17.0' end From fbb5212037535fc84cd2c3ab73d9b638d44fb312 Mon Sep 17 00:00:00 2001 From: Belen Bustamante Date: Wed, 21 Oct 2020 16:19:39 -0700 Subject: [PATCH 020/249] Add lightstep gh action --- .github/workflows/lightstep.yml | 29 +++++++++++++++++++++++++++++ .lightstep.yml | 10 ++++++++++ vmpooler.gemspec | 2 +- 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/lightstep.yml create mode 100644 .lightstep.yml diff --git a/.github/workflows/lightstep.yml b/.github/workflows/lightstep.yml new file mode 100644 index 0000000..d32f2b1 --- /dev/null +++ b/.github/workflows/lightstep.yml @@ -0,0 +1,29 @@ +name: Verify Pre-Deploy Status + +on: pull_request + +jobs: + build: + runs-on: ubuntu-latest + + steps: + # Checkout repo + - name: Checkout + uses: actions/checkout@v2 + + # Run checks + - name: Lightstep Pre-Deploy Check + uses: lightstep/lightstep-action-predeploy@v0.1.4 + id: lightstep-predeploy + with: + lightstep_api_key: ${{ secrets.LIGHTSTEP_API_KEY }} + pagerduty_api_token: ${{ secrets.PAGERDUTY_API_TOKEN }} + + # Output status as a comment + - name: Add a Comment + uses: unsplash/comment-on-pr@master + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + msg: ${{ steps.lightstep-predeploy.outputs.lightstep_predeploy_md }} + check_for_duplicate_msg: true diff --git a/.lightstep.yml b/.lightstep.yml new file mode 100644 index 0000000..c3d2ebc --- /dev/null +++ b/.lightstep.yml @@ -0,0 +1,10 @@ +organization: Puppet Inc +project: puppet-inc-prod +conditions: + # API /status error >0% + - SZcJVQfy +integrations: + pagerduty: + # VMPooler service + service: P714ID4 + diff --git a/vmpooler.gemspec b/vmpooler.gemspec index f6fedeb..39127d3 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -45,7 +45,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'pry' s.add_development_dependency 'rack-test', '>= 0.6' s.add_development_dependency 'rspec', '>= 3.2' - s.add_development_dependency 'rubocop' + s.add_development_dependency 'rubocop', '< 1.0' s.add_development_dependency 'simplecov', '>= 0.11.2' s.add_development_dependency 'yarjuf', '>= 2.0' end From 0a323f6052fcb08e885950f525b6f87df027425a Mon Sep 17 00:00:00 2001 From: Samuel Date: Fri, 23 Oct 2020 11:40:11 -0500 Subject: [PATCH 021/249] (maint) Speedup the tagging method (#422) * (maint) Speedup the tagging method While looking at the instrumentation data for the ABS queue processor, I noticed a lot of time spent in the HTTP PUT method, which in the code was easy to isolate, as it is only used via the vmpooler tagging functions ie the API /vm/foobar/ with 'tag' key-value pairs. While I'm not sure the original hset() make sense to me, there was an easy way to speed them up by using pipelined. I would expect a very good speed increase with this turned on. * tag rubocop to <1.0 because the 1.0 version returns 130 new offenses --- lib/vmpooler/api/helpers.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index 1fd4325..a667dcb 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -114,11 +114,13 @@ module Vmpooler end def export_tags(backend, hostname, tags) - tags.each_pair do |tag, value| - next if value.nil? or value.empty? + backend.pipelined do + tags.each_pair do |tag, value| + next if value.nil? or value.empty? - backend.hset('vmpooler__vm__' + hostname, 'tag:' + tag, value) - backend.hset('vmpooler__tag__' + Date.today.to_s, hostname + ':' + tag, value) + backend.hset('vmpooler__vm__' + hostname, 'tag:' + tag, value) + backend.hset('vmpooler__tag__' + Date.today.to_s, hostname + ':' + tag, value) + end end end From 2a6d610b7abc397b44d506a8f17bba78b0a9efd5 Mon Sep 17 00:00:00 2001 From: Belen Bustamante Date: Fri, 23 Oct 2020 09:50:31 -0700 Subject: [PATCH 022/249] Rubocop fix --- lib/vmpooler.rb | 20 +++--- lib/vmpooler/api/dashboard.rb | 36 +++++----- lib/vmpooler/api/helpers.rb | 24 +++---- lib/vmpooler/api/v1.rb | 80 ++++++++++----------- lib/vmpooler/metrics/dummy_statsd.rb | 3 - lib/vmpooler/metrics/graphite.rb | 2 + lib/vmpooler/metrics/promstats.rb | 2 + lib/vmpooler/metrics/statsd.rb | 8 ++- lib/vmpooler/pool_manager.rb | 103 +++++++++++++-------------- lib/vmpooler/providers/dummy.rb | 58 ++++++--------- lib/vmpooler/providers/vsphere.rb | 45 +++++------- spec/rbvmomi_helper.rb | 2 +- spec/unit/pool_manager_spec.rb | 14 ++-- spec/unit/providers/vsphere_spec.rb | 8 +-- vmpooler.gemspec | 2 +- 15 files changed, 182 insertions(+), 225 deletions(-) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index bbc68f8..cee8db8 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -53,17 +53,13 @@ module Vmpooler # Bail out if someone attempts to start vmpooler with dummy authentication # without enbaling debug mode. - if parsed_config.key? :auth - if parsed_config[:auth]['provider'] == 'dummy' - unless ENV['VMPOOLER_DEBUG'] - warning = [ - 'Dummy authentication should not be used outside of debug mode', - 'please set environment variable VMPOOLER_DEBUG to \'true\' if you want to use dummy authentication' - ] + if parsed_config.key?(:auth) && parsed_config[:auth]['provider'] == 'dummy' && !ENV['VMPOOLER_DEBUG'] + warning = [ + 'Dummy authentication should not be used outside of debug mode', + 'please set environment variable VMPOOLER_DEBUG to \'true\' if you want to use dummy authentication' + ] - raise warning.join(";\s") - end - end + raise warning.join(";\s") end # Set some configuration defaults @@ -140,14 +136,14 @@ module Vmpooler parsed_config[:pool_names] << pool['name'] pool['ready_ttl'] ||= parsed_config[:config]['ready_ttl'] if pool['alias'] - if pool['alias'].is_a?(Array) + if pool['alias'].instance_of?(Array) pool['alias'].each do |pool_alias| parsed_config[:alias] ||= {} parsed_config[:alias][pool_alias] = [pool['name']] unless parsed_config[:alias].key? pool_alias parsed_config[:alias][pool_alias] << pool['name'] unless parsed_config[:alias][pool_alias].include? pool['name'] parsed_config[:pool_names] << pool_alias end - elsif pool['alias'].is_a?(String) + elsif pool['alias'].instance_of?(String) parsed_config[:alias][pool['alias']] = pool['name'] parsed_config[:pool_names] << pool['alias'] end diff --git a/lib/vmpooler/api/dashboard.rb b/lib/vmpooler/api/dashboard.rb index 4c56798..f5e65cf 100644 --- a/lib/vmpooler/api/dashboard.rb +++ b/lib/vmpooler/api/dashboard.rb @@ -128,34 +128,32 @@ module Vmpooler pools.each do |pool| running = running_hash[pool['name']] - pool['major'] = Regexp.last_match[1] if pool['name'] =~ /^(\w+)\-/ + 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 graph_url - begin - buffer = URI.parse(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'] ||= [] + if params[:history] && graph_url + begin + buffer = URI.parse(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'] ||= [] - 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 + 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 - rescue StandardError end + rescue StandardError end end JSON.pretty_generate(result) diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index 1fd4325..0f23842 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -13,12 +13,12 @@ module Vmpooler def valid_token?(backend) return false unless has_token? - backend.exists?('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN']) ? true : false + backend.exists?("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}") ? true : false end def validate_token(backend) if valid_token?(backend) - backend.hset('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN'], 'last', Time.now) + backend.hset("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}last#{Time.now}") return true end @@ -117,8 +117,8 @@ module Vmpooler tags.each_pair do |tag, value| next if value.nil? or value.empty? - backend.hset('vmpooler__vm__' + hostname, 'tag:' + tag, value) - backend.hset('vmpooler__tag__' + Date.today.to_s, hostname + ':' + tag, value) + backend.hset("vmpooler__vm__#{hostname}", "tag:#{tag}", value) + backend.hset("vmpooler__tag__#{Date.today}", "#{hostname}:#{tag}", value) end end @@ -145,7 +145,7 @@ module Vmpooler def hostname_shorten(hostname, domain=nil) if domain && hostname =~ /^[\w-]+\.#{domain}$/ - hostname = hostname[/[^\.]+/] + hostname = hostname[/[^.]+/] end hostname @@ -251,17 +251,15 @@ module Vmpooler tags = {} - backend.hgetall('vmpooler__tag__' + date_str).each do |key, value| + backend.hgetall("vmpooler__tag__#{date_str}").each do |key, value| hostname = 'unknown' tag = 'unknown' - if key =~ /\:/ + if key =~ /:/ hostname, tag = key.split(':', 2) end - if opts[:only] - next unless tag == opts[:only] - end + next if opts[:only] && tag != opts[:only] tags[tag] ||= {} tags[tag][value] ||= 0 @@ -319,7 +317,7 @@ module Vmpooler } } - task[:count][:total] = backend.hlen('vmpooler__' + task_str + '__' + date_str).to_i + task[:count][:total] = backend.hlen("vmpooler__#{task_str}__#{date_str}").to_i if task[:count][:total] > 0 if opts[:bypool] == true @@ -328,11 +326,11 @@ module Vmpooler task[:count][:pool] = {} task[:duration][:pool] = {} - backend.hgetall('vmpooler__' + task_str + '__' + date_str).each do |key, value| + backend.hgetall("vmpooler__#{task_str}__#{date_str}").each do |key, value| pool = 'unknown' hostname = 'unknown' - if key =~ /\:/ + if key =~ /:/ pool, hostname = key.split(':') else hostname = key diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 066af86..1f4a23e 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -174,13 +174,13 @@ module Vmpooler if Vmpooler::API.settings.config[:auth] and has_token? validate_token(backend) - backend.hset('vmpooler__vm__' + vm, 'token:token', request.env['HTTP_X_AUTH_TOKEN']) - backend.hset('vmpooler__vm__' + vm, 'token:user', - backend.hget('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN'], 'user') + backend.hset("vmpooler__vm__#{vm}", 'token:token', request.env['HTTP_X_AUTH_TOKEN']) + backend.hset("vmpooler__vm__#{vm}", 'token:user', + backend.hget("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}", 'user') ) if config['vm_lifetime_auth'].to_i > 0 - backend.hset('vmpooler__vm__' + vm, 'lifetime', config['vm_lifetime_auth'].to_i) + backend.hset("vmpooler__vm__#{vm}", 'lifetime', config['vm_lifetime_auth'].to_i) end end end @@ -205,11 +205,11 @@ module Vmpooler vmname, vmpool, vmtemplate = fetch_single_vm(requested) if !vmname failed = true - metrics.increment('checkout.empty.' + requested) + metrics.increment("checkout.empty.#{requested}") break else vms << [vmpool, vmname, vmtemplate] - metrics.increment('checkout.success.' + vmtemplate) + metrics.increment("checkout.success.#{vmtemplate}") end end end @@ -337,7 +337,7 @@ module Vmpooler payload&.each do |poolname, count| next unless count.to_i > config['max_ondemand_instances_per_request'] - metrics.increment('ondemandrequest_fail.toomanyrequests.' + poolname) + metrics.increment("ondemandrequest_fail.toomanyrequests.#{poolname}") return true end false @@ -380,7 +380,7 @@ module Vmpooler if Vmpooler::API.settings.config[:auth] and has_token? backend.hset("vmpooler__odrequest__#{request_id}", 'token:token', request.env['HTTP_X_AUTH_TOKEN']) backend.hset("vmpooler__odrequest__#{request_id}", 'token:user', - backend.hget('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN'], 'user')) + backend.hget("vmpooler__token__#{request.env['HTTP_X_AUTH_TOKEN']}", 'user')) end result['domain'] = config['domain'] if config['domain'] @@ -542,9 +542,9 @@ module Vmpooler if subpool.include?(p['name']) true elsif !p['alias'].nil? - if p['alias'].is_a?(Array) + if p['alias'].instance_of?(Array) (p['alias'] & subpool).any? - elsif p['alias'].is_a?(String) + elsif p['alias'].instance_of?(String) subpool.include?(p['alias']) end end @@ -727,14 +727,14 @@ module Vmpooler result = { 'ok' => false } if Vmpooler::API.settings.config[:auth] - token = backend.hgetall('vmpooler__token__' + params[:token]) + token = backend.hgetall("vmpooler__token__#{params[:token]}") if not token.nil? and not token.empty? status 200 pools.each do |pool| - backend.smembers('vmpooler__running__' + pool['name']).each do |vm| - if backend.hget('vmpooler__vm__' + vm, 'token:token') == params[:token] + backend.smembers("vmpooler__running__#{pool['name']}").each do |vm| + if backend.hget("vmpooler__vm__#{vm}", 'token:token') == params[:token] token['vms'] ||= {} token['vms']['running'] ||= [] token['vms']['running'].push(vm) @@ -760,7 +760,7 @@ module Vmpooler need_auth! - if backend.del('vmpooler__token__' + params[:token]).to_i > 0 + if backend.del("vmpooler__token__#{params[:token]}").to_i > 0 status 200 result['ok'] = true end @@ -783,8 +783,8 @@ module Vmpooler o = [('a'..'z'), ('0'..'9')].map(&:to_a).flatten result['token'] = o[rand(25)] + (0...31).map { o[rand(o.length)] }.join - backend.hset('vmpooler__token__' + result['token'], 'user', @auth.username) - backend.hset('vmpooler__token__' + result['token'], 'created', Time.now) + backend.hset("vmpooler__token__#{result['token']}", 'user', @auth.username) + backend.hset("vmpooler__token__#{result['token']}", 'created', Time.now) status 200 result['ok'] = true @@ -823,7 +823,7 @@ module Vmpooler else result[:bad_templates] = invalid invalid.each do |bad_template| - metrics.increment('ondemandrequest_fail.invalid.' + bad_template) + metrics.increment("ondemandrequest_fail.invalid.#{bad_template}") end status 404 end @@ -858,7 +858,7 @@ module Vmpooler else result[:bad_templates] = invalid invalid.each do |bad_template| - metrics.increment('ondemandrequest_fail.invalid.' + bad_template) + metrics.increment("ondemandrequest_fail.invalid.#{bad_template}") end status 404 end @@ -904,7 +904,7 @@ module Vmpooler result = atomically_allocate_vms(payload) else invalid.each do |bad_template| - metrics.increment('checkout.invalid.' + bad_template) + metrics.increment("checkout.invalid.#{bad_template}") end status 404 end @@ -980,7 +980,8 @@ module Vmpooler result['ok'] = true status 202 - if request_hash['status'] == 'ready' + case request_hash['status'] + when 'ready' result['ready'] = true Parsing.get_platform_pool_count(request_hash['requested']) do |platform_alias, pool, _count| instances = backend.smembers("vmpooler__#{request_id}__#{platform_alias}__#{pool}") @@ -993,10 +994,10 @@ module Vmpooler end result['domain'] = config['domain'] if config['domain'] status 200 - elsif request_hash['status'] == 'failed' + when 'failed' result['message'] = "The request failed to provision instances within the configured ondemand_request_ttl '#{config['ondemand_request_ttl']}'" status 200 - elsif request_hash['status'] == 'deleted' + when 'deleted' result['message'] = 'The request has been deleted' status 200 else @@ -1059,7 +1060,7 @@ module Vmpooler result = atomically_allocate_vms(payload) else invalid.each do |bad_template| - metrics.increment('checkout.invalid.' + bad_template) + metrics.increment("checkout.invalid.#{bad_template}") end status 404 end @@ -1082,7 +1083,7 @@ module Vmpooler params[:hostname] = hostname_shorten(params[:hostname], config['domain']) - rdata = backend.hgetall('vmpooler__vm__' + params[:hostname]) + rdata = backend.hgetall("vmpooler__vm__#{params[:hostname]}") unless rdata.empty? status 200 result['ok'] = true @@ -1155,12 +1156,12 @@ module Vmpooler params[:hostname] = hostname_shorten(params[:hostname], config['domain']) - rdata = backend.hgetall('vmpooler__vm__' + params[:hostname]) + rdata = backend.hgetall("vmpooler__vm__#{params[:hostname]}") unless rdata.empty? need_token! if rdata['token:token'] - if backend.srem('vmpooler__running__' + rdata['template'], params[:hostname]) - backend.sadd('vmpooler__completed__' + rdata['template'], params[:hostname]) + if backend.srem("vmpooler__running__#{rdata['template']}", params[:hostname]) + backend.sadd("vmpooler__completed__#{rdata['template']}", params[:hostname]) status 200 result['ok'] = true @@ -1184,7 +1185,7 @@ module Vmpooler params[:hostname] = hostname_shorten(params[:hostname], config['domain']) - if backend.exists?('vmpooler__vm__' + params[:hostname]) + if backend.exists?("vmpooler__vm__#{params[:hostname]}") begin jdata = JSON.parse(request.body.read) rescue StandardError @@ -1212,13 +1213,8 @@ module Vmpooler end when 'tags' - unless arg.is_a?(Hash) - failure.push("You provided tags (#{arg}) as something other than a hash.") - end - - if config['allowed_tags'] - failure.push("You provided unsuppored tags (#{arg}).") if not (arg.keys - config['allowed_tags']).empty? - end + failure.push("You provided tags (#{arg}) as something other than a hash.") unless arg.is_a?(Hash) + failure.push("You provided unsuppored tags (#{arg}).") if config['allowed_tags'] && !(arg.keys - config['allowed_tags']).empty? else failure.push("Unknown argument #{arg}.") end @@ -1235,7 +1231,7 @@ module Vmpooler arg = arg.to_i - backend.hset('vmpooler__vm__' + params[:hostname], param, arg) + backend.hset("vmpooler__vm__#{params[:hostname]}", param, arg) when 'tags' filter_tags(arg) export_tags(backend, params[:hostname], arg) @@ -1261,11 +1257,11 @@ module Vmpooler params[:hostname] = hostname_shorten(params[:hostname], config['domain']) - if ((params[:size].to_i > 0 )and (backend.exists?('vmpooler__vm__' + params[:hostname]))) + if ((params[:size].to_i > 0 )and (backend.exists?("vmpooler__vm__#{params[:hostname]}"))) result[params[:hostname]] = {} result[params[:hostname]]['disk'] = "+#{params[:size]}gb" - backend.sadd('vmpooler__tasks__disk', params[:hostname] + ':' + params[:size]) + backend.sadd('vmpooler__tasks__disk', "#{params[:hostname]}:#{params[:size]}") status 202 result['ok'] = true @@ -1285,13 +1281,13 @@ module Vmpooler params[:hostname] = hostname_shorten(params[:hostname], config['domain']) - if backend.exists?('vmpooler__vm__' + params[:hostname]) + if backend.exists?("vmpooler__vm__#{params[:hostname]}") result[params[:hostname]] = {} o = [('a'..'z'), ('0'..'9')].map(&:to_a).flatten result[params[:hostname]]['snapshot'] = o[rand(25)] + (0...31).map { o[rand(o.length)] }.join - backend.sadd('vmpooler__tasks__snapshot', params[:hostname] + ':' + result[params[:hostname]]['snapshot']) + backend.sadd('vmpooler__tasks__snapshot', "#{params[:hostname]}:#{result[params[:hostname]]['snapshot']}") status 202 result['ok'] = true @@ -1311,8 +1307,8 @@ module Vmpooler params[:hostname] = hostname_shorten(params[:hostname], config['domain']) - unless backend.hget('vmpooler__vm__' + params[:hostname], 'snapshot:' + params[:snapshot]).to_i.zero? - backend.sadd('vmpooler__tasks__snapshot-revert', params[:hostname] + ':' + params[:snapshot]) + unless backend.hget("vmpooler__vm__#{params[:hostname]}", "snapshot:#{params[:snapshot]}").to_i.zero? + backend.sadd('vmpooler__tasks__snapshot-revert', "#{params[:hostname]}:#{params[:snapshot]}") status 202 result['ok'] = true diff --git a/lib/vmpooler/metrics/dummy_statsd.rb b/lib/vmpooler/metrics/dummy_statsd.rb index 96f2eaa..ad866b0 100644 --- a/lib/vmpooler/metrics/dummy_statsd.rb +++ b/lib/vmpooler/metrics/dummy_statsd.rb @@ -5,9 +5,6 @@ module Vmpooler class DummyStatsd < Metrics attr_reader :server, :port, :prefix - def initialize(*) - end - def increment(*) true end diff --git a/lib/vmpooler/metrics/graphite.rb b/lib/vmpooler/metrics/graphite.rb index 128805d..88e66e3 100644 --- a/lib/vmpooler/metrics/graphite.rb +++ b/lib/vmpooler/metrics/graphite.rb @@ -7,6 +7,7 @@ module Vmpooler class Graphite < Metrics attr_reader :server, :port, :prefix + # rubocop:disable Lint/MissingSuper def initialize(logger, params = {}) raise ArgumentError, "Graphite server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty? @@ -15,6 +16,7 @@ module Vmpooler @prefix = params['prefix'] || 'vmpooler' @logger = logger end + # rubocop:enable Lint/MissingSuper def increment(label) log label, 1 diff --git a/lib/vmpooler/metrics/promstats.rb b/lib/vmpooler/metrics/promstats.rb index bee432f..253879c 100644 --- a/lib/vmpooler/metrics/promstats.rb +++ b/lib/vmpooler/metrics/promstats.rb @@ -23,6 +23,7 @@ module Vmpooler @p_metrics = {} @torun = [] + # rubocop:disable Lint/MissingSuper def initialize(logger, params = {}) @prefix = params['prefix'] || 'vmpooler' @prometheus_prefix = params['prometheus_prefix'] || 'vmpooler' @@ -32,6 +33,7 @@ module Vmpooler # Setup up prometheus registry and data structures @prometheus = Prometheus::Client.registry end +# rubocop:enable Lint/MissingSuper =begin # rubocop:disable Style/BlockComments The Metrics table is used to register metrics and translate/interpret the incoming metrics. diff --git a/lib/vmpooler/metrics/statsd.rb b/lib/vmpooler/metrics/statsd.rb index 85b705d..b469c38 100644 --- a/lib/vmpooler/metrics/statsd.rb +++ b/lib/vmpooler/metrics/statsd.rb @@ -8,6 +8,7 @@ module Vmpooler class Statsd < Metrics attr_reader :server, :port, :prefix + # rubocop:disable Lint/MissingSuper def initialize(logger, params = {}) raise ArgumentError, "Statsd server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty? @@ -17,21 +18,22 @@ module Vmpooler @server = ::Statsd.new(host, @port) @logger = logger end + # rubocop:enable Lint/MissingSuper def increment(label) - server.increment(prefix + '.' + label) + server.increment("#{prefix}.#{label}") rescue StandardError => e @logger.log('s', "[!] Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}") end def gauge(label, value) - server.gauge(prefix + '.' + label, value) + server.gauge("#{prefix}.#{label}", value) rescue StandardError => e @logger.log('s', "[!] Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}") end def timing(label, duration) - server.timing(prefix + '.' + label, duration) + server.timing("#{prefix}.#{label}", duration) rescue StandardError => e @logger.log('s', "[!] Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}") end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index a7773f1..6e969ff 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -108,7 +108,7 @@ module Vmpooler $logger.log('d', "[!] [#{pool}] '#{vm}' no longer exists. Removing from pending.") end - def fail_pending_vm(vm, pool, timeout, redis, exists = true) + def fail_pending_vm(vm, pool, timeout, redis, exists: true) clone_stamp = redis.hget("vmpooler__vm__#{vm}", 'clone') time_since_clone = (Time.now - Time.parse(clone_stamp)) / 60 @@ -117,7 +117,7 @@ module Vmpooler request_id = redis.hget("vmpooler__vm__#{vm}", 'request_id') pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias') if request_id redis.multi - redis.smove('vmpooler__pending__' + pool, 'vmpooler__completed__' + pool, vm) + redis.smove("vmpooler__pending__#{pool}", "vmpooler__completed__#{pool}", vm) redis.zadd('vmpooler__odcreate__task', 1, "#{pool_alias}:#{pool}:1:#{request_id}") if request_id redis.exec $metrics.increment("errors.markedasfailed.#{pool}") @@ -133,15 +133,16 @@ module Vmpooler end def move_pending_vm_to_ready(vm, pool, redis, request_id = nil) - clone_time = redis.hget('vmpooler__vm__' + vm, 'clone') + clone_time = redis.hget("vmpooler__vm__#{vm}", 'clone') finish = format('%