From ef4ca261d05ef26863f7fa4ba200079d20146deb Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Fri, 17 Jul 2020 16:19:57 -0700 Subject: [PATCH] Fix vmpooler folder purging This commit updates folder purging references to ensure that provider name references are referring to the named provider, rather than the provider type. Without this change folder purging fails because it cannot identify target folders. --- lib/vmpooler/pool_manager.rb | 21 +++++++------- spec/unit/pool_manager_spec.rb | 52 ++++++++++++++++++++++------------ 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 2380c36..35b4226 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -539,15 +539,14 @@ module Vmpooler def purge_unused_vms_and_folders global_purge = $config[:config]['purge_unconfigured_folders'] providers = $config[:providers].keys - providers.each do |provider| - provider_purge = $config[:providers][provider]['purge_unconfigured_folders'] - provider_purge = global_purge if provider_purge.nil? + providers.each do |provider_key| + provider_purge = $config[:providers][provider_key]['purge_unconfigured_folders'] || global_purge if provider_purge Thread.new do begin - purge_vms_and_folders($providers[provider.to_s]) + purge_vms_and_folders(provider_key) rescue StandardError => e - $logger.log('s', "[!] failed while purging provider #{provider} VMs and folders with an error: #{e}") + $logger.log('s', "[!] failed while purging provider #{provider_key} VMs and folders with an error: #{e}") end end end @@ -556,14 +555,13 @@ module Vmpooler end # Return a list of pool folders - def pool_folders(provider) - provider_name = provider.name + def pool_folders(provider_name) folders = {} $config[:pools].each do |pool| - next unless pool['provider'] == provider_name + next unless pool['provider'] == provider_name.to_s folder_parts = pool['folder'].split('/') - datacenter = provider.get_target_datacenter_from_config(pool['name']) + datacenter = $providers[provider_name.to_s].get_target_datacenter_from_config(pool['name']) folders[folder_parts.pop] = "#{datacenter}/vm/#{folder_parts.join('/')}" end folders @@ -577,8 +575,9 @@ module Vmpooler base.uniq end - def purge_vms_and_folders(provider) - configured_folders = pool_folders(provider) + def purge_vms_and_folders(provider_name) + provider = $providers[provider_name.to_s] + configured_folders = pool_folders(provider_name) base_folders = get_base_folders(configured_folders) whitelist = provider.provider_config['folder_whitelist'] provider.purge_unconfigured_folders(base_folders, configured_folders, whitelist) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 446d1ba..204a2d7 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -1418,16 +1418,24 @@ EOT ) } - it 'should return a list of pool folders' do - expect(provider).to receive(:get_target_datacenter_from_config).with(pool).and_return(datacenter) + context 'when evaluating pool folders' do + before do + expect(subject).not_to be_nil + # Inject mock provider into global variable - Note this is a code smell + $providers = { provider_name => provider } + end - expect(subject.pool_folders(provider)).to eq(expected_response) - end + it 'should return a list of pool folders' do + expect(provider).to receive(:get_target_datacenter_from_config).with(pool).and_return(datacenter) - it 'should raise an error when the provider fails to get the datacenter' do - expect(provider).to receive(:get_target_datacenter_from_config).with(pool).and_raise('mockerror') + expect(subject.pool_folders(provider_name)).to eq(expected_response) + end - expect{ subject.pool_folders(provider) }.to raise_error(RuntimeError, 'mockerror') + it 'should raise an error when the provider fails to get the datacenter' do + expect(provider).to receive(:get_target_datacenter_from_config).with(pool).and_raise('mockerror') + + expect{ subject.pool_folders(provider_name) }.to raise_error(RuntimeError, 'mockerror') + end end end @@ -1456,20 +1464,28 @@ EOT ) } - it 'should run purge_unconfigured_folders' do - expect(subject).to receive(:pool_folders).and_return(configured_folders) - expect(provider).to receive(:purge_unconfigured_folders).with(base_folders, configured_folders, whitelist) - expect(provider).to receive(:provider_config).and_return({}) + context 'when purging folders' do + before do + expect(subject).not_to be_nil + # Inject mock provider into global variable - Note this is a code smell + $providers = { provider_name => provider } + end - subject.purge_vms_and_folders(provider) - end + it 'should run purge_unconfigured_folders' do + expect(subject).to receive(:pool_folders).and_return(configured_folders) + expect(provider).to receive(:purge_unconfigured_folders).with(base_folders, configured_folders, whitelist) + expect(provider).to receive(:provider_config).and_return({}) - it 'should raise any errors' do - expect(subject).to receive(:pool_folders).and_return(configured_folders) - expect(provider).to receive(:purge_unconfigured_folders).with(base_folders, configured_folders, whitelist).and_raise('mockerror') - expect(provider).to receive(:provider_config).and_return({}) + subject.purge_vms_and_folders(provider_name) + end - expect{ subject.purge_vms_and_folders(provider) }.to raise_error(RuntimeError, 'mockerror') + it 'should raise any errors' do + expect(subject).to receive(:pool_folders).and_return(configured_folders) + expect(provider).to receive(:purge_unconfigured_folders).with(base_folders, configured_folders, whitelist).and_raise('mockerror') + expect(provider).to receive(:provider_config).and_return({}) + + expect{ subject.purge_vms_and_folders(provider_name) }.to raise_error(RuntimeError, 'mockerror') + end end end