Skip to content

Commit

Permalink
Fix vmpooler folder purging
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mattkirby committed Aug 3, 2020
1 parent 807daef commit ef4ca26
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 29 deletions.
21 changes: 10 additions & 11 deletions lib/vmpooler/pool_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -539,15 +539,14 @@ def component_to_test(match, labels_string)
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
Expand All @@ -556,14 +555,13 @@ def purge_unused_vms_and_folders
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
Expand All @@ -577,8 +575,9 @@ def get_base_folders(folders)
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)
Expand Down
52 changes: 34 additions & 18 deletions spec/unit/pool_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1418,16 +1418,24 @@
)
}

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

Expand Down Expand Up @@ -1456,20 +1464,28 @@
)
}

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

Expand Down

0 comments on commit ef4ca26

Please sign in to comment.