From b2ac53fa764f97919903cb804e332fb5e91209fd Mon Sep 17 00:00:00 2001 From: suckatrash Date: Tue, 13 Oct 2020 13:16:59 -0700 Subject: [PATCH 001/232] (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 002/232] (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 003/232] 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 004/232] (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 005/232] 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('%