Move vSphere specific methods out of VMPooler

VMPooler has the vSphere provider taken out, moving some vSphere related
methods to the provider:

1) pool_folders
2) get_base_folders

And the related spec tests.
At the same time renaming some configuration and code items
to remove harmful terminology.
Note this version of the vsphere provider needs to run on vmpooler
that also contain the renaming changes (version >2.1)

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)
 

lib/vmpooler/providers/vsphere.rb

@@ -51,8 +51,8 @@ module Vmpooler
           'vsphere'
         end
 
-        def folder_configured?(folder_title, base_folder, configured_folders, whitelist)
+        def folder_configured?(folder_title, base_folder, configured_folders, allowlist)
-          return true if whitelist&.include?(folder_title)
+          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

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
+  describe '#purge_vms_and_resources' do
-    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) {
+    let(:allowlist) { nil }
-      {
-        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(: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
+      it 'should run purge_unconfigured_resources' do
-        expect(subject).to receive(:pool_folders).and_return(configured_folders)
+        expect(provider).to receive(:purge_unconfigured_resources).with(allowlist)
-        expect(provider).to receive(:purge_unconfigured_folders).with(base_folders, configured_folders, whitelist)
+        allow(provider).to receive(:provider_config).and_return({})
-        expect(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_resources).with(allowlist).and_raise('mockerror')
-        expect(provider).to receive(:purge_unconfigured_folders).with(base_folders, configured_folders, whitelist).and_raise('mockerror')
+        allow(provider).to receive(:provider_config).and_return({})
-        expect(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

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
+    context 'with a allowlist set' do
-      let(:whitelist) { [ other_folder ] }
+      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
+    context 'with string allowlist value' do
-      let(:whitelist) { 'whitelist' }
+      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
 

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'