diff --git a/Gemfile.lock b/Gemfile.lock index 5bde0f7..23c1771 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,7 +136,7 @@ GEM rubocop-ast (>= 1.0.1) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 2.0) - rubocop-ast (1.14.0) + rubocop-ast (1.15.0) parser (>= 3.0.1.1) ruby-progressbar (1.11.0) ruby2_keywords (0.0.5) @@ -160,7 +160,7 @@ GEM thrift (0.15.0) tilt (2.0.10) unicode-display_width (1.8.0) - vmpooler (2.0.0) + vmpooler (2.1.0) concurrent-ruby (~> 1.1) connection_pool (~> 2.2) net-ldap (~> 0.16) @@ -197,7 +197,7 @@ DEPENDENCIES rubocop (~> 1.1.0) simplecov (>= 0.11.2) thor (~> 1.0, >= 1.0.1) - vmpooler (~> 2.0) + vmpooler (~> 2.1) vmpooler-provider-vsphere! yarjuf (>= 2.0) diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index b860743..c572d48 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -51,8 +51,8 @@ module Vmpooler 'vsphere' end - def folder_configured?(folder_title, base_folder, configured_folders, whitelist) - return true if whitelist&.include?(folder_title) + def folder_configured?(folder_title, base_folder, configured_folders, allowlist) + return true if allowlist&.include?(folder_title) return false unless configured_folders.keys.include?(folder_title) return false unless configured_folders[folder_title] == base_folder @@ -119,7 +119,30 @@ module Vmpooler try >= max_tries ? raise : retry end - def purge_unconfigured_folders(base_folders, configured_folders, whitelist) + # Return a list of pool folders + def pool_folders(provider_name) + folders = {} + $config[:pools].each do |pool| + next unless pool['provider'] == provider_name.to_s + + folder_parts = pool['folder'].split('/') + datacenter = get_target_datacenter_from_config(pool['name']) + folders[folder_parts.pop] = "#{datacenter}/vm/#{folder_parts.join('/')}" + end + folders + end + + def get_base_folders(folders) + base = [] + folders.each do |_key, value| + base << value + end + base.uniq + end + + def purge_unconfigured_resources(allowlist) + configured_folders = pool_folders(name) + base_folders = get_base_folders(configured_folders) @connection_pool.with_metrics do |pool_object| connection = ensured_vsphere_connection(pool_object) @@ -129,7 +152,7 @@ module Vmpooler folder_children.each do |folder_hash| folder_hash.each do |folder_title, folder_object| - destroy_folder_and_children(folder_object) unless folder_configured?(folder_title, base_folder, configured_folders, whitelist) + destroy_folder_and_children(folder_object) unless folder_configured?(folder_title, base_folder, configured_folders, allowlist) end end end diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 24dae84..4f551eb 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -61,7 +61,7 @@ EOT --- :config: :providers: - :mock: + :vsphere: :pools: - name: '#{pool}' size: 1 @@ -1060,7 +1060,7 @@ EOT end end - describe '#purge_unused_vms_and_folders' do + describe '#purge_unused_vms_and_resources' do let(:config) { YAML.load(<<-EOT --- :config: {} @@ -1074,104 +1074,46 @@ EOT } it 'should return when purging is not enabled' do - expect(subject.purge_unused_vms_and_folders).to be_nil + expect(subject.purge_unused_vms_and_resources).to be_nil end context 'with purging enabled globally' do before(:each) do - config[:config]['purge_unconfigured_folders'] = true + config[:config]['purge_unconfigured_resources'] = true expect(Thread).to receive(:new).and_yield end it 'should run a purge for each provider' do - expect(subject).to receive(:purge_vms_and_folders) + expect(subject).to receive(:purge_vms_and_resources) - subject.purge_unused_vms_and_folders + subject.purge_unused_vms_and_resources end it 'should log when purging fails' do - expect(subject).to receive(:purge_vms_and_folders).and_raise(RuntimeError,'MockError') + expect(subject).to receive(:purge_vms_and_resources).and_raise(RuntimeError,'MockError') expect(logger).to receive(:log).with('s', '[!] failed while purging provider mock VMs and folders with an error: MockError') - subject.purge_unused_vms_and_folders + subject.purge_unused_vms_and_resources end end context 'with purging enabled on the provider' do before(:each) do - config[:providers][:mock]['purge_unconfigured_folders'] = true + config[:providers][:mock]['purge_unconfigured_resources'] = true expect(Thread).to receive(:new).and_yield end it 'should run a purge for the provider' do - expect(subject).to receive(:purge_vms_and_folders) + expect(subject).to receive(:purge_vms_and_resources) - subject.purge_unused_vms_and_folders + subject.purge_unused_vms_and_resources end end end - describe '#pool_folders' do - let(:folder_name) { 'myinstance' } - let(:folder_base) { 'vmpooler' } - let(:folder) { [folder_base,folder_name].join('/') } - let(:datacenter) { 'dc1' } + describe '#purge_vms_and_resources' do let(:provider_name) { 'mock_provider' } - let(:expected_response) { - { - folder_name => "#{datacenter}/vm/#{folder_base}" - } - } - let(:config) { YAML.load(<<-EOT ---- -:providers: - :mock: -:pools: - - name: '#{pool}' - folder: '#{folder}' - size: 1 - datacenter: '#{datacenter}' - provider: '#{provider_name}' - - name: '#{pool}2' - folder: '#{folder}' - size: 1 - datacenter: '#{datacenter}' - provider: '#{provider_name}2' -EOT - ) - } - - 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 - - it 'should return a list of pool folders' do - expect(provider).to receive(:get_target_datacenter_from_config).with(pool).and_return(datacenter) - - expect(subject.pool_folders(provider_name)).to eq(expected_response) - end - - 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 - - describe '#purge_vms_and_folders' do - let(:folder_name) { 'myinstance' } - let(:folder_base) { 'vmpooler' } - let(:datacenter) { 'dc1' } - let(:full_folder_path) { "#{datacenter}/vm/folder_base" } - let(:configured_folders) { { folder_name => full_folder_path } } - let(:base_folders) { [ full_folder_path ] } - let(:folder) { [folder_base,folder_name].join('/') } - let(:provider_name) { 'mock_provider' } - let(:whitelist) { nil } + let(:allowlist) { nil } let(:config) { YAML.load(<<-EOT --- :config: {} @@ -1179,9 +1121,7 @@ EOT :mock_provider: {} :pools: - name: '#{pool}' - folder: '#{folder}' size: 1 - datacenter: '#{datacenter}' provider: '#{provider_name}' EOT ) @@ -1194,20 +1134,18 @@ EOT $providers = { provider_name => 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 run purge_unconfigured_resources' do + expect(provider).to receive(:purge_unconfigured_resources).with(allowlist) + allow(provider).to receive(:provider_config).and_return({}) - subject.purge_vms_and_folders(provider_name) + subject.purge_vms_and_resources(provider_name) end 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(provider).to receive(:purge_unconfigured_resources).with(allowlist).and_raise('mockerror') + allow(provider).to receive(:provider_config).and_return({}) - expect{ subject.purge_vms_and_folders(provider_name) }.to raise_error(RuntimeError, 'mockerror') + expect{ subject.purge_vms_and_resources(provider_name) }.to raise_error(RuntimeError, 'mockerror') end end end diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb index b05457c..f2a1975 100644 --- a/spec/unit/providers/vsphere_spec.rb +++ b/spec/unit/providers/vsphere_spec.rb @@ -104,36 +104,36 @@ EOT let(:other_folder) { 'folder2' } let(:base_folder) { 'dc1/vm/base' } let(:configured_folders) { { folder_title => base_folder } } - let(:whitelist) { nil } + let(:allowlist) { nil } it 'should return true when configured_folders includes the folder_title' do - expect(subject.folder_configured?(folder_title, base_folder, configured_folders, whitelist)).to be true + expect(subject.folder_configured?(folder_title, base_folder, configured_folders, allowlist)).to be true end it 'should return false when title is not in configured_folders' do - expect(subject.folder_configured?(other_folder, base_folder, configured_folders, whitelist)).to be false + expect(subject.folder_configured?(other_folder, base_folder, configured_folders, allowlist)).to be false end context 'with another base folder' do let(:base_folder) { 'dc2/vm/base' } let(:configured_folders) { { folder_title => 'dc1/vm/base' } } it 'should return false' do - expect(subject.folder_configured?(folder_title, base_folder, configured_folders, whitelist)).to be false + expect(subject.folder_configured?(folder_title, base_folder, configured_folders, allowlist)).to be false end end - context 'with a whitelist set' do - let(:whitelist) { [ other_folder ] } + context 'with a allowlist set' do + let(:allowlist) { [ other_folder ] } it 'should return true' do - expect(subject.folder_configured?(other_folder, base_folder, configured_folders, whitelist)).to be true + expect(subject.folder_configured?(other_folder, base_folder, configured_folders, allowlist)).to be true end end - context 'with string whitelist value' do - let(:whitelist) { 'whitelist' } + context 'with string allowlist value' do + let(:allowlist) { 'allowlist' } it 'should raise an error' do - expect(whitelist).to receive(:include?).and_raise('mockerror') + expect(allowlist).to receive(:include?).and_raise('mockerror') - expect{ subject.folder_configured?(other_folder, base_folder, configured_folders, whitelist) }.to raise_error(RuntimeError, 'mockerror') + expect{ subject.folder_configured?(other_folder, base_folder, configured_folders, allowlist) }.to raise_error(RuntimeError, 'mockerror') end end end @@ -297,12 +297,61 @@ EOT end end - describe '#purge_unconfigured_folders' do + describe '#pool_folders' do + let(:pool) { 'pool1' } + let(:folder_name) { 'myinstance' } + let(:folder_base) { 'vmpooler' } + let(:folder) { [folder_base,folder_name].join('/') } + let(:datacenter) { 'dc1' } + let(:provider_name) { 'mock_provider' } + let(:expected_response) { + { + folder_name => "#{datacenter}/vm/#{folder_base}" + } + } + context 'when evaluating pool folders' do + before do + expect(subject).not_to be_nil + #replace top-level global config + $config = YAML.load(<<-EOT +--- +:providers: + :mock: +:pools: + - name: '#{pool}' + folder: '#{folder}' + size: 1 + datacenter: '#{datacenter}' + provider: '#{provider_name}' + - name: '#{pool}2' + folder: '#{folder}' + size: 1 + datacenter: '#{datacenter}' + provider: '#{provider_name}2' + EOT + ) + end + + it 'should return a list of pool folders' do + expect(subject).to receive(:get_target_datacenter_from_config).with(pool).and_return(datacenter) + + expect(subject.pool_folders(provider_name)).to eq(expected_response) + end + + it 'should raise an error when the provider fails to get the datacenter' do + expect(subject).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 + + describe '#purge_unconfigured_resources' do let(:folder_title) { 'folder1' } let(:base_folder) { 'dc1/vm/base' } let(:folder_object) { mock_RbVmomi_VIM_Folder({ :name => base_folder }) } let(:child_folder) { mock_RbVmomi_VIM_Folder({ :name => folder_title }) } - let(:whitelist) { nil } + let(:allowlist) { nil } let(:base_folders) { [ base_folder ] } let(:configured_folders) { { folder_title => base_folder } } let(:folder_children) { [ folder_title => child_folder ] } @@ -310,6 +359,7 @@ EOT before(:each) do allow(subject).to receive(:connect_to_vsphere).and_return(connection) + allow(subject).to receive(:pool_folders).and_return(configured_folders) end context 'with an empty folder' do @@ -317,7 +367,7 @@ EOT expect(subject).to receive(:get_folder_children).with(base_folder, connection).and_return(empty_list) expect(subject).to_not receive(:destroy_folder_and_children) - subject.purge_unconfigured_folders(base_folders, configured_folders, whitelist) + subject.purge_unconfigured_resources(allowlist) end end @@ -325,7 +375,7 @@ EOT expect(subject).to receive(:get_folder_children).with(base_folder, connection).and_return(folder_children) allow(subject).to receive(:folder_configured?).and_return(true) - subject.purge_unconfigured_folders(base_folders, configured_folders, whitelist) + subject.purge_unconfigured_resources(allowlist) end context 'with a folder that is not configured' do @@ -337,14 +387,14 @@ EOT it 'should destroy the folder and children' do expect(subject).to receive(:destroy_folder_and_children).with(child_folder).and_return(nil) - subject.purge_unconfigured_folders(base_folders, configured_folders, whitelist) + subject.purge_unconfigured_resources(allowlist) end end it 'should raise any errors' do expect(subject).to receive(:get_folder_children).and_throw('mockerror') - expect{ subject.purge_unconfigured_folders(base_folders, configured_folders, whitelist) }.to raise_error(/mockerror/) + expect{ subject.purge_unconfigured_resources(allowlist) }.to raise_error(/mockerror/) end end diff --git a/vmpooler-provider-vsphere.gemspec b/vmpooler-provider-vsphere.gemspec index a0d332b..6c7febe 100644 --- a/vmpooler-provider-vsphere.gemspec +++ b/vmpooler-provider-vsphere.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |s| s.require_paths = ["lib"] s.add_dependency 'rbvmomi', '>= 2.1', '< 4.0' - s.add_development_dependency 'vmpooler', '~> 2.0' + s.add_development_dependency 'vmpooler', '~> 2.1' # renaming done in version 2.1 # Testing dependencies s.add_development_dependency 'climate_control', '>= 0.2.0'