From 199bf4a07013bb26033201d61688a1baae620561 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 31 Mar 2017 13:40:47 -0700 Subject: [PATCH] (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 | 29 +++++-------- spec/unit/pool_manager_spec.rb | 74 ++++------------------------------ 2 files changed, 19 insertions(+), 84 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index c8c033b..87c0588 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/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? - ensure - sock.close + _check_pending_vm(vm, pool, timeout, provider) + rescue => err + $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 - move_pending_vm_to_ready(vm, pool, host) - rescue - fail_pending_vm(vm, pool, timeout) + if provider.vm_ready?(pool, vm) + move_pending_vm_to_ready(vm, pool, host) + else + fail_pending_vm(vm, pool, timeout) + end end def remove_nonexistent_vm(vm, pool) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 6a6c226..df17f5c 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/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(subject).to receive(:open_socket).and_return(nil) + expect(provider).to receive(:vm_ready?).with(pool,vm).and_return(true) 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 - expect(provider).to receive(:find_vm).and_return(host) - expect(subject).to receive(:open_socket).and_raise(SocketError,'getaddrinfo: No such host is known') + it 'calls fail_pending_vm if host is not ready' do + expect(provider).to receive(:vm_ready?).with(pool,vm).and_return(false) expect(subject).to receive(:fail_pending_vm).with(vm, pool, timeout) subject._check_pending_vm(vm, pool, timeout, provider)