From 6db71d85890fbca533f11a52588255ee79bec77f Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Wed, 27 Oct 2021 16:19:02 -0400 Subject: [PATCH 1/2] Update Dockerfile_local to rebuild faster This makes it so that cached layers can be used when all that is changing is VMPooler's code, and not its gems. --- docker/Dockerfile_local | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docker/Dockerfile_local b/docker/Dockerfile_local index c1b3f4e..2480e16 100644 --- a/docker/Dockerfile_local +++ b/docker/Dockerfile_local @@ -10,9 +10,6 @@ FROM jruby:9.2-jdk -COPY docker/docker-entrypoint.sh /usr/local/bin/ -COPY ./ ./ - ENV RACK_ENV=production RUN apt-get update -qq && \ @@ -21,9 +18,18 @@ RUN apt-get update -qq && \ apt-get autoremove -y && \ rm -rf /var/lib/apt/lists/* +COPY docker/docker-entrypoint.sh /usr/local/bin/ +COPY ./Gemfile ./ +COPY ./vmpooler.gemspec ./ +COPY ./lib/vmpooler/version.rb ./lib/vmpooler/version.rb + RUN gem install bundler && \ - bundle install && \ - gem build vmpooler.gemspec && \ + bundle config set --local jobs 3 && \ + bundle install + +COPY ./ ./ + +RUN gem build vmpooler.gemspec && \ gem install vmpooler*.gem && \ chmod +x /usr/local/bin/docker-entrypoint.sh From a0caa41a549a6797a7b17ac9f02f1311d0e590bc Mon Sep 17 00:00:00 2001 From: Gene Liverman Date: Thu, 14 Oct 2021 15:08:55 -0400 Subject: [PATCH 2/2] (DIO-2675) Undo pool size template overrides This implements a delete method for pooltemplate and poolsize. The API removes the override from Redis and then adds an entry in Redis that causes the pool manager to wake up and process the removal of the override. To facilitate this, a new variable has been created in lib/vmpooler.rb to hold a copy of the original / pre-override config. This supplemental copy of the pools is then indexed for use as a reference. When pool manager wakes up to process an override removal, it looks up the pre-override value from the config via the new variables mentioned above. Just as with entering overrides, no restart is needed. Template and pool size changes are logged so that anyone watching or reviewing the logs can see what happened when. The new API endpoints also return values for both the pre-revert and post-revert value. --- docs/API.md | 44 ++++++++++++ lib/vmpooler.rb | 9 +++ lib/vmpooler/api/v1.rb | 97 ++++++++++++++++++++++++++ lib/vmpooler/pool_manager.rb | 59 ++++++++++++++-- spec/integration/api/v1/config_spec.rb | 95 +++++++++++++++++++++++++ spec/unit/pool_manager_spec.rb | 96 +++++++++++++++++++++++++ spec/unit/vmpooler_spec.rb | 15 ++++ 7 files changed, 409 insertions(+), 6 deletions(-) diff --git a/docs/API.md b/docs/API.md index 038d4e9..ee6fa26 100644 --- a/docs/API.md +++ b/docs/API.md @@ -743,6 +743,28 @@ $ curl -X POST -H "Content-Type: application/json" -d '{"debian-7-i386":"2","deb } ``` +##### DELETE /config/poolsize/<pool> + +Delete an overridden pool size. This results in the values from VMPooler's config being used. + +Return codes: +* 200 - when nothing was changed but no error occurred +* 201 - size reset successful +* 401 - when not authorized +* 404 - pool does not exist +* 405 - The endpoint is disabled because experimental features are disabled + +``` +$ curl -X DELETE -u jdoe --url vmpooler.example.com/api/v1/poolsize/almalinux-8-x86_64 +``` +```json +{ + "ok": true, + "pool_size_before_overrides": 2, + "pool_size_before_reset": 4 +} +``` + ##### POST /config/pooltemplate Change the template configured for a pool, and replenish the pool with instances built from the new template. @@ -775,6 +797,28 @@ $ curl -X POST -H "Content-Type: application/json" -d '{"debian-7-i386":"templat } ``` +##### DELETE /config/pooltemplate/<pool> + +Delete an overridden pool template. This results in the values from VMPooler's config being used. + +Return codes: +* 200 - when nothing was changed but no error occurred +* 201 - template reset successful +* 401 - when not authorized +* 404 - pool does not exist +* 405 - The endpoint is disabled because experimental features are disabled + +``` +$ curl -X DELETE -u jdoe --url vmpooler.example.com/api/v1/pooltemplate/almalinux-8-x86_64 +``` +```json +{ + "ok": true, + "template_before_overrides": "templates/almalinux-8-x86_64-0.0.2", + "template_before_reset": "templates/almalinux-8-x86_64-0.0.3-beta" +} +``` + ##### POST /poolreset Clear all pending and ready instances in a pool, and deploy replacements diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 3403896..7d39f18 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -133,8 +133,17 @@ module Vmpooler parsed_config[:pools] = load_pools_from_redis(redis) end + # Marshal.dump is paired with Marshal.load to create a copy that has its own memory space + # so that each can be edited independently + # rubocop:disable Security/MarshalLoad + + # retain a copy of the pools that were observed at startup + serialized_pools = Marshal.dump(parsed_config[:pools]) + parsed_config[:pools_at_startup] = Marshal.load(serialized_pools) + # Create an index of pools by title parsed_config[:pool_index] = pool_index(parsed_config[:pools]) + # rubocop:enable Security/MarshalLoad parsed_config[:pools].each do |pool| parsed_config[:pool_names] << pool['name'] diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 2fffee5..f209de2 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -28,6 +28,10 @@ module Vmpooler Vmpooler::API.settings.config[:pools] end + def pools_at_startup + Vmpooler::API.settings.config[:pools_at_startup] + end + def pool_exists?(template) Vmpooler::API.settings.config[:pool_names].include?(template) end @@ -289,6 +293,32 @@ module Vmpooler puts 'd', "[!] [#{poolname}] failed while evaluating usage labels on '#{vmname}' with an error: #{e}" end + def reset_pool_size(poolname) + result = { 'ok' => false } + + pool_index = pool_index(pools) + + pools_updated = 0 + sync_pool_sizes + + pool_size_now = pools[pool_index[poolname]]['size'].to_i + pool_size_original = pools_at_startup[pool_index[poolname]]['size'].to_i + result['pool_size_before_reset'] = pool_size_now + result['pool_size_before_overrides'] = pool_size_original + + unless pool_size_now == pool_size_original + pools[pool_index[poolname]]['size'] = pool_size_original + backend.hdel('vmpooler__config__poolsize', poolname) + backend.sadd('vmpooler__pool__undo_size_override', poolname) + pools_updated += 1 + status 201 + end + + status 200 unless pools_updated > 0 + result['ok'] = true + result + end + def update_pool_size(payload) result = { 'ok' => false } @@ -309,6 +339,33 @@ module Vmpooler result end + def reset_pool_template(poolname) + result = { 'ok' => false } + + pool_index_live = pool_index(pools) + pool_index_original = pool_index(pools_at_startup) + + pools_updated = 0 + sync_pool_templates + + template_now = pools[pool_index_live[poolname]]['template'] + template_original = pools_at_startup[pool_index_original[poolname]]['template'] + result['template_before_reset'] = template_now + result['template_before_overrides'] = template_original + + unless template_now == template_original + pools[pool_index_live[poolname]]['template'] = template_original + backend.hdel('vmpooler__config__template', poolname) + backend.sadd('vmpooler__pool__undo_template_override', poolname) + pools_updated += 1 + status 201 + end + + status 200 unless pools_updated > 0 + result['ok'] = true + result + end + def update_pool_template(payload) result = { 'ok' => false } @@ -1375,6 +1432,26 @@ module Vmpooler JSON.pretty_generate(result) end + delete "#{api_prefix}/config/poolsize/:pool/?" do + content_type :json + result = { 'ok' => false } + + if config['experimental_features'] + need_token! if Vmpooler::API.settings.config[:auth] + + if pool_exists?(params[:pool]) + result = reset_pool_size(params[:pool]) + else + metrics.increment('config.invalid.unknown') + status 404 + end + else + status 405 + end + + JSON.pretty_generate(result) + end + post "#{api_prefix}/config/poolsize/?" do content_type :json result = { 'ok' => false } @@ -1406,6 +1483,26 @@ module Vmpooler JSON.pretty_generate(result) end + delete "#{api_prefix}/config/pooltemplate/:pool/?" do + content_type :json + result = { 'ok' => false } + + if config['experimental_features'] + need_token! if Vmpooler::API.settings.config[:auth] + + if pool_exists?(params[:pool]) + result = reset_pool_template(params[:pool]) + else + metrics.increment('config.invalid.unknown') + status 404 + end + else + status 405 + end + + JSON.pretty_generate(result) + end + post "#{api_prefix}/config/pooltemplate/?" do content_type :json result = { 'ok' => false } diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 668d1c0..1c9551e 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -784,6 +784,10 @@ module Vmpooler # - Fires when a pool reset is requested # - Additional options # :poolname + # :undo_override + # - Fires when a pool override removal is requested + # - Additional options + # :poolname # def sleep_with_wakeup_events(loop_delay, wakeup_period = 5, options = {}) exit_by = Time.now + loop_delay @@ -826,6 +830,11 @@ module Vmpooler break if pending end + if options[:undo_override] + break if redis.sismember('vmpooler__pool__undo_template_override', options[:poolname]) + break if redis.sismember('vmpooler__pool__undo_size_override', options[:poolname]) + end + if options[:pending_vm] pending_vm_count = redis.scard("vmpooler__pending__#{options[:poolname]}") break unless pending_vm_count == 0 @@ -880,7 +889,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, pending_vm: true, pool_reset: 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, pending_vm: true, pool_reset: true, undo_override: true) unless maxloop == 0 break if loop_count >= maxloop @@ -1040,15 +1049,18 @@ module Vmpooler return if mutex.locked? @redis.with_metrics do |redis| - poolsize = redis.hget('vmpooler__config__poolsize', pool['name']) - break if poolsize.nil? + pool_size_requested = redis.hget('vmpooler__config__poolsize', pool['name']) + break if pool_size_requested.nil? - poolsize = Integer(poolsize) - break if poolsize == pool['size'] + pool_size_requested = Integer(pool_size_requested) + pool_size_currently = pool['size'] + break if pool_size_requested == pool_size_currently mutex.synchronize do - pool['size'] = poolsize + pool['size'] = pool_size_requested end + + $logger.log('s', "[*] [#{pool['name']}] size updated from #{pool_size_currently} to #{pool_size_requested}") end end @@ -1066,6 +1078,38 @@ module Vmpooler end end + def undo_override(pool, provider) + poolname = pool['name'] + mutex = pool_mutex(poolname) + return if mutex.locked? + + @redis.with_metrics do |redis| + break unless redis.sismember('vmpooler__pool__undo_template_override', poolname) + + redis.srem('vmpooler__pool__undo_template_override', poolname) + template_now = pool['template'] + template_original = $config[:pools_at_startup][$config[:pool_index][poolname]]['template'] + + mutex.synchronize do + update_pool_template(pool, provider, template_original, template_now, redis) + end + end + + @redis.with_metrics do |redis| + break unless redis.sismember('vmpooler__pool__undo_size_override', poolname) + + redis.srem('vmpooler__pool__undo_size_override', poolname) + pool_size_now = pool['size'] + pool_size_original = $config[:pools_at_startup][$config[:pool_index][poolname]]['size'] + + mutex.synchronize do + pool['size'] = pool_size_original + end + + $logger.log('s', "[*] [#{poolname}] size updated from #{pool_size_now} to #{pool_size_original}") + end + end + def create_inventory(pool, provider, pool_check_response) inventory = {} begin @@ -1300,6 +1344,9 @@ module Vmpooler # Reset a pool when poolreset is requested from the API reset_pool(pool) + # Undo overrides submitted via the api + undo_override(pool, provider) + pool_check_response end diff --git a/spec/integration/api/v1/config_spec.rb b/spec/integration/api/v1/config_spec.rb index 79874c8..62968bd 100644 --- a/spec/integration/api/v1/config_spec.rb +++ b/spec/integration/api/v1/config_spec.rb @@ -26,6 +26,10 @@ describe Vmpooler::API::V1 do {'name' => 'pool1', 'size' => 5, 'template' => 'templates/pool1', 'clone_target' => 'default_cluster'}, {'name' => 'pool2', 'size' => 10} ], + pools_at_startup: [ + {'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' ] @@ -45,6 +49,47 @@ describe Vmpooler::API::V1 do create_token('abcdefghijklmnopqrstuvwxyz012345', 'jdoe', current_time) end + describe 'DELETE /config/pooltemplate/:pool' do + it 'resets a pool template' do + post "#{prefix}/config/pooltemplate", '{"pool1":"templates/new_template"}' + delete "#{prefix}/config/pooltemplate/pool1" + expect_json(ok = true, http = 201) + + expected = { + ok: true, + template_before_reset: 'templates/new_template', + template_before_overrides: 'templates/pool1' + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'succeeds when the pool has not been overridden' do + delete "#{prefix}/config/pooltemplate/pool1" + expect_json(ok = true, http = 200) + end + + it 'fails on nonexistent pools' do + delete "#{prefix}/config/pooltemplate/poolpoolpool" + expect_json(ok = false, http = 404) + end + + context 'with experimental features disabled' do + before(:each) do + config[:config]['experimental_features'] = false + end + + it 'should return 405' do + delete "#{prefix}/config/pooltemplate/pool1" + expect_json(ok = false, http = 405) + + expected = { ok: false } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + end + + end + describe 'POST /config/pooltemplate' do it 'updates a pool template' do post "#{prefix}/config/pooltemplate", '{"pool1":"templates/new_template"}' @@ -142,6 +187,56 @@ describe Vmpooler::API::V1 do end + describe 'DELETE /config/poolsize' do + it 'resets a pool size' do + post "#{prefix}/config/poolsize", '{"pool1":"2"}' + delete "#{prefix}/config/poolsize/pool1" + expect_json(ok = true, http = 201) + + expected = { + ok: true, + pool_size_before_reset: 2, + pool_size_before_overrides: 5 + } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'fails when a specified pool does not exist' do + delete "#{prefix}/config/poolsize/pool10" + expect_json(ok = false, http = 404) + expected = { ok: false } + + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + + it 'succeeds when a pool has not been overridden' do + delete "#{prefix}/config/poolsize/pool1" + expect_json(ok = true, http = 200) + expected = { + ok: true, + pool_size_before_reset: 5, + pool_size_before_overrides: 5 + } + + 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 + delete "#{prefix}/config/poolsize/pool1" + expect_json(ok = false, http = 405) + + expected = { ok: false } + expect(last_response.body).to eq(JSON.pretty_generate(expected)) + end + end + end + describe 'POST /config/poolsize' do it 'changes a pool size' do post "#{prefix}/config/poolsize", '{"pool1":"2"}' diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index cc5777b..3bc9c68 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -3159,6 +3159,54 @@ EOT end end + describe 'with the undo_override wakeup option' do + let(:wakeup_option) {{ + :undo_override => true, + :poolname => pool + }} + + let(:wakeup_period) { -1 } # A negative number forces the wakeup evaluation to always occur + + context 'when a undoing a template override is requested' do + before(:each) do + redis_connection_pool.with do |redis| + redis.sadd('vmpooler__pool__undo_template_override', pool) + allow(redis).to receive(:hget) + end + end + + it 'should sleep until the undo override request is detected' do + redis_connection_pool.with do |redis| + expect(subject).to receive(:sleep).at_least(2).times + expect(subject).to receive(:sleep).at_most(3).times + expect(redis).to receive(:sismember).with('vmpooler__pool__undo_template_override', pool).and_return(false,false,true) + expect(redis).to receive(:sismember).with('vmpooler__pool__undo_size_override', pool).and_return(false,false) + end + + subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option) + end + end + + context 'when a undoing a size override is requested' do + before(:each) do + redis_connection_pool.with do |redis| + redis.sadd('vmpooler__pool__undo_size_override', pool) + allow(redis).to receive(:hget) + end + end + + it 'should sleep until the undo override request is detected' do + redis_connection_pool.with do |redis| + expect(subject).to receive(:sleep).exactly(3).times + expect(redis).to receive(:sismember).with('vmpooler__pool__undo_template_override', pool).and_return(false,false,false) + expect(redis).to receive(:sismember).with('vmpooler__pool__undo_size_override', pool).and_return(false,false,true) + end + + subject.sleep_with_wakeup_events(loop_delay, wakeup_period, wakeup_option) + end + end + end + describe 'with the pending_vm wakeup option' do let(:wakeup_option) {{ :pending_vm => true, @@ -3477,6 +3525,54 @@ EOT end end + describe 'undo_override' do + let(:mutex) { Mutex.new } + let(:original_template) { 'templates/template1' } + let(:override_template) { 'templates/template2' } + let(:original_size) { 2 } + let(:override_size) { 10 } + let(:config) { YAML.load(<<-EOT +--- +:config: + task_limit: 5 +:providers: + :mock: +:pools: + - name: '#{pool}' + size: #{override_size} + template: '#{override_template}' +:pool_index: + '#{pool}': 0 +:pools_at_startup: + - name: '#{pool}' + size: #{original_size} + template: '#{original_template}' +EOT + ) + } + + before(:each) do + redis_connection_pool.with do |redis| + redis.sadd('vmpooler__pool__undo_template_override', pool) + redis.sadd('vmpooler__pool__undo_size_override', pool) + # allow(redis).to receive(:hget) + end + end + + it 'should revert to the original template and pool size' do + redis_connection_pool.with do |redis| + expect(redis).to receive(:sismember).with('vmpooler__pool__undo_template_override', pool).and_return(true) + expect(redis).to receive(:srem).with('vmpooler__pool__undo_template_override', pool).and_return(true) + expect(subject).to receive(:update_pool_template).with(config[:pools][0], provider, original_template, override_template, redis) + + expect(redis).to receive(:sismember).with('vmpooler__pool__undo_size_override', pool).and_return(true) + expect(redis).to receive(:srem).with('vmpooler__pool__undo_size_override', pool).and_return(true) + end + + subject.undo_override(config[:pools][0], provider) + end + end + describe '#create_inventory' do it 'should log an error if one occurs' do diff --git a/spec/unit/vmpooler_spec.rb b/spec/unit/vmpooler_spec.rb index ac9f620..19aed91 100644 --- a/spec/unit/vmpooler_spec.rb +++ b/spec/unit/vmpooler_spec.rb @@ -20,6 +20,21 @@ describe 'Vmpooler' do expect(Vmpooler.config[:pools]).to eq(default_config[:pools]) end end + + it 'keeps a copy of the original pools at startup' do + Dir.chdir(fixtures_dir) do + configuration = Vmpooler.config + expect(configuration[:pools]).to eq(configuration[:pools_at_startup]) + end + end + + it 'the copy is a separate object and not a reference' do + Dir.chdir(fixtures_dir) do + configuration = Vmpooler.config + configuration[:pools][0]['template'] = 'sam' + expect(configuration[:pools]).not_to eq(configuration[:pools_at_startup]) + end + end end context 'when config variable is set' do