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

Provide configurable retries for connect on SSH communicator #13628

Merged
merged 1 commit into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/vagrant/machine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,8 @@ def ssh_info
info[:forward_x11] = @config.ssh.forward_x11
info[:forward_env] = @config.ssh.forward_env
info[:connect_timeout] = @config.ssh.connect_timeout
info[:connect_retries] = @config.ssh.connect_retries
info[:connect_retry_delay] = @config.ssh.connect_retry_delay

info[:ssh_command] = @config.ssh.ssh_command if @config.ssh.ssh_command

Expand Down
5 changes: 3 additions & 2 deletions plugins/communicators/ssh/communicator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,8 @@ def connect(**opts)
raise Vagrant::Errors::SSHNotReady if ssh_info.nil?

# Default some options
opts[:retries] = 5 if !opts.key?(:retries)
opts[:retries] = ssh_info[:connect_retries] if !opts.key?(:retries)
opts[:retry_delay] = ssh_info[:connect_retry_delay] if !opts.key?(:retry_delay)

# Set some valid auth methods. We disable the auth methods that
# we're not using if we don't have the right auth info.
Expand Down Expand Up @@ -487,7 +488,7 @@ def connect(**opts)
timeout = 60

@logger.info("Attempting SSH connection...")
connection = retryable(tries: opts[:retries], on: SSH_RETRY_EXCEPTIONS) do
connection = retryable(tries: opts[:retries], on: SSH_RETRY_EXCEPTIONS, sleep: opts[:retry_delay]) do
Timeout.timeout(timeout) do
begin
# This logger will get the Net-SSH log data for us.
Expand Down
32 changes: 32 additions & 0 deletions plugins/kernel_v2/config/ssh_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
module VagrantPlugins
module Kernel_V2
class SSHConnectConfig < Vagrant.plugin("2", :config)
DEFAULT_SSH_CONNECT_RETRIES = 5
DEFAULT_SSH_CONNECT_RETRY_DELAY = 2
DEFAULT_SSH_CONNECT_TIMEOUT = 15

attr_accessor :host
attr_accessor :port
attr_accessor :config
attr_accessor :connect_retries
attr_accessor :connect_retry_delay
attr_accessor :connect_timeout
attr_accessor :private_key_path
attr_accessor :username
Expand All @@ -28,6 +32,8 @@ def initialize
@host = UNSET_VALUE
@port = UNSET_VALUE
@config = UNSET_VALUE
@connect_retries = UNSET_VALUE
@connect_retry_delay = UNSET_VALUE
@connect_timeout = UNSET_VALUE
@private_key_path = UNSET_VALUE
@username = UNSET_VALUE
Expand Down Expand Up @@ -61,6 +67,8 @@ def finalize!
@config = nil if @config == UNSET_VALUE
@disable_deprecated_algorithms = false if @disable_deprecated_algorithms == UNSET_VALUE
@connect_timeout = DEFAULT_SSH_CONNECT_TIMEOUT if @connect_timeout == UNSET_VALUE
@connect_retries = DEFAULT_SSH_CONNECT_RETRIES if @connect_retries == UNSET_VALUE
@connect_retry_delay = DEFAULT_SSH_CONNECT_RETRY_DELAY if @connect_retry_delay == UNSET_VALUE

if @private_key_path && !@private_key_path.is_a?(Array)
@private_key_path = [@private_key_path]
Expand Down Expand Up @@ -147,6 +155,30 @@ def validate(machine)
given: @connect_timeout.to_s)
end

if !@connect_retries.is_a?(Integer)
errors << I18n.t(
"vagrant.config.ssh.connect_retries_invalid_type",
given: @connect_retries.class.name
)
elsif @connect_retries < 0
errors << I18n.t(
"vagrant.config.ssh.connect_retries_invalid_value",
given: @connect_retries.to_s
)
end

if !@connect_retry_delay.is_a?(Numeric)
errors << I18n.t(
"vagrant.config.ssh.connect_retry_delay_invalid_type",
given: @connect_retry_delay.class.name
)
elsif @connect_retry_delay < 0
errors << I18n.t(
"vagrant.config.ssh.connect_retry_delay_invalid_value",
given: @connect_retry_delay.to_s
)
end

if @key_type != :auto && !Vagrant::Util::Keypair.valid_type?(@key_type)
errors << I18n.t(
"vagrant.config.ssh.connect_invalid_key_type",
Expand Down
12 changes: 11 additions & 1 deletion templates/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2070,7 +2070,17 @@ en:
The `connect_timeout` key only accepts values of Integer type. Received
`%{given}` type which cannot be converted to an Integer type.
connect_timeout_invalid_value: |-
The `connect_timeout` key only accepts values greater than 1 (received `%{given}`)
The `connect_timeout` key only accepts values greater than or equal to 1 (received `%{given}`)
connect_retries_invalid_type: |-
The `connect_retries` key only accepts values of Integer type. Received
`%{given}` type which cannot be converted to an Integer type.
connect_retries_invalid_value: |-
The `connect_retries` key only accepts values greater than or equal to 0 (received `%{given}`)
connect_retry_delay_invalid_type: |-
The `connect_retry_delay` key only accepts values of Numeric type. Received
`%{given}` type which cannot be converted to a Numeric type.
connect_retry_delay_invalid_value: |-
The `connect_retry_delay` key only accepts values greater than or equal to 0 (received `%{given}`)
connect_invalid_key_type: |-
Invalid SSH key type set ('%{given}'). Supported types: %{supported}
triggers:
Expand Down
68 changes: 68 additions & 0 deletions test/unit/plugins/communicators/ssh/communicator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,74 @@
communicator.send(:connect)
end
end

context "with connect_retries configured" do
before do
expect(machine).to receive(:ssh_info).and_return(
host: '127.0.0.1',
port: 2222,
connect_retries: 3
)
end

it "should retry the connect three times" do
expect(Net::SSH).to receive(:start).and_raise(Errno::EACCES).twice
expect(Net::SSH).to receive(:start)

communicator.send(:connect)
end

it "should override from passed options" do
expect(Net::SSH).to receive(:start).and_raise(Errno::EACCES).thrice
expect(Net::SSH).to receive(:start)

communicator.send(:connect, retries: 4)
end
end

context "with connect_retry_delay configured" do
let(:retries) { 3 }
let(:sleep_delay) { 5 }

before do
expect(machine).to receive(:ssh_info).and_return(
host: '127.0.0.1',
port: 2222,
connect_retries: retries,
connect_retry_delay: sleep_delay
)
end

it "should sleep twice while retrying" do
expect(Net::SSH).to receive(:start).and_raise(Errno::EACCES).twice
expect(Net::SSH).to receive(:start)

expect(communicator).to receive(:sleep).with(sleep_delay).twice

communicator.send(:connect)
end

it "should overrride from passed options" do
expect(Net::SSH).to receive(:start).and_raise(Errno::EACCES).twice
expect(Net::SSH).to receive(:start)

expect(communicator).to receive(:sleep).with(20).twice

communicator.send(:connect, retry_delay: 20)
end

context "when no retries are configured" do
let(:retries) { 0 }

it "should not sleep" do
expect(Net::SSH).to receive(:start).and_raise(Errno::EACCES)
expect(communicator).not_to receive(:sleep)

expect { communicator.send(:connect) }.to raise_error(Vagrant::Errors::SSHConnectEACCES)
end
end

end
end

describe "#insecure_key?" do
Expand Down
120 changes: 119 additions & 1 deletion test/unit/plugins/kernel_v2/config/ssh_connect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
to eq(described_class.const_get(:DEFAULT_SSH_CONNECT_TIMEOUT))
end


it "should be set to provided value" do
subject.connect_timeout = timeout_value
subject.finalize!
Expand Down Expand Up @@ -192,4 +191,123 @@
end
end
end

describe "#connect_retries" do
let(:retry_value) { 1 }

it "should default to the default value" do
subject.finalize!
expect(subject.connect_retries).
to eq(described_class.const_get(:DEFAULT_SSH_CONNECT_RETRIES))
end

it "should be set to the provided value" do
subject.connect_retries = retry_value
subject.finalize!
expect(subject.connect_retries).to eq(retry_value)
end

it "should properly validate" do
subject.connect_retries = retry_value
subject.finalize!
expect(subject.validate(machine)).to be_empty
end

context "when value is a float" do
let(:retry_value) { 2.3 }

it "should not raise an error" do
subject.connect_retries = retry_value
expect { subject.finalize! }.not_to raise_error
end

it "should not validate" do
subject.connect_retries = retry_value
subject.finalize!
expect(subject.validate(machine)).not_to be_empty
end
end

context "when value is not numeric" do
let(:retry_value) { "2" }

it "should not raise an error" do
subject.connect_retries = retry_value
expect { subject.finalize! }.not_to raise_error
end

it "should not validate" do
subject.connect_retries = retry_value
subject.finalize!
expect(subject.validate(machine)).not_to be_empty
end
end

context "when value is less than 0" do
let(:retry_value) { -1 }

it "should not validate" do
subject.connect_retries = retry_value
subject.finalize!
expect(subject.validate(machine)).not_to be_empty
end
end
end

describe "#connect_retry_delay" do
let(:delay_value) { 1 }

it "should default to the default value" do
subject.finalize!
expect(subject.connect_retry_delay).
to eq(described_class.const_get(:DEFAULT_SSH_CONNECT_RETRY_DELAY))
end

it "should be set to the provided value" do
subject.connect_retry_delay = delay_value
subject.finalize!
expect(subject.connect_retry_delay).to eq(delay_value)
end

it "should properly validate" do
subject.connect_retry_delay = delay_value
subject.finalize!
expect(subject.validate(machine)).to be_empty
end

context "when value is a float" do
let(:delay_value) { 2.3 }

it "should validate" do
subject.connect_retry_delay = delay_value
subject.finalize!
expect(subject.validate(machine)).to be_empty
end
end

context "when value is not numeric" do
let(:delay_value) { "2" }

it "should not raise an error" do
subject.connect_retry_delay = delay_value
expect { subject.finalize! }.not_to raise_error
end

it "should not validate" do
subject.connect_retry_delay = delay_value
subject.finalize!
expect(subject.validate(machine)).not_to be_empty
end
end

context "when value is less than 0" do
let(:delay_value) { -1 }

it "should not validate" do
subject.connect_retry_delay = delay_value
subject.finalize!
expect(subject.validate(machine)).not_to be_empty
end
end
end
end
6 changes: 6 additions & 0 deletions website/content/docs/vagrantfile/ssh_settings.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ defaults are typically fine, but you can fine tune whatever you would like.
compression setting when ssh'ing into a machine. If this is not set, it will
default to `true` and `Compression=yes` will be enabled with ssh.

- `config.ssh.connect_retries` (integer) - Number of times to attempt to establish an
an SSH connection to the guest. Defaults to `5`.

- `config.ssh.connect_retry_delay` (numeric) - Number of seconds to wait between
retries when attempting to establish an SSH connection to the guest. Defaults to `2`.

- `config.ssh.connect_timeout` (integer) - Number of seconds to wait for establishing
an SSH connection to the guest. Defaults to `15`.

Expand Down
2 changes: 1 addition & 1 deletion website/data/intro-nav-data.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
{
"title": "Getting Started",
"href": "https://developer.hashicorp.com/vagrant/tutorials/get-started",
"href": "https://developer.hashicorp.com/vagrant/tutorials/get-started"
},
{
"title": "Contributing",
Expand Down