diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f019a1f8..100bc760d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## 0.6.0 (unreleased) + - SSH connection is retried 5 times if there is a connection refused. + Related to GH-140. - If `http_proxy` environmental variable is set, it will be used as the proxy box adding via http. - Remove `config.ssh.password`. It hasn't been used for a few versions diff --git a/lib/vagrant/ssh.rb b/lib/vagrant/ssh.rb index c54466237..e83497aba 100644 --- a/lib/vagrant/ssh.rb +++ b/lib/vagrant/ssh.rb @@ -7,6 +7,8 @@ module Vagrant # replace the process with SSH itself, run a specific set of commands, # upload files, or even check if a host is up. class SSH + include Util::Retryable + # Reference back up to the environment which this SSH object belongs # to attr_accessor :env @@ -57,14 +59,16 @@ module Vagrant opts = opts.dup opts[:forward_agent] = true if env.config.ssh.forward_agent - Net::SSH.start(env.config.ssh.host, - env.config.ssh.username, - opts.merge( :port => port, - :keys => [env.config.ssh.private_key_path], - :user_known_hosts_file => [], - :paranoid => false, - :config => false)) do |ssh| - yield SSH::Session.new(ssh) + retryable(:tries => 5, :on => Errno::ECONNREFUSED) do + Net::SSH.start(env.config.ssh.host, + env.config.ssh.username, + opts.merge( :port => port, + :keys => [env.config.ssh.private_key_path], + :user_known_hosts_file => [], + :paranoid => false, + :config => false)) do |ssh| + yield SSH::Session.new(ssh) + end end end @@ -72,16 +76,11 @@ module Vagrant # or StringIO, and `to` is expected to be a path. This method simply forwards # the arguments to `Net::SCP#upload!` so view that for more information. def upload!(from, to) - tries = 5 - - begin + retryable(:tries => 5, :on => IOError) do execute do |ssh| scp = Net::SCP.new(ssh.session) scp.upload!(from, to) end - rescue IOError - retry if (tries -= 1) > 0 - raise end end diff --git a/lib/vagrant/util.rb b/lib/vagrant/util.rb index 83bb78662..bcfb27841 100644 --- a/lib/vagrant/util.rb +++ b/lib/vagrant/util.rb @@ -6,6 +6,7 @@ module Vagrant autoload :PlainLogger, 'vagrant/util/plain_logger' autoload :Platform, 'vagrant/util/platform' autoload :ResourceLogger, 'vagrant/util/resource_logger' + autoload :Retryable, 'vagrant/util/retryable' autoload :StackedProcRunner, 'vagrant/util/stacked_proc_runner' autoload :TemplateRenderer, 'vagrant/util/template_renderer' end diff --git a/lib/vagrant/util/retryable.rb b/lib/vagrant/util/retryable.rb new file mode 100644 index 000000000..2247cfe42 --- /dev/null +++ b/lib/vagrant/util/retryable.rb @@ -0,0 +1,22 @@ +module Vagrant + module Util + module Retryable + # Retries a given block a specified number of times in the + # event the specified exception is raised. If the retries + # run out, the final exception is raised. + # + # This code is adapted slightly from the following blog post: + # http://blog.codefront.net/2008/01/14/retrying-code-blocks-in-ruby-on-exceptions-whatever/ + def retryable(opts=nil) + opts = { :tries => 1, :on => Exception }.merge(opts || {}) + + begin + return yield + rescue opts[:on] + retry if (opts[:tries] -= 1) > 0 + raise + end + end + end + end +end diff --git a/test/vagrant/ssh_test.rb b/test/vagrant/ssh_test.rb index e4106e122..55990dc0c 100644 --- a/test/vagrant/ssh_test.rb +++ b/test/vagrant/ssh_test.rb @@ -170,13 +170,6 @@ class SshTest < Test::Unit::TestCase @ssh.expects(:execute).yields(ssh).once @ssh.upload!("foo", "bar") end - - should "retry 5 times" do - @ssh.expects(:execute).times(5).raises(IOError) - assert_raises(IOError) { - @ssh.upload!("foo", "bar") - } - end end context "checking if host is up" do @@ -204,7 +197,7 @@ class SshTest < Test::Unit::TestCase end should "return false if the connection is refused" do - Net::SSH.expects(:start).raises(Errno::ECONNREFUSED) + Net::SSH.expects(:start).times(5).raises(Errno::ECONNREFUSED) assert_nothing_raised { assert !@ssh.up? } diff --git a/test/vagrant/util/retryable_test.rb b/test/vagrant/util/retryable_test.rb new file mode 100644 index 000000000..e8238b73e --- /dev/null +++ b/test/vagrant/util/retryable_test.rb @@ -0,0 +1,38 @@ +require "test_helper" + +class RetryableUtilTest < Test::Unit::TestCase + setup do + @klass = Class.new do + extend Vagrant::Util::Retryable + end + end + + should "retry specified number of times if exception is raised" do + proc = mock("proc") + proc.expects(:call).twice + + assert_raises(RuntimeError) { + @klass.retryable(:tries => 2, :on => RuntimeError) do + proc.call + raise "An error" + end + } + end + + should "only retry on specified exception" do + proc = mock("proc") + proc.expects(:call).once + + assert_raises(StandardError) { + @klass.retryable(:tries => 5, :on => RuntimeError) do + proc.call + raise StandardError.new + end + } + end + + should "return the value of the block" do + result = @klass.retryable { 7 } + assert_equal 7, result + end +end