Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to work with Instances over SSH tunnel. #1091

Merged
merged 4 commits into from Aug 9, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 68 additions & 5 deletions lib/kitchen/transport/ssh.rb
Expand Up @@ -19,6 +19,7 @@
require "kitchen"

require "net/ssh"
require "net/ssh/gateway"
require "net/scp"
require "timeout"
require "benchmark"
Expand Down Expand Up @@ -53,6 +54,9 @@ class Ssh < Kitchen::Transport::Base
default_config :connection_retry_sleep, 1
default_config :max_wait_until_ready, 600

default_config :ssh_gateway, nil
default_config :ssh_gateway_username, nil

default_config :ssh_key, nil
expand_path_for :ssh_key

Expand Down Expand Up @@ -142,6 +146,11 @@ def login_command
if options.key?(:forward_agent)
args += %W[ -o ForwardAgent=#{options[:forward_agent] ? "yes" : "no"} ]
end
if ssh_gateway
gateway_command = "ssh -q #{ssh_gateway_username}@#{ssh_gateway} nc #{hostname} #{port}"
# Should support other ports than 22 for ssh gateways
args += %W[ -o ProxyCommand=#{gateway_command} -p 22 ]
end
Array(options[:keys]).each { |ssh_key| args += %W[ -i #{ssh_key} ] }
args += %W[ -p #{port} ]
args += %W[ #{username}@#{hostname} ]
Expand Down Expand Up @@ -226,6 +235,49 @@ def wait_until_ready
# @api private
attr_reader :port

# @return [String] The ssh gateway to use when connecting to the
# remote SSH host
# @api private
attr_reader :ssh_gateway

# @return [String] The username to use when using an ssh gateway
# @api private
attr_reader :ssh_gateway_username

# Establish an SSH session on the remote host using a gateway host.
#
# @param opts [Hash] retry options
# @option opts [Integer] :retries the number of times to retry before
# failing
# @option opts [Float] :delay the number of seconds to wait until
# attempting a retry
# @option opts [String] :message an optional message to be logged on
# debug (overriding the default) when a rescuable exception is raised
# @return [Net::SSH::Connection::Session] the SSH connection session
# @api private
def establish_connection_via_gateway(opts)
logger.debug("[SSH] opening connection to #{self} via " \
"#{ssh_gateway_username}@#{ssh_gateway}")
Net::SSH::Gateway.new(ssh_gateway,
ssh_gateway_username, options).ssh(hostname, username, options)
rescue *RESCUE_EXCEPTIONS_ON_ESTABLISH => e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great if we could reuse the retry logic accross the two connection strategies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried. Look at this commit d171410. But rubocop was very unhappy -(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you hat the gateway and non-gateway connection with the retry all in one method there. I think you did the right thing be making separate methods but I'd suggest extracting the retry logic which appears to be identical in both to be something like:

def retry_connection(opts)
  yield
# RESCUE CODE HERE
end

Then

def establish_connection_via_gateway(opts)
  retry_connection(opts) do
    Net::SSH::Gateway.new(ssh_gateway,
        ssh_gateway_username, options).ssh(hostname, username, options)
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint! Moved the code. Although now we have comments describing parameters identical for three methods -(

if (opts[:retries] -= 1) > 0
message = if opts[:message]
logger.debug("[SSH] connection failed (#{e.inspect})")
opts[:message]
else
"[SSH] connection failed, retrying in #{opts[:delay]} seconds " \
"(#{e.inspect})"
end
logger.info(message)
sleep(opts[:delay])
retry
else
logger.warn("[SSH] connection failed, terminating (#{e.inspect})")
raise SshFailed, "SSH session could not be established"
end
end

# Establish an SSH session on the remote host.
#
# @param opts [Hash] retry options
Expand Down Expand Up @@ -298,6 +350,8 @@ def init_options(options)
@connection_retry_sleep = @options.delete(:connection_retry_sleep)
@max_ssh_sessions = @options.delete(:max_ssh_sessions)
@max_wait_until_ready = @options.delete(:max_wait_until_ready)
@ssh_gateway = @options.delete(:ssh_gateway)
@ssh_gateway_username = @options.delete(:ssh_gateway_username)
end

# Returns a connection session, or establishes one when invoked the
Expand All @@ -307,10 +361,17 @@ def init_options(options)
# @return [Net::SSH::Connection::Session] the SSH connection session
# @api private
def session(retry_options = {})
@session ||= establish_connection({
:retries => connection_retries.to_i,
:delay => connection_retry_sleep.to_i
}.merge(retry_options))
if ssh_gateway
@session ||= establish_connection_via_gateway({
:retries => connection_retries.to_i,
:delay => connection_retry_sleep.to_i
}.merge(retry_options))
else
@session ||= establish_connection({
:retries => connection_retries.to_i,
:delay => connection_retry_sleep.to_i
}.merge(retry_options))
end
end

# String representation of object, reporting its connection details and
Expand Down Expand Up @@ -347,7 +408,9 @@ def connection_options(data)
:connection_retries => data[:connection_retries],
:connection_retry_sleep => data[:connection_retry_sleep],
:max_ssh_sessions => data[:max_ssh_sessions],
:max_wait_until_ready => data[:max_wait_until_ready]
:max_wait_until_ready => data[:max_wait_until_ready],
:ssh_gateway => data[:ssh_gateway],
:ssh_gateway_username => data[:ssh_gateway_username]
}

opts[:keys_only] = true if data[:ssh_key]
Expand Down
1 change: 1 addition & 0 deletions test-kitchen.gemspec
Expand Up @@ -26,6 +26,7 @@ Gem::Specification.new do |gem|
gem.add_dependency "mixlib-shellout", ">= 1.2", "< 3.0"
gem.add_dependency "net-scp", "~> 1.1"
gem.add_dependency "net-ssh", ">= 2.9", "< 4.0"
gem.add_dependency "net-ssh-gateway", "~> 1.2.0"
gem.add_dependency "safe_yaml", "~> 1.0"
gem.add_dependency "thor", "~> 0.18"
gem.add_dependency "mixlib-install", "~> 1.0", ">= 1.0.4"
Expand Down