From 8562daf85ecf774989348e431f3d90d863360b32 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 19 Oct 2018 15:18:03 -0700 Subject: [PATCH] Prevent overly verbose output from SSH communicator If the type of error changes on retry the messages will effectively spam the user display with alternating messages. Log each message sent and only re-display each message once within 10 seconds. --- plugins/communicators/ssh/communicator.rb | 10 ++--- .../communicators/ssh/communicator_test.rb | 43 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index 1ae8b9047..deb241144 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -75,8 +75,7 @@ module VagrantPlugins ssh_auth_type = "password" if ssh_info[:password] @machine.ui.detail("SSH auth method: #{ssh_auth_type}") - last_message = nil - last_message_repeat_at = 0 + previous_messages = {} while true message = nil begin @@ -123,14 +122,13 @@ module VagrantPlugins if message message_at = Time.now.to_f show_message = true - if last_message == message - show_message = (message_at - last_message_repeat_at) > 10.0 + if previous_messages[message] + show_message = (message_at - previous_messages[message]) > 10.0 end if show_message @machine.ui.detail("Warning: #{message} Retrying...") - last_message = message - last_message_repeat_at = message_at + previous_messages[message] = message_at end end end diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index a236c1931..cfbcad89b 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -116,6 +116,49 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do expect(communicator.wait_for_ready(0.6)).to eq(true) end end + + context "when printing message to the user" do + before do + allow(machine).to receive(:ssh_info). + and_return(host: '10.1.2.3', port: 22).ordered + allow(communicator).to receive(:connect) + allow(communicator).to receive(:ready?).and_return(true) + end + + it "should print message" do + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHConnectionTimeout) + expect(ui).to receive(:detail).with(/timeout/) + communicator.wait_for_ready(0.5) + end + + it "should not print the same message twice" do + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHConnectionTimeout) + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHConnectionTimeout) + expect(ui).to receive(:detail).with(/timeout/) + expect(ui).not_to receive(:detail).with(/timeout/) + communicator.wait_for_ready(0.5) + end + + it "should print different messages" do + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHConnectionTimeout) + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHDisconnected) + expect(ui).to receive(:detail).with(/timeout/) + expect(ui).to receive(:detail).with(/disconnect/) + communicator.wait_for_ready(0.5) + end + + it "should not print different messages twice" do + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHConnectionTimeout) + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHDisconnected) + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHConnectionTimeout) + expect(communicator).to receive(:connect).and_raise(Vagrant::Errors::SSHDisconnected) + expect(ui).to receive(:detail).with(/timeout/) + expect(ui).to receive(:detail).with(/disconnect/) + expect(ui).not_to receive(:detail).with(/timeout/) + expect(ui).not_to receive(:detail).with(/disconnect/) + communicator.wait_for_ready(0.5) + end + end end end