Merge pull request #467 from puppetlabs/fix-purge

Move vsphere specific methods out of vmpooler

docs/configuration.md

@@ -170,9 +170,12 @@ This can also be set per pool.
 
 ### PURGE\_UNCONFIGURED\_FOLDERS
 
-Enable purging of VMs and folders detected within the base folder path that are not configured for the provider
+Deprecated, see PURGE\_UNCONFIGURED\_RESOURCES
-Only a single layer of folders and their child VMs are evaluated from detected base folder paths
+
-A base folder path for 'vmpooler/redhat-7' would be 'vmpooler'
+### PURGE\_UNCONFIGURED\_RESOURCES
+
+Enable purging of VMs (and other resources) detected within the provider that are not explicitly configured.
+Implementation is provider-dependent
 When enabled in the global configuration then purging is enabled for all providers
 Expects a boolean value
 (optional; default: false)

lib/vmpooler.rb

@@ -31,7 +31,7 @@ module Vmpooler
     if ENV['VMPOOLER_CONFIG']
       config_string = ENV['VMPOOLER_CONFIG']
       # Parse the YAML config into a Hash
-      # Whitelist the Symbol class
+      # Allow the Symbol class
       parsed_config = YAML.safe_load(config_string, [Symbol])
     else
       # Take the name of the config file either from an ENV variable or from the filepath argument
@@ -81,10 +81,13 @@ module Vmpooler
     parsed_config[:config]['retry_factor']                       = string_to_int(ENV['RETRY_FACTOR']) if ENV['RETRY_FACTOR']
     parsed_config[:config]['create_folders']                     = true?(ENV['CREATE_FOLDERS']) if ENV['CREATE_FOLDERS']
     parsed_config[:config]['experimental_features']              = ENV['EXPERIMENTAL_FEATURES'] if ENV['EXPERIMENTAL_FEATURES']
-    parsed_config[:config]['purge_unconfigured_folders']         = ENV['PURGE_UNCONFIGURED_FOLDERS'] if ENV['PURGE_UNCONFIGURED_FOLDERS']
     parsed_config[:config]['usage_stats']                        = ENV['USAGE_STATS'] if ENV['USAGE_STATS']
     parsed_config[:config]['request_logger']                     = ENV['REQUEST_LOGGER'] if ENV['REQUEST_LOGGER']
     parsed_config[:config]['create_template_delta_disks']        = ENV['CREATE_TEMPLATE_DELTA_DISKS'] if ENV['CREATE_TEMPLATE_DELTA_DISKS']
+    parsed_config[:config]['purge_unconfigured_resources']       = ENV['PURGE_UNCONFIGURED_RESOURCES'] if ENV['PURGE_UNCONFIGURED_RESOURCES']
+    parsed_config[:config]['purge_unconfigured_resources']       = ENV['PURGE_UNCONFIGURED_FOLDERS'] if ENV['PURGE_UNCONFIGURED_FOLDERS']
+    # ENV PURGE_UNCONFIGURED_FOLDERS deprecated, will be removed in version 3
+    puts '[!] [deprecation] rename ENV var \'PURGE_UNCONFIGURED_FOLDERS\' to \'PURGE_UNCONFIGURED_RESOURCES\'' if ENV['PURGE_UNCONFIGURED_FOLDERS']
     set_linked_clone(parsed_config)
 
     parsed_config[:redis]                            = parsed_config[:redis] || {}

lib/vmpooler/pool_manager.rb

@@ -478,15 +478,15 @@ module Vmpooler
       dereference_mutex(vm)
     end
 
-    def purge_unused_vms_and_folders
+    def purge_unused_vms_and_resources
-      global_purge = $config[:config]['purge_unconfigured_folders']
+      global_purge = $config[:config]['purge_unconfigured_resources']
       providers = $config[:providers].keys
       providers.each do |provider_key|
-        provider_purge = $config[:providers][provider_key]['purge_unconfigured_folders'] || global_purge
+        provider_purge = $config[:providers][provider_key]['purge_unconfigured_resources'] || global_purge
         if provider_purge
           Thread.new do
             begin
-              purge_vms_and_folders(provider_key)
+              purge_vms_and_resources(provider_key)
             rescue StandardError => e
               $logger.log('s', "[!] failed while purging provider #{provider_key} VMs and folders with an error: #{e}")
             end
@@ -496,33 +496,16 @@ module Vmpooler
       nil
     end
 
-    # Return a list of pool folders
+    def purge_vms_and_resources(provider_name)
-    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 = $providers[provider_name.to_s].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_vms_and_folders(provider_name)
       provider = $providers[provider_name.to_s]
-      configured_folders = pool_folders(provider_name)
+      # Deprecated, will be removed in version 3
-      base_folders = get_base_folders(configured_folders)
+      if provider.provider_config['folder_whitelist']
-      whitelist = provider.provider_config['folder_whitelist']
+        $logger.log('d', "[!] [deprecation] rename configuration 'folder_whitelist' to 'resources_allowlist' for provider #{provider_name}")
-      provider.purge_unconfigured_folders(base_folders, configured_folders, whitelist)
+        allowlist = provider.provider_config['folder_whitelist']
+      else
+        allowlist = provider.provider_config['resources_allowlist']
+      end
+      provider.purge_unconfigured_resources(allowlist)
     end
 
     def create_vm_disk(pool_name, vm, disk_size, provider)
@@ -1606,7 +1589,7 @@ module Vmpooler
         end
       end
 
-      purge_unused_vms_and_folders
+      purge_unused_vms_and_resources
 
       loop_count = 1
       loop do

lib/vmpooler/providers/base.rb

@@ -237,8 +237,15 @@ module Vmpooler
           raise("#{self.class.name} does not implement get_target_datacenter_from_config")
         end
 
-        def purge_unconfigured_folders(_base_folders, _configured_folders, _whitelist)
+        def purge_unconfigured_resources(_allowlist)
-          raise("#{self.class.name} does not implement purge_unconfigured_folders")
+          raise("#{self.class.name} does not implement purge_unconfigured_resources")
+        end
+
+        # DEPRECATED if a provider does not implement the new method, it will hit this base class method
+        # and return a deprecation message
+        def purge_unconfigured_folders(_deprecated, _deprecated2, allowlist)
+          logger.log('s', '[!] purge_unconfigured_folders was renamed to purge_unconfigured_resources, please update your provider implementation')
+          purge_unconfigured_resources(allowlist)
         end
       end
     end

lib/vmpooler/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Vmpooler
-  VERSION = '2.0.0'
+  VERSION = '2.1.0'
 end

spec/unit/env_config.rb

@@ -28,7 +28,7 @@ describe 'Vmpooler' do
         ['create_template_delta_disks', test_bool, nil],
         ['create_linked_clones', test_bool, nil],
         ['experimental_features', test_bool, nil],
-        ['purge_unconfigured_folders', test_bool, nil],
+        ['purge_unconfigured_resources', test_bool, nil],
         ['usage_stats', test_bool, nil],
         ['request_logger', test_bool, nil],
         ['extra_config', test_string, nil],

spec/unit/pool_manager_spec.rb

@@ -1061,7 +1061,7 @@ EOT
     end
   end
 
-  describe '#purge_unused_vms_and_folders' do
+  describe '#purge_unused_vms_and_resources' do
     let(:config) { YAML.load(<<-EOT
 ---
 :config: {}
@@ -1075,104 +1075,51 @@ 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) {
-      {
-        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: {}
@@ -1195,20 +1142,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)
+        expect(provider).to receive(:provider_config).and_return({}).twice
-        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')
+        expect(provider).to receive(:provider_config).and_return({}).twice
-        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

vmpooler.yaml.example

@@ -11,20 +11,21 @@
 #     For multiple providers, specify one of the supported backing services (vsphere or dummy)
 #     (optional: will default to it's parent :key: name eg. 'vsphere')
 #
-#   - purge_unconfigured_folders
+#   - purge_unconfigured_folders DEPRECATED, use purge_unconfigured_resources
-#     Enable purging of VMs and folders detected within the base folder path that are not configured for the provider
+#   - purge_unconfigured_resources
-#     Only a single layer of folders and their child VMs are evaluated from detected base folder paths
+#     Enable purging of resources (typically VMs) or other items based on the provider. Provides a high-level cleanup
-#     Nested child folders will not be destroyed. An optional whitelist can be provided to exclude folders
+#     mechanism for things that are live but not found in the vmpooler config ie in a pool config. See the provider's
-#     A base folder path for 'vmpooler/redhat-7' would be 'vmpooler'
+#     implementation for more details.
-#     Setting this on the provider will enable folder purging for the provider
+#     Setting this on the provider will enable purging for the provider
 #     Expects a boolean value
 #     (optional; default: false)
 #
-#   - folder_whitelist
+#   - folder_whitelist DEPRECATED, use resources_allowlist
-#     Specify folders that are within the base folder path, not in the configuration, and should not be destroyed
+#   - resources_allowlist
-#     To exclude 'vmpooler/myfolder' from being destroyed when the base path is 'vmpooler' you would specify 'myfolder' in the whitelist
+#     Specify items names that should be ignored when purging. See the provider's
-#     This option is only evaluated when 'purge_unconfigured_folders' is enabled
+#     implementation for more details.
-#     Expects an array of strings specifying the whitelisted folders by name
+#     This option is only evaluated when 'purge_unconfigured_resources' is enabled
+#     Expects an array of strings specifying the allowed items by name
 #     (optional; default: nil)
 #
 # If you want to support more than one provider with different parameters (server, username or passwords) you have to specify the
@@ -518,10 +519,11 @@
 #     Expects a boolean value
 #     (optional; default: false)
 #
-#   - purge_unconfigured_folders (vSphere Provider only)
+#   - purge_unconfigured_folders DEPRECATED, use purge_unconfigured_resources
-#     Enable purging of VMs and folders detected within the base folder path that are not configured for the provider
+#   - purge_unconfigured_resources
-#     Only a single layer of folders and their child VMs are evaluated from detected base folder paths
+#     Enable purging of resources (typically VMs) or other items based on the provider. Provides a high-level cleanup
-#     A base folder path for 'vmpooler/redhat-7' would be 'vmpooler'
+#     mechanism for things that are live but not found in the vmpooler config ie in a pool config. See the provider's
+#     implementation for more details.
 #     When enabled in the global configuration then purging is enabled for all providers
 #     Expects a boolean value
 #     (optional; default: false)
@@ -579,7 +581,8 @@
 #
 # Example:
 
-:config: site_name: 'vmpooler'
+:config:
+  site_name: 'vmpooler'
   logfile: '/var/log/vmpooler.log'
   task_limit: 10
   timeout: 15