From 1b17cceb01385c2f4569fe7ae7f29317fdd59f54 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Mon, 2 Jul 2018 09:53:41 -0700 Subject: [PATCH 1/2] Ensure template deltas are created once This commit updates how template delta disk creation is evaluated. Without this change template deltas are created for every template on each applicatoin startup. This change updates this behavior to instead run template delta disk creation only once per template configured for a pool. Without this change it is possible to get a template to a state where the XML depth is too great to be read with default settings and the template requires a new clone to resolve. --- lib/vmpooler/pool_manager.rb | 7 +++++-- spec/unit/pool_manager_spec.rb | 21 +++++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index a00acb4..fe9739b 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -628,6 +628,11 @@ module Vmpooler prepare_template(pool, provider) prepared_template = $redis.hget('vmpooler__template__prepared', pool['name']) end + elsif not prepared_template == pool['template'] + mutex.synchronize do + prepare_template(pool, provider) + prepared_template = $redis.hget('vmpooler__template__prepared', pool['name']) + end end return if configured_template.nil? return if configured_template == prepared_template @@ -896,8 +901,6 @@ module Vmpooler $redis.set('vmpooler__tasks__clone', 0) # Clear out vmpooler__migrations since stale entries may be left after a restart $redis.del('vmpooler__migration') - # Ensure template deltas are created on each startup - $redis.del('vmpooler__template__prepared') # Copy vSphere settings to correct location. This happens with older configuration files if !$config[:vsphere].nil? && ($config[:providers].nil? || $config[:providers][:vsphere].nil?) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index be176d6..bf44a01 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -1830,11 +1830,29 @@ EOT end context 'when prepared template is nil' do - before(:each) do + + it 'should prepare the template' do expect(redis).to receive(:hget).with('vmpooler__template__prepared', pool).and_return(nil) + expect(subject).to receive(:prepare_template).with(config[:pools][0], provider) + + subject.evaluate_template(config[:pools][0], provider) + end + + it 'should not prepare the template again' do + expect(redis).to receive(:hget).with('vmpooler__template__prepared', pool).and_return(current_template) + expect(subject).to_not receive(:prepare_template).with(config[:pools][0], provider) + + subject.evaluate_template(config[:pools][0], provider) + end + end + + context 'when the configured pool template does not match the prepared template' do + before(:each) do + config[:pools][0]['template'] = new_template end it 'should prepare the template' do + expect(redis).to receive(:hget).with('vmpooler__template__prepared', pool).and_return(current_template) expect(subject).to receive(:prepare_template).with(config[:pools][0], provider) subject.evaluate_template(config[:pools][0], provider) @@ -2291,7 +2309,6 @@ EOT it 'should run startup tasks only once' do expect(redis).to receive(:set).with('vmpooler__tasks__clone', 0).once expect(redis).to receive(:del).with('vmpooler__migration').once - expect(redis).to receive(:del).with('vmpooler__template__prepared').once subject.execute!(maxloop,0) end From 70156ba7f76301d3d38da011f2b606e95b9dc141 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Mon, 2 Jul 2018 14:12:17 -0700 Subject: [PATCH 2/2] Do not prepare template when config_template is set --- lib/vmpooler/pool_manager.rb | 10 ++++++---- spec/unit/pool_manager_spec.rb | 11 ++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index fe9739b..ef6a5cd 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -628,10 +628,12 @@ module Vmpooler prepare_template(pool, provider) prepared_template = $redis.hget('vmpooler__template__prepared', pool['name']) end - elsif not prepared_template == pool['template'] - mutex.synchronize do - prepare_template(pool, provider) - prepared_template = $redis.hget('vmpooler__template__prepared', pool['name']) + elsif prepared_template != pool['template'] + if configured_template.nil? + mutex.synchronize do + prepare_template(pool, provider) + prepared_template = $redis.hget('vmpooler__template__prepared', pool['name']) + end end end return if configured_template.nil? diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index bf44a01..b6d082a 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -1849,14 +1849,23 @@ EOT context 'when the configured pool template does not match the prepared template' do before(:each) do config[:pools][0]['template'] = new_template + expect(redis).to receive(:hget).with('vmpooler__template__prepared', pool).and_return(current_template) end it 'should prepare the template' do - expect(redis).to receive(:hget).with('vmpooler__template__prepared', pool).and_return(current_template) expect(subject).to receive(:prepare_template).with(config[:pools][0], provider) subject.evaluate_template(config[:pools][0], provider) end + + context 'if configured_template is provided' do + it 'should not run prepare_template' do + expect(redis).to receive(:hget).with('vmpooler__config__template', pool).and_return(current_template) + expect(subject).to_not receive(:prepare_template) + + subject.evaluate_template(config[:pools][0], provider) + end + end end context 'when a new template is requested' do