(POOLER-52) Use a Connection Pooler for vSphere connections

Previously the vSphere Provider would share a single vSphere connection for all
pools under management.  This would cause issues in large environments as this
would cause errors to be thrown or operations to slow down.  This commit
modifies the vSphere Provider to use a connection pool when communicating with
the vSphere API
- Uses the GenericConnectionPool object to manage the connection pool
- Uses a default connection pool size of:
	Whatever is biggest from:
	- How many pools this provider services
	- Maximum number of cloning tasks allowed
	- Need at least 2 connections so that a pool can have inventory functions
	  performed while cloning etc.
- A large connection_pool_timeout is used as a connection object is consumed
  during a VM clone, which can take up to 2 minutes
- Removes the `get_connection` method as that is now obsolete due to the
  connection pool
- Removes the `close` method as it is now obsolete
- Modified the spec tests slightly, to stop mocking get_connection as it no
  longer exists, and set a super low pool timeout so that if a test fails, it
  will fail quickly instead of taking the default time of 60+ seconds

lib/vmpooler/providers/vsphere.rb

@@ -2,159 +2,188 @@ module Vmpooler
   class PoolManager
     class Provider
       class VSphere < Vmpooler::PoolManager::Provider::Base
+        def initialize(config, logger, metrics, name, options)
+          super(config, logger, metrics, name, options)
+
+          task_limit = global_config[:config].nil? || global_config[:config]['task_limit'].nil? ? 10 : global_config[:config]['task_limit'].to_i
+          # The default connection pool size is:
+          # Whatever is biggest from:
+          #   - How many pools this provider services
+          #   - Maximum number of cloning tasks allowed
+          #   - Need at least 2 connections so that a pool can have inventory functions performed while cloning etc.
+          default_connpool_size = [provided_pools.count, task_limit, 2].max
+          connpool_size = provider_config['connection_pool_size'].nil? ? default_connpool_size : provider_config['connection_pool_size'].to_i
+          # The default connection pool timeout should be quite large - 60 seconds
+          connpool_timeout = provider_config['connection_pool_timeout'].nil? ? 60 : provider_config['connection_pool_timeout'].to_i
+          logger.log('d', "[#{name}] ConnPool - Creating a connection pool of size #{connpool_size} with timeout #{connpool_timeout}")
+          @connection_pool = Vmpooler::PoolManager::GenericConnectionPool.new(
+            metrics: metrics,
+            metric_prefix: "#{name}_provider_connection_pool",
+            size: connpool_size,
+            timeout: connpool_timeout
+          ) do
+            logger.log('d', "[#{name}] Connection Pool - Creating a connection object")
+            new_conn = connect_to_vsphere
+
+            new_conn
+          end
+        end
+
         def name
           'vsphere'
         end
 
         def vms_in_pool(pool_name)
-          connection = get_connection
-
-          foldername = pool_config(pool_name)['folder']
-          folder_object = find_folder(foldername, connection)
-
           vms = []
+          @connection_pool.with_metrics do |connection|
+            foldername = pool_config(pool_name)['folder']
+            folder_object = find_folder(foldername, connection)
 
-          return vms if folder_object.nil?
+            return vms if folder_object.nil?
 
-          folder_object.childEntity.each do |vm|
+            folder_object.childEntity.each do |vm|
-            vms << { 'name' => vm.name }
+              vms << { 'name' => vm.name }
+            end
           end
-
           vms
         end
 
         def get_vm_host(_pool_name, vm_name)
-          connection = get_connection
-
-          vm_object = find_vm(vm_name, connection)
-          return nil if vm_object.nil?
-
           host_name = nil
-          host_name = vm_object.summary.runtime.host.name if vm_object.summary && vm_object.summary.runtime && vm_object.summary.runtime.host
 
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
+            return host_name if vm_object.nil?
+
+            host_name = vm_object.summary.runtime.host.name if vm_object.summary && vm_object.summary.runtime && vm_object.summary.runtime.host
+          end
           host_name
         end
 
         def find_least_used_compatible_host(_pool_name, vm_name)
-          connection = get_connection
+          hostname = nil
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
 
-          vm_object = find_vm(vm_name, connection)
+            return hostname if vm_object.nil?
+            host_object = find_least_used_vpshere_compatible_host(vm_object)
 
-          return nil if vm_object.nil?
+            return hostname if host_object.nil?
-          host_object = find_least_used_vpshere_compatible_host(vm_object)
+            hostname = host_object[0].name
-
+          end
-          return nil if host_object.nil?
+          hostname
-          host_object[0].name
         end
 
         def migrate_vm_to_host(pool_name, vm_name, dest_host_name)
           pool = pool_config(pool_name)
           raise("Pool #{pool_name} does not exist for the provider #{name}") if pool.nil?
 
-          connection = get_connection
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
+            raise("VM #{vm_name} does not exist in Pool #{pool_name} for the provider #{name}") if vm_object.nil?
 
-          vm_object = find_vm(vm_name, connection)
+            target_cluster_name = get_target_cluster_from_config(pool_name)
-          raise("VM #{vm_name} does not exist in Pool #{pool_name} for the provider #{name}") if vm_object.nil?
+            cluster = find_cluster(target_cluster_name, connection)
+            raise("Pool #{pool_name} specifies cluster #{target_cluster_name} which does not exist for the provider #{name}") if cluster.nil?
 
-          target_cluster_name = get_target_cluster_from_config(pool_name)
+            # Go through each host and initiate a migration when the correct host name is found
-          cluster = find_cluster(target_cluster_name, connection)
+            cluster.host.each do |host|
-          raise("Pool #{pool_name} specifies cluster #{target_cluster_name} which does not exist for the provider #{name}") if cluster.nil?
+              if host.name == dest_host_name
-
+                migrate_vm_host(vm_object, host)
-          # Go through each host and initiate a migration when the correct host name is found
+                return true
-          cluster.host.each do |host|
+              end
-            if host.name == dest_host_name
-              migrate_vm_host(vm_object, host)
-              return true
             end
           end
-
           false
         end
 
         def get_vm(_pool_name, vm_name)
-          connection = get_connection
+          vm_hash = nil
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
+            return vm_hash if vm_object.nil?
 
-          vm_object = find_vm(vm_name, connection)
+            vm_folder_path = get_vm_folder_path(vm_object)
-          return nil if vm_object.nil?
+            # Find the pool name based on the folder path
-
+            pool_name = nil
-          vm_folder_path = get_vm_folder_path(vm_object)
+            template_name = nil
-          # Find the pool name based on the folder path
+            global_config[:pools].each do |pool|
-          pool_name = nil
+              if pool['folder'] == vm_folder_path
-          template_name = nil
+                pool_name = pool['name']
-          global_config[:pools].each do |pool|
+                template_name = pool['template']
-            if pool['folder'] == vm_folder_path
+              end
-              pool_name = pool['name']
-              template_name = pool['template']
             end
-          end
 
-          generate_vm_hash(vm_object, template_name, pool_name)
+            vm_hash = generate_vm_hash(vm_object, template_name, pool_name)
+          end
+          vm_hash
         end
 
         def create_vm(pool_name, new_vmname)
           pool = pool_config(pool_name)
           raise("Pool #{pool_name} does not exist for the provider #{name}") if pool.nil?
+          vm_hash = nil
+          @connection_pool.with_metrics do |connection|
+            # Assume all pool config is valid i.e. not missing
+            template_path = pool['template']
+            target_folder_path = pool['folder']
+            target_datastore = pool['datastore']
+            target_cluster_name = get_target_cluster_from_config(pool_name)
 
-          connection = get_connection
+            # Extract the template VM name from the full path
+            raise("Pool #{pool_name} did specify a full path for the template for the provider #{name}") unless template_path =~ /\//
+            templatefolders = template_path.split('/')
+            template_name = templatefolders.pop
 
-          # Assume all pool config is valid i.e. not missing
+            # Get the actual objects from vSphere
-          template_path = pool['template']
+            template_folder_object = find_folder(templatefolders.join('/'), connection)
-          target_folder_path = pool['folder']
+            raise("Pool #{pool_name} specifies a template folder of #{templatefolders.join('/')} which does not exist for the provider #{name}") if template_folder_object.nil?
-          target_datastore = pool['datastore']
-          target_cluster_name = get_target_cluster_from_config(pool_name)
 
-          # Extract the template VM name from the full path
+            template_vm_object = template_folder_object.find(template_name)
-          raise("Pool #{pool_name} did specify a full path for the template for the provider #{name}") unless template_path =~ /\//
+            raise("Pool #{pool_name} specifies a template VM of #{template_name} which does not exist for the provider #{name}") if template_vm_object.nil?
-          templatefolders = template_path.split('/')
-          template_name = templatefolders.pop
 
-          # Get the actual objects from vSphere
+            # Annotate with creation time, origin template, etc.
-          template_folder_object = find_folder(templatefolders.join('/'), connection)
+            # Add extraconfig options that can be queried by vmtools
-          raise("Pool #{pool_name} specifies a template folder of #{templatefolders.join('/')} which does not exist for the provider #{name}") if template_folder_object.nil?
+            config_spec = RbVmomi::VIM.VirtualMachineConfigSpec(
+              annotation: JSON.pretty_generate(
+                name: new_vmname,
+                created_by: provider_config['username'],
+                base_template: template_path,
+                creation_timestamp: Time.now.utc
+              ),
+              extraConfig: [
+                { key: 'guestinfo.hostname', value: new_vmname }
+              ]
+            )
 
-          template_vm_object = template_folder_object.find(template_name)
+            # Choose a cluster/host to place the new VM on
-          raise("Pool #{pool_name} specifies a template VM of #{template_name} which does not exist for the provider #{name}") if template_vm_object.nil?
+            target_host_object = find_least_used_host(target_cluster_name, connection)
 
-          # Annotate with creation time, origin template, etc.
+            # Put the VM in the specified folder and resource pool
-          # Add extraconfig options that can be queried by vmtools
+            relocate_spec = RbVmomi::VIM.VirtualMachineRelocateSpec(
-          config_spec = RbVmomi::VIM.VirtualMachineConfigSpec(
+              datastore: find_datastore(target_datastore, connection),
-            annotation: JSON.pretty_generate(
+              host: target_host_object,
+              diskMoveType: :moveChildMostDiskBacking
+            )
+
+            # Create a clone spec
+            clone_spec = RbVmomi::VIM.VirtualMachineCloneSpec(
+              location: relocate_spec,
+              config: config_spec,
+              powerOn: true,
+              template: false
+            )
+
+            # Create the new VM
+            new_vm_object = template_vm_object.CloneVM_Task(
+              folder: find_folder(target_folder_path, connection),
               name: new_vmname,
-              created_by: provider_config['username'],
+              spec: clone_spec
-              base_template: template_path,
+            ).wait_for_completion
-              creation_timestamp: Time.now.utc
-            ),
-            extraConfig: [
-              { key: 'guestinfo.hostname', value: new_vmname }
-            ]
-          )
 
-          # Choose a cluster/host to place the new VM on
+            vm_hash = generate_vm_hash(new_vm_object, template_path, pool_name)
-          target_host_object = find_least_used_host(target_cluster_name, connection)
+          end
-
+          vm_hash
-          # Put the VM in the specified folder and resource pool
-          relocate_spec = RbVmomi::VIM.VirtualMachineRelocateSpec(
-            datastore: find_datastore(target_datastore, connection),
-            host: target_host_object,
-            diskMoveType: :moveChildMostDiskBacking
-          )
-
-          # Create a clone spec
-          clone_spec = RbVmomi::VIM.VirtualMachineCloneSpec(
-            location: relocate_spec,
-            config: config_spec,
-            powerOn: true,
-            template: false
-          )
-
-          # Create the new VM
-          new_vm_object = template_vm_object.CloneVM_Task(
-            folder: find_folder(target_folder_path, connection),
-            name: new_vmname,
-            spec: clone_spec
-          ).wait_for_completion
-
-          generate_vm_hash(new_vm_object, template_path, pool_name)
         end
 
         def create_disk(pool_name, vm_name, disk_size)
@@ -164,62 +193,58 @@ module Vmpooler
           datastore_name = pool['datastore']
           raise("Pool #{pool_name} does not have a datastore defined for the provider #{name}") if datastore_name.nil?
 
-          connection = get_connection
+          @connection_pool.with_metrics do |connection|
-
+            vm_object = find_vm(vm_name, connection)
-          vm_object = find_vm(vm_name, connection)
+            raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil?
-          raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil?
-
-          add_disk(vm_object, disk_size, datastore_name, connection)
 
+            add_disk(vm_object, disk_size, datastore_name, connection)
+          end
           true
         end
 
         def create_snapshot(pool_name, vm_name, new_snapshot_name)
-          connection = get_connection
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
+            raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil?
 
-          vm_object = find_vm(vm_name, connection)
+            old_snap = find_snapshot(vm_object, new_snapshot_name)
-          raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil?
+            raise("Snapshot #{new_snapshot_name} for VM #{vm_name} in pool #{pool_name} already exists for the provider #{name}") unless old_snap.nil?
-
-          old_snap = find_snapshot(vm_object, new_snapshot_name)
-          raise("Snapshot #{new_snapshot_name} for VM #{vm_name} in pool #{pool_name} already exists for the provider #{name}") unless old_snap.nil?
-
-          vm_object.CreateSnapshot_Task(
-            name: new_snapshot_name,
-            description: 'vmpooler',
-            memory: true,
-            quiesce: true
-          ).wait_for_completion
 
+            vm_object.CreateSnapshot_Task(
+              name: new_snapshot_name,
+              description: 'vmpooler',
+              memory: true,
+              quiesce: true
+            ).wait_for_completion
+          end
           true
         end
 
         def revert_snapshot(pool_name, vm_name, snapshot_name)
-          connection = get_connection
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
+            raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil?
 
-          vm_object = find_vm(vm_name, connection)
+            snapshot_object = find_snapshot(vm_object, snapshot_name)
-          raise("VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if vm_object.nil?
+            raise("Snapshot #{snapshot_name} for VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if snapshot_object.nil?
-
-          snapshot_object = find_snapshot(vm_object, snapshot_name)
-          raise("Snapshot #{snapshot_name} for VM #{vm_name} in pool #{pool_name} does not exist for the provider #{name}") if snapshot_object.nil?
-
-          snapshot_object.RevertToSnapshot_Task.wait_for_completion
 
+            snapshot_object.RevertToSnapshot_Task.wait_for_completion
+          end
           true
         end
 
         def destroy_vm(_pool_name, vm_name)
-          connection = get_connection
+          @connection_pool.with_metrics do |connection|
+            vm_object = find_vm(vm_name, connection)
+            # If a VM doesn't exist then it is effectively deleted
+            return true if vm_object.nil?
 
-          vm_object = find_vm(vm_name, connection)
+            # Poweroff the VM if it's running
-          # If a VM doesn't exist then it is effectively deleted
+            vm_object.PowerOffVM_Task.wait_for_completion if vm_object.runtime && vm_object.runtime.powerState && vm_object.runtime.powerState == 'poweredOn'
-          return true if vm_object.nil?
-
-          # Poweroff the VM if it's running
-          vm_object.PowerOffVM_Task.wait_for_completion if vm_object.runtime && vm_object.runtime.powerState && vm_object.runtime.powerState == 'poweredOn'
-
-          # Kill it with fire
-          vm_object.Destroy_Task.wait_for_completion
 
+            # Kill it with fire
+            vm_object.Destroy_Task.wait_for_completion
+          end
           true
         end
 
@@ -263,16 +288,6 @@ module Vmpooler
         DISK_TYPE = 'thin'.freeze
         DISK_MODE = 'persistent'.freeze
 
-        def get_connection
-          begin
-            @connection.serviceInstance.CurrentTime
-          rescue
-            @connection = connect_to_vsphere
-          end
-
-          @connection
-        end
-
         def connect_to_vsphere
           max_tries = global_config[:config]['max_tries'] || 3
           retry_factor = global_config[:config]['retry_factor'] || 10
@@ -667,10 +682,6 @@ module Vmpooler
           relospec = RbVmomi::VIM.VirtualMachineRelocateSpec(host: host)
           vm.RelocateVM_Task(spec: relospec).wait_for_completion
         end
-
-        def close
-          @connection.close
-        end
       end
     end
   end

spec/unit/providers/vsphere_spec.rb

@@ -53,6 +53,8 @@ describe 'Vmpooler::PoolManager::Provider::VSphere' do
     username: "vcenter_user"
     password: "vcenter_password"
     insecure: true
+    # Drop the connection pool timeout way down for spec tests so they fail fast
+    connection_pool_timeout: 1
 :pools:
   - name: '#{poolname}'
     alias: [ 'mockpool' ]
@@ -84,7 +86,7 @@ EOT
     let(:pool_config) { config[:pools][0] }
 
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
     end
 
     context 'Given a pool folder that is missing' do
@@ -93,7 +95,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.vms_in_pool(poolname)
       end
@@ -111,7 +113,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.vms_in_pool(poolname)
       end
@@ -140,7 +142,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.vms_in_pool(poolname)
       end
@@ -155,7 +157,7 @@ EOT
 
   describe '#get_vm_host' do
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       expect(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object)
     end
 
@@ -163,7 +165,7 @@ EOT
       let(:vm_object) { nil }
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.get_vm_host(poolname,vmname)
       end
@@ -185,7 +187,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.get_vm_host(poolname,vmname)
       end
@@ -208,7 +210,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.get_vm_host(poolname,vmname)
       end
@@ -223,7 +225,7 @@ EOT
     let(:vm_object) { nil }
 
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       expect(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object)
     end
 
@@ -231,7 +233,7 @@ EOT
       let(:vm_object) { nil }
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.find_least_used_compatible_host(poolname,vmname)
       end
@@ -250,7 +252,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.find_least_used_compatible_host(poolname,vmname)
       end
@@ -272,7 +274,7 @@ EOT
       end
 
       it 'should get a connection' do
-        expect(subject).to receive(:get_connection).and_return(connection)
+        expect(subject).to receive(:connect_to_vsphere).and_return(connection)
 
         subject.find_least_used_compatible_host(poolname,vmname)
       end
@@ -293,7 +295,7 @@ EOT
 
     before(:each) do
       config[:pools][0]['clone_target'] = cluster_name
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       allow(subject).to receive(:find_vm).and_return(vm_object)
     end
 
@@ -389,7 +391,7 @@ EOT
   describe '#get_vm' do
     let(:vm_object) { nil }
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       expect(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object)
     end
 
@@ -510,7 +512,7 @@ EOT
     let(:new_vm_object)  { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname }) }
 
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       allow(connection.serviceInstance).to receive(:find_datacenter).and_return(datacenter_object)
     end
 
@@ -585,7 +587,7 @@ EOT
     let(:datastorename) { 'datastore0' }
     let(:disk_size) { 10 }
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       allow(subject).to receive(:find_vm).with(vmname, connection).and_return(vm_object)
     end
 
@@ -643,7 +645,7 @@ EOT
     let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname, :snapshot_tree => snapshot_tree }) }
 
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       allow(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object)
     end
 
@@ -698,7 +700,7 @@ EOT
     let(:vm_object) { mock_RbVmomi_VIM_VirtualMachine({ :name => vmname, :snapshot_tree => snapshot_tree }) }
 
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
       allow(subject).to receive(:find_vm).with(vmname,connection).and_return(vm_object)
     end
 
@@ -747,7 +749,7 @@ EOT
     let(:destroy_task) { mock_RbVmomi_VIM_Task() }
 
     before(:each) do
-      allow(subject).to receive(:get_connection).and_return(connection)
+      allow(subject).to receive(:connect_to_vsphere).and_return(connection)
     end
 
     context 'Given a missing VM name' do
@@ -876,57 +878,6 @@ EOT
   end
 
   # vSphere helper methods
-  describe '#get_connection' do
-    before(:each) do
-      # NOTE - Using instance_variable_set is a code smell of code that is not testable
-      subject.instance_variable_set("@connection",connection)
-    end
-
-    context 'when connection is ok' do
-      it 'should not attempt to reconnect' do
-        expect(subject).to receive(:connect_to_vsphere).exactly(0).times
-
-        subject.get_connection()
-      end
-
-      it 'should return a connection' do
-        result = subject.get_connection()
-
-        expect(result).to be(connection)
-      end
-    end
-
-    context 'when connection has broken' do
-      before(:each) do
-        expect(connection.serviceInstance).to receive(:CurrentTime).and_raise(RuntimeError,'MockConnectionError')
-      end
-
-      it 'should not increment the connect.open metric' do
-        # https://github.com/puppetlabs/vmpooler/issues/195
-        expect(metrics).to receive(:increment).with('connect.open').exactly(0).times
-        allow(subject).to receive(:connect_to_vsphere)
-
-        subject.get_connection()
-      end
-
-      it 'should call connect_to_vsphere to reconnect' do
-        allow(metrics).to receive(:increment)
-        expect(subject).to receive(:connect_to_vsphere).with(no_args)
-
-        subject.get_connection()
-      end
-
-      it 'should return a new connection' do
-        new_connection = mock_RbVmomi_VIM_Connection(connection_options)
-        expect(subject).to receive(:connect_to_vsphere).with(no_args).and_return(new_connection)
-
-        result = subject.get_connection()
-
-        expect(result).to be(new_connection)
-      end
-    end
-  end
-
   describe '#connect_to_vsphere' do
     before(:each) do
       allow(RbVmomi::VIM).to receive(:connect).and_return(connection)
@@ -2828,30 +2779,4 @@ EOT
       expect(subject.migrate_vm_host(vm_object,host_object)).to eq('RELOCATE_RESULT')
     end
   end
-
-  describe '#close' do
-    context 'no connection has been made' do
-      before(:each) do
-        # NOTE - Using instance_variable_set is a code smell of code that is not testable
-        subject.instance_variable_set("@connection",nil)
-      end
-
-      it 'should not error' do
-        pending('https://github.com/puppetlabs/vmpooler/issues/211')
-        subject.close
-      end
-    end
-
-    context 'on an open connection' do
-      before(:each) do
-        # NOTE - Using instance_variable_set is a code smell of code that is not testable
-        subject.instance_variable_set("@connection",connection)
-      end
-
-      it 'should close the underlying connection object' do
-        expect(connection).to receive(:close)
-        subject.close
-      end
-    end
-  end
 end