diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 6e4a44338..79901d503 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -7,6 +7,7 @@ module VagrantPlugins attr_accessor :port attr_accessor :guest_port attr_accessor :max_tries + attr_accessor :retry_delay attr_accessor :timeout attr_accessor :transport attr_accessor :ssl_peer_verification @@ -18,6 +19,7 @@ module VagrantPlugins @port = UNSET_VALUE @guest_port = UNSET_VALUE @max_tries = UNSET_VALUE + @retry_delay = UNSET_VALUE @timeout = UNSET_VALUE @transport = UNSET_VALUE @ssl_peer_verification = UNSET_VALUE @@ -31,20 +33,22 @@ module VagrantPlugins is_ssl = @transport == :ssl @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE @guest_port = (is_ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE - @max_tries = 20 if @max_tries == UNSET_VALUE - @timeout = 1800 if @timeout == UNSET_VALUE + @max_tries = 20 if @max_tries == UNSET_VALUE + @retry_delay = 2 if @retry_delay == UNSET_VALUE + @timeout = 1800 if @timeout == UNSET_VALUE @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end def validate(machine) errors = [] - errors << "winrm.username cannot be nil." if @username.nil? - errors << "winrm.password cannot be nil." if @password.nil? - errors << "winrm.port cannot be nil." if @port.nil? - errors << "winrm.guest_port cannot be nil." if @guest_port.nil? - errors << "winrm.max_tries cannot be nil." if @max_tries.nil? - errors << "winrm.timeout cannot be nil." if @timeout.nil? + errors << "winrm.username cannot be nil." if @username.nil? + errors << "winrm.password cannot be nil." if @password.nil? + errors << "winrm.port cannot be nil." if @port.nil? + errors << "winrm.guest_port cannot be nil." if @guest_port.nil? + errors << "winrm.max_tries cannot be nil." if @max_tries.nil? + errors << "winrm.retry_delay cannot be nil." if @max_tries.nil? + errors << "winrm.timeout cannot be nil." if @timeout.nil? unless @ssl_peer_verification == true || @ssl_peer_verification == false errors << "winrm.ssl_peer_verification must be a boolean." end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index ae14e0f23..d9d5f28ce 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -88,7 +88,7 @@ module VagrantPlugins end def execute_shell_with_retry(command, shell, &block) - retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: 10) do + retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: @config.retry_delay) do @logger.debug("#{shell} executing:\n#{command}") output = session.send(shell, command) do |out, err| block.call(:stdout, out) if block_given? && out @@ -124,7 +124,6 @@ module VagrantPlugins endpoint: endpoint, message: exception.message when WinRM::WinRMHTTPTransportError - case exception.status_code raise Errors::ExecutionError, shell: shell, command: command, diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 22a50072d..de7b1dc19 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -12,6 +12,8 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| c.username = 'username' c.password = 'password' + c.max_tries = 3 + c.retry_delay = 0 c.finalize! end } @@ -28,10 +30,13 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do expect(subject.powershell("dir")[:exitcode]).to eq(0) end - it "should raise auth error when WinRM exception has a response code of 401" do - # The default settings might an account lockout - 20 auth failures! - expect(session).to receive(:powershell).with(/^dir.+/).exactly(20).times.and_raise( - WinRM::WinRMAuthorizationError.new("Oh no!!", 401)) + it "should retry when a WinRMAuthorizationError is received" do + expect(session).to receive(:powershell).with(/^dir.+/).exactly(3).times.and_raise( + # Note: The initialize for WinRMAuthorizationError may require a status_code as + # the second argument in a future WinRM release. Currently it doesn't track the + # status code. + WinRM::WinRMAuthorizationError.new("Oh no!! Unauthrorized") + ) expect { subject.powershell("dir") }.to raise_error( VagrantPlugins::CommunicatorWinRM::Errors::AuthenticationFailed) end