From f580f9915e13cd7fe0b0115a2f75876f1200d03f Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 21 Dec 2015 11:35:33 -0800 Subject: [PATCH 1/2] (MAINT) Make Port Check Configurable This change makes the port and port timeout configurable via the YAML config file. Before these were hard coded to 22 and 5, respectively. Tests have been added to verify the default is chosen when not configured and the configuration is picked up when supplied. --- lib/vmpooler/pool_manager.rb | 11 +++++++++-- spec/vmpooler/pool_manager_spec.rb | 31 ++++++++++++++++++++++++++++-- vmpooler.yaml.example | 12 ++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index eb9504f..c7960e0 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,5 +1,12 @@ module Vmpooler class PoolManager + + # Defaults + # port number to verify pending VM is ready + DEFAULT_PORT_NO = 22 + # timeout, in seconds, when connecting to DEFAULT_PORT_NO + DEFAULT_PORT_TIMEOUT = 5 + def initialize(config, logger, redis, graphite=nil) $config = config @@ -32,8 +39,8 @@ module Vmpooler if host begin - Timeout.timeout(5) do - TCPSocket.new vm, 22 + Timeout.timeout($config[:config]['port_timeout'] || DEFAULT_PORT_TIMEOUT) do + TCPSocket.new vm, ($config[:config]['port'] || DEFAULT_PORT_NO) end move_pending_vm_to_ready(vm, pool, host) rescue diff --git a/spec/vmpooler/pool_manager_spec.rb b/spec/vmpooler/pool_manager_spec.rb index a402bf4..0f40a54 100644 --- a/spec/vmpooler/pool_manager_spec.rb +++ b/spec/vmpooler/pool_manager_spec.rb @@ -33,16 +33,43 @@ describe 'Pool Manager' do end - context 'host is in pool' do + context 'host is in pool (defaults)' do let(:vm_finder) { double('vm_finder') } let(:tcpsocket) { double('TCPSocket') } + let(:config) { {config: {}} } it 'calls move_pending_vm_to_ready' do stub_const("TCPSocket", tcpsocket) allow(pool_helper).to receive(:find_vm).and_return(vm_finder) allow(vm_finder).to receive(:summary).and_return(nil) - allow(tcpsocket).to receive(:new).and_return(true) + # ensure the default port was used + allow(tcpsocket).to receive(:new).with(String, 22).and_return(true) + + expect(vm_finder).to receive(:summary).once + expect(redis).not_to receive(:hget).with(String, 'clone') + + subject._check_pending_vm(vm, pool, timeout) + end + end + + context 'host is in pool (port config)' do + let(:vm_finder) { double('vm_finder') } + let(:tcpsocket) { double('TCPSocket') } + # port and port timeout are now configurable. + # update the config mock + let(:port_no) { 99 } + let(:config) { {config: { + 'port' => port_no, + 'port_timeout' => 2}} } + + it 'calls move_pending_vm_to_ready' do + stub_const("TCPSocket", tcpsocket) + + allow(pool_helper).to receive(:find_vm).and_return(vm_finder) + allow(vm_finder).to receive(:summary).and_return(nil) + # ensure the port config made it to the TCPSocket call: + allow(tcpsocket).to receive(:new).with(String, port_no).and_return(true) expect(vm_finder).to receive(:summary).once expect(redis).not_to receive(:hget).with(String, 'clone') diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index 6e1c5d4..12aaa7d 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -161,6 +161,16 @@ # # - domain # If set, returns a top-level 'domain' JSON key in POST requests +# +# - port +# The port number used to verify a host is ready and available. +# (optional; default: 22) +# +# - port_timeout +# How long (in seconds) to wait for connection to port before canceling. +# This is not used in marking a VM dead or stale. +# (optional; default: 5) +# # Example: @@ -176,6 +186,8 @@ - 'created_by' - 'project' domain: 'company.com' + port: 22 + port_timeout: 5 # :pools: # From 38f962f51479297e9a10cb9b96f5714ecedc5976 Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 21 Dec 2015 11:54:22 -0800 Subject: [PATCH 2/2] (MAINT) Rename New Port Config Rename port to check_pending_port and port_timeout to check_pending_timeout. The default settings have also been moved to a location where other defaults are being set. --- lib/vmpooler.rb | 3 +++ lib/vmpooler/pool_manager.rb | 10 ++-------- spec/vmpooler/pool_manager_spec.rb | 27 ++++----------------------- vmpooler.yaml.example | 8 ++++---- 4 files changed, 13 insertions(+), 35 deletions(-) diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index a9eb4eb..8f4a946 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -33,6 +33,9 @@ module Vmpooler parsed_config[:config]['vm_checktime'] ||= 15 parsed_config[:config]['vm_lifetime'] ||= 24 + parsed_config[:config]['check_pending_port'] ||= 22 + parsed_config[:config]['check_pending_timeout'] ||= 5 + # Create an index of pool aliases parsed_config[:pools].each do |pool| if pool['alias'] diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index c7960e0..30844dc 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,12 +1,6 @@ module Vmpooler class PoolManager - # Defaults - # port number to verify pending VM is ready - DEFAULT_PORT_NO = 22 - # timeout, in seconds, when connecting to DEFAULT_PORT_NO - DEFAULT_PORT_TIMEOUT = 5 - def initialize(config, logger, redis, graphite=nil) $config = config @@ -39,8 +33,8 @@ module Vmpooler if host begin - Timeout.timeout($config[:config]['port_timeout'] || DEFAULT_PORT_TIMEOUT) do - TCPSocket.new vm, ($config[:config]['port'] || DEFAULT_PORT_NO) + Timeout.timeout($config[:config]['check_pending_timeout']) do + TCPSocket.new vm, $config[:config]['check_pending_port'] end move_pending_vm_to_ready(vm, pool, host) rescue diff --git a/spec/vmpooler/pool_manager_spec.rb b/spec/vmpooler/pool_manager_spec.rb index 0f40a54..2937b62 100644 --- a/spec/vmpooler/pool_manager_spec.rb +++ b/spec/vmpooler/pool_manager_spec.rb @@ -33,35 +33,15 @@ describe 'Pool Manager' do end - context 'host is in pool (defaults)' do - let(:vm_finder) { double('vm_finder') } - let(:tcpsocket) { double('TCPSocket') } - let(:config) { {config: {}} } - - it 'calls move_pending_vm_to_ready' do - stub_const("TCPSocket", tcpsocket) - - allow(pool_helper).to receive(:find_vm).and_return(vm_finder) - allow(vm_finder).to receive(:summary).and_return(nil) - # ensure the default port was used - allow(tcpsocket).to receive(:new).with(String, 22).and_return(true) - - expect(vm_finder).to receive(:summary).once - expect(redis).not_to receive(:hget).with(String, 'clone') - - subject._check_pending_vm(vm, pool, timeout) - end - end - - context 'host is in pool (port config)' do + context 'host is in pool' do let(:vm_finder) { double('vm_finder') } let(:tcpsocket) { double('TCPSocket') } # port and port timeout are now configurable. # update the config mock let(:port_no) { 99 } let(:config) { {config: { - 'port' => port_no, - 'port_timeout' => 2}} } + 'check_pending_port' => port_no, + 'check_pending_timeout' => 2}} } it 'calls move_pending_vm_to_ready' do stub_const("TCPSocket", tcpsocket) @@ -77,6 +57,7 @@ describe 'Pool Manager' do subject._check_pending_vm(vm, pool, timeout) end end + end describe '#move_vm_to_ready' do diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index 12aaa7d..6c5d95c 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -162,11 +162,11 @@ # - domain # If set, returns a top-level 'domain' JSON key in POST requests # -# - port +# - check_pending_port # The port number used to verify a host is ready and available. # (optional; default: 22) # -# - port_timeout +# - check_pending_timeout # How long (in seconds) to wait for connection to port before canceling. # This is not used in marking a VM dead or stale. # (optional; default: 5) @@ -186,8 +186,8 @@ - 'created_by' - 'project' domain: 'company.com' - port: 22 - port_timeout: 5 + check_pending_port: 22 + check_pending_timeout: 5 # :pools: #