From 4f79fe59c667eddf549497bd628a878c7fc29f5f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 20 Mar 2025 17:27:29 -0700 Subject: [PATCH] Provide configurable retries for connect on SSH communicator Adds two new options to the SSH connect configuration: `connect_retries` and `connect_retry_delay`. Provides user configurable values used when establishing the SSH connection. Previous behavior would retry generally by the default value without a pause between attempts. Updated behavior will retry the number of times set within the config (unless value is provided directly to connect call) and each retry will pause based on the delay value set in the config (unless value is provided directly to the connect call). --- lib/vagrant/machine.rb | 2 + plugins/communicators/ssh/communicator.rb | 5 +- plugins/kernel_v2/config/ssh_connect.rb | 32 +++++ templates/locales/en.yml | 12 +- .../communicators/ssh/communicator_test.rb | 68 ++++++++++ .../kernel_v2/config/ssh_connect_test.rb | 120 +++++++++++++++++- .../content/docs/vagrantfile/ssh_settings.mdx | 6 + website/data/intro-nav-data.json | 2 +- 8 files changed, 242 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index bd0b197bc..3e839c5f2 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -487,6 +487,8 @@ module Vagrant 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 diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index d36d9ed38..64f00bfb8 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -458,7 +458,8 @@ module VagrantPlugins 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. @@ -487,7 +488,7 @@ module VagrantPlugins 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. diff --git a/plugins/kernel_v2/config/ssh_connect.rb b/plugins/kernel_v2/config/ssh_connect.rb index a305025d1..6bd8bfe5d 100644 --- a/plugins/kernel_v2/config/ssh_connect.rb +++ b/plugins/kernel_v2/config/ssh_connect.rb @@ -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 @@ -28,6 +32,8 @@ module VagrantPlugins @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 @@ -61,6 +67,8 @@ module VagrantPlugins @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] @@ -147,6 +155,30 @@ module VagrantPlugins 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", diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a84181c8d..63741e9fc 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -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: diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index bc5cf979b..a27625fca 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -1028,6 +1028,74 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do 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 diff --git a/test/unit/plugins/kernel_v2/config/ssh_connect_test.rb b/test/unit/plugins/kernel_v2/config/ssh_connect_test.rb index 9969a2918..0a1e04623 100644 --- a/test/unit/plugins/kernel_v2/config/ssh_connect_test.rb +++ b/test/unit/plugins/kernel_v2/config/ssh_connect_test.rb @@ -143,7 +143,6 @@ describe VagrantPlugins::Kernel_V2::SSHConnectConfig do 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! @@ -192,4 +191,123 @@ describe VagrantPlugins::Kernel_V2::SSHConnectConfig do 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 diff --git a/website/content/docs/vagrantfile/ssh_settings.mdx b/website/content/docs/vagrantfile/ssh_settings.mdx index 605650923..67951dade 100644 --- a/website/content/docs/vagrantfile/ssh_settings.mdx +++ b/website/content/docs/vagrantfile/ssh_settings.mdx @@ -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`. diff --git a/website/data/intro-nav-data.json b/website/data/intro-nav-data.json index 2be5f7daf..730f944c8 100644 --- a/website/data/intro-nav-data.json +++ b/website/data/intro-nav-data.json @@ -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",