(POOLER-70) Update check_pending_vm for VM Provider

Previously the Pool Manager would use vSphere objects directly.  This commit
- Modifies the pool_manager to use the VM provider methods instead
- Removes the open_socket method and tests as it is only required in the vSphere
  VM provider

lib/vmpooler/pool_manager.rb

@@ -22,34 +22,27 @@ module Vmpooler
     # Check the state of a VM
     def check_pending_vm(vm, pool, timeout, provider)
       Thread.new do
-        _check_pending_vm(vm, pool, timeout, provider)
-      end
-    end
-
-    def open_socket(host, domain=nil, timeout=5, port=22, &block)
-      Timeout.timeout(timeout) do
-        target_host = host
-        target_host = "#{host}.#{domain}" if domain
-        sock = TCPSocket.new target_host, port
         begin
-          yield sock if block_given?
+          _check_pending_vm(vm, pool, timeout, provider)
-        ensure
+        rescue => err
-          sock.close
+          $logger.log('s', "[!] [#{pool}] '#{vm}' errored while checking a pending vm : #{err}")
+          fail_pending_vm(vm, pool, timeout)
+          raise
         end
       end
     end
 
     def _check_pending_vm(vm, pool, timeout, provider)
-      host = provider.find_vm(vm)
+      host = provider.get_vm(pool, vm)
-
       if ! host
         fail_pending_vm(vm, pool, timeout, false)
         return
       end
-      open_socket vm
+      if provider.vm_ready?(pool, vm)
-      move_pending_vm_to_ready(vm, pool, host)
+        move_pending_vm_to_ready(vm, pool, host)
-    rescue
+      else
-      fail_pending_vm(vm, pool, timeout)
+        fail_pending_vm(vm, pool, timeout)
+      end
     end
 
     def remove_nonexistent_vm(vm, pool)

spec/unit/pool_manager_spec.rb

@@ -37,8 +37,6 @@ EOT
   subject { Vmpooler::PoolManager.new(config, logger, redis, metrics) }
 
   describe '#check_pending_vm' do
-    let(:provider) { double('provider') }
-
     before do
       expect(subject).not_to be_nil
     end
@@ -51,72 +49,14 @@ EOT
     end
   end
 
-  describe '#open_socket' do
-    let(:TCPSocket) { double('tcpsocket') }
-    let(:socket) { double('tcpsocket') }
-    let(:hostname) { 'host' }
-    let(:domain) { 'domain.local'}
-    let(:default_socket) { 22 }
-
-    before do
-      expect(subject).not_to be_nil
-      allow(socket).to receive(:close)
-    end
-
-    it 'opens socket with defaults' do
-      expect(TCPSocket).to receive(:new).with(hostname,default_socket).and_return(socket)
-
-      expect(subject.open_socket(hostname)).to eq(nil)
-    end
-
-    it 'yields the socket if a block is given' do
-      expect(TCPSocket).to receive(:new).with(hostname,default_socket).and_return(socket)
-
-      expect{ |socket| subject.open_socket(hostname,nil,nil,default_socket,&socket) }.to yield_control.exactly(1).times 
-    end
-
-    it 'closes the opened socket' do
-      expect(TCPSocket).to receive(:new).with(hostname,default_socket).and_return(socket)
-      expect(socket).to receive(:close)
-
-      expect(subject.open_socket(hostname)).to eq(nil)
-    end
-
-    it 'opens a specific socket' do
-      expect(TCPSocket).to receive(:new).with(hostname,80).and_return(socket)
-
-      expect(subject.open_socket(hostname,nil,nil,80)).to eq(nil)
-    end
-
-    it 'uses a specific domain with the hostname' do
-      expect(TCPSocket).to receive(:new).with("#{hostname}.#{domain}",default_socket).and_return(socket)
-
-      expect(subject.open_socket(hostname,domain)).to eq(nil)
-    end
-
-    it 'raises error if host is not resolvable' do
-      expect(TCPSocket).to receive(:new).with(hostname,default_socket).and_raise(SocketError,'getaddrinfo: No such host is known')
-
-      expect { subject.open_socket(hostname,nil,1) }.to raise_error(SocketError)
-    end
-
-    it 'raises error if socket is not listening' do
-      expect(TCPSocket).to receive(:new).with(hostname,default_socket).and_raise(SocketError,'No connection could be made because the target machine actively refused it')
-
-      expect { subject.open_socket(hostname,nil,1) }.to raise_error(SocketError)
-    end
-  end
-
   describe '#_check_pending_vm' do
-    let(:provider) { double('provider') }
-
     before do
       expect(subject).not_to be_nil
     end
 
     context 'host does not exist or not in pool' do
       it 'calls fail_pending_vm' do
-        expect(provider).to receive(:find_vm).and_return(nil)
+        expect(provider).to receive(:get_vm).with(pool,vm).and_return(nil)
         expect(subject).to receive(:fail_pending_vm).with(vm, pool, timeout, false) 
 
         subject._check_pending_vm(vm, pool, timeout, provider)
@@ -124,17 +64,19 @@ EOT
     end
 
     context 'host is in pool' do
+      before do
+        expect(provider).to receive(:get_vm).with(pool,vm).and_return(host)
+      end
+
       it 'calls move_pending_vm_to_ready if host is ready' do
-        expect(provider).to receive(:find_vm).and_return(host)
+        expect(provider).to receive(:vm_ready?).with(pool,vm).and_return(true)
-        expect(subject).to receive(:open_socket).and_return(nil)
         expect(subject).to receive(:move_pending_vm_to_ready).with(vm, pool, host)
 
         subject._check_pending_vm(vm, pool, timeout, provider)
       end
 
-      it 'calls fail_pending_vm if an error is raised' do
+      it 'calls fail_pending_vm if host is not ready' do
-        expect(provider).to receive(:find_vm).and_return(host)
+        expect(provider).to receive(:vm_ready?).with(pool,vm).and_return(false)
-        expect(subject).to receive(:open_socket).and_raise(SocketError,'getaddrinfo: No such host is known')
         expect(subject).to receive(:fail_pending_vm).with(vm, pool, timeout)
 
         subject._check_pending_vm(vm, pool, timeout, provider)