From 33b04b384cb94b00b8afcff042a06fa498077d8b Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Thu, 22 Aug 2019 14:34:08 -0700 Subject: [PATCH 001/371] Add CODEOWNERS file to vmpooler --- CODEOWNERS | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 CODEOWNERS diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000..a113f39 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,9 @@ + +# This will cause QE to be assigned review of any opened PRs against +# the branches containing this file. +# See https://help.github.com/en/articles/about-code-owners for info on how to +# take ownership of parts of the code base that should be reviewed by another +# team. + +* @puppetlabs/qe-team + From 79ac9ad37e03f9121261189bcba97b0515c92139 Mon Sep 17 00:00:00 2001 From: Andrew Makousky Date: Fri, 30 Aug 2019 17:58:24 -0500 Subject: [PATCH 002/371] (POOLER-148) Fix undefined variable bug in _check_ready_vm. The host['boottime'] variable in the function _check_ready_vm no longer has its parent object in reference due to the refactoring in pull request #269. So in order to get the same information without the performance impact from duplicate object lookups, we get similar information from the time that the VM is ready. --- lib/vmpooler/pool_manager.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index f2242ff..34a76b0 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -156,8 +156,14 @@ module Vmpooler $redis.hset('vmpooler__vm__' + vm, 'check', Time.now) # Check if the hosts TTL has expired if ttl > 0 - # host['boottime'] may be nil if host is not powered on - if ((Time.now - host['boottime']) / 60).to_s[/^\d+\.\d{1}/].to_f > ttl + # 'boottime' may be nil if host is not powered on + boottime = $redis.hget("vmpooler__vm__#{vm}", 'ready') + if boottime + boottime = Time.parse(boottime) + else + boottime = Time.at(0) + end + if ((Time.now - boottime) / 60).to_s[/^\d+\.\d{1}/].to_f > ttl $redis.smove('vmpooler__ready__' + pool_name, 'vmpooler__completed__' + pool_name, vm) $logger.log('d', "[!] [#{pool_name}] '#{vm}' reached end of TTL after #{ttl} minutes, removed from 'ready' queue") From a2548d11e8c5404f6ab728c0d59a3bcb836d406b Mon Sep 17 00:00:00 2001 From: Andrew Makousky Date: Wed, 4 Sep 2019 01:32:20 +0000 Subject: [PATCH 003/371] (POOLER-148) Improve code comment. Co-Authored-By: Samuel --- lib/vmpooler/pool_manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 34a76b0..3e59e44 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -156,7 +156,7 @@ module Vmpooler $redis.hset('vmpooler__vm__' + vm, 'check', Time.now) # Check if the hosts TTL has expired if ttl > 0 - # 'boottime' may be nil if host is not powered on + # if 'boottime' is nil, set bootime to beginning of unix epoch, forces TTL to be assumed expired boottime = $redis.hget("vmpooler__vm__#{vm}", 'ready') if boottime boottime = Time.parse(boottime) From 30bf2074fe410d18baaf2927f17e6fd886d0ba4d Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Mon, 21 Oct 2019 14:24:34 -0700 Subject: [PATCH 004/371] (POOLER-150) Synchronize checkout operations for API This commit adds a shared mutex to vmpooler API so that checkout requests can be synchronized across threads. Without this change it is possible in some scenarios for vmpooler to allocate the same SUT to different API requests for a VM. --- CHANGELOG.md | 1 + bin/vmpooler | 3 ++- lib/vmpooler/api.rb | 3 ++- lib/vmpooler/api/v1.rb | 30 ++++++++++++--------- spec/integration/api/v1/vm_spec.rb | 2 ++ spec/integration/api/v1/vm_template_spec.rb | 2 ++ 6 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef80643..3f7f5da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ git logs & PR history. ### Fixed - Correctly detect create\_linked\_clone on a pool level (POOLER-147) +- Ensure that checkout operations are synchronized # [0.7.0](https://github.com/puppetlabs/vmpooler/compare/0.6.3...0.7.0) diff --git a/bin/vmpooler b/bin/vmpooler index b84a139..29a4e4e 100755 --- a/bin/vmpooler +++ b/bin/vmpooler @@ -24,7 +24,8 @@ if torun.include? 'api' api = Thread.new do thr = Vmpooler::API.new redis = Vmpooler.new_redis(redis_host, redis_port, redis_password) - thr.helpers.configure(config, redis, metrics) + checkoutlock = Mutex.new + thr.helpers.configure(config, redis, metrics, checkoutlock) thr.helpers.execute! end torun_threads << api diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index 239155b..0b1fbe6 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -36,10 +36,11 @@ module Vmpooler use Vmpooler::API::Reroute use Vmpooler::API::V1 - def configure(config, redis, metrics) + def configure(config, redis, metrics, checkoutlock) self.settings.set :config, config self.settings.set :redis, redis self.settings.set :metrics, metrics + self.settings.set :checkoutlock, checkoutlock end def execute! diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 4b84e3e..1f63fc7 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -36,6 +36,10 @@ module Vmpooler validate_token(backend) end + def checkoutlock + Vmpooler::API::settings.checkoutlock + end + def fetch_single_vm(template) template_backends = [template] aliases = Vmpooler::API.settings.config[:alias] @@ -67,21 +71,23 @@ module Vmpooler end end - template_backends.each do |template_backend| - vms = backend.smembers("vmpooler__ready__#{template_backend}") - next if vms.empty? - vms.reverse.each do |vm| - ready = vm_ready?(vm, config['domain']) - if ready - backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm) - return [vm, template_backend, template] - else - backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__completed__#{template_backend}", vm) - metrics.increment("checkout.nonresponsive.#{template_backend}") + checkoutlock.synchronize do + template_backends.each do |template_backend| + vms = backend.smembers("vmpooler__ready__#{template_backend}") + next if vms.empty? + vms.reverse.each do |vm| + ready = vm_ready?(vm, config['domain']) + if ready + backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__running__#{template_backend}", vm) + return [vm, template_backend, template] + else + backend.smove("vmpooler__ready__#{template_backend}", "vmpooler__completed__#{template_backend}", vm) + metrics.increment("checkout.nonresponsive.#{template_backend}") + end end end + [nil, nil, nil] end - [nil, nil, nil] end def return_vm_to_ready_state(template, vm) diff --git a/spec/integration/api/v1/vm_spec.rb b/spec/integration/api/v1/vm_spec.rb index 9bbe08a..78be432 100644 --- a/spec/integration/api/v1/vm_spec.rb +++ b/spec/integration/api/v1/vm_spec.rb @@ -29,12 +29,14 @@ describe Vmpooler::API::V1 do } let(:current_time) { Time.now } let(:vmname) { 'abcdefghijkl' } + let(:checkoutlock) { Mutex.new } before(:each) do app.settings.set :config, config app.settings.set :redis, redis app.settings.set :metrics, metrics app.settings.set :config, auth: false + app.settings.set :checkoutlock, checkoutlock create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) end diff --git a/spec/integration/api/v1/vm_template_spec.rb b/spec/integration/api/v1/vm_template_spec.rb index d94b537..9620f1f 100644 --- a/spec/integration/api/v1/vm_template_spec.rb +++ b/spec/integration/api/v1/vm_template_spec.rb @@ -29,12 +29,14 @@ describe Vmpooler::API::V1 do let(:current_time) { Time.now } let(:socket) { double('socket') } + let(:checkoutlock) { Mutex.new } before(:each) do app.settings.set :config, config app.settings.set :redis, redis app.settings.set :metrics, metrics app.settings.set :config, auth: false + app.settings.set :checkoutlock, checkoutlock create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) end From 0e1a6da0433fb83b3df7a9f5490cf57e31afdfcf Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Wed, 23 Oct 2019 03:52:08 -0700 Subject: [PATCH 005/371] Simplify declaration of checkoutlock mutex This commit updates the way that checkoutlock is defined so it is not passed through bin/vmpooler. Without this change there's an unnecessary layer the mutex passes through. --- bin/vmpooler | 3 +-- lib/vmpooler/api.rb | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/bin/vmpooler b/bin/vmpooler index 29a4e4e..b84a139 100755 --- a/bin/vmpooler +++ b/bin/vmpooler @@ -24,8 +24,7 @@ if torun.include? 'api' api = Thread.new do thr = Vmpooler::API.new redis = Vmpooler.new_redis(redis_host, redis_port, redis_password) - checkoutlock = Mutex.new - thr.helpers.configure(config, redis, metrics, checkoutlock) + thr.helpers.configure(config, redis, metrics) thr.helpers.execute! end torun_threads << api diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index 0b1fbe6..22bf64e 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -36,11 +36,11 @@ module Vmpooler use Vmpooler::API::Reroute use Vmpooler::API::V1 - def configure(config, redis, metrics, checkoutlock) + def configure(config, redis, metrics) self.settings.set :config, config self.settings.set :redis, redis self.settings.set :metrics, metrics - self.settings.set :checkoutlock, checkoutlock + self.settings.set :checkoutlock, Mutex.new end def execute! From 2ca6d49aebad8fa9a354d41161e26dcf96142ca0 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Wed, 23 Oct 2019 12:21:39 -0700 Subject: [PATCH 006/371] (QENG-7530) Make VM names more human readable Prior to this commit hostnames for VMs provisioned by vmpooler were 15 random characters. This is difficult for humans to tell apart. This commit updates the naming to use the `spicy-proton` gem to generate adjective noun pair names for the VMs, which I think would be easier for humans to tell apart, as well as fun and memorable to say. The random name should not exceed 15 characters in order to prevent issues with NETBIOS, etc as discussed in the attached ticket. --- Gemfile | 1 + lib/vmpooler/pool_manager.rb | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index dfa75dd..11def9f 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,7 @@ gem 'net-ldap', '~> 0.16' gem 'statsd-ruby', '~> 1.4.0', :require => 'statsd' gem 'connection_pool', '~> 2.2' gem 'nokogiri', '~> 1.8' +gem 'spicy-proton', '2.1.1' group :development do gem 'pry' diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 3e59e44..6c07bde 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,4 +1,5 @@ require 'vmpooler/providers' +require 'spicy-proton' module Vmpooler class PoolManager @@ -29,6 +30,9 @@ module Vmpooler @vm_mutex = {} + # Name generator for generating host names + @name_generator = Spicy::Proton.new + # load specified providers from config file load_used_providers end @@ -265,8 +269,8 @@ module Vmpooler def _clone_vm(pool_name, provider) # Generate a randomized hostname - o = [('a'..'z'), ('0'..'9')].map(&:to_a).flatten - new_vmname = $config[:config]['prefix'] + o[rand(25)] + (0...14).map { o[rand(o.length)] }.join + random_name = [@name_generator.adjective(max: 7), @name_generator.noun(max: 7)].join('-') + new_vmname = $config[:config]['prefix'] + random_name # Add VM to Redis inventory ('pending' pool) $redis.sadd('vmpooler__pending__' + pool_name, new_vmname) From 8c9b69b85d171e505e6cc6ecc2cbca963a958a88 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Thu, 24 Oct 2019 23:47:08 +0000 Subject: [PATCH 007/371] (GEM) update vmpooler version to 0.7.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 fc884b9..dbe4c0c 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.7.1'.freeze + VERSION = '0.7.2'.freeze end From 85665a08566c6ccd15734cfcef33fbe80340642f Mon Sep 17 00:00:00 2001 From: Jenkins Date: Fri, 25 Oct 2019 00:08:48 +0000 Subject: [PATCH 008/371] (GEM) update vmpooler version to 0.8.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 dbe4c0c..e2665c9 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.7.2'.freeze + VERSION = '0.8.0'.freeze end From fa5aa8d0beb1b14a9b64f2e7f8461446c68c212c Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Fri, 25 Oct 2019 09:00:01 -0700 Subject: [PATCH 009/371] Add spicy-proton to vmpooler.gemspec This commit adds the spicy-proton gem to vmpooler.gemspec. Without this change the spicy-proton gem is in the Gemfile, but not the gemspec, causing issues when deploying from gem. --- Gemfile | 2 +- vmpooler.gemspec | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 11def9f..43046e4 100644 --- a/Gemfile +++ b/Gemfile @@ -12,7 +12,7 @@ gem 'net-ldap', '~> 0.16' gem 'statsd-ruby', '~> 1.4.0', :require => 'statsd' gem 'connection_pool', '~> 2.2' gem 'nokogiri', '~> 1.8' -gem 'spicy-proton', '2.1.1' +gem 'spicy-proton', '~> 2.1' group :development do gem 'pry' diff --git a/vmpooler.gemspec b/vmpooler.gemspec index c8b0433..596218f 100644 --- a/vmpooler.gemspec +++ b/vmpooler.gemspec @@ -28,4 +28,5 @@ Gem::Specification.new do |s| s.add_dependency 'statsd-ruby', '~> 1.4' s.add_dependency 'connection_pool', '~> 2.2' s.add_dependency 'nokogiri', '~> 1.8' + s.add_dependency 'spicy-proton', '~> 2.1' end From 3732ed750ef9df824bd5e140d8d91d11b910f95c Mon Sep 17 00:00:00 2001 From: Jenkins Date: Fri, 25 Oct 2019 16:35:08 +0000 Subject: [PATCH 010/371] (GEM) update vmpooler version to 0.8.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 e2665c9..a9750a2 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.8.0'.freeze + VERSION = '0.8.1'.freeze end From 3bde636803ae483ffb94bf8e85ff89cefb8266ef Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Wed, 30 Oct 2019 10:52:01 -0700 Subject: [PATCH 011/371] Update changelog for 0.8.1 release --- CHANGELOG.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f7f5da..1a1cc48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,22 @@ The format is based on Tracking in this Changelog began for this project with the tagging of version 0.1.0. If you're looking for changes from before this, refer to the project's git logs & PR history. -# [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.7.0...master) +# [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.8.1...master) + +# [0.8.1](https://github.com/puppetlabs/vmpooler/compare/0.7.2...0.8.1) + +### Added +- Make VM names human readable + +# [0.7.2](https://github.com/puppetlabs/vmpooler/compare/0.7.1...0.7.2) + +### Fixed +- Synchronize checkout operations across API threads (POOLER-150) + +# [0.7.1](https://github.com/puppetlabs/vmpooler/compare/0.7.0...0.7.1) ### Fixed - Correctly detect create\_linked\_clone on a pool level (POOLER-147) -- Ensure that checkout operations are synchronized # [0.7.0](https://github.com/puppetlabs/vmpooler/compare/0.6.3...0.7.0) From 625472df35cbb5499a87efde97fc4a77f7d462a5 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Fri, 1 Nov 2019 09:09:34 -0700 Subject: [PATCH 012/371] (QENG-7530) Fix hostname_shorten regex Prior to this commit the hostname_shorten regex wouldn't match the updated human readable hostnames because they contain dashes. This commit updates the regex to capture dashes in the hostname, and adds a few specs to verify that behavior. --- lib/vmpooler/api/helpers.rb | 2 +- spec/unit/api/helpers_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index 54636ca..6fdd2f6 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -140,7 +140,7 @@ module Vmpooler end def hostname_shorten(hostname, domain=nil) - if domain && hostname =~ /^\w+\.#{domain}$/ + if domain && hostname =~ /^[\w-]+\.#{domain}$/ hostname = hostname[/[^\.]+/] end diff --git a/spec/unit/api/helpers_spec.rb b/spec/unit/api/helpers_spec.rb index e026cf7..62fed34 100644 --- a/spec/unit/api/helpers_spec.rb +++ b/spec/unit/api/helpers_spec.rb @@ -19,6 +19,8 @@ describe Vmpooler::API::Helpers do ['example.com', 'not-example.com', 'example.com'], ['example.com', 'example.com', 'example.com'], ['sub.example.com', 'example.com', 'sub'], + ['adjective-noun.example.com', 'example.com', 'adjective-noun'], + ['abc123.example.com', 'example.com', 'abc123'], ['example.com', nil, 'example.com'] ].each do |hostname, domain, expected| it { expect(subject.hostname_shorten(hostname, domain)).to eq expected } From 35475546ef663d19eb44201fef96807e9fc76833 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Mon, 4 Nov 2019 14:57:53 -0800 Subject: [PATCH 013/371] Update rubocop configs Previously, there were some rubocop rule names that were causing rubocop to fail to run entirely. This commit updates the rubocop configs to match the new rubocop rule names so that we can see all the issues that need correcting. --- .rubocop.yml | 2 +- .rubocop_todo.yml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6c94c01..e588c52 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -58,7 +58,7 @@ Style/RegexpLiteral: # In some cases readability is better without these cops enabled Style/ConditionalAssignment: Enabled: false -Next: +Style/Next: Enabled: false Metrics/ParameterLists: Max: 10 diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3d824d7..2c162d6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -55,7 +55,7 @@ Layout/EmptyLinesAroundModuleBody: # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: special_inside_parentheses, consistent, align_braces -Layout/IndentHash: +Layout/IndentFirstHashElement: Exclude: - 'lib/vmpooler/api/helpers.rb' - 'lib/vmpooler/api/v1.rb' @@ -246,7 +246,7 @@ Style/PerlBackrefs: # NamePrefix: is_, has_, have_ # NamePrefixBlacklist: is_, has_, have_ # NameWhitelist: is_a? -Style/PredicateName: +Naming/PredicateName: Exclude: - 'spec/**/*' - 'lib/vmpooler/api/helpers.rb' @@ -289,7 +289,7 @@ Style/TernaryParentheses: # Offense count: 2 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: snake_case, camelCase -Style/VariableName: +Naming/VariableName: Exclude: - 'lib/vmpooler/api/v1.rb' From 019ed021b0dc3b1f21b745897c34f6c6ab54e368 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Fri, 1 Nov 2019 15:26:51 -0700 Subject: [PATCH 014/371] (QENG-7530) Add check for unique hostnames Prior to this commit the pooler had no awareness of the complete set of hostnames that are currently in use. This meant that it was possible to allocate the same hostname twice, which would result in the original host with that hostname becoming unreachable. This commit adds a check for the existence of the `vmpooler__vm__` key before attempting to clone the vm. This should prevent duplicate hostnames. If the hostname is already taken, `_clone_vm` will retry with a new random hostname multiple times before raising an exception. --- lib/vmpooler/pool_manager.rb | 28 ++++++++++++++++++++++++++-- spec/unit/pool_manager_spec.rb | 2 ++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 6c07bde..25a5027 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -267,10 +267,34 @@ module Vmpooler end end - def _clone_vm(pool_name, provider) + def generate_and_check_hostname(pool_name) # Generate a randomized hostname random_name = [@name_generator.adjective(max: 7), @name_generator.noun(max: 7)].join('-') - new_vmname = $config[:config]['prefix'] + random_name + hostname = $config[:config]['prefix'] + random_name + available = $redis.hlen('vmpooler__vm__' + hostname) == 0 + + return hostname, available + end + + def find_unique_hostname(pool_name) + hostname_retries = 0 + max_hostname_retries = 3 + while hostname_retries < max_hostname_retries + hostname, available = generate_and_check_hostname(pool_name) + break if available + + hostname_retries += 1 + $metrics.increment("errors.duplicatehostname.#{pool_name}") + $logger.log('s', "[!] [#{pool_name}] Generated hostname #{hostname} was not unique (attempt \##{hostname_retries} of #{max_hostname_retries})") + end + + raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless available + + hostname + end + + def _clone_vm(pool_name, provider) + new_vmname = find_unique_hostname(pool_name) # Add VM to Redis inventory ('pending' pool) $redis.sadd('vmpooler__pending__' + pool_name, new_vmname) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 3b41e62..2fa9b3e 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -608,6 +608,7 @@ EOT expect(metrics).to receive(:timing).with(/clone\./,/0/) expect(provider).to receive(:create_vm).with(pool, String) allow(logger).to receive(:log) + expect(subject).to receive(:find_unique_hostname).with(pool).and_return(vm) end it 'should create a cloning VM' do @@ -649,6 +650,7 @@ EOT before(:each) do expect(provider).to receive(:create_vm).with(pool, String).and_raise('MockError') allow(logger).to receive(:log) + expect(subject).to receive(:find_unique_hostname).with(pool).and_return(vm) end it 'should not create a cloning VM' do From 8a17c5fa379750c9ad65292f13bffbd1c932aed7 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Wed, 6 Nov 2019 20:34:28 +0000 Subject: [PATCH 015/371] (GEM) update vmpooler version to 0.8.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 a9750a2..3054b4a 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.8.1'.freeze + VERSION = '0.8.2'.freeze end From 86dbc783ef848ee2ec2489d6c513b1fbd45d874a Mon Sep 17 00:00:00 2001 From: Brandon High Date: Mon, 11 Nov 2019 15:28:13 -0800 Subject: [PATCH 016/371] Update CHANGELOG for 0.8.2 This commit updates the CHANGELOG, and fixes some Markdown linter issues with the CHANGELOG. --- CHANGELOG.md | 76 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a1cc48..5ed56da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,107 +10,139 @@ The format is based on Tracking in this Changelog began for this project with the tagging of version 0.1.0. If you're looking for changes from before this, refer to the project's git logs & PR history. -# [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.8.1...master) -# [0.8.1](https://github.com/puppetlabs/vmpooler/compare/0.7.2...0.8.1) +## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.8.2...master) + +## [0.8.2](https://github.com/puppetlabs/vmpooler/compare/0.8.1...0.8.2) ### Added + +- Uniqueness check on generated hostnames to prevent collisions + +### Fixed + +- Update hostname_shorten regex that was causing problems with returning VMs +- Rubocop linter fixes + +## [0.8.1](https://github.com/puppetlabs/vmpooler/compare/0.7.2...0.8.1) + +### Added + - Make VM names human readable -# [0.7.2](https://github.com/puppetlabs/vmpooler/compare/0.7.1...0.7.2) +## [0.7.2](https://github.com/puppetlabs/vmpooler/compare/0.7.1...0.7.2) ### Fixed + - Synchronize checkout operations across API threads (POOLER-150) -# [0.7.1](https://github.com/puppetlabs/vmpooler/compare/0.7.0...0.7.1) +## [0.7.1](https://github.com/puppetlabs/vmpooler/compare/0.7.0...0.7.1) ### Fixed + - Correctly detect create\_linked\_clone on a pool level (POOLER-147) -# [0.7.0](https://github.com/puppetlabs/vmpooler/compare/0.6.3...0.7.0) +## [0.7.0](https://github.com/puppetlabs/vmpooler/compare/0.6.3...0.7.0) ### Added + - Add capability to disable linked clones for vsphere provider (POOLER-147) - Add running host to VM data returned from /vm/hostname (POOLER-142) -# [0.6.3](https://github.com/puppetlabs/vmpooler/compare/0.6.2...0.6.3) +## [0.6.3](https://github.com/puppetlabs/vmpooler/compare/0.6.2...0.6.3) ### Added + - Add capability to configure pool cluster via config api (POOLER-143) -# [0.6.2](https://github.com/puppetlabs/vmpooler/compare/0.6.1...0.6.2) +## [0.6.2](https://github.com/puppetlabs/vmpooler/compare/0.6.1...0.6.2) ### Added + - Validate a machine responds to vm\_ready? at checkout (POOLER-140) -# [0.6.1](https://github.com/puppetlabs/vmpooler/compare/0.6.0...0.6.1) +## [0.6.1](https://github.com/puppetlabs/vmpooler/compare/0.6.0...0.6.1) ### Added + - Vmpooler /status legacy api optimization -# [0.6.0](https://github.com/puppetlabs/vmpooler/compare/0.5.1...0.6.0) +## [0.6.0](https://github.com/puppetlabs/vmpooler/compare/0.5.1...0.6.0) ### Fixed + - Ensure migrations and pending evaluations are processed FIFO (POOLER-141) ### Added + - Vmpooler pool statistic endpoint optimization ### Fixed - - Ensure a checked out VM stays in a queue during checkout (POOLER-140) -# [0.5.1](https://github.com/puppetlabs/vmpooler/compare/0.5.0...0.5.1) +- Ensure a checked out VM stays in a queue during checkout (POOLER-140) -# [0.5.0](https://github.com/puppetlabs/vmpooler/compare/0.4.0...0.5.0) +## [0.5.1](https://github.com/puppetlabs/vmpooler/compare/0.5.0...0.5.1) + +## [0.5.0](https://github.com/puppetlabs/vmpooler/compare/0.4.0...0.5.0) ### Fixed - - Eliminate window for checked out VM to be discovered (POOLER-139) -# [0.4.0](https://github.com/puppetlabs/vmpooler/compare/0.3.0...0.4.0) +- Eliminate window for checked out VM to be discovered (POOLER-139) + +## [0.4.0](https://github.com/puppetlabs/vmpooler/compare/0.3.0...0.4.0) ### Fixed - - Improve support for configuration via environment variables (POOLER-137) - - Support multiple pool backends per alias (POOLER-138) - - Remove redis server testing requirement -# [0.3.0](https://github.com/puppetlabs/vmpooler/compare/0.2.2...0.3.0) +- Improve support for configuration via environment variables (POOLER-137) +- Support multiple pool backends per alias (POOLER-138) +- Remove redis server testing requirement + +## [0.3.0](https://github.com/puppetlabs/vmpooler/compare/0.2.2...0.3.0) ### Fixed + - Sync pool size before dashboard is displayed (POOLER-132) - Remove a failed VM from the ready queue (POOLER-133) - Begin checking ready VMs to ensure alive after 1 minute by default - Ensure that metric nodes for vm usage stats are consistent ### Added + - Add capability to ship VM usage metrics (POOLER-134) -# [0.2.2](https://github.com/puppetlabs/vmpooler/compare/0.2.1...0.2.2) +## [0.2.2](https://github.com/puppetlabs/vmpooler/compare/0.2.1...0.2.2) ### Fixed + - Return label used to request VMs when fulfilling VM requests (POOLER-131) -# [0.2.1](https://github.com/puppetlabs/vmpooler/compare/0.2.0...0.2.1) +## [0.2.1](https://github.com/puppetlabs/vmpooler/compare/0.2.0...0.2.1) ### Fixed + - Better handle delta disk creation errors (POOLER-130) ### Added + - Re-write check\_pool in pool\_manager to improve readability - Add a docker-compose file for testing vmpooler - Add capability to weight backends when an alias spans multiple backends (POOLER-129) -# [0.2.0](https://github.com/puppetlabs/vmpooler/compare/0.1.0...0.2.0) +## [0.2.0](https://github.com/puppetlabs/vmpooler/compare/0.1.0...0.2.0) ### Fixed + - (POOLER-128) VM specific mutex objects are not dereferenced when a VM is destroyed - A VM that is being destroyed is reported as discovered ### Added + - Adds a new mechanism to load providers from any gem or file path -# [0.1.0](https://github.com/puppetlabs/vmpooler/compare/4c858d012a262093383e57ea6db790521886d8d4...master) +## [0.1.0](https://github.com/puppetlabs/vmpooler/compare/4c858d012a262093383e57ea6db790521886d8d4...master) ### Fixed + - Remove unused method `find_pool` and related pending tests - Setting `max_tries` results in an infinite loop (POOLER-124) - Do not evaluate folders as VMs in `get_pool_vms` (POOLER-40) From f6fdfe42d77cc4b108a5e7d0ac7adb6f4e7a8cc5 Mon Sep 17 00:00:00 2001 From: Sean Millichamp Date: Tue, 26 Nov 2019 13:48:53 -0500 Subject: [PATCH 017/371] Support nested host folders in find_cluster() Search the root and any subfolders for cluster or host resources. --- lib/vmpooler/providers/vsphere.rb | 15 ++++++++- spec/rbvmomi_helper.rb | 28 +++++++++++++++- spec/unit/providers/vsphere_spec.rb | 52 ++++++++++++++++++++++------- 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index d0ca8c6..53524fe 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -839,7 +839,20 @@ module Vmpooler def find_cluster(cluster, connection, datacentername) datacenter = connection.serviceInstance.find_datacenter(datacentername) raise("Datacenter #{datacentername} does not exist") if datacenter.nil? - datacenter.hostFolder.children.find { |cluster_object| cluster_object.name == cluster } + + # In the event the cluster is not a direct descendent of the + # datacenter, we use a ContainerView to leverage its recursive + # search. This will find clusters which are, for example, in + # folders under the datacenter. This will also find standalone + # hosts which are not part of a cluster. + cv = connection.serviceContent.viewManager.CreateContainerView( + container: datacenter.hostFolder, + type: ['ComputeResource', 'ClusterComputeResource'], + recursive: true, + ) + cluster = cv.view.find { |cluster_object| cluster_object.name == cluster } + cv.DestroyView + cluster end def get_cluster_host_utilization(cluster, model = nil) diff --git a/spec/rbvmomi_helper.rb b/spec/rbvmomi_helper.rb index 88da343..05754d9 100644 --- a/spec/rbvmomi_helper.rb +++ b/spec/rbvmomi_helper.rb @@ -24,7 +24,29 @@ MockContainerView = Struct.new( # https://www.vmware.com/support/developer/vc-sdk/visdk400pubs/ReferenceGuide/vim.view.ContainerView.html # From ContainerView :container, :recursive, :type -) +) do + def _search_tree(layer) + results = [] + + layer.children.each do |child| + if type.any? { |t| child.is_a?(RbVmomi::VIM.const_get(t)) } + results << child + end + + if recursive && child.respond_to?(:children) + results += _search_tree(child) + end + end + results + end + + def view + _search_tree(container) + end + + def DestroyView + end +end MockDatacenter = Struct.new( # https://www.vmware.com/support/developer/vc-sdk/visdk41pubs/ApiReference/vim.Datacenter.html @@ -384,6 +406,10 @@ def mock_RbVmomi_VIM_ComputeResource(options = {}) mock.host << mock_host end + allow(mock).to receive(:is_a?) do |expected_type| + expected_type == RbVmomi::VIM::ComputeResource + end + mock end diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb index 7e445c2..1d166d2 100644 --- a/spec/unit/providers/vsphere_spec.rb +++ b/spec/unit/providers/vsphere_spec.rb @@ -2887,6 +2887,7 @@ EOT describe '#find_cluster' do let(:cluster) {'cluster'} + let(:host) { 'host' } let(:missing_cluster) {'missing_cluster'} context 'no clusters in the datacenter' do @@ -2909,10 +2910,11 @@ EOT :datacenters => [ { :name => datacenter_name, :hostfolder_tree => { - 'cluster1' => {:object_type => 'compute_resource'}, - 'cluster2' => {:object_type => 'compute_resource'}, - cluster => {:object_type => 'compute_resource'}, - 'cluster3' => {:object_type => 'compute_resource'}, + 'cluster1' => {:object_type => 'cluster_compute_resource'}, + 'cluster2' => {:object_type => 'cluster_compute_resource'}, + cluster => {:object_type => 'cluster_compute_resource'}, + 'cluster3' => {:object_type => 'cluster_compute_resource'}, + host => {:object_type => 'compute_resource'}, } } ] @@ -2926,6 +2928,13 @@ EOT expect(result.name).to eq(cluster) end + it 'should return the single host when found' do + result = subject.find_cluster(host,connection,datacenter_name) + + expect(result).to_not be_nil + expect(result.name).to eq(host) + end + it 'should return nil if the cluster is not found' do expect(subject.find_cluster(missing_cluster,connection,datacenter_name)).to be_nil end @@ -2937,14 +2946,15 @@ EOT :datacenters => [ { :name => 'AnotherDC', :hostfolder_tree => { - 'cluster1' => {:object_type => 'compute_resource'}, - 'cluster2' => {:object_type => 'compute_resource'}, + 'cluster1' => {:object_type => 'cluster_compute_resource'}, + 'cluster2' => {:object_type => 'cluster_compute_resource'}, } }, { :name => datacenter_name, :hostfolder_tree => { - cluster => {:object_type => 'compute_resource'}, - 'cluster3' => {:object_type => 'compute_resource'}, + cluster => {:object_type => 'cluster_compute_resource'}, + 'cluster3' => {:object_type => 'cluster_compute_resource'}, + host => {:object_type => 'compute_resource'} } } ] @@ -2958,6 +2968,13 @@ EOT expect(result.name).to eq(cluster) end + it 'should return the single host when found' do + result = subject.find_cluster(host,connection,datacenter_name) + + expect(result).to_not be_nil + expect(result.name).to eq(host) + end + it 'should return nil if the cluster is not found' do expect(subject.find_cluster(missing_cluster,connection,'AnotherDC')).to be_nil end @@ -2969,13 +2986,18 @@ EOT :datacenters => [ { :name => datacenter_name, :hostfolder_tree => { - 'cluster1' => {:object_type => 'compute_resource'}, + 'cluster1' => {:object_type => 'cluster_compute_resource'}, 'folder2' => { :children => { - cluster => {:object_type => 'compute_resource'}, + cluster => {:object_type => 'cluster_compute_resource'}, } }, - 'cluster3' => {:object_type => 'compute_resource'}, + 'cluster3' => {:object_type => 'cluster_compute_resource'}, + 'folder4' => { + :children => { + host => {:object_type => 'compute_resource'}, + } + } } } ] @@ -2983,13 +3005,19 @@ EOT }} it 'should return the cluster when found' do - pending('https://github.com/puppetlabs/vmpooler/issues/205') result = subject.find_cluster(cluster,connection,datacenter_name) expect(result).to_not be_nil expect(result.name).to eq(cluster) end + it 'should return the host when found' do + result = subject.find_cluster(host,connection,datacenter_name) + + expect(result).to_not be_nil + expect(result.name).to eq(host) + end + it 'should return nil if the cluster is not found' do expect(subject.find_cluster(missing_cluster,connection,datacenter_name)).to be_nil end From ae10bd4e226922b8ab46583fe3e7568de3faae53 Mon Sep 17 00:00:00 2001 From: Samuel Date: Thu, 5 Dec 2019 16:35:30 +0000 Subject: [PATCH 018/371] (POOLER-123) Implement a max TTL (#349) * (POOLER-123) Implement a max TTL Before this change, we could checkout a vm and set the lifetime to a very high number which would esssentially keep the vm running forever. Now implementing a config setting max_lifetime_upper_limit which enforces a maximum lifetime in hours both for initial checkout and extending a running vm * (POOLER-123) Improve PUT vm endpoint error messaging Prior to this commit the PUT vm endpoint didn't give any useful information about why a user's request failed. This commit updates PUT to output a more helpful set of error messages in the `failure` key that gets returned in the JSON response. * (POOLER-123) Update max_lifetime_upper_limit key This commit switches the max_lifetime_upper_limit key from being a symbol to being a string, which is what the config hash seems to contain. * (maint) Add option to disable Redis persistence in docker-compose This commit is just a handy little command override to the redis container to prevent persistence. --- docker/docker-compose.yml | 2 ++ lib/vmpooler/api/v1.rb | 30 ++++++++++++---- spec/helpers.rb | 4 +++ spec/integration/api/v1/vm_hostname_spec.rb | 38 ++++++++++++++++++++- vmpooler.yaml.example | 6 ++++ 5 files changed, 73 insertions(+), 7 deletions(-) diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index b007866..55e3383 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -22,6 +22,8 @@ services: - redislocal redislocal: image: redis + # Uncomment this if you don't want the redis data to persist + #command: "redis-server --save '' --appendonly no" ports: - "6379:6379" networks: diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 1f63fc7..2c39b45 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -875,7 +875,7 @@ module Vmpooler status 404 result = { 'ok' => false } - failure = false + failure = [] params[:hostname] = hostname_shorten(params[:hostname], config['domain']) @@ -892,24 +892,42 @@ module Vmpooler when 'lifetime' need_token! if Vmpooler::API.settings.config[:auth] + # in hours, defaults to one week + max_lifetime_upper_limit = config['max_lifetime_upper_limit'] + if max_lifetime_upper_limit + max_lifetime_upper_limit = max_lifetime_upper_limit.to_i + if arg.to_i >= max_lifetime_upper_limit + failure.push("You provided a lifetime (#{arg}) that exceeds the configured maximum of #{max_lifetime_upper_limit}.") + else + # also make sure we do not extend past max_lifetime_upper_limit + rdata = backend.hgetall('vmpooler__vm__' + params[:hostname]) + running = ((Time.now - Time.parse(rdata['checkout'])) / 60 / 60).round(2) + unless running + arg.to_i < max_lifetime_upper_limit + failure.push("You provided a lifetime (#{arg}) that will extend the current lifetime past the configured maximum of #{max_lifetime_upper_limit}.") + end + end + end + + # validate lifetime is within boundaries unless arg.to_i > 0 - failure = true + failure.push("You provided a lifetime (#{arg}) but you must provide a positive number.") end when 'tags' unless arg.is_a?(Hash) - failure = true + failure.push("You provided tags (#{arg}) as something other than a hash.") end if config['allowed_tags'] - failure = true if not (arg.keys - config['allowed_tags']).empty? + failure.push("You provided unsuppored tags (#{arg}).") if not (arg.keys - config['allowed_tags']).empty? end else - failure = true + failure.push("Unknown argument #{arg}.") end end - if failure + if failure.size > 0 status 400 + result['failure'] = failure else jdata.each do |param, arg| case param diff --git a/spec/helpers.rb b/spec/helpers.rb index bd3c748..589e00f 100644 --- a/spec/helpers.rb +++ b/spec/helpers.rb @@ -99,6 +99,10 @@ def fetch_vm(vm) redis.hgetall("vmpooler__vm__#{vm}") end +def set_vm_data(vm, key, value) + redis.hset("vmpooler__vm__#{vm}", key, value) +end + def snapshot_revert_vm(vm, snapshot = '12345678901234567890123456789012') redis.sadd('vmpooler__tasks__snapshot-revert', "#{vm}:#{snapshot}") redis.hset("vmpooler__vm__#{vm}", "snapshot:#{snapshot}", "1") diff --git a/spec/integration/api/v1/vm_hostname_spec.rb b/spec/integration/api/v1/vm_hostname_spec.rb index 1e6ac19..81aa6d0 100644 --- a/spec/integration/api/v1/vm_hostname_spec.rb +++ b/spec/integration/api/v1/vm_hostname_spec.rb @@ -20,12 +20,14 @@ describe Vmpooler::API::V1 do config: { 'site_name' => 'test pooler', 'vm_lifetime_auth' => 2, + }, pools: [ {'name' => 'pool1', 'size' => 5}, {'name' => 'pool2', 'size' => 10} ], alias: { 'poolone' => 'pool1' }, + auth: false } } @@ -34,7 +36,6 @@ describe Vmpooler::API::V1 do before(:each) do app.settings.set :config, config app.settings.set :redis, redis - app.settings.set :config, auth: false create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) end @@ -122,6 +123,41 @@ describe Vmpooler::API::V1 do vm = fetch_vm('testhost') expect(vm['lifetime']).to be_nil end + + it 'does not enforce a lifetime' do + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"lifetime":"20000"}' + expect_json(ok = true, http = 200) + + vm = fetch_vm('testhost') + expect(vm['lifetime']).to eq("20000") + end + + it 'does not allow a lifetime to be initially past config 168' do + app.settings.set :config, + { :config => { 'max_lifetime_upper_limit' => 168 } } + create_vm('testhost') + + put "#{prefix}/vm/testhost", '{"lifetime":"200"}' + expect_json(ok = false, http = 400) + + vm = fetch_vm('testhost') + expect(vm['lifetime']).to be_nil + end + + it 'does not allow a lifetime to be extended past config 168' do + app.settings.set :config, + { :config => { 'max_lifetime_upper_limit' => 168 } } + create_vm('testhost') + + set_vm_data('testhost', "checkout", (Time.now - (69*60*60))) + put "#{prefix}/vm/testhost", '{"lifetime":"100"}' + expect_json(ok = false, http = 400) + + vm = fetch_vm('testhost') + expect(vm['lifetime']).to be_nil + end end context '(auth configured)' do diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index f6781b6..e29a8ce 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -498,6 +498,12 @@ # Expects a boolean value # (optional; default: false) # +# - max_lifetime_upper_limit +# Sets a lifetime upper limit (in hours) for how long the vm lifetime can be set via the API. Lifetime can be set and extended +# so this configuration is used to enforce an upper limit to both the initial lifetime request and/or the extended +# lifetime (by checking how long it has already been running). +# (optional; default: unlimited) +# # Example: :config: From f581d065aeb6ea7f961ecd2ca2817fdfaa63f7eb Mon Sep 17 00:00:00 2001 From: John O'Connor Date: Wed, 11 Dec 2019 15:24:57 +0000 Subject: [PATCH 019/371] (QENG-7531) Add Marked as Failed Stat This is a useful measure for monitoring the health of pools that we don't capture yet. --- lib/vmpooler/pool_manager.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 25a5027..bd1a9ff 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -104,6 +104,7 @@ module Vmpooler if time_since_clone > timeout if exists $redis.smove('vmpooler__pending__' + pool, 'vmpooler__completed__' + pool, vm) + $metrics.increment("errors.markedasfailed.#{pool}") $logger.log('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes") else remove_nonexistent_vm(vm, pool) From 94eacdd7af5e33ce16fdf48739344142ea1e83c8 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Thu, 12 Dec 2019 22:24:58 +0000 Subject: [PATCH 020/371] (GEM) update vmpooler version to 0.9.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 3054b4a..c92e482 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.8.2'.freeze + VERSION = '0.9.0'.freeze end From c4f3a49782a3d7b1e116f165caf623ef6d970f3c Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 24 Jan 2020 16:03:40 -0800 Subject: [PATCH 021/371] Generate a wider set of legal names Previously, we restricted the adjective and noun portion of the name each to 7 characters to ensure that the final name would not be more than 15 after adding a hyphen. Given that the _total_ length is what matters, we can generate a noun up to 11 characters (to ensure we leave room for a hyphen and a 3 letter adjective) and adjust our acceptable adjective size accordingly. This lets many more names be generated than would otherwise, while still respecting the 15 character limit. Due to the limited set of 11 letter nouns and corresponding 3 letter adjectives, as well as some complex combinatorics, setting the noun length to 11 causes a net increase in conflicts. We therefore actually set it to 10, which causes a net decrease in conflicts. We favor generating longer nouns rather than longer adjectives (by selecting the noun first) because longer adjectives tend to be more unwieldy words, and thus more awkward to say and generally less fun. --- lib/vmpooler/pool_manager.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index bd1a9ff..e61406d 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -269,8 +269,17 @@ module Vmpooler end def generate_and_check_hostname(pool_name) - # Generate a randomized hostname - random_name = [@name_generator.adjective(max: 7), @name_generator.noun(max: 7)].join('-') + # Generate a randomized hostname. The total name must no longer than 15 + # character including the hyphen. The shortest adjective in the corpus is + # three characters long. Therefore, we can technically select a noun up to 11 + # characters long and still be guaranteed to have an available adjective. + # Because of the limited set of 11 letter nouns and corresponding 3 + # letter adjectives, we actually limit the noun to 10 letters to avoid + # inviting more conflicts. We favor selecting a longer noun rather than a + # longer adjective because longer adjectives tend to be less fun. + noun = @name_generator.noun(max: 10) + adjective = @name_generator.adjective(max: 14-noun.length) + random_name = [adjective, noun].join('-') hostname = $config[:config]['prefix'] + random_name available = $redis.hlen('vmpooler__vm__' + hostname) == 0 From 1407dd55755386ee05ca83614ce0263acaa31c13 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Tue, 28 Jan 2020 01:14:59 +0000 Subject: [PATCH 022/371] (GEM) update vmpooler version to 0.9.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 c92e482..49a189b 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.9.0'.freeze + VERSION = '0.9.1'.freeze end From 52b60b074cba9c76b15d297a09d964d40429d850 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Tue, 11 Feb 2020 16:34:19 -0800 Subject: [PATCH 023/371] (POOLER-153) Add endpoint for resetting a pool This commit adds a capability to vmpooler to reset a pool, deleting its ready and pending instances and replacing them with fresh ones. Without this change vmpooler does not offer a mechanism to reset a pool without also changing its template. --- docs/API.md | 26 ++++++ lib/vmpooler/api/v1.rb | 49 +++++++++++ lib/vmpooler/pool_manager.rb | 24 +++++- spec/integration/api/v1/poolreset.rb | 117 +++++++++++++++++++++++++++ spec/unit/pool_manager_spec.rb | 22 +++++ 5 files changed, 237 insertions(+), 1 deletion(-) create mode 100644 spec/integration/api/v1/poolreset.rb diff --git a/docs/API.md b/docs/API.md index 560d4b1..d30b329 100644 --- a/docs/API.md +++ b/docs/API.md @@ -773,3 +773,29 @@ $ curl -X POST -H "Content-Type: application/json" -d '{"debian-7-i386":"templat "ok": true } ``` + +##### POST /poolreset + +Clear all pending and ready instances in a pool, and deploy replacements + +All pool reset requests must be for pools that exist in the vmpooler configuration running, or a 404 code will be returned. + +When a pool reset is requested a 201 status will be returned. + +A pool reset will cause vmpooler manager to log that it has cleared ready and pending instances. + +For poolreset to be available it is necessary to enable experimental features. Additionally, the request must be performed with an authentication token when authentication is configured. + +Responses: +* 201 - Pool reset requested received +* 400 - An invalid configuration was provided causing requested changes to fail +* 404 - An unknown error occurred +* 405 - The endpoint is disabled because experimental features are disabled +``` +$ curl -X POST -H "Content-Type: application/json" -d '{"debian-7-i386":"1"}' --url https://vmpooler.example.com/api/v1/poolreset +``` +```json +{ + "ok": true +} +``` diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 1f63fc7..bff7c19 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -200,6 +200,17 @@ module Vmpooler result end + def reset_pool(payload) + result = { 'ok' => false } + + payload.each do |poolname, count| + backend.sadd('vmpooler__poolreset', poolname) + end + status 201 + result['ok'] = true + result + end + def update_clone_target(payload) result = { 'ok' => false } @@ -1063,6 +1074,44 @@ module Vmpooler JSON.pretty_generate(result) end + post "#{api_prefix}/poolreset/?" do + content_type :json + result = { 'ok' => false } + + if config['experimental_features'] + need_token! if Vmpooler::API.settings.config[:auth] + + begin + payload = JSON.parse(request.body.read) + if payload + invalid = invalid_templates(payload) + if invalid.empty? + result = reset_pool(payload) + else + invalid.each do |bad_pool| + metrics.increment("poolreset.invalid.#{bad_pool}") + end + result[:bad_pools] = invalid + status 400 + end + else + metrics.increment('poolreset.invalid.unknown') + status 404 + end + rescue JSON::ParserError + status 400 + result = { + 'ok' => false, + 'message' => 'JSON payload could not be parsed' + } + end + else + status 405 + end + + JSON.pretty_generate(result) + end + post "#{api_prefix}/config/clonetarget/?" do content_type :json result = { 'ok' => false } diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 6c07bde..80d7898 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -680,6 +680,10 @@ module Vmpooler # - Fires when a template configuration update is requested # - Additional options # :poolname + # :pool_reset + # - Fires when a pool reset is requested + # - Additional options + # :poolname # def sleep_with_wakeup_events(loop_delay, wakeup_period = 5, options = {}) exit_by = Time.now + loop_delay @@ -726,6 +730,10 @@ module Vmpooler end end + if options[:pool_reset] + break if $redis.sismember('vmpooler__poolreset', options[:poolname]) + end + end break if time_passed?(:exit_by, exit_by) @@ -763,7 +771,7 @@ module Vmpooler loop_delay = (loop_delay * loop_delay_decay).to_i loop_delay = loop_delay_max if loop_delay > loop_delay_max end - sleep_with_wakeup_events(loop_delay, loop_delay_min, pool_size_change: true, poolname: pool['name'], pool_template_change: true, clone_target_change: true) + sleep_with_wakeup_events(loop_delay, loop_delay_min, pool_size_change: true, poolname: pool['name'], pool_template_change: true, clone_target_change: true, pool_reset: true) unless maxloop.zero? break if loop_count >= maxloop @@ -917,6 +925,17 @@ module Vmpooler end end + def reset_pool(pool) + poolname = pool['name'] + return unless $redis.sismember('vmpooler__poolreset', poolname) + $redis.srem('vmpooler__poolreset', poolname) + mutex = pool_mutex(poolname) + mutex.synchronize do + drain_pool(poolname) + $logger.log('s', "[*] [#{poolname}] reset has cleared ready and pending instances") + end + end + def create_inventory(pool, provider, pool_check_response) inventory = {} begin @@ -1125,6 +1144,9 @@ module Vmpooler # Remove VMs in excess of the configured pool size remove_excess_vms(pool) + # Reset a pool when poolreset is requested from the API + reset_pool(pool) + pool_check_response end diff --git a/spec/integration/api/v1/poolreset.rb b/spec/integration/api/v1/poolreset.rb new file mode 100644 index 0000000..9c87c6c --- /dev/null +++ b/spec/integration/api/v1/poolreset.rb @@ -0,0 +1,117 @@ +require 'spec_helper' +require 'rack/test' + +describe Vmpooler::API::V1 do + include Rack::Test::Methods + + def app() + Vmpooler::API + end + + let(:config) { + { + config: { + 'site_name' => 'test pooler', + 'vm_lifetime_auth' => 2, + 'experimental_features' => true + }, + pools: [ + {'name' => 'pool1', 'size' => 5, 'template' => 'templates/pool1', 'clone_target' => 'default_cluster'}, + {'name' => 'pool2', 'size' => 10} + ], + statsd: { 'prefix' => 'stats_prefix'}, + alias: { 'poolone' => 'pool1' }, + pool_names: [ 'pool1', 'pool2', 'poolone' ] + } + } + + describe '/poolreset' do + let(:prefix) { '/api/v1' } + let(:metrics) { Vmpooler::DummyStatsd.new } + + let(:current_time) { Time.now } + + before(:each) do + app.settings.set :config, config + app.settings.set :redis, redis + app.settings.set :metrics, metrics + app.settings.set :config, auth: false + create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) + end + + describe 'POST /poolreset' do + it 'refreshes ready and pending instances from a pool' do + post "#{prefix}/poolreset", '{"pool1":"1"}' + expect_json(ok = true, http = 201) + + expected = { ok: true } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'fails on nonexistent pools' do + post "#{prefix}/poolreset", '{"poolpoolpool":"1"}' + expect_json(ok = false, http = 400) + end + + it 'resets multiple pools' do + post "#{prefix}/poolreset", '{"pool1":"1","pool2":"1"}' + expect_json(ok = true, http = 201) + + expected = { ok: true } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'fails when not all pools exist' do + post "#{prefix}/poolreset", '{"pool1":"1","pool3":"1"}' + expect_json(ok = false, http = 400) + + expected = { + ok: false, + bad_pools: ['pool3'] + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + context 'with experimental features disabled' do + before(:each) do + config[:config]['experimental_features'] = false + end + + it 'should return 405' do + post "#{prefix}/poolreset", '{"pool1":"1"}' + expect_json(ok = false, http = 405) + + expected = { ok: false } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + end + + it 'should return 400 for invalid json' do + post "#{prefix}/poolreset", '{"pool1":"1}' + expect_json(ok = false, http = 400) + + expected = { ok: false } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'should return 400 with a bad pool name' do + post "#{prefix}/poolreset", '{"pool11":"1"}' + expect_json(ok = false, http = 400) + + expected = { ok: false } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'should return 404 when there is no payload' do + post "#{prefix}/poolreset" + expect_json(ok = false, http = 404) + + expected = { ok: false } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + end + end +end diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 3b41e62..8cc9d05 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -2874,6 +2874,28 @@ EOT end end end + + describe 'with the pool_reset wakeup option' do + let(:wakeup_option) {{ + :pool_reset => true, + :poolname => pool + }} + + let(:wakeup_period) { -1 } # A negative number forces the wakeup evaluation to always occur + + context 'when a pool reset is requested' do + before(:each) do + redis.sadd('vmpooler__poolreset', pool) + end + + it 'should sleep until the reset request is detected' do + expect(subject).to receive(:sleep).exactly(3).times + expect(redis).to receive(:sismember).with('vmpooler__poolreset', pool).and_return(false,false,true) + + subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option) + end + end + end end describe "#check_pool" do From 0a21ac563d3619c55da3795b4c3b0623da3b5279 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Wed, 12 Feb 2020 20:53:08 -0800 Subject: [PATCH 024/371] Update travis tests to use latest ruby versions This commit updates travis to use ruby 2.4.9, 2.5.7, and jruby 9.2.9.0 for tests. Test with latest z releases of ruby versions --- .travis.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 978a549..cd54d84 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,22 +4,22 @@ language: ruby matrix: include: - - rvm: 2.4.5 + - rvm: 2.4.9 env: "CHECK=rubocop" - - rvm: 2.4.5 + - rvm: 2.4.9 env: "CHECK=test" - - rvm: 2.5.3 + - rvm: 2.5.7 env: "CHECK=test" - - rvm: jruby-9.2.5.0 + - rvm: jruby-9.2.9.0 env: "CHECK=test" # Remove the allow_failures section once # Rubocop is required for Travis to pass a build allow_failures: - - rvm: 2.4.5 + - rvm: 2.4.9 env: "CHECK=rubocop" install: From 6f30d7b973d0327c8d82ea2c576fd42a7c6739a4 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Fri, 14 Feb 2020 10:04:38 -0800 Subject: [PATCH 025/371] Update changelog for 0.10.0 release --- CHANGELOG.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ed56da..15a9841 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,27 @@ Tracking in this Changelog began for this project with the tagging of version 0. If you're looking for changes from before this, refer to the project's git logs & PR history. -## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.8.2...master) +## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.10.0...master) + +## [0.10.0](https://github.com/puppetlabs/vmpooler/compare/0.9.1...0.10.0) + +### Added + +- API endpoint for resetting a pool clearing ready and pending instances (POOLER-153) + +## [0.9.1](https://github.com/puppetlabs/vmpooler/compare/0.9.0...0.9.1) + +### Added + +- Generate a wider set of human readable machine names + +## [0.9.0](https://github.com/puppetlabs/vmpooler/compare/0.8.2...0.9.0) + +### Added + +- Support nested folders in find_cluster() +- Add capability to set a max machine TTL (POOLER-123) +- Add marked as failed stat ## [0.8.2](https://github.com/puppetlabs/vmpooler/compare/0.8.1...0.8.2) From 39dd692db1ece4a31927d5ae78bbfd68d8503496 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Fri, 14 Feb 2020 10:09:53 -0800 Subject: [PATCH 026/371] Bump version.rb to 0.10.0 I probably should have been using some automation so that I didn't tag before bumping this version. Whoops! --- 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 49a189b..705d25c 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.9.1'.freeze + VERSION = '0.10.0'.freeze end From 81db9fb515e2fc0fc6235669cbdbf37078a381be Mon Sep 17 00:00:00 2001 From: Brandon High Date: Fri, 14 Feb 2020 10:15:42 -0800 Subject: [PATCH 027/371] Bump version.rb and CHANGELOG.md to 0.10.1 This commit is to fix a mistake made the in the tagging and release process. --- CHANGELOG.md | 4 ++++ lib/vmpooler/version.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15a9841..92f6a0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ git logs & PR history. ## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.10.0...master) +## [0.10.1](https://github.com/puppetlabs/vmpooler/compare/0.10.0...0.10.1) + +- No changes, version bump only + ## [0.10.0](https://github.com/puppetlabs/vmpooler/compare/0.9.1...0.10.0) ### Added diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 705d25c..3c0a81a 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.10.0'.freeze + VERSION = '0.10.1'.freeze end From 7ac03c6c9405a0d19cdf6185fae932741a6197a5 Mon Sep 17 00:00:00 2001 From: Jenkins Date: Fri, 14 Feb 2020 18:24:38 +0000 Subject: [PATCH 028/371] (GEM) update vmpooler version to 0.10.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 3c0a81a..9434008 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.10.1'.freeze + VERSION = '0.10.2'.freeze end From 1fe80194e3d4e04b73c376cdf2b303a85f0b696a Mon Sep 17 00:00:00 2001 From: Brandon High Date: Tue, 3 Mar 2020 14:00:00 -0800 Subject: [PATCH 029/371] (POOLER-154) Delay vm host update until after migration completes Prior to this commit the vsphere migration code updated the redis value for where the VM is running (`vmpooler__vm__#{vm_name}/host`) before attempting the migration. This meant that if the migration failed, there was no record of what the original host for the VM was. Additionally, the VM was marked as migrated before the migration happened which didn't reflect reality if the migration failed. This commit moves the redis update during migration to after the migration has completed. This means that if an exception is thrown in the migration code, the original host won't be lost and the host won't be considered as migrated when it was not. --- lib/vmpooler/providers/vsphere.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index 53524fe..2ffe409 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -979,10 +979,10 @@ module Vmpooler def migrate_vm_to_new_host(pool_name, vm_name, vm_hash, connection) $redis.sadd('vmpooler__migration', vm_name) target_host_name = select_next_host(pool_name, @provider_hosts, vm_hash['architecture']) - $redis.hset("vmpooler__vm__#{vm_name}", 'host', target_host_name) - $redis.hset("vmpooler__vm__#{vm_name}", 'migrated', true) target_host_object = find_host_by_dnsname(connection, target_host_name) finish = migrate_vm_and_record_timing(pool_name, vm_name, vm_hash, target_host_object, target_host_name) + $redis.hset("vmpooler__vm__#{vm_name}", 'host', target_host_name) + $redis.hset("vmpooler__vm__#{vm_name}", 'migrated', true) logger.log('s', "[>] [#{pool_name}] '#{vm_name}' migrated from #{vm_hash['host_name']} to #{target_host_name} in #{finish} seconds") ensure $redis.srem('vmpooler__migration', vm_name) From cec7183fdc9e92abf40f98f1f1b1806766cb6847 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Wed, 4 Mar 2020 11:44:36 -0800 Subject: [PATCH 030/371] Release 0.10.3 Updates for release 0.10.3, tagging will be handled by the job. --- CHANGELOG.md | 9 ++++++++- lib/vmpooler/version.rb | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92f6a0d..4e16d5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,14 @@ Tracking in this Changelog began for this project with the tagging of version 0. If you're looking for changes from before this, refer to the project's git logs & PR history. -## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.10.0...master) +## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.10.3...master) + +## [0.10.3](https://github.com/puppetlabs/vmpooler/compare/0.10.2...0.10.3) + +- Fix Redis update during migration to better reflect actual state (POOLER-154) + +## [0.10.2](https://github.com/puppetlabs/vmpooler/compare/0.10.1...0.10.2) +- Version bump (no code changes) ## [0.10.1](https://github.com/puppetlabs/vmpooler/compare/0.10.0...0.10.1) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 9434008..2b2306c 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.10.2'.freeze + VERSION = '0.10.3'.freeze end From 9201a0befbdbb41675912a8df3084e84f7d9661b Mon Sep 17 00:00:00 2001 From: Brandon High Date: Wed, 4 Mar 2020 12:06:54 -0800 Subject: [PATCH 031/371] Rubocop 0.80 updates to rubocop configs Arbitrary annoying name changes to cops and new cops that I don't think we would care about: https://rubocop.readthedocs.io/en/latest/cops_style/#stylehasheachmethods https://rubocop.readthedocs.io/en/latest/cops_style/#stylehashtransformkeys https://rubocop.readthedocs.io/en/latest/cops_style/#stylehashtransformvalues --- .rubocop.yml | 13 ++++++++++++- .rubocop_todo.yml | 26 +++----------------------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index e588c52..a79ebcf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,7 +17,7 @@ Style/Documentation: Enabled: false # Line length is not useful -Metrics/LineLength: +Layout/LineLength: Enabled: false # Empty method definitions over more than one line is ok @@ -69,3 +69,14 @@ Style/GuardClause: Layout/EndOfLine: EnforcedStyle: lf +# Added in 0.80, don't really care about the change +Style/HashEachMethods: + Enabled: false + +# Added in 0.80, don't really care about the change +Style/HashTransformKeys: + Enabled: false + +# Added in 0.80, don't really care about the change +Style/HashTransformValues: + Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2c162d6..6105e46 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -10,7 +10,7 @@ # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: with_first_parameter, with_fixed_indentation -Layout/AlignParameters: +Layout/ParameterAlignment: Exclude: - 'lib/vmpooler/api/v1.rb' @@ -55,7 +55,7 @@ Layout/EmptyLinesAroundModuleBody: # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: special_inside_parentheses, consistent, align_braces -Layout/IndentFirstHashElement: +Layout/FirstHashElementIndentation: Exclude: - 'lib/vmpooler/api/helpers.rb' - 'lib/vmpooler/api/v1.rb' @@ -96,12 +96,6 @@ Layout/SpaceAroundOperators: Exclude: - 'lib/vmpooler/api/v1.rb' -# Offense count: 2 -# Cop supports --auto-correct. -Layout/SpaceInsideBrackets: - Exclude: - - 'lib/vmpooler/api/v1.rb' - # Offense count: 8 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, EnforcedStyleForEmptyBraces, SupportedStylesForEmptyBraces. @@ -125,7 +119,7 @@ Lint/AssignmentInCondition: - 'lib/vmpooler/api/v1.rb' # Offense count: 2 -Lint/HandleExceptions: +Lint/SuppressedException: Exclude: - 'lib/vmpooler/api/dashboard.rb' @@ -147,12 +141,6 @@ Lint/UselessAssignment: - 'lib/vmpooler/api/dashboard.rb' - 'lib/vmpooler/api/helpers.rb' -# Offense count: 2 -# Cop supports --auto-correct. -Performance/RedundantMatch: - Exclude: - - 'lib/vmpooler/api/v1.rb' - # Offense count: 6 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. @@ -162,14 +150,6 @@ Style/AndOr: - 'lib/vmpooler/api/helpers.rb' - 'lib/vmpooler/api/v1.rb' -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: braces, no_braces, context_dependent -Style/BracesAroundHashParameters: - Exclude: - - 'lib/vmpooler/api/v1.rb' - # Offense count: 1 Style/CaseEquality: Exclude: From b613a2dc07d78e31bbe5ab8607fd20b666888f7c Mon Sep 17 00:00:00 2001 From: Brandon High Date: Wed, 4 Mar 2020 12:14:23 -0800 Subject: [PATCH 032/371] Update CHANGELOG for 0.10.3 (again) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e16d5e..3d5ef30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ git logs & PR history. ## [0.10.3](https://github.com/puppetlabs/vmpooler/compare/0.10.2...0.10.3) - Fix Redis update during migration to better reflect actual state (POOLER-154) +- Update Rubocop config, now we can actually see all the failing cops ## [0.10.2](https://github.com/puppetlabs/vmpooler/compare/0.10.1...0.10.2) - Version bump (no code changes) From 692937713254193805b505cd63c9f04bc333d697 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Wed, 4 Mar 2020 12:46:13 -0800 Subject: [PATCH 033/371] Decrement version in version.rb I always forget that the job both bumps this version and tags, so undoing the bump I did previously. --- 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 2b2306c..9434008 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.10.3'.freeze + VERSION = '0.10.2'.freeze end From dfa2f8620aa335780ff0b6624e0a142c4370234e Mon Sep 17 00:00:00 2001 From: Jenkins Date: Wed, 4 Mar 2020 20:47:38 +0000 Subject: [PATCH 034/371] (GEM) update vmpooler version to 0.10.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 9434008..2b2306c 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,3 @@ module Vmpooler - VERSION = '0.10.2'.freeze + VERSION = '0.10.3'.freeze end From 8bb89b604dbe455fe1ee28b2e143bc9a1b2e160f Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Wed, 4 Mar 2020 16:35:52 -0800 Subject: [PATCH 035/371] (POOLER-157) Add extra_config option to vmpooler This commit adds the extra_config option to vmpooler to allow specifying additional configuration files to load from. Without this change vmpooler does not offer a mechanism to provide additional configuration files for the application. --- CHANGELOG.md | 10 +++++----- docs/configuration.md | 7 +++++++ lib/vmpooler.rb | 8 ++++++++ spec/unit/env_config.rb | 1 + vmpooler.yaml.example | 6 ++++++ 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d5ef30..7f42c55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,19 +13,19 @@ git logs & PR history. ## [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.10.3...master) +### Added + +- Option to provide path to additional configuration files (POOLER-157) + ## [0.10.3](https://github.com/puppetlabs/vmpooler/compare/0.10.2...0.10.3) - Fix Redis update during migration to better reflect actual state (POOLER-154) - Update Rubocop config, now we can actually see all the failing cops ## [0.10.2](https://github.com/puppetlabs/vmpooler/compare/0.10.1...0.10.2) -- Version bump (no code changes) -## [0.10.1](https://github.com/puppetlabs/vmpooler/compare/0.10.0...0.10.1) -- No changes, version bump only - -## [0.10.0](https://github.com/puppetlabs/vmpooler/compare/0.9.1...0.10.0) +## [0.10.2](https://github.com/puppetlabs/vmpooler/compare/0.9.1...0.10.2) ### Added diff --git a/docs/configuration.md b/docs/configuration.md index 3ce13de..26d485b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -176,6 +176,13 @@ https://jenkins.example.com/job/platform\_puppet-agent-extra\_puppet-agent-integ Expects a boolean value (optional; default: false) +### EXTRA\_CONFIG + +Specify additional application configuration files +The argument can accept a full path to a file, or multiple files comma separated. +Expects a string value +(optional) + ## API options ### AUTH\_PROVIDER diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index e94f1c2..ddc1582 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -27,6 +27,14 @@ module Vmpooler # Take the name of the config file either from an ENV variable or from the filepath argument config_file = ENV['VMPOOLER_CONFIG_FILE'] || filepath parsed_config = YAML.load_file(config_file) if File.exist? config_file + parsed_config[:config]['extra_config'] = ENV['EXTRA_CONFIG'] if ENV['EXTRA_CONFIG'] + if parsed_config[:config]['extra_config'] + extra_configs = parsed_config[:config]['extra_config'].split(',') + extra_configs.each do |config| + extra_config = YAML.load_file(config) + parsed_config.merge!(extra_config) + end + end end parsed_config ||= { config: {} } diff --git a/spec/unit/env_config.rb b/spec/unit/env_config.rb index 1dd1bdf..91d79f6 100644 --- a/spec/unit/env_config.rb +++ b/spec/unit/env_config.rb @@ -30,6 +30,7 @@ describe 'Vmpooler' do ['experimental_features', test_bool, nil], ['purge_unconfigured_folders', test_bool, nil], ['usage_stats', test_bool, nil], + ['extra_config', test_string, nil], ] test_cases.each do |key, value, default| diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index e29a8ce..c2d9c5b 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -504,6 +504,12 @@ # lifetime (by checking how long it has already been running). # (optional; default: unlimited) # +# - extra_config +# Specify additional application configuration files +# The argument can accept a full path to a file, or multiple files comma separated. +# Expects a string value +# (optional) +# # Example: :config: From 0314357cb9d3b7b2189b51e26526bbe27df81c46 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Thu, 5 Mar 2020 08:43:10 -0800 Subject: [PATCH 036/371] Remove duplicate of 0.10.2 from CHANGELOG --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f42c55..e6e4f42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,9 +22,6 @@ git logs & PR history. - Fix Redis update during migration to better reflect actual state (POOLER-154) - Update Rubocop config, now we can actually see all the failing cops -## [0.10.2](https://github.com/puppetlabs/vmpooler/compare/0.10.1...0.10.2) - - ## [0.10.2](https://github.com/puppetlabs/vmpooler/compare/0.9.1...0.10.2) ### Added From 29519006faf72c173f4b833aebe5d0d7c43afddf Mon Sep 17 00:00:00 2001 From: Brandon High Date: Thu, 5 Mar 2020 10:32:11 -0800 Subject: [PATCH 037/371] Fix Rubocop "safe" auto-corrections Generated using `bundle exec rubocop --safe --auto-correct` --- bin/vmpooler | 4 +- lib/vmpooler.rb | 15 +- lib/vmpooler/api/dashboard.rb | 11 +- lib/vmpooler/api/helpers.rb | 41 +++-- lib/vmpooler/api/v1.rb | 92 +++++----- lib/vmpooler/dashboard.rb | 1 - lib/vmpooler/generic_connection_pool.rb | 12 +- lib/vmpooler/graphite.rb | 8 +- lib/vmpooler/pool_manager.rb | 219 +++++++++++++----------- lib/vmpooler/providers.rb | 20 +-- lib/vmpooler/providers/base.rb | 6 +- lib/vmpooler/providers/dummy.rb | 26 +-- lib/vmpooler/providers/vsphere.rb | 158 +++++++++-------- lib/vmpooler/statsd.rb | 16 +- 14 files changed, 327 insertions(+), 302 deletions(-) diff --git a/bin/vmpooler b/bin/vmpooler index b84a139..d9155f5 100755 --- a/bin/vmpooler +++ b/bin/vmpooler @@ -49,6 +49,4 @@ if ENV['VMPOOLER_DEBUG'] end end -torun_threads.each do |th| - th.join -end +torun_threads.each(&:join) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index ddc1582..bbdc3b6 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -58,7 +58,7 @@ module Vmpooler parsed_config[:config]['task_limit'] = string_to_int(ENV['TASK_LIMIT']) || parsed_config[:config]['task_limit'] || 10 parsed_config[:config]['migration_limit'] = string_to_int(ENV['MIGRATION_LIMIT']) if ENV['MIGRATION_LIMIT'] parsed_config[:config]['vm_checktime'] = string_to_int(ENV['VM_CHECKTIME']) || parsed_config[:config]['vm_checktime'] || 1 - parsed_config[:config]['vm_lifetime'] = string_to_int(ENV['VM_LIFETIME']) || parsed_config[:config]['vm_lifetime'] || 24 + parsed_config[:config]['vm_lifetime'] = string_to_int(ENV['VM_LIFETIME']) || parsed_config[:config]['vm_lifetime'] || 24 parsed_config[:config]['prefix'] = ENV['PREFIX'] || parsed_config[:config]['prefix'] || '' parsed_config[:config]['logfile'] = ENV['LOGFILE'] if ENV['LOGFILE'] @@ -130,10 +130,8 @@ module Vmpooler end end - if parsed_config[:tagfilter] - parsed_config[:tagfilter].keys.each do |tag| - parsed_config[:tagfilter][tag] = Regexp.new(parsed_config[:tagfilter][tag]) - end + parsed_config[:tagfilter]&.keys&.each do |tag| + parsed_config[:tagfilter][tag] = Regexp.new(parsed_config[:tagfilter][tag]) end parsed_config[:uptime] = Time.now @@ -179,7 +177,7 @@ module Vmpooler def self.pool_index(pools) pools_hash = {} index = 0 - for pool in pools + pools.each do |pool| pools_hash[pool['name']] = index index += 1 end @@ -190,11 +188,12 @@ module Vmpooler # Returns a integer if input is a string return if s.nil? return unless s =~ /\d/ - return Integer(s) + + Integer(s) end def self.true?(obj) - obj.to_s.downcase == "true" + obj.to_s.downcase == 'true' end def self.set_linked_clone(parsed_config) diff --git a/lib/vmpooler/api/dashboard.rb b/lib/vmpooler/api/dashboard.rb index 2288b79..c0dbe6c 100644 --- a/lib/vmpooler/api/dashboard.rb +++ b/lib/vmpooler/api/dashboard.rb @@ -1,7 +1,6 @@ module Vmpooler class API class Dashboard < Sinatra::Base - helpers do include Vmpooler::API::Helpers end @@ -21,9 +20,11 @@ module Vmpooler if config[:graphs] return false unless config[:graphs]['server'] + @graph_server = config[:graphs]['server'] elsif config[:graphite] return false unless config[:graphite]['server'] + @graph_server = config[:graphite]['server'] else false @@ -36,9 +37,11 @@ module Vmpooler if config[:graphs] return 'vmpooler' unless config[:graphs]['prefix'] + @graph_prefix = config[:graphs]['prefix'] elsif config[:graphite] return false unless config[:graphite]['prefix'] + @graph_prefix = config[:graphite]['prefix'] else false @@ -48,12 +51,14 @@ module Vmpooler # what is the base URL for viewable graphs? def graph_url return false unless graph_server && graph_prefix + @graph_url ||= "http://#{graph_server}/render?target=#{graph_prefix}" end # return a full URL to a viewable graph for a given metrics target (graphite syntax) def graph_link(target = '') return '' unless graph_url + graph_url + target end @@ -100,7 +105,7 @@ module Vmpooler end end end - rescue + rescue StandardError end else pools.each do |pool| @@ -147,7 +152,7 @@ module Vmpooler end end end - rescue + rescue StandardError end end end diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index 6fdd2f6..72d11ad 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -71,6 +71,7 @@ module Vmpooler ) return true if ldap.bind + return false end @@ -90,7 +91,7 @@ module Vmpooler auth[:ldap]['user_object'], search_base, username_str, - password_str, + password_str ) return true if result == true end @@ -101,7 +102,7 @@ module Vmpooler auth[:ldap]['user_object'], ldap_base, username_str, - password_str, + password_str ) return result end @@ -124,6 +125,7 @@ module Vmpooler tags.each_pair do |tag, value| next unless filter = Vmpooler::API.settings.config[:tagfilter][tag] + tags[tag] = value.match(filter).captures.join if value.match(filter) end @@ -161,7 +163,7 @@ module Vmpooler backend.scard(key + pool['name']) end end - res.inject(0){ |m, x| m+x }.to_i + res.inject(0) { |m, x| m + x }.to_i end # Takes the pools and a key to run scard on @@ -201,12 +203,12 @@ module Vmpooler def get_capacity_metrics(pools, backend) capacity = { current: 0, - total: 0, + total: 0, percent: 0 } pools.each do |pool| - capacity[:total] += pool['size'].to_i + capacity[:total] += pool['size'].to_i end capacity[:current] = get_total_across_pools_redis_scard(pools, 'vmpooler__ready__', backend) @@ -220,16 +222,16 @@ module Vmpooler def get_queue_metrics(pools, backend) queue = { - pending: 0, - cloning: 0, - booting: 0, - ready: 0, - running: 0, + pending: 0, + cloning: 0, + booting: 0, + ready: 0, + running: 0, completed: 0, - total: 0 + total: 0 } - queue[:pending] = get_total_across_pools_redis_scard(pools,'vmpooler__pending__', backend) + queue[:pending] = get_total_across_pools_redis_scard(pools, 'vmpooler__pending__', backend) queue[:ready] = get_total_across_pools_redis_scard(pools, 'vmpooler__ready__', backend) queue[:running] = get_total_across_pools_redis_scard(pools, 'vmpooler__running__', backend) queue[:completed] = get_total_across_pools_redis_scard(pools, 'vmpooler__completed__', backend) @@ -306,11 +308,11 @@ module Vmpooler task = { duration: { average: 0, - min: 0, - max: 0, - total: 0 + min: 0, + max: 0, + total: 0 }, - count: { + count: { total: 0 } } @@ -450,7 +452,7 @@ module Vmpooler def pool_index(pools) pools_hash = {} index = 0 - for pool in pools + pools.each do |pool| pools_hash[pool['name']] = index index += 1 end @@ -461,13 +463,14 @@ module Vmpooler prepared_template = backend.hget('vmpooler__template__prepared', pool['name']) return false if prepared_template.nil? return true if pool['template'] == prepared_template + return false end def is_integer?(x) Integer(x) true - rescue + rescue StandardError false end @@ -487,7 +490,7 @@ module Vmpooler def vm_ready?(vm_name, domain = nil) begin open_socket(vm_name, domain) - rescue => _err + rescue StandardError => _e return false end diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 17e3710..589b707 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -37,22 +37,24 @@ module Vmpooler end def checkoutlock - Vmpooler::API::settings.checkoutlock + Vmpooler::API.settings.checkoutlock end def fetch_single_vm(template) template_backends = [template] aliases = Vmpooler::API.settings.config[:alias] if aliases - template_backends = template_backends + aliases[template] if aliases[template].is_a?(Array) + template_backends += aliases[template] if aliases[template].is_a?(Array) template_backends << aliases[template] if aliases[template].is_a?(String) pool_index = pool_index(pools) weighted_pools = {} template_backends.each do |t| next unless pool_index.key? t + index = pool_index[t] clone_target = pools[index]['clone_target'] || config['clone_target'] next unless config.key?('backend_weight') + weight = config['backend_weight'][clone_target] if weight weighted_pools[t] = weight @@ -75,6 +77,7 @@ module Vmpooler template_backends.each do |template_backend| vms = backend.smembers("vmpooler__ready__#{template_backend}") next if vms.empty? + vms.reverse.each do |vm| ready = vm_ready?(vm, config['domain']) if ready @@ -104,7 +107,7 @@ module Vmpooler 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.hget('vmpooler__token__' + request.env['HTTP_X_AUTH_TOKEN'], 'user') ) if config['vm_lifetime_auth'].to_i > 0 @@ -136,14 +139,14 @@ module Vmpooler metrics.increment('checkout.empty.' + requested) break else - vms << [ vmpool, vmname, vmtemplate ] + vms << [vmpool, vmname, vmtemplate] metrics.increment('checkout.success.' + vmtemplate) end end end if failed - vms.each do |(vmpool, vmname, vmtemplate)| + vms.each do |(vmpool, vmname, _vmtemplate)| return_vm_to_ready_state(vmpool, vmname) end status 503 @@ -203,7 +206,7 @@ module Vmpooler def reset_pool(payload) result = { 'ok' => false } - payload.each do |poolname, count| + payload.each do |poolname, _count| backend.sadd('vmpooler__poolreset', poolname) end status 201 @@ -234,42 +237,36 @@ module Vmpooler def sync_pool_templates pool_index = pool_index(pools) template_configs = backend.hgetall('vmpooler__config__template') - unless template_configs.nil? - template_configs.each do |poolname, template| + template_configs&.each do |poolname, template| if pool_index.include? poolname unless pools[pool_index[poolname]]['template'] == template pools[pool_index[poolname]]['template'] = template end end - end end end def sync_pool_sizes pool_index = pool_index(pools) poolsize_configs = backend.hgetall('vmpooler__config__poolsize') - unless poolsize_configs.nil? - poolsize_configs.each do |poolname, size| + poolsize_configs&.each do |poolname, size| if pool_index.include? poolname unless pools[pool_index[poolname]]['size'] == size.to_i pools[pool_index[poolname]]['size'] == size.to_i end end - end end end def sync_clone_targets pool_index = pool_index(pools) clone_target_configs = backend.hgetall('vmpooler__config__clone_target') - unless clone_target_configs.nil? - clone_target_configs.each do |poolname, clone_target| + clone_target_configs&.each do |poolname, clone_target| if pool_index.include? poolname unless pools[pool_index[poolname]]['clone_target'] == clone_target pools[pool_index[poolname]]['clone_target'] == clone_target end end - end end end @@ -368,36 +365,38 @@ module Vmpooler pending_hash = get_list_across_pools_redis_scard(pools, 'vmpooler__pending__', backend) lastBoot_hash = get_list_across_pools_redis_hget(pools, 'vmpooler__lastboot', backend) - pools.each do |pool| - # REMIND: move this out of the API and into the back-end - ready = ready_hash[pool['name']] - running = running_hash[pool['name']] - pending = pending_hash[pool['name']] - max = pool['size'] - lastBoot = lastBoot_hash[pool['name']] - aka = pool['alias'] + unless views and not views.include?("pools") + pools.each do |pool| + # REMIND: move this out of the API and into the back-end + ready = ready_hash[pool['name']] + running = running_hash[pool['name']] + pending = pending_hash[pool['name']] + max = pool['size'] + lastBoot = lastBoot_hash[pool['name']] + aka = pool['alias'] - result[:pools][pool['name']] = { - ready: ready, - running: running, - pending: pending, - max: max, - lastBoot: lastBoot - } + result[:pools][pool['name']] = { + ready: ready, + running: running, + pending: pending, + max: max, + lastBoot: lastBoot + } - if aka - result[:pools][pool['name']][:alias] = aka + if aka + result[:pools][pool['name']][:alias] = aka + end + + # for backwards compatibility, include separate "empty" stats in "status" block + if ready == 0 + result[:status][:empty] ||= [] + result[:status][:empty].push(pool['name']) + + result[:status][:ok] = false + result[:status][:message] = "Found #{result[:status][:empty].length} empty pools." + end end - - # for backwards compatibility, include separate "empty" stats in "status" block - if ready == 0 - result[:status][:empty] ||= [] - result[:status][:empty].push(pool['name']) - - result[:status][:ok] = false - result[:status][:message] = "Found #{result[:status][:empty].length} empty pools." - end - end unless views and not views.include?("pools") + end result[:status][:uptime] = (Time.now - Vmpooler::API.settings.config[:uptime]).round(1) if Vmpooler::API.settings.config[:uptime] @@ -442,7 +441,6 @@ module Vmpooler if aka result[:pools][pool['name']][:alias] = aka end - end ready_hash = get_list_across_pools_redis_scard(poolscopy, 'vmpooler__ready__', backend) @@ -456,7 +454,7 @@ module Vmpooler get "#{api_prefix}/totalrunning/?" do content_type :json queue = { - running: 0, + running: 0 } queue[:running] = get_total_across_pools_redis_scard(pools, 'vmpooler__running__', backend) @@ -753,7 +751,7 @@ module Vmpooler def invalid_pool(payload) invalid = [] - payload.each do |pool, clone_target| + payload.each do |pool, _clone_target| invalid << pool unless pool_exists?(pool) end invalid @@ -837,7 +835,7 @@ module Vmpooler # Look up IP address of the hostname begin ipAddress = TCPSocket.gethostbyname(params[:hostname])[3] - rescue + rescue StandardError ipAddress = "" end @@ -893,7 +891,7 @@ module Vmpooler if backend.exists('vmpooler__vm__' + params[:hostname]) begin jdata = JSON.parse(request.body.read) - rescue + rescue StandardError halt 400, JSON.pretty_generate(result) end diff --git a/lib/vmpooler/dashboard.rb b/lib/vmpooler/dashboard.rb index b875465..a1b4346 100644 --- a/lib/vmpooler/dashboard.rb +++ b/lib/vmpooler/dashboard.rb @@ -1,6 +1,5 @@ module Vmpooler class Dashboard < Sinatra::Base - def config Vmpooler.config end diff --git a/lib/vmpooler/generic_connection_pool.rb b/lib/vmpooler/generic_connection_pool.rb index ca18576..b90f3ca 100644 --- a/lib/vmpooler/generic_connection_pool.rb +++ b/lib/vmpooler/generic_connection_pool.rb @@ -20,15 +20,15 @@ module Vmpooler start = Time.now conn = checkout(options) timespan_ms = ((Time.now - start) * 1000).to_i - @metrics.gauge(@metric_prefix + '.available', @available.length) unless @metrics.nil? - @metrics.timing(@metric_prefix + '.waited', timespan_ms) unless @metrics.nil? + @metrics&.gauge(@metric_prefix + '.available', @available.length) + @metrics&.timing(@metric_prefix + '.waited', timespan_ms) begin Thread.handle_interrupt(Exception => :immediate) do yield conn end ensure checkin - @metrics.gauge(@metric_prefix + '.available', @available.length) unless @metrics.nil? + @metrics&.gauge(@metric_prefix + '.available', @available.length) end end end @@ -38,13 +38,13 @@ module Vmpooler start = Time.now conn = checkout(options) timespan_ms = ((Time.now - start) * 1000).to_i - @metrics.gauge(@metric_prefix + '.available', @available.length) unless @metrics.nil? - @metrics.timing(@metric_prefix + '.waited', timespan_ms) unless @metrics.nil? + @metrics&.gauge(@metric_prefix + '.available', @available.length) + @metrics&.timing(@metric_prefix + '.waited', timespan_ms) begin yield conn ensure checkin - @metrics.gauge(@metric_prefix + '.available', @available.length) unless @metrics.nil? + @metrics&.gauge(@metric_prefix + '.available', @available.length) end end end diff --git a/lib/vmpooler/graphite.rb b/lib/vmpooler/graphite.rb index e7f28c3..c505f2c 100644 --- a/lib/vmpooler/graphite.rb +++ b/lib/vmpooler/graphite.rb @@ -5,9 +5,7 @@ module Vmpooler attr_reader :server, :port, :prefix def initialize(params = {}) - if params['server'].nil? || params['server'].empty? - raise ArgumentError, "Graphite server is required. Config: #{params.inspect}" - end + raise ArgumentError, "Graphite server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty? @server = params['server'] @port = params['port'] || 2003 @@ -35,8 +33,8 @@ module Vmpooler socket.close end end - rescue => err - $stderr.puts "Failure logging #{path} to graphite server [#{server}:#{port}]: #{err}" + rescue StandardError => e + warn "Failure logging #{path} to graphite server [#{server}:#{port}]: #{e}" end end end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 37063de..a39be30 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -63,7 +63,7 @@ module Vmpooler $redis.del("vmpooler__pool__#{pool}") end end - return + nil end # Check the state of a VM @@ -71,8 +71,8 @@ module Vmpooler Thread.new do begin _check_pending_vm(vm, pool, timeout, provider) - rescue => err - $logger.log('s', "[!] [#{pool}] '#{vm}' #{timeout} #{provider} errored while checking a pending vm : #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool}] '#{vm}' #{timeout} #{provider} errored while checking a pending vm : #{e}") fail_pending_vm(vm, pool, timeout) raise end @@ -82,6 +82,7 @@ module Vmpooler def _check_pending_vm(vm, pool, timeout, provider) mutex = vm_mutex(vm) return if mutex.locked? + mutex.synchronize do if provider.vm_ready?(pool, vm) move_pending_vm_to_ready(vm, pool) @@ -111,8 +112,8 @@ module Vmpooler end end true - rescue => err - $logger.log('d', "Fail pending VM failed with an error: #{err}") + rescue StandardError => e + $logger.log('d', "Fail pending VM failed with an error: #{e}") false end @@ -134,8 +135,9 @@ module Vmpooler def vm_still_ready?(pool_name, vm_name, provider) # Check if the VM is still ready/available return true if provider.vm_ready?(pool_name, vm_name) + raise("VM #{vm_name} is not ready") - rescue + rescue StandardError move_vm_queue(pool_name, vm_name, 'ready', 'completed', "is unreachable, removed from 'ready' queue") end @@ -143,8 +145,8 @@ module Vmpooler Thread.new do begin _check_ready_vm(vm, pool_name, ttl, provider) - rescue => err - $logger.log('s', "[!] [#{pool_name}] '#{vm}' failed while checking a ready vm : #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool_name}] '#{vm}' failed while checking a ready vm : #{e}") raise end end @@ -154,6 +156,7 @@ module Vmpooler # Periodically check that the VM is available mutex = vm_mutex(vm) return if mutex.locked? + mutex.synchronize do check_stamp = $redis.hget('vmpooler__vm__' + vm, 'check') return if check_stamp && (((Time.now - Time.parse(check_stamp)) / 60) <= $config[:config]['vm_checktime']) @@ -201,22 +204,24 @@ module Vmpooler # Check if the hostname has magically changed from underneath Pooler vm_hash = provider.get_vm(pool_name, vm) return unless vm_hash.is_a? Hash + hostname = vm_hash['hostname'] return if hostname.nil? return if hostname.empty? return if hostname == vm + $redis.smove('vmpooler__ready__' + pool_name, 'vmpooler__completed__' + pool_name, vm) $logger.log('d', "[!] [#{pool_name}] '#{vm}' has mismatched hostname #{hostname}, removed from 'ready' queue") - return true + true end def check_running_vm(vm, pool, ttl, provider) Thread.new do begin _check_running_vm(vm, pool, ttl, provider) - rescue => err - $logger.log('s', "[!] [#{pool}] '#{vm}' failed while checking VM with an error: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool}] '#{vm}' failed while checking VM with an error: #{e}") raise end end @@ -225,6 +230,7 @@ module Vmpooler def _check_running_vm(vm, pool, ttl, provider) mutex = vm_mutex(vm) return if mutex.locked? + mutex.synchronize do # Check that VM is within defined lifetime checkouttime = $redis.hget('vmpooler__active__' + pool, vm) @@ -245,7 +251,7 @@ module Vmpooler if host return else - move_vm_queue(pool, vm, 'running', 'completed', "is no longer in inventory, removing from running") + move_vm_queue(pool, vm, 'running', 'completed', 'is no longer in inventory, removing from running') end end end @@ -261,14 +267,14 @@ module Vmpooler Thread.new do begin _clone_vm(pool_name, provider) - rescue => err - $logger.log('s', "[!] [#{pool_name}] failed while cloning VM with an error: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool_name}] failed while cloning VM with an error: #{e}") raise end end end - def generate_and_check_hostname(pool_name) + def generate_and_check_hostname(_pool_name) # Generate a randomized hostname. The total name must no longer than 15 # character including the hyphen. The shortest adjective in the corpus is # three characters long. Therefore, we can technically select a noun up to 11 @@ -278,12 +284,12 @@ module Vmpooler # inviting more conflicts. We favor selecting a longer noun rather than a # longer adjective because longer adjectives tend to be less fun. noun = @name_generator.noun(max: 10) - adjective = @name_generator.adjective(max: 14-noun.length) + adjective = @name_generator.adjective(max: 14 - noun.length) random_name = [adjective, noun].join('-') hostname = $config[:config]['prefix'] + random_name available = $redis.hlen('vmpooler__vm__' + hostname) == 0 - return hostname, available + [hostname, available] end def find_unique_hostname(pool_name) @@ -297,9 +303,9 @@ module Vmpooler $metrics.increment("errors.duplicatehostname.#{pool_name}") $logger.log('s', "[!] [#{pool_name}] Generated hostname #{hostname} was not unique (attempt \##{hostname_retries} of #{max_hostname_retries})") end - + raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless available - + hostname end @@ -322,11 +328,11 @@ module Vmpooler $logger.log('s', "[+] [#{pool_name}] '#{new_vmname}' cloned in #{finish} seconds") $metrics.timing("clone.#{pool_name}", finish) - rescue => _err + rescue StandardError => _e $redis.srem("vmpooler__pending__#{pool_name}", new_vmname) expiration_ttl = $config[:redis]['data_ttl'].to_i * 60 * 60 $redis.expire("vmpooler__vm__#{new_vmname}", expiration_ttl) - raise _err + raise _e ensure $redis.decr('vmpooler__tasks__clone') end @@ -337,8 +343,8 @@ module Vmpooler Thread.new do begin _destroy_vm(vm, pool, provider) - rescue => err - $logger.log('d', "[!] [#{pool}] '#{vm}' failed while destroying the VM with an error: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool}] '#{vm}' failed while destroying the VM with an error: #{e}") raise end end @@ -347,6 +353,7 @@ module Vmpooler def _destroy_vm(vm, pool, provider) mutex = vm_mutex(vm) return if mutex.locked? + mutex.synchronize do $redis.hdel('vmpooler__active__' + pool, vm) $redis.hset('vmpooler__vm__' + vm, 'destroy', Time.now) @@ -370,11 +377,13 @@ module Vmpooler def get_vm_usage_labels(vm) return unless $config[:config]['usage_stats'] + checkout = $redis.hget("vmpooler__vm__#{vm}", 'checkout') return if checkout.nil? + jenkins_build_url = $redis.hget("vmpooler__vm__#{vm}", 'tag:jenkins_build_url') user = $redis.hget("vmpooler__vm__#{vm}", 'token:user') || 'unauthenticated' - poolname = $redis.hget("vmpooler__vm__#{vm}", "template") + poolname = $redis.hget("vmpooler__vm__#{vm}", 'template') unless jenkins_build_url user = user.gsub('.', '_') @@ -404,25 +413,24 @@ module Vmpooler poolname ] - metric_parts = metric_parts.reject { |s| s.nil? } + metric_parts = metric_parts.reject(&:nil?) metric_parts = metric_parts.map { |s| s.gsub('.', '_') } $metrics.increment(metric_parts.join('.')) - rescue => err - logger.log('d', "[!] [#{poolname}] failed while evaluating usage labels on '#{vm}' with an error: #{err}") + rescue StandardError => e + logger.log('d', "[!] [#{poolname}] failed while evaluating usage labels on '#{vm}' with an error: #{e}") end def component_to_test(match, labels_string) return if labels_string.nil? + labels_string_parts = labels_string.split(',') labels_string_parts.each do |part| key, value = part.split('=') next if value.nil? - if key == match - return value - end + return value if key == match end - return + nil end def purge_unused_vms_and_folders @@ -435,13 +443,13 @@ module Vmpooler Thread.new do begin purge_vms_and_folders($providers[provider.to_s]) - rescue => err - $logger.log('s', "[!] failed while purging provider #{provider.to_s} VMs and folders with an error: #{err}") + rescue StandardError => e + $logger.log('s', "[!] failed while purging provider #{provider} VMs and folders with an error: #{e}") end end end end - return + nil end # Return a list of pool folders @@ -450,6 +458,7 @@ module Vmpooler folders = {} $config[:pools].each do |pool| next unless pool['provider'] == provider_name + folder_parts = pool['folder'].split('/') datacenter = provider.get_target_datacenter_from_config(pool['name']) folders[folder_parts.pop] = "#{datacenter}/vm/#{folder_parts.join('/')}" @@ -459,7 +468,7 @@ module Vmpooler def get_base_folders(folders) base = [] - folders.each do |key, value| + folders.each do |_key, value| base << value end base.uniq @@ -476,8 +485,8 @@ module Vmpooler Thread.new do begin _create_vm_disk(pool_name, vm, disk_size, provider) - rescue => err - $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while creating disk: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while creating disk: #{e}") raise end end @@ -512,8 +521,8 @@ module Vmpooler Thread.new do begin _create_vm_snapshot(pool_name, vm, snapshot_name, provider) - rescue => err - $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while creating snapshot: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while creating snapshot: #{e}") raise end end @@ -541,8 +550,8 @@ module Vmpooler Thread.new do begin _revert_vm_snapshot(pool_name, vm, snapshot_name, provider) - rescue => err - $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while reverting snapshot: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while reverting snapshot: #{e}") raise end end @@ -574,14 +583,14 @@ module Vmpooler # ie. ["vsphere", "dummy"] def used_providers pools = config[:pools] || [] - @used_providers ||= (pools.map { |pool| pool[:provider] || pool['provider'] }.compact + default_providers ).uniq + @used_providers ||= (pools.map { |pool| pool[:provider] || pool['provider'] }.compact + default_providers).uniq end # @return [Array] - returns a list of providers that should always be loaded # note: vsphere is the default if user does not specify although this should not be # if vsphere is to no longer be loaded by default please remove def default_providers - @default_providers ||= %w( vsphere dummy ) + @default_providers ||= %w[vsphere dummy] end def get_pool_name_for_vm(vm_name) @@ -594,6 +603,7 @@ module Vmpooler def get_provider_for_pool(pool_name) pool = $config[:pools].find { |pool| pool['name'] == pool_name } return nil unless pool + provider_name = pool.fetch('provider', nil) $providers[provider_name] end @@ -609,6 +619,7 @@ module Vmpooler unless maxloop.zero? break if loop_count >= maxloop + loop_count += 1 end end @@ -627,8 +638,8 @@ module Vmpooler raise("Missing Provider for vm #{vm_name} in pool #{pool_name}") if provider.nil? create_vm_disk(pool_name, vm_name, disk_size, provider) - rescue => err - $logger.log('s', "[!] [disk_manager] disk creation appears to have failed: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [disk_manager] disk creation appears to have failed: #{e}") end end end @@ -644,6 +655,7 @@ module Vmpooler unless maxloop.zero? break if loop_count >= maxloop + loop_count += 1 end end @@ -663,8 +675,8 @@ module Vmpooler raise("Missing Provider for vm #{vm_name} in pool #{pool_name}") if provider.nil? create_vm_snapshot(pool_name, vm_name, snapshot_name, provider) - rescue => err - $logger.log('s', "[!] [snapshot_manager] snapshot create appears to have failed: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [snapshot_manager] snapshot create appears to have failed: #{e}") end end @@ -680,8 +692,8 @@ module Vmpooler raise("Missing Provider for vm #{vm_name} in pool #{pool_name}") if provider.nil? revert_vm_snapshot(pool_name, vm_name, snapshot_name, provider) - rescue => err - $logger.log('s', "[!] [snapshot_manager] snapshot revert appears to have failed: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [snapshot_manager] snapshot revert appears to have failed: #{e}") end end end @@ -694,8 +706,8 @@ module Vmpooler $redis.srem("vmpooler__migrating__#{pool_name}", vm_name) provider.migrate_vm(pool_name, vm_name) end - rescue => err - $logger.log('s', "[x] [#{pool_name}] '#{vm_name}' migration failed with an error: #{err}") + rescue StandardError => e + $logger.log('s', "[x] [#{pool_name}] '#{vm_name}' migration failed with an error: #{e}") end end end @@ -724,17 +736,11 @@ module Vmpooler wakeup_by = Time.now + wakeup_period return if time_passed?(:exit_by, exit_by) - if options[:pool_size_change] - initial_ready_size = $redis.scard("vmpooler__ready__#{options[:poolname]}") - end + initial_ready_size = $redis.scard("vmpooler__ready__#{options[:poolname]}") if options[:pool_size_change] - if options[:clone_target_change] - initial_clone_target = $redis.hget("vmpooler__pool__#{options[:poolname]}", options[:clone_target]) - end + initial_clone_target = $redis.hget("vmpooler__pool__#{options[:poolname]}", options[:clone_target]) if options[:clone_target_change] - if options[:pool_template_change] - initial_template = $redis.hget('vmpooler__template__prepared', options[:poolname]) - end + initial_template = $redis.hget('vmpooler__template__prepared', options[:poolname]) if options[:pool_template_change] loop do sleep(1) @@ -751,7 +757,7 @@ module Vmpooler end if options[:clone_target_change] - clone_target = $redis.hget("vmpooler__config__clone_target}", options[:poolname]) + clone_target = $redis.hget('vmpooler__config__clone_target}', options[:poolname]) if clone_target break unless clone_target == initial_clone_target end @@ -795,6 +801,7 @@ module Vmpooler loop_delay = loop_delay_min provider = get_provider_for_pool(pool['name']) raise("Could not find provider '#{pool['provider']}") if provider.nil? + sync_pool_template(pool) loop do result = _check_pool(pool, provider) @@ -809,11 +816,12 @@ module Vmpooler unless maxloop.zero? break if loop_count >= maxloop + loop_count += 1 end end - rescue => err - $logger.log('s', "[!] [#{pool['name']}] Error while checking the pool: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool['name']}] Error while checking the pool: #{e}") raise end end @@ -828,19 +836,13 @@ module Vmpooler end def dereference_mutex(vmname) - if @vm_mutex.delete(vmname) - return true - else - return - end + true if @vm_mutex.delete(vmname) end def sync_pool_template(pool) pool_template = $redis.hget('vmpooler__config__template', pool['name']) if pool_template - unless pool['template'] == pool_template - pool['template'] = pool_template - end + pool['template'] = pool_template unless pool['template'] == pool_template end end @@ -850,8 +852,8 @@ module Vmpooler begin provider.create_template_delta_disks(pool) $redis.sadd('vmpooler__template__deltas', pool['template']) - rescue => err - $logger.log('s', "[!] [#{pool['name']}] failed while preparing a template with an error. As a result vmpooler could not create the template delta disks. Either a template delta disk already exists, or the template delta disk creation failed. The error is: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool['name']}] failed while preparing a template with an error. As a result vmpooler could not create the template delta disks. Either a template delta disk already exists, or the template delta disk creation failed. The error is: #{e}") end end end @@ -863,6 +865,7 @@ module Vmpooler prepared_template = $redis.hget('vmpooler__template__prepared', pool['name']) configured_template = $redis.hget('vmpooler__config__template', pool['name']) return if mutex.locked? + if prepared_template.nil? mutex.synchronize do prepare_template(pool, provider) @@ -878,6 +881,7 @@ module Vmpooler end return if configured_template.nil? return if configured_template == prepared_template + mutex.synchronize do update_pool_template(pool, provider, configured_template, prepared_template) end @@ -913,9 +917,11 @@ module Vmpooler def update_clone_target(pool) mutex = pool_mutex(pool['name']) return if mutex.locked? + clone_target = $redis.hget('vmpooler__config__clone_target', pool['name']) return if clone_target.nil? return if clone_target == pool['clone_target'] + $logger.log('s', "[*] [#{pool['name']}] clone updated from #{pool['clone_target']} to #{clone_target}") mutex.synchronize do pool['clone_target'] = clone_target @@ -930,9 +936,11 @@ module Vmpooler total = $redis.scard("vmpooler__pending__#{pool['name']}") + ready return if total.nil? return if total == 0 + mutex = pool_mutex(pool['name']) return if mutex.locked? return unless ready > pool['size'] + mutex.synchronize do difference = ready - pool['size'] difference.times do @@ -950,10 +958,13 @@ module Vmpooler def update_pool_size(pool) mutex = pool_mutex(pool['name']) return if mutex.locked? + poolsize = $redis.hget('vmpooler__config__poolsize', pool['name']) return if poolsize.nil? + poolsize = Integer(poolsize) return if poolsize == pool['size'] + mutex.synchronize do pool['size'] = poolsize end @@ -962,6 +973,7 @@ module Vmpooler def reset_pool(pool) poolname = pool['name'] return unless $redis.sismember('vmpooler__poolreset', poolname) + $redis.srem('vmpooler__poolreset', poolname) mutex = pool_mutex(poolname) mutex.synchronize do @@ -992,9 +1004,9 @@ module Vmpooler inventory[vm['name']] = 1 end end - rescue => err - $logger.log('s', "[!] [#{pool['name']}] _check_pool failed with an error while running create_inventory: #{err}") - raise(err) + rescue StandardError => e + $logger.log('s', "[!] [#{pool['name']}] _check_pool failed with an error while running create_inventory: #{e}") + raise(e) end inventory end @@ -1006,8 +1018,8 @@ module Vmpooler vm_lifetime = $redis.hget('vmpooler__vm__' + vm, 'lifetime') || $config[:config]['vm_lifetime'] || 12 pool_check_response[:checked_running_vms] += 1 check_running_vm(vm, pool_name, vm_lifetime, provider) - rescue => err - $logger.log('d', "[!] [#{pool_name}] _check_pool with an error while evaluating running VMs: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] _check_pool with an error while evaluating running VMs: #{e}") end else move_vm_queue(pool_name, vm, 'running', 'completed', 'is a running VM but is missing from inventory. Marking as completed.') @@ -1021,8 +1033,8 @@ module Vmpooler begin pool_check_response[:checked_ready_vms] += 1 check_ready_vm(vm, pool_name, pool_ttl || 0, provider) - rescue => err - $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating ready VMs: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating ready VMs: #{e}") end else move_vm_queue(pool_name, vm, 'ready', 'completed', 'is a ready VM but is missing from inventory. Marking as completed.') @@ -1037,8 +1049,8 @@ module Vmpooler begin pool_check_response[:checked_pending_vms] += 1 check_pending_vm(vm, pool_name, pool_timeout, provider) - rescue => err - $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating pending VMs: #{err}") + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating pending VMs: #{e}") end else fail_pending_vm(vm, pool_name, pool_timeout, false) @@ -1052,11 +1064,11 @@ module Vmpooler begin pool_check_response[:destroyed_vms] += 1 destroy_vm(vm, pool_name, provider) - rescue => err + rescue StandardError => e $redis.srem("vmpooler__completed__#{pool_name}", vm) $redis.hdel("vmpooler__active__#{pool_name}", vm) $redis.del("vmpooler__vm__#{vm}") - $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating completed VMs: #{err}") + $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating completed VMs: #{e}") end else $logger.log('s', "[!] [#{pool_name}] '#{vm}' not found in inventory, removed from 'completed' queue") @@ -1068,22 +1080,18 @@ module Vmpooler end def check_discovered_pool_vms(pool_name) - begin - $redis.smembers("vmpooler__discovered__#{pool_name}").reverse.each do |vm| - %w[pending ready running completed].each do |queue| - if $redis.sismember("vmpooler__#{queue}__#{pool_name}", vm) - $logger.log('d', "[!] [#{pool_name}] '#{vm}' found in '#{queue}', removed from 'discovered' queue") - $redis.srem("vmpooler__discovered__#{pool_name}", vm) - end - end - - if $redis.sismember("vmpooler__discovered__#{pool_name}", vm) - $redis.smove("vmpooler__discovered__#{pool_name}", "vmpooler__completed__#{pool_name}", vm) + $redis.smembers("vmpooler__discovered__#{pool_name}").reverse.each do |vm| + %w[pending ready running completed].each do |queue| + if $redis.sismember("vmpooler__#{queue}__#{pool_name}", vm) + $logger.log('d', "[!] [#{pool_name}] '#{vm}' found in '#{queue}', removed from 'discovered' queue") + $redis.srem("vmpooler__discovered__#{pool_name}", vm) end end - rescue => err - $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating discovered VMs: #{err}") + + $redis.smove("vmpooler__discovered__#{pool_name}", "vmpooler__completed__#{pool_name}", vm) if $redis.sismember("vmpooler__discovered__#{pool_name}", vm) end + rescue StandardError => e + $logger.log('d', "[!] [#{pool_name}] _check_pool failed with an error while evaluating discovered VMs: #{e}") end def check_migrating_pool_vms(pool_name, provider, pool_check_response, inventory) @@ -1092,8 +1100,8 @@ module Vmpooler begin pool_check_response[:migrated_vms] += 1 migrate_vm(vm, pool_name, provider) - rescue => err - $logger.log('s', "[x] [#{pool_name}] '#{vm}' failed to migrate: #{err}") + rescue StandardError => e + $logger.log('s', "[x] [#{pool_name}] '#{vm}' failed to migrate: #{e}") end end end @@ -1101,6 +1109,7 @@ module Vmpooler def repopulate_pool_vms(pool_name, provider, pool_check_response, pool_size) return if pool_mutex(pool_name).locked? + ready = $redis.scard("vmpooler__ready__#{pool_name}") total = $redis.scard("vmpooler__pending__#{pool_name}") + ready @@ -1120,8 +1129,8 @@ module Vmpooler $redis.incr('vmpooler__tasks__clone') pool_check_response[:cloned_vms] += 1 clone_vm(pool_name, provider) - rescue => err - $logger.log('s', "[!] [#{pool_name}] clone failed during check_pool with an error: #{err}") + rescue StandardError => e + $logger.log('s', "[!] [#{pool_name}] clone failed during check_pool with an error: #{e}") $redis.decr('vmpooler__tasks__clone') raise end @@ -1142,7 +1151,7 @@ module Vmpooler begin inventory = create_inventory(pool, provider, pool_check_response) - rescue => err + rescue StandardError => e return(pool_check_response) end @@ -1195,6 +1204,7 @@ module Vmpooler provider_klass = Vmpooler::PoolManager::Provider provider_klass.constants.each do |classname| next unless classname.to_s.casecmp(provider_class) == 0 + return provider_klass.const_get(classname).new(config, logger, metrics, provider_name, options) end raise("Provider '#{provider_class}' is unknown for pool with provider name '#{provider_name}'") if provider.nil? @@ -1257,8 +1267,8 @@ module Vmpooler end begin $providers[provider_name] = create_provider_object($config, $logger, $metrics, provider_class, provider_name, {}) if $providers[provider_name].nil? - rescue => err - $logger.log('s', "Error while creating provider for pool #{pool['name']}: #{err}") + rescue StandardError => e + $logger.log('s', "Error while creating provider for pool #{pool['name']}: #{e}") raise end end @@ -1294,6 +1304,7 @@ module Vmpooler unless maxloop.zero? break if loop_count >= maxloop + loop_count += 1 end end diff --git a/lib/vmpooler/providers.rb b/lib/vmpooler/providers.rb index 9a5d955..6a1ff79 100644 --- a/lib/vmpooler/providers.rb +++ b/lib/vmpooler/providers.rb @@ -2,26 +2,25 @@ require 'pathname' module Vmpooler class Providers - # @param names [Array] - an array of names or string name of a provider # @return [Array] - list of provider files loaded # ie. ["lib/vmpooler/providers/base.rb", "lib/vmpooler/providers/dummy.rb", "lib/vmpooler/providers/vsphere.rb"] def self.load_by_name(names) names = Array(names) - instance = self.new - names.map {|name| instance.load_from_gems(name)}.flatten + instance = new + names.map { |name| instance.load_from_gems(name) }.flatten end # @return [Array] - array of provider files # ie. ["lib/vmpooler/providers/base.rb", "lib/vmpooler/providers/dummy.rb", "lib/vmpooler/providers/vsphere.rb"] # although these files can come from any gem def self.load_all_providers - self.new.load_from_gems + new.load_from_gems end # @return [Array] - returns an array of gem names that contain a provider def self.installed_providers - self.new.vmpooler_provider_gem_list.map(&:name) + new.vmpooler_provider_gem_list.map(&:name) end # @return [Array] returns a list of vmpooler providers gem plugin specs @@ -50,7 +49,7 @@ module Vmpooler # @return [String] - the relative path to the vmpooler provider dir # this is used when searching gems for this path def provider_path - File.join('lib','vmpooler','providers') + File.join('lib', 'vmpooler', 'providers') end # Add constants to array to skip over classes, ie. Vmpooler::PoolManager::Provider::Dummy @@ -81,8 +80,6 @@ module Vmpooler @plugin_map ||= Hash[plugin_classes.map { |gem| [gem.send(:name), gem] }] end - - # Internal: Retrieve a list of available gem paths from RubyGems. # # Returns an Array of Pathname objects. @@ -90,11 +87,11 @@ module Vmpooler dirs = [] if has_rubygems? dirs = gemspecs.map do |spec| - lib_path = File.expand_path(File.join(spec.full_gem_path,provider_path)) - lib_path if File.exists? lib_path + lib_path = File.expand_path(File.join(spec.full_gem_path, provider_path)) + lib_path if File.exist? lib_path end + included_lib_dirs end - dirs.reject { |dir| dir.nil? }.uniq + dirs.reject(&:nil?).uniq end # Internal: Check if RubyGems is loaded and available. @@ -114,6 +111,5 @@ module Vmpooler Gem.searcher.init_gemspecs end end - end end diff --git a/lib/vmpooler/providers/base.rb b/lib/vmpooler/providers/base.rb index d30564f..d77d852 100644 --- a/lib/vmpooler/providers/base.rb +++ b/lib/vmpooler/providers/base.rb @@ -222,7 +222,7 @@ module Vmpooler # [Hash] pool : Configuration for the pool # returns # nil when successful. Raises error when encountered - def create_template_delta_disks(pool) + def create_template_delta_disks(_pool) raise("#{self.class.name} does not implement create_template_delta_disks") end @@ -230,11 +230,11 @@ module Vmpooler # [String] provider_name : Name of the provider # returns # Hash of folders - def get_target_datacenter_from_config(provider_name) + def get_target_datacenter_from_config(_provider_name) raise("#{self.class.name} does not implement get_target_datacenter_from_config") end - def purge_unconfigured_folders(base_folders, configured_folders, whitelist) + def purge_unconfigured_folders(_base_folders, _configured_folders, _whitelist) raise("#{self.class.name} does not implement purge_unconfigured_folders") end end diff --git a/lib/vmpooler/providers/dummy.rb b/lib/vmpooler/providers/dummy.rb index bc924a0..2ca21ee 100644 --- a/lib/vmpooler/providers/dummy.rb +++ b/lib/vmpooler/providers/dummy.rb @@ -73,10 +73,10 @@ module Vmpooler return current_vm['vm_host'] if provider_config['migratevm_couldmove_percent'].nil? # Only migrate if migratevm_couldmove_percent is met - return current_vm['vm_host'] if 1 + rand(100) > provider_config['migratevm_couldmove_percent'] + return current_vm['vm_host'] if rand(1..100) > provider_config['migratevm_couldmove_percent'] # Simulate a 10 node cluster and randomly pick a different one - new_host = 'HOST' + (1 + rand(10)).to_s while new_host == current_vm['vm_host'] + new_host = 'HOST' + rand(1..10).to_s while new_host == current_vm['vm_host'] new_host end @@ -93,7 +93,7 @@ module Vmpooler # Inject clone failure unless provider_config['migratevm_fail_percent'].nil? - raise('Dummy Failure for migratevm_fail_percent') if 1 + rand(100) <= provider_config['migratevm_fail_percent'] + raise('Dummy Failure for migratevm_fail_percent') if rand(1..100) <= provider_config['migratevm_fail_percent'] end @write_lock.synchronize do @@ -114,7 +114,7 @@ module Vmpooler # Randomly power off the VM unless dummy['powerstate'] != 'PoweredOn' || provider_config['getvm_poweroff_percent'].nil? - if 1 + rand(100) <= provider_config['getvm_poweroff_percent'] + if rand(1..100) <= provider_config['getvm_poweroff_percent'] @write_lock.synchronize do dummy = get_dummy_vm(pool_name, vm_name) dummy['powerstate'] = 'PoweredOff' @@ -126,7 +126,7 @@ module Vmpooler # Randomly rename the host unless dummy['hostname'] != dummy['name'] || provider_config['getvm_rename_percent'].nil? - if 1 + rand(100) <= provider_config['getvm_rename_percent'] + if rand(1..100) <= provider_config['getvm_rename_percent'] @write_lock.synchronize do dummy = get_dummy_vm(pool_name, vm_name) dummy['hostname'] = 'DUMMY' + dummy['name'] @@ -194,7 +194,7 @@ module Vmpooler begin # Inject clone failure unless provider_config['createvm_fail_percent'].nil? - raise('Dummy Failure for createvm_fail_percent') if 1 + rand(100) <= provider_config['createvm_fail_percent'] + raise('Dummy Failure for createvm_fail_percent') if rand(1..100) <= provider_config['createvm_fail_percent'] end # Assert the VM is ready for use @@ -202,7 +202,7 @@ module Vmpooler vm['dummy_state'] = 'RUNNING' write_backing_file end - rescue => _err + rescue StandardError => _e @write_lock.synchronize do remove_dummy_vm(pool_name, dummy_hostname) write_backing_file @@ -227,7 +227,7 @@ module Vmpooler # Inject create failure unless provider_config['createdisk_fail_percent'].nil? - raise('Dummy Failure for createdisk_fail_percent') if 1 + rand(100) <= provider_config['createdisk_fail_percent'] + raise('Dummy Failure for createdisk_fail_percent') if rand(1..100) <= provider_config['createdisk_fail_percent'] end @write_lock.synchronize do @@ -253,7 +253,7 @@ module Vmpooler # Inject create failure unless provider_config['createsnapshot_fail_percent'].nil? - raise('Dummy Failure for createsnapshot_fail_percent') if 1 + rand(100) <= provider_config['createsnapshot_fail_percent'] + raise('Dummy Failure for createsnapshot_fail_percent') if rand(1..100) <= provider_config['createsnapshot_fail_percent'] end @write_lock.synchronize do @@ -280,7 +280,7 @@ module Vmpooler # Inject create failure unless provider_config['revertsnapshot_fail_percent'].nil? - raise('Dummy Failure for revertsnapshot_fail_percent') if 1 + rand(100) <= provider_config['revertsnapshot_fail_percent'] + raise('Dummy Failure for revertsnapshot_fail_percent') if rand(1..100) <= provider_config['revertsnapshot_fail_percent'] end end @@ -318,7 +318,7 @@ module Vmpooler # Inject destroy VM failure unless provider_config['destroyvm_fail_percent'].nil? - raise('Dummy Failure for migratevm_fail_percent') if 1 + rand(100) <= provider_config['destroyvm_fail_percent'] + raise('Dummy Failure for migratevm_fail_percent') if rand(1..100) <= provider_config['destroyvm_fail_percent'] end # 'Destroy' the VM @@ -352,7 +352,7 @@ module Vmpooler sleep(2) unless provider_config['vmready_fail_percent'].nil? - raise('Dummy Failure for vmready_fail_percent') if 1 + rand(100) <= provider_config['vmready_fail_percent'] + raise('Dummy Failure for vmready_fail_percent') if rand(1..100) <= provider_config['vmready_fail_percent'] end @write_lock.synchronize do @@ -370,6 +370,7 @@ module Vmpooler def remove_dummy_vm(pool_name, vm_name) return if @dummylist['pool'][pool_name].nil? + new_poollist = @dummylist['pool'][pool_name].delete_if { |vm| vm['name'] == vm_name } @dummylist['pool'][pool_name] = new_poollist end @@ -395,6 +396,7 @@ module Vmpooler def write_backing_file dummyfilename = provider_config['filename'] return if dummyfilename.nil? + File.open(dummyfilename, 'w') { |file| file.write(YAML.dump(@dummylist)) } end end diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index 2ffe409..b4763dd 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -50,7 +50,8 @@ module Vmpooler end return false unless configured_folders.keys.include?(folder_title) return false unless configured_folders[folder_title] == base_folder - return true + + true end def destroy_vm_and_log(vm_name, vm_object, pool, data_ttl) @@ -69,7 +70,7 @@ module Vmpooler logger.log('s', "[!] [#{pool}] '#{vm_name}' is a folder, bailing on destroying") raise('Expected VM, but received a folder object') end - vm_object.PowerOffVM_Task.wait_for_completion if vm_object.runtime && vm_object.runtime.powerState && vm_object.runtime.powerState == 'poweredOn' + vm_object.PowerOffVM_Task.wait_for_completion if vm_object.runtime&.powerState && vm_object.runtime.powerState == 'poweredOn' vm_object.Destroy_Task.wait_for_completion finish = format('%.2f', Time.now - start) @@ -77,9 +78,9 @@ module Vmpooler metrics.timing("destroy.#{pool}", finish) rescue RuntimeError raise - rescue => err + rescue StandardError => e try += 1 - logger.log('s', "[!] [#{pool}] failed to destroy '#{vm_name}' with an error: #{err}") + logger.log('s', "[!] [#{pool}] failed to destroy '#{vm_name}' with an error: #{e}") try >= max_tries ? raise : retry end @@ -104,7 +105,7 @@ module Vmpooler max_tries = 3 logger.log('s', "[-] [#{folder_object.name}] removing unconfigured folder") folder_object.Destroy_Task.wait_for_completion - rescue + rescue StandardError try += 1 try >= max_tries ? raise : retry end @@ -118,9 +119,7 @@ module Vmpooler unless folder_children.empty? folder_children.each do |folder_hash| folder_hash.each do |folder_title, folder_object| - unless folder_configured?(folder_title, base_folder, configured_folders, whitelist) - destroy_folder_and_children(folder_object) - end + destroy_folder_and_children(folder_object) unless folder_configured?(folder_title, base_folder, configured_folders, whitelist) end end end @@ -132,8 +131,8 @@ module Vmpooler folders = [] propSpecs = { - :entity => self, - :inventoryPath => folder_name + entity: self, + inventoryPath: folder_name } folder_object = connection.searchIndex.FindByInventoryPath(propSpecs) @@ -141,6 +140,7 @@ module Vmpooler folder_object.childEntity.each do |folder| next unless folder.is_a? RbVmomi::VIM::Folder + folders << { folder.name => folder } end @@ -171,9 +171,9 @@ module Vmpooler target[dc]['checking'] = true hosts_hash = find_least_used_hosts(cluster, datacenter, percentage) target[dc] = hosts_hash - rescue => _err + rescue StandardError => _e target[dc] = {} - raise(_err) + raise(_e) ensure target[dc]['check_time_finished'] = Time.now end @@ -188,36 +188,36 @@ module Vmpooler cluster = get_target_cluster_from_config(pool_name) raise("cluster for pool #{pool_name} cannot be identified") if cluster.nil? raise("datacenter for pool #{pool_name} cannot be identified") if datacenter.nil? + dc = "#{datacenter}_#{cluster}" unless target.key?(dc) select_target_hosts(target, cluster, datacenter) return end - if target[dc].key?('checking') - wait_for_host_selection(dc, target, loop_delay, max_age) - end + wait_for_host_selection(dc, target, loop_delay, max_age) if target[dc].key?('checking') if target[dc].key?('check_time_finished') - if now - target[dc]['check_time_finished'] > max_age - select_target_hosts(target, cluster, datacenter) - end + select_target_hosts(target, cluster, datacenter) if now - target[dc]['check_time_finished'] > max_age end end def wait_for_host_selection(dc, target, maxloop = 0, loop_delay = 1, max_age = 60) loop_count = 1 - until target.key?(dc) and target[dc].key?('check_time_finished') + until target.key?(dc) && target[dc].key?('check_time_finished') sleep(loop_delay) unless maxloop.zero? break if loop_count >= maxloop + loop_count += 1 end end return unless target[dc].key?('check_time_finished') + loop_count = 1 while Time.now - target[dc]['check_time_finished'] > max_age sleep(loop_delay) unless maxloop.zero? break if loop_count >= maxloop + loop_count += 1 end end @@ -228,10 +228,12 @@ module Vmpooler cluster = get_target_cluster_from_config(pool_name) raise("cluster for pool #{pool_name} cannot be identified") if cluster.nil? raise("datacenter for pool #{pool_name} cannot be identified") if datacenter.nil? + dc = "#{datacenter}_#{cluster}" @provider_hosts_lock.synchronize do if architecture raise("there is no candidate in vcenter that meets all the required conditions, that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory") unless target[dc].key?('architectures') + host = target[dc]['architectures'][architecture].shift target[dc]['architectures'][architecture] << host if target[dc]['hosts'].include?(host) @@ -241,12 +243,11 @@ module Vmpooler return host else raise("there is no candidate in vcenter that meets all the required conditions, that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory") unless target[dc].key?('hosts') + host = target[dc]['hosts'].shift target[dc]['hosts'] << host target[dc]['architectures'].each do |arch| - if arch.include?(host) - target[dc]['architectures'][arch] = arch.partition { |v| v != host }.flatten - end + target[dc]['architectures'][arch] = arch.partition { |v| v != host }.flatten if arch.include?(host) end return host end @@ -258,11 +259,13 @@ module Vmpooler cluster = get_target_cluster_from_config(pool_name) raise("cluster for pool #{pool_name} cannot be identified") if cluster.nil? raise("datacenter for pool #{pool_name} cannot be identified") if datacenter.nil? + dc = "#{datacenter}_#{cluster}" raise("there is no candidate in vcenter that meets all the required conditions, that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory") unless target[dc].key?('hosts') return true if target[dc]['hosts'].include?(parent_host) return true if target[dc]['architectures'][architecture].include?(parent_host) - return false + + false end def get_vm(pool_name, vm_name) @@ -280,6 +283,7 @@ module Vmpooler def create_vm(pool_name, new_vmname) pool = pool_config(pool_name) raise("Pool #{pool_name} does not exist for the provider #{name}") if pool.nil? + vm_hash = nil @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) @@ -322,7 +326,7 @@ module Vmpooler host_object = find_host_by_dnsname(connection, target_host) relocate_spec.host = host_object else - # Choose a cluster/host to place the new VM on + # Choose a cluster/host to place the new VM on target_cluster_object = find_cluster(target_cluster_name, connection, target_datacenter_name) relocate_spec.pool = target_cluster_object.resourcePool end @@ -337,14 +341,12 @@ module Vmpooler begin vm_target_folder = find_vm_folder(pool_name, connection) - if vm_target_folder.nil? and @config[:config].key?('create_folders') and @config[:config]['create_folders'] == true - vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name) - end - rescue => _err - if @config[:config].key?('create_folders') and @config[:config]['create_folders'] == true + vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name) if vm_target_folder.nil? && @config[:config].key?('create_folders') && (@config[:config]['create_folders'] == true) + rescue StandardError => _e + if @config[:config].key?('create_folders') && (@config[:config]['create_folders'] == true) vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name) else - raise(_err) + raise(_e) end end @@ -418,7 +420,7 @@ module Vmpooler return true if vm_object.nil? # Poweroff the VM if it's running - vm_object.PowerOffVM_Task.wait_for_completion if vm_object.runtime && vm_object.runtime.powerState && vm_object.runtime.powerState == 'poweredOn' + vm_object.PowerOffVM_Task.wait_for_completion if vm_object.runtime&.powerState && vm_object.runtime.powerState == 'poweredOn' # Kill it with fire vm_object.Destroy_Task.wait_for_completion @@ -429,7 +431,7 @@ module Vmpooler def vm_ready?(_pool_name, vm_name) begin open_socket(vm_name, global_config[:config]['domain']) - rescue => _err + rescue StandardError => _e return false end @@ -464,9 +466,9 @@ module Vmpooler pool_configuration = pool_config(pool_name) return nil if pool_configuration.nil? - hostname = vm_object.summary.guest.hostName if vm_object.summary && vm_object.summary.guest && vm_object.summary.guest.hostName - boottime = vm_object.runtime.bootTime if vm_object.runtime && vm_object.runtime.bootTime - powerstate = vm_object.runtime.powerState if vm_object.runtime && vm_object.runtime.powerState + hostname = vm_object.summary.guest.hostName if vm_object.summary&.guest && vm_object.summary.guest.hostName + boottime = vm_object.runtime.bootTime if vm_object.runtime&.bootTime + powerstate = vm_object.runtime.powerState if vm_object.runtime&.powerState hash = { 'name' => vm_object.name, @@ -474,7 +476,7 @@ module Vmpooler 'template' => pool_configuration['template'], 'poolname' => pool_name, 'boottime' => boottime, - 'powerstate' => powerstate, + 'powerstate' => powerstate } hash @@ -492,9 +494,9 @@ module Vmpooler def vsphere_connection_ok?(connection) _result = connection.serviceInstance.CurrentTime - return true - rescue - return false + true + rescue StandardError + false end def connect_to_vsphere @@ -507,10 +509,11 @@ module Vmpooler password: provider_config['password'], insecure: provider_config['insecure'] || false metrics.increment('connect.open') - return connection - rescue => err + connection + rescue StandardError => e metrics.increment('connect.fail') - raise err if try >= max_tries + raise e if try >= max_tries + sleep(try * retry_factor) try += 1 retry @@ -610,6 +613,7 @@ module Vmpooler def find_datastore(datastorename, connection, datacentername) datacenter = connection.serviceInstance.find_datacenter(datacentername) raise("Datacenter #{datacentername} does not exist") if datacenter.nil? + datacenter.find_datastore(datastorename) end @@ -625,9 +629,7 @@ module Vmpooler devices = find_disk_devices(vm) devices.keys.sort.each do |device| - if devices[device]['children'].length < 15 - return find_device(vm, devices[device]['device'].deviceInfo.label) - end + return find_device(vm, devices[device]['device'].deviceInfo.label) if devices[device]['children'].length < 15 end nil @@ -667,6 +669,7 @@ module Vmpooler devices.keys.sort.each do |c| next unless controller.key == devices[c]['device'].key + used_unit_numbers.push(devices[c]['device'].scsiCtlrUnitNumber) devices[c]['children'].each do |disk| used_unit_numbers.push(disk.unitNumber) @@ -674,12 +677,10 @@ module Vmpooler end (0..15).each do |scsi_id| - if used_unit_numbers.grep(scsi_id).length <= 0 - available_unit_numbers.push(scsi_id) - end + available_unit_numbers.push(scsi_id) if used_unit_numbers.grep(scsi_id).length <= 0 end - available_unit_numbers.sort[0] + available_unit_numbers.min end # Finds a folder object by inventory path @@ -692,17 +693,19 @@ module Vmpooler # Returns nil when the object found is not a folder pool_configuration = pool_config(pool_name) return nil if pool_configuration.nil? + folder = pool_configuration['folder'] datacenter = get_target_datacenter_from_config(pool_name) return nil if datacenter.nil? propSpecs = { - :entity => self, - :inventoryPath => "#{datacenter}/vm/#{folder}" + entity: self, + inventoryPath: "#{datacenter}/vm/#{folder}" } folder_object = connection.searchIndex.FindByInventoryPath(propSpecs) return nil unless folder_object.class == RbVmomi::VIM::Folder + folder_object end @@ -752,6 +755,7 @@ module Vmpooler def cpu_utilization_for(host) cpu_usage = host.summary.quickStats.overallCpuUsage return nil if cpu_usage.nil? + cpu_size = host.summary.hardware.cpuMhz * host.summary.hardware.numCpuCores (cpu_usage.to_f / cpu_size.to_f) * 100 end @@ -759,6 +763,7 @@ module Vmpooler def memory_utilization_for(host) memory_usage = host.summary.quickStats.overallMemoryUsage return nil if memory_usage.nil? + memory_size = host.summary.hardware.memorySize / 1024 / 1024 (memory_usage.to_f / memory_size.to_f) * 100 end @@ -769,13 +774,13 @@ module Vmpooler end def build_compatible_hosts_lists(hosts, percentage) - hosts_with_arch_versions = hosts.map { |h| + hosts_with_arch_versions = hosts.map do |h| { - 'utilization' => h[0], - 'host_object' => h[1], - 'architecture' => get_host_cpu_arch_version(h[1]) + 'utilization' => h[0], + 'host_object' => h[1], + 'architecture' => get_host_cpu_arch_version(h[1]) } - } + end versions = hosts_with_arch_versions.map { |host| host['architecture'] }.uniq architectures = {} versions.each do |version| @@ -796,12 +801,13 @@ module Vmpooler def select_least_used_hosts(hosts, percentage) raise('Provided hosts list to select_least_used_hosts is empty') if hosts.empty? + average_utilization = get_average_cluster_utilization(hosts) least_used_hosts = [] hosts.each do |host| least_used_hosts << host if host[0] <= average_utilization end - hosts_to_select = (hosts.count * (percentage / 100.0)).to_int + hosts_to_select = (hosts.count * (percentage / 100.0)).to_int hosts_to_select = hosts.count - 1 if percentage == 100 least_used_hosts.sort[0..hosts_to_select].map { |host| host[1].name } end @@ -811,8 +817,10 @@ module Vmpooler connection = ensured_vsphere_connection(pool_object) cluster_object = find_cluster(cluster, connection, datacentername) raise("Cluster #{cluster} cannot be found") if cluster_object.nil? + target_hosts = get_cluster_host_utilization(cluster_object) raise("there is no candidate in vcenter that meets all the required conditions, that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory'") if target_hosts.empty? + architectures = build_compatible_hosts_lists(target_hosts, percentage) least_used_hosts = select_least_used_hosts(target_hosts, percentage) { @@ -825,6 +833,7 @@ module Vmpooler def find_host_by_dnsname(connection, dnsname) host_object = connection.searchIndex.FindByDnsName(dnsName: dnsname, vmSearch: false) return nil if host_object.nil? + host_object end @@ -832,7 +841,8 @@ module Vmpooler cluster_object = find_cluster(cluster, connection, datacentername) target_hosts = get_cluster_host_utilization(cluster_object) raise("There is no host candidate in vcenter that meets all the required conditions, check that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory'") if target_hosts.empty? - least_used_host = target_hosts.sort[0][1] + + least_used_host = target_hosts.min[1] least_used_host end @@ -848,7 +858,7 @@ module Vmpooler cv = connection.serviceContent.viewManager.CreateContainerView( container: datacenter.hostFolder, type: ['ComputeResource', 'ClusterComputeResource'], - recursive: true, + recursive: true ) cluster = cv.view.find { |cluster_object| cluster_object.name == cluster } cv.DestroyView @@ -870,7 +880,8 @@ module Vmpooler cluster = source_host.parent target_hosts = get_cluster_host_utilization(cluster, model) raise("There is no host candidate in vcenter that meets all the required conditions, check that the cluster has available hosts in a 'green' status, not in maintenance mode and not overloaded CPU and memory'") if target_hosts.empty? - target_host = target_hosts.sort[0][1] + + target_host = target_hosts.min[1] [target_host, target_host.name] end @@ -891,13 +902,14 @@ module Vmpooler # Returns nil when a VM, or pool configuration, cannot be found pool_configuration = pool_config(pool_name) return nil if pool_configuration.nil? + folder = pool_configuration['folder'] datacenter = get_target_datacenter_from_config(pool_name) return nil if datacenter.nil? propSpecs = { - :entity => self, - :inventoryPath => "#{datacenter}/vm/#{folder}/#{vmname}" + entity: self, + inventoryPath: "#{datacenter}/vm/#{folder}/#{vmname}" } connection.searchIndex.FindByInventoryPath(propSpecs) @@ -929,8 +941,10 @@ module Vmpooler def get_vm_details(pool_name, vm_name, connection) vm_object = find_vm(pool_name, vm_name, connection) return nil if vm_object.nil? - parent_host_object = vm_object.summary.runtime.host if vm_object.summary && vm_object.summary.runtime && vm_object.summary.runtime.host + + parent_host_object = vm_object.summary.runtime.host if vm_object.summary&.runtime && vm_object.summary.runtime.host raise('Unable to determine which host the VM is running on') if parent_host_object.nil? + parent_host = parent_host_object.name architecture = get_host_cpu_arch_version(parent_host_object) { @@ -944,6 +958,7 @@ module Vmpooler migration_limit = config[:config]['migration_limit'] return false unless migration_limit.is_a? Integer return true if migration_limit > 0 + false end @@ -969,9 +984,9 @@ module Vmpooler else logger.log('s', "[ ] [#{pool_name}] '#{vm_name}' is running on #{vm_hash['host_name']}") end - rescue => _err + rescue StandardError => _e logger.log('s', "[!] [#{pool_name}] '#{vm_name}' is running on #{vm_hash['host_name']}") - raise _err + raise _e end end end @@ -1008,8 +1023,9 @@ module Vmpooler def create_folder(connection, new_folder, datacenter) dc = connection.serviceInstance.find_datacenter(datacenter) - folder_object = dc.vmFolder.traverse(new_folder, type=RbVmomi::VIM::Folder, create=true) + folder_object = dc.vmFolder.traverse(new_folder, type = RbVmomi::VIM::Folder, create = true) raise("Cannot create folder #{new_folder}") if folder_object.nil? + folder_object end @@ -1018,8 +1034,8 @@ module Vmpooler raise('cannot find datacenter') if datacenter.nil? propSpecs = { - :entity => self, - :inventoryPath => "#{datacenter}/vm/#{pool['template']}" + entity: self, + inventoryPath: "#{datacenter}/vm/#{pool['template']}" } template_vm_object = connection.searchIndex.FindByInventoryPath(propSpecs) @@ -1041,12 +1057,14 @@ module Vmpooler return false unless template.include?('/') return false if template[0] == '/' return false if template[-1] == '/' - return true + + true end def get_disk_backing(pool) return :moveChildMostDiskBacking if linked_clone?(pool) - return :moveAllDiskBackingsAndConsolidate + + :moveAllDiskBackingsAndConsolidate end def linked_clone?(pool) diff --git a/lib/vmpooler/statsd.rb b/lib/vmpooler/statsd.rb index 141649f..0726ce6 100644 --- a/lib/vmpooler/statsd.rb +++ b/lib/vmpooler/statsd.rb @@ -6,9 +6,7 @@ module Vmpooler attr_reader :server, :port, :prefix def initialize(params = {}) - if params['server'].nil? || params['server'].empty? - raise ArgumentError, "Statsd server is required. Config: #{params.inspect}" - end + raise ArgumentError, "Statsd server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty? host = params['server'] @port = params['port'] || 8125 @@ -18,20 +16,20 @@ module Vmpooler def increment(label) server.increment(prefix + '.' + label) - rescue => err - $stderr.puts "Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}" + rescue StandardError => e + warn "Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}" end def gauge(label, value) server.gauge(prefix + '.' + label, value) - rescue => err - $stderr.puts "Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}" + rescue StandardError => e + warn "Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}" end def timing(label, duration) server.timing(prefix + '.' + label, duration) - rescue => err - $stderr.puts "Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}" + rescue StandardError => e + warn "Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}" end end end From f22a84f26f62639c205ebf977e8aee5f180fdecd Mon Sep 17 00:00:00 2001 From: Brandon High Date: Thu, 5 Mar 2020 11:16:43 -0800 Subject: [PATCH 038/371] "Unsafe" rubocop fixes Adds the remaining "unsafe" fixes that aren't included in #359 --- bin/vmpooler | 1 + lib/vmpooler.rb | 6 ++++-- lib/vmpooler/api.rb | 2 ++ lib/vmpooler/api/dashboard.rb | 2 ++ lib/vmpooler/api/helpers.rb | 2 ++ lib/vmpooler/api/reroute.rb | 2 ++ lib/vmpooler/api/v1.rb | 4 +++- lib/vmpooler/dashboard.rb | 2 ++ lib/vmpooler/dummy_statsd.rb | 2 ++ lib/vmpooler/generic_connection_pool.rb | 2 ++ lib/vmpooler/graphite.rb | 2 ++ lib/vmpooler/logger.rb | 2 ++ lib/vmpooler/pool_manager.rb | 4 +++- lib/vmpooler/providers.rb | 4 +++- lib/vmpooler/providers/base.rb | 2 ++ lib/vmpooler/providers/dummy.rb | 2 ++ lib/vmpooler/providers/vsphere.rb | 8 +++++--- lib/vmpooler/statsd.rb | 2 ++ lib/vmpooler/version.rb | 4 +++- 19 files changed, 46 insertions(+), 9 deletions(-) diff --git a/bin/vmpooler b/bin/vmpooler index b84a139..bdf6d9a 100755 --- a/bin/vmpooler +++ b/bin/vmpooler @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'vmpooler' diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index ddc1582..40332a5 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler require 'date' require 'json' @@ -41,7 +43,7 @@ module Vmpooler # Bail out if someone attempts to start vmpooler with dummy authentication # without enbaling debug mode. - if parsed_config.has_key? :auth + if parsed_config.key? :auth if parsed_config[:auth]['provider'] == 'dummy' unless ENV['VMPOOLER_DEBUG'] warning = [ @@ -94,7 +96,7 @@ module Vmpooler parsed_config[:graphite]['port'] = string_to_int(ENV['GRAPHITE_PORT']) if ENV['GRAPHITE_PORT'] parsed_config[:auth] = parsed_config[:auth] || {} if ENV['AUTH_PROVIDER'] - if parsed_config.has_key? :auth + if parsed_config.key? :auth parsed_config[:auth]['provider'] = ENV['AUTH_PROVIDER'] if ENV['AUTH_PROVIDER'] parsed_config[:auth][:ldap] = parsed_config[:auth][:ldap] || {} if parsed_config[:auth]['provider'] == 'ldap' parsed_config[:auth][:ldap]['host'] = ENV['LDAP_HOST'] if ENV['LDAP_HOST'] diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index 22bf64e..cf7e8ab 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class API < Sinatra::Base def initialize diff --git a/lib/vmpooler/api/dashboard.rb b/lib/vmpooler/api/dashboard.rb index 2288b79..9d3ba00 100644 --- a/lib/vmpooler/api/dashboard.rb +++ b/lib/vmpooler/api/dashboard.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class API class Dashboard < Sinatra::Base diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index 6fdd2f6..539d51c 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class API diff --git a/lib/vmpooler/api/reroute.rb b/lib/vmpooler/api/reroute.rb index 9c68663..bc44106 100644 --- a/lib/vmpooler/api/reroute.rb +++ b/lib/vmpooler/api/reroute.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class API class Reroute < Sinatra::Base diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 17e3710..b969d6f 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class API class V1 < Sinatra::Base @@ -936,7 +938,7 @@ module Vmpooler end end - if failure.size > 0 + if !failure.empty? status 400 result['failure'] = failure else diff --git a/lib/vmpooler/dashboard.rb b/lib/vmpooler/dashboard.rb index b875465..6359127 100644 --- a/lib/vmpooler/dashboard.rb +++ b/lib/vmpooler/dashboard.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class Dashboard < Sinatra::Base diff --git a/lib/vmpooler/dummy_statsd.rb b/lib/vmpooler/dummy_statsd.rb index a555268..fa23833 100644 --- a/lib/vmpooler/dummy_statsd.rb +++ b/lib/vmpooler/dummy_statsd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class DummyStatsd attr_reader :server, :port, :prefix diff --git a/lib/vmpooler/generic_connection_pool.rb b/lib/vmpooler/generic_connection_pool.rb index ca18576..6c74783 100644 --- a/lib/vmpooler/generic_connection_pool.rb +++ b/lib/vmpooler/generic_connection_pool.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'connection_pool' module Vmpooler diff --git a/lib/vmpooler/graphite.rb b/lib/vmpooler/graphite.rb index e7f28c3..7261511 100644 --- a/lib/vmpooler/graphite.rb +++ b/lib/vmpooler/graphite.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rubygems' unless defined?(Gem) module Vmpooler diff --git a/lib/vmpooler/logger.rb b/lib/vmpooler/logger.rb index f481246..f8a9644 100644 --- a/lib/vmpooler/logger.rb +++ b/lib/vmpooler/logger.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rubygems' unless defined?(Gem) module Vmpooler diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 37063de..c361c41 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'vmpooler/providers' require 'spicy-proton' @@ -54,7 +56,7 @@ module Vmpooler pool_keys.each do |k| to_set[k] = pool[k] end - to_set['alias'] = pool['alias'].join(',') if to_set.has_key?('alias') + to_set['alias'] = pool['alias'].join(',') if to_set.key?('alias') $redis.hmset("vmpooler__pool__#{pool['name']}", to_set.to_a.flatten) unless to_set.empty? end previously_configured_pools.each do |pool| diff --git a/lib/vmpooler/providers.rb b/lib/vmpooler/providers.rb index 9a5d955..7da9d13 100644 --- a/lib/vmpooler/providers.rb +++ b/lib/vmpooler/providers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'pathname' module Vmpooler @@ -38,7 +40,7 @@ module Vmpooler # we don't exactly know if the provider name matches the main file name that should be loaded # so we use globs to get everything like the name # this could mean that vsphere5 and vsphere6 are loaded when only vsphere5 is used - Dir.glob(File.join(gem_path, "*#{name}*.rb")).each do |file| + Dir.glob(File.join(gem_path, "*#{name}*.rb")).sort.each do |file| require file end end diff --git a/lib/vmpooler/providers/base.rb b/lib/vmpooler/providers/base.rb index d30564f..fa730bf 100644 --- a/lib/vmpooler/providers/base.rb +++ b/lib/vmpooler/providers/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler class PoolManager class Provider diff --git a/lib/vmpooler/providers/dummy.rb b/lib/vmpooler/providers/dummy.rb index bc924a0..b221859 100644 --- a/lib/vmpooler/providers/dummy.rb +++ b/lib/vmpooler/providers/dummy.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'yaml' require 'vmpooler/providers/base' diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index 2ffe409..71b6659 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'vmpooler/providers/base' module Vmpooler @@ -481,9 +483,9 @@ module Vmpooler end # vSphere helper methods - ADAPTER_TYPE = 'lsiLogic'.freeze - DISK_TYPE = 'thin'.freeze - DISK_MODE = 'persistent'.freeze + ADAPTER_TYPE = 'lsiLogic' + DISK_TYPE = 'thin' + DISK_MODE = 'persistent' def ensured_vsphere_connection(connection_pool_object) connection_pool_object[:connection] = connect_to_vsphere unless vsphere_connection_ok?(connection_pool_object[:connection]) diff --git a/lib/vmpooler/statsd.rb b/lib/vmpooler/statsd.rb index 141649f..4843bec 100644 --- a/lib/vmpooler/statsd.rb +++ b/lib/vmpooler/statsd.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rubygems' unless defined?(Gem) require 'statsd' diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index 2b2306c..fb27c85 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Vmpooler - VERSION = '0.10.3'.freeze + VERSION = '0.10.3' end From b4f42cd4b1b471dd41a657914f3db920c94cf960 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Thu, 5 Mar 2020 11:50:56 -0800 Subject: [PATCH 039/371] Disable Naming/VariableName for propSpecs I assume `propSpecs` is a reference to the VMWare API so using camelCase is intentional. --- lib/vmpooler/providers/vsphere.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index 2aabbb3..c5b37ff 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -132,11 +132,11 @@ module Vmpooler def get_folder_children(folder_name, connection) folders = [] - propSpecs = { + propSpecs = { # rubocop:disable Naming/VariableName entity: self, inventoryPath: folder_name } - folder_object = connection.searchIndex.FindByInventoryPath(propSpecs) + folder_object = connection.searchIndex.FindByInventoryPath(propSpecs) # rubocop:disable Naming/VariableName return folders if folder_object.nil? @@ -700,12 +700,12 @@ module Vmpooler datacenter = get_target_datacenter_from_config(pool_name) return nil if datacenter.nil? - propSpecs = { + propSpecs = { # rubocop:disable Naming/VariableName entity: self, inventoryPath: "#{datacenter}/vm/#{folder}" } - folder_object = connection.searchIndex.FindByInventoryPath(propSpecs) + folder_object = connection.searchIndex.FindByInventoryPath(propSpecs) # rubocop:disable Naming/VariableName return nil unless folder_object.class == RbVmomi::VIM::Folder folder_object @@ -891,12 +891,12 @@ module Vmpooler get_snapshot_list(vm.snapshot.rootSnapshotList, snapshotname) if vm.snapshot end - def build_propSpecs(datacenter, folder, vmname) - propSpecs = { + def build_propSpecs(datacenter, folder, vmname) # rubocop:disable Naming/MethodName + propSpecs = { # rubocop:disable Naming/VariableName entity => self, :inventoryPath => "#{datacenter}/vm/#{folder}/#{vmname}" } - propSpecs + propSpecs # rubocop:disable Naming/VariableName end def find_vm(pool_name, vmname, connection) @@ -909,12 +909,12 @@ module Vmpooler datacenter = get_target_datacenter_from_config(pool_name) return nil if datacenter.nil? - propSpecs = { + propSpecs = { # rubocop:disable Naming/VariableName entity: self, inventoryPath: "#{datacenter}/vm/#{folder}/#{vmname}" } - connection.searchIndex.FindByInventoryPath(propSpecs) + connection.searchIndex.FindByInventoryPath(propSpecs) # rubocop:disable Naming/VariableName end def get_base_vm_container_from(connection) @@ -1035,12 +1035,12 @@ module Vmpooler datacenter = get_target_datacenter_from_config(pool['name']) raise('cannot find datacenter') if datacenter.nil? - propSpecs = { + propSpecs = { # rubocop:disable Naming/VariableName entity: self, inventoryPath: "#{datacenter}/vm/#{pool['template']}" } - template_vm_object = connection.searchIndex.FindByInventoryPath(propSpecs) + template_vm_object = connection.searchIndex.FindByInventoryPath(propSpecs) # rubocop:disable Naming/VariableName raise("Pool #{pool['name']} specifies a template VM of #{pool['template']} which does not exist for the provider #{name}") if template_vm_object.nil? template_vm_object From a5a2740762b6aeceea54aa896095af47d0acc9d9 Mon Sep 17 00:00:00 2001 From: Brandon High Date: Thu, 5 Mar 2020 12:01:34 -0800 Subject: [PATCH 040/371] Consistent Style/FormatStringToken This commit fixes the various calls to `format` to consistently use the keyword token style. Hopefully this is more understandable and explicit? --- lib/vmpooler/pool_manager.rb | 12 ++++++------ lib/vmpooler/providers/vsphere.rb | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 22d7764..6913f58 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -121,7 +121,7 @@ module Vmpooler def move_pending_vm_to_ready(vm, pool) clone_time = $redis.hget('vmpooler__vm__' + vm, 'clone') - finish = format('%.2f', Time.now - Time.parse(clone_time)) if clone_time + finish = format('%