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

Added config.ssh.retries and config.ssh.retry_interval to allow for SSH retries #13047

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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

# Default some options
opts[:retries] = 5 if !opts.key?(:retries)
opts[:retries] = if ssh_info.key?(:retries)
Copy link
Contributor

Choose a reason for hiding this comment

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

the ssh_info should probably already have the default value for the retries (coming from the config). So, there is no need to default this again. Instead, this can be opts[:retries] = ssh_info[:retries] if !opts.key?(:retries) (same for the retry_interval below). This also implies that the internal caller of this method can override the user defined config.

This also means that the ssh_info method in the Machine class needs to be updated to account for this new config.

ssh_info[:retries]
elsif !opts.key?(:retries)
5
end
opts[:retry_interval] = if ssh_info.key?(:retry_interval)
ssh_info[:retry_interval]
else
10
end

# 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 @@ -429,7 +438,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_interval]) do
Timeout.timeout(timeout) do
begin
# This logger will get the Net-SSH log data for us.
Expand Down
28 changes: 28 additions & 0 deletions plugins/kernel_v2/config/ssh_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module VagrantPlugins
module Kernel_V2
class SSHConnectConfig < Vagrant.plugin("2", :config)
DEFAULT_SSH_CONNECT_TIMEOUT = 15
DEFAULT_SSH_RETRIES = 5
DEFAULT_SSH_RETRY_INTERVAL = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

This number can probably come down some. The winrm communicator uses 2.


attr_accessor :host
attr_accessor :port
Expand All @@ -18,6 +20,8 @@ class SSHConnectConfig < Vagrant.plugin("2", :config)
attr_accessor :dsa_authentication
attr_accessor :extra_args
attr_accessor :remote_user
attr_accessor :retries
attr_accessor :retry_interval

def initialize
@host = UNSET_VALUE
Expand All @@ -35,6 +39,8 @@ def initialize
@dsa_authentication = UNSET_VALUE
@extra_args = UNSET_VALUE
@remote_user = UNSET_VALUE
@retries = UNSET_VALUE
@retry_interval = UNSET_VALUE
end

def finalize!
Expand All @@ -52,6 +58,8 @@ def finalize!
@extra_args = nil if @extra_args == UNSET_VALUE
@config = nil if @config == UNSET_VALUE
@connect_timeout = DEFAULT_SSH_CONNECT_TIMEOUT if @connect_timeout == UNSET_VALUE
@retries = DEFAULT_SSH_RETRIES if @retries == UNSET_VALUE
@retry_interval = DEFAULT_SSH_RETRY_INTERVAL if @retry_interval == UNSET_VALUE

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

if !@retries.is_a?(Integer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at the validate method for the winrm communicator. It has some better validations steps. For example, it's ok to not have any retries or to have a retry interval between 0 and 1.

errors << I18n.t(
"vagrant.config.ssh.retries_invalid_type",
given: @retries.class.name)
elsif @retries < 1
errors << I18n.t(
"vagrant.config.ssh.retries_invalid_value",
given: @retries.to_s)
end

if !@retry_interval.is_a?(Integer)
errors << I18n.t(
"vagrant.config.ssh.retry_interval_invalid_type",
given: @retry_interval.class.name)
elsif @retry_interval < 1
errors << I18n.t(
"vagrant.config.ssh.retry_interval_invalid_value",
given: @retry_interval.to_s)
end

errors
end
end
Expand Down
10 changes: 10 additions & 0 deletions templates/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2014,6 +2014,16 @@ en:
`%{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}`)
retries_invalid_type: |-
The `retries` key only accepts values of Integer type. Received
`%{given}` type which cannot be converted to an Integer type.
retries_invalid_value: |-
The `retries` key only accepts values greater than or equal to 1 (received `%{given}`)
retry_interval_invalid_type: |-
The `retry_interval` key only accepts values of Integer type. Received
`%{given}` type which cannot be converted to an Integer type.
retry_interval_invalid_value: |-
The `retry_interval` key only accepts values greater than or equal to 1 (received `%{given}`)
triggers:
bad_trigger_type: |-
The type '%{type}' defined for trigger '%{trigger}' is not valid. Must be one of the following types: '%{types}'
Expand Down
10 changes: 10 additions & 0 deletions website/content/docs/vagrantfile/ssh_settings.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,16 @@ defaults are typically fine, but you can fine tune whatever you would like.
net-ssh library (ignored by the `ssh` executable) and should not be used in general.
This defaults to the value of `config.ssh.username`.

- `config.ssh.retries` (integer) - Number of times to retry establishing an SSH
connection before failing. The default value is 5. This value should be >= 1.

- `config.ssh.retry_interval` (integer) - Number seconds to sleep between SSH connection
retries. Using this setting together with `config.ssh.retries` can be useful when waiting
for hosts to boot where they are pingable first, yet the SSH daemon takes a long time to boot.
It is also useful when running provisioning scripts that reboot hosts, allowing time for the
hosts to fully boot and SSH to be available.
The default value is 10. This value should be >= 1.

- `config.ssh.shell` (string) - The shell to use when executing SSH commands from
Vagrant. By default this is `bash -l`.

Expand Down