Skip to content

Commit

Permalink
Merge pull request #1440 from kevpl/revert_bkr1155
Browse files Browse the repository at this point in the history 
(BKR-1155) Revert "Allow configurable ssh connection preference"
  • Loading branch information
@rishijavia
rishijavia committed Jul 26, 2017
2 parents d455033 + 0e9e499 commit 19ff349
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 76 deletions.
2 changes: 1 addition & 1 deletion lib/beaker/host.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def connection
# create new connection object if necessary
@connection ||= SshConnection.connect( { :ip => self['ip'], :vmhostname => self['vmhostname'], :hostname => @name },
self['user'],
self['ssh'], { :logger => @logger, :ssh_connection_preference => self[:ssh_connection_preference] } )
self['ssh'], { :logger => @logger } )
# update connection information
if self['ip'] && (@connection.ip != self['ip'])
@connection.ip = self['ip']
Expand Down
25 changes: 1 addition & 24 deletions lib/beaker/hypervisor.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def self.create(type, hosts_to_provision, options)
end

hypervisor = hyper_class.new(hosts_to_provision, options)
self.set_ssh_connection_preference(hosts_to_provision, hypervisor)
hypervisor.provision if options[:provision]

hypervisor
end

Expand All @@ -65,29 +65,6 @@ def cleanup
nil
end

DEFAULT_CONNECTION_PREFERENCE = ['ip', 'vmhostname', 'hostname']
#SSH connection method preference. Can be overwritten by hypervisor to change the order
def connection_preference
DEFAULT_CONNECTION_PREFERENCE
end

#Check if overriding method returns correct array with ip, vmhostname hostname as elements
def self.set_ssh_connection_preference(hosts_to_provision, hypervisor)
if hypervisor.connection_preference.sort == DEFAULT_CONNECTION_PREFERENCE.sort
hosts_to_provision.each{ |h| h[:ssh_connection_preference] = hypervisor.connection_preference}
else
raise ArgumentError, <<-HEREDOC
Hypervisor's overriding connection_pereference method is not matching the API.
Make sure your hypervisor's connection_preference returns an array
containing the following elements in any order you prefer:
"ip", "hostname", "vmhostname"
Please check hypervisor.rb file's "self.connection_preference" method for an example
HEREDOC
end
end

#Proxy package managers on tests hosts created by this hypervisor, runs before validation and configuration.
def proxy_package_manager
if @options[:package_proxy]
Expand Down
23 changes: 15 additions & 8 deletions lib/beaker/ssh_connection.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Beaker
class SshConnection

attr_accessor :logger
attr_accessor :ip, :vmhostname, :hostname, :ssh_connection_preference
attr_accessor :ip, :vmhostname, :hostname

RETRYABLE_EXCEPTIONS = [
SocketError,
Expand All @@ -33,7 +33,6 @@ def initialize name_hash, user = nil, ssh_opts = {}, options = {}
@ssh_opts = ssh_opts
@logger = options[:logger]
@options = options
@ssh_connection_preference = @options[:ssh_connection_preference]
end

def self.connect name_hash, user = 'root', ssh_opts = {}, options = {}
Expand Down Expand Up @@ -67,13 +66,21 @@ def connect_block host, user, ssh_opts
# connect to the host
def connect
#try three ways to connect to host (vmhostname, ip, hostname)
# Try each method in turn until we succeed
methods = @ssh_connection_preference.dup
while (not @ssh) && (not methods.empty?) do
@ssh = connect_block(instance_variable_get("@#{methods.shift}"), @user, @ssh_opts)
methods = []
if @vmhostname
@ssh ||= connect_block(@vmhostname, @user, @ssh_opts)
methods << "vmhostname (#{@vmhostname})"
end
unless @ssh
@logger.error "Failed to connect to #{@hostname}, attempted #{@ssh_connection_preference.join(', ')}"
if @ip && !@ssh
@ssh ||= connect_block(@ip, @user, @ssh_opts)
methods << "ip (#{@ip})"
end
if @hostname && !@ssh
@ssh ||= connect_block(@hostname, @user, @ssh_opts)
methods << "hostname (#{@hostname})"
end
if not @ssh
@logger.error "Failed to connect to #{@hostname}, attempted #{methods.join(', ')}"
raise RuntimeError, "Cannot connect to #{@hostname}"
end
@ssh
Expand Down
29 changes: 4 additions & 25 deletions spec/beaker/hypervisor/hypervisor_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,15 @@

module Beaker
describe Hypervisor do
let( :hosts ) { make_hosts( { :platform => 'el-5' } ) }

context "#create" do
let( :hypervisor ) { Beaker::Hypervisor }

it "includes custom hypervisor and call set_ssh_connection_preference" do
allow(hypervisor).to receive(:set_ssh_connection_preference).with([], hypervisor)
expect{ hypervisor.create('custom_hypervisor', [], make_opts() )}.to raise_error(RuntimeError, "Invalid hypervisor: custom_hypervisor")
end

it "sets ssh connection preference if connection_preference method is not overwritten" do
hypervisor.create('none', hosts, make_opts())
expect(hosts[0][:ssh_connection_preference]).to eq(['ip', 'vmhostname', 'hostname'])
end

it "tests ssh connection methods array for valid elements" do
allow(hypervisor).to receive(:connection_preference).and_return(['my', 'invalid', 'method_name'])
expect{ hypervisor.set_ssh_connection_preference(hosts, hypervisor)}.to raise_error(ArgumentError, /overriding/)
end

it "sets to new preference if connection_preference is overridden" do
allow(hypervisor).to receive(:connection_preference).and_return(['vmhostname', 'hostname', 'ip'])
hypervisor.set_ssh_connection_preference(hosts, hypervisor)
expect(hosts[0][:ssh_connection_preference]).to eq(['vmhostname', 'hostname', 'ip'])
end
let( :hypervisor ) { Beaker::Hypervisor }

it "includes custom hypervisor" do
expect{ hypervisor.create('custom_hypervisor', [], make_opts() )}.to raise_error(RuntimeError, "Invalid hypervisor: custom_hypervisor")
end

context "#configure" do
let( :options ) { make_opts.merge({ 'logger' => double().as_null_object }) }
let( :hosts ) { make_hosts( { :platform => 'el-5' } ) }
let( :hypervisor ) { Beaker::Hypervisor.new( hosts, options ) }

context 'if :timesync option set true on host' do
Expand Down
37 changes: 19 additions & 18 deletions spec/beaker/ssh_connection_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Beaker
describe SshConnection do
let( :user ) { 'root' }
let( :ssh_opts ) { { keepalive: true, keepalive_interval: 2 } }
let( :options ) { { :logger => double('logger').as_null_object, :ssh_connection_preference => ["ip", "vmhostname", "hostname"]} }
let( :options ) { { :logger => double('logger').as_null_object } }
let( :ip ) { "default.ip.address" }
let( :vmhostname ){ "vmhostname" }
let( :hostname) { "my_host" }
Expand All @@ -18,24 +18,25 @@ module Beaker

it 'self.connect creates connects and returns a proxy for that connection' do
# grrr
expect( Net::SSH ).to receive(:start).with( ip, user, ssh_opts ).and_return(true)
expect( Net::SSH ).to receive(:start).with( vmhostname, user, ssh_opts ).and_return(true)
connection_constructor = SshConnection.connect name_hash, user, ssh_opts, options
expect( connection_constructor ).to be_a_kind_of SshConnection
end

it 'connect creates a new connection' do
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts).and_return(true)
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts).and_return(true)
connection.connect
end

it 'connect caches its connection' do
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts ).once.and_return true
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts ).once.and_return true
connection.connect
connection.connect
end

it 'attempts to connect by vmhostname address if ip connection fails' do
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts).and_return(false)
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts).and_return(true).once
it 'attempts to connect by ip address if vmhostname connection fails' do
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts).and_return(false)
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts).and_return(true).once
expect( Net::SSH ).to receive( :start ).with( hostname, user, ssh_opts).never
connection.connect
end
Expand All @@ -52,7 +53,7 @@ module Beaker

it 'runs ssh close' do
mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

allow( mock_ssh).to receive( :closed? ).once.and_return(false)
Expand All @@ -62,7 +63,7 @@ module Beaker

it 'sets the @ssh variable to nil' do
mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

allow( mock_ssh).to receive( :closed? ).once.and_return(false)
Expand All @@ -75,7 +76,7 @@ module Beaker
it 'calls ssh shutdown & re-raises if ssh close fails with an unexpected Error' do
mock_ssh = Object.new
allow( mock_ssh ).to receive( :close ) { raise StandardError }
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

allow( mock_ssh).to receive( :closed? ).once.and_return(false)
Expand All @@ -89,7 +90,7 @@ module Beaker
describe '#execute' do
it 'retries if failed with a retryable exception' do
mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

allow( subject ).to receive( :close )
Expand All @@ -101,7 +102,7 @@ module Beaker

it 'raises an error if it fails both times' do
mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

allow( subject ).to receive( :close )
Expand All @@ -114,7 +115,7 @@ module Beaker
describe '#request_terminal_for' do
it 'fails correctly by raising Net::SSH::Exception' do
mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

mock_channel = Object.new
Expand All @@ -127,7 +128,7 @@ module Beaker
describe '#register_stdout_for' do
before :each do
@mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { @mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { @mock_ssh }
connection.connect

@data = '7 of clubs'
Expand Down Expand Up @@ -163,7 +164,7 @@ module Beaker

before :each do
@mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { @mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { @mock_ssh }
connection.connect

@data = '3 of spades'
Expand Down Expand Up @@ -202,7 +203,7 @@ module Beaker

it 'assigns the output\'s exit code correctly from the data' do
mock_ssh = Object.new
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { mock_ssh }
connection.connect

data = '10 of jeromes'
Expand Down Expand Up @@ -235,7 +236,7 @@ module Beaker
@mock_scp = Object.new
allow( @mock_scp ).to receive( :upload! )
allow( @mock_ssh ).to receive( :scp ).and_return( @mock_scp )
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { @mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { @mock_ssh }
connection.connect
end

Expand All @@ -262,7 +263,7 @@ module Beaker
@mock_scp = Object.new
allow( @mock_scp ).to receive( :download! )
allow( @mock_ssh ).to receive( :scp ).and_return( @mock_scp )
expect( Net::SSH ).to receive( :start ).with( ip, user, ssh_opts) { @mock_ssh }
expect( Net::SSH ).to receive( :start ).with( vmhostname, user, ssh_opts) { @mock_ssh }
connection.connect
end

Expand Down

0 comments on commit 19ff349

Please sign in to comment.