Fixes #10229: Add timeout for changing hostname on windows

Prior to this commit, if Windows was slow to reboot, Vagrant would fail
to find the right IP address to upload the wait_for_reboot script to.
This commit fixes this race condition by adding a timeout to ensure that
Vagrant can retry. It also properly catches an exception in the winrm
ready? method for checking if a guest is properly ready for
communications.

plugins/communicators/winrm/communicator.rb

@@ -109,7 +109,7 @@ module VagrantPlugins
 
         @logger.info("WinRM is ready!")
         return true
-      rescue Errors::TransientError => e
+      rescue Errors::TransientError, VagrantPlugins::CommunicatorWinRM::Errors::WinRMNotReady => e
         # We catch a `TransientError` which would signal that something went
         # that might work if we wait and retry.
         @logger.info("WinRM not up: #{e.inspect}")

plugins/guests/windows/cap/change_host_name.rb

@@ -1,7 +1,10 @@
+require "log4r"
+
 module VagrantPlugins
   module GuestWindows
     module Cap
       module ChangeHostName
+        MAX_REBOOT_DURATION = 120
 
         def self.change_host_name(machine, name)
           change_host_name_and_wait(machine, name, machine.config.vm.graceful_halt_timeout)
@@ -11,6 +14,7 @@ module VagrantPlugins
           # If the configured name matches the current name, then bail
           # We cannot use %ComputerName% because it truncates at 15 chars
           return if machine.communicate.test("if ([System.Net.Dns]::GetHostName() -eq '#{name}') { exit 0 } exit 1")
+          @logger = Log4r::Logger.new("vagrant::windows::change_host_name")
 
           # Rename and reboot host if rename succeeded
           script = <<-EOH
@@ -27,13 +31,24 @@ module VagrantPlugins
             error_class: Errors::RenameComputerFailed,
             error_key: :rename_computer_failed)
 
+
+          wait_remaining = MAX_REBOOT_DURATION
+          begin
             # Don't continue until the machine has shutdown and rebooted
             if machine.guest.capability?(:wait_for_reboot)
               machine.guest.capability(:wait_for_reboot)
             else
+              @logger.debug("No wait_for_reboot capability, sleeping for #{sleep_timeout} instead...")
               # use graceful_halt_timeout only if guest cannot wait for reboot
               sleep(sleep_timeout)
             end
+          rescue Vagrant::Errors::MachineGuestNotReady => e
+            raise if wait_remaining < 0
+            @logger.warn("Machine not ready, cannot wait for reboot yet. Trying again")
+            sleep(5)
+            wait_remaining -= 5
+            retry
+          end
         end
       end
     end

plugins/providers/hyperv/provider.rb

@@ -38,6 +38,7 @@ module VagrantPlugins
         # This method will load in our driver, so we call it now to
         # initialize it.
         machine_id_changed
+        @logger = Log4r::Logger.new("vagrant::hyperv::provider")
       end
 
       def action(name)
@@ -83,16 +84,27 @@ module VagrantPlugins
         "Hyper-V (#{id})"
       end
 
+      # @return [Hash]
       def ssh_info
         # We can only SSH into a running machine
         return nil if state.id != :running
 
         # Read the IP of the machine using Hyper-V APIs
-        network = @driver.read_guest_ip
-        return nil if !network["ip"]
+        guest_ip = nil
+
+        begin
+          network_info = @driver.read_guest_ip
+          guest_ip = network_info["ip"]
+        rescue Errors::PowerShellError
+          @logger.warn("Failed to read guest IP.")
+        end
+
+        return nil if !guest_ip
+
+        @logger.debug("IP: #{guest_ip}")
 
         {
-          host: network["ip"],
+          host: guest_ip,
           port: 22,
         }
       end

test/unit/plugins/providers/hyperv/provider_test.rb

@@ -3,7 +3,12 @@ require_relative "../../../base"
 require Vagrant.source_root.join("plugins/providers/hyperv/provider")
 
 describe VagrantPlugins::HyperV::Provider do
-  let(:machine) { double("machine") }
+  let(:driver){ double("driver") }
+  let(:provider){ double("provider", driver: driver) }
+  let(:provider_config){ double("provider_config", ip_address_timeout: ip_address_timeout) }
+  let(:ip_address_timeout){ 1 }
+  let(:machine){ double("machine", provider: provider, provider_config: provider_config) }
+
   let(:platform)   { double("platform") }
   let(:powershell) { double("powershell") }
 
@@ -103,4 +108,28 @@ describe VagrantPlugins::HyperV::Provider do
       expect(subject.state.id).to eq(:bar)
     end
   end
+
+  describe "#ssh_info" do
+    let(:result) { "127.0.0.1" }
+    let(:exit_code) { 0 }
+    let(:ssh_info) {{:host=>result,:port=>22}}
+
+    before do
+      allow(VagrantPlugins::HyperV::Driver).to receive(:new).and_return(driver)
+      allow(machine).to receive(:action).with(:read_state).and_return(machine_state_id: :running)
+    end
+
+    it "returns nil if a PowerShellError is returned from the driver" do
+      allow(driver).to receive(:read_guest_ip)
+        .and_raise(VagrantPlugins::HyperV::Errors::PowerShellError, script: anything, stderr: anything)
+      expect(subject.ssh_info).to eq(nil)
+    end
+
+    it "should receive a valid address" do
+      allow(driver).to receive(:execute).with(:get_network_config).and_return(result)
+
+      allow(driver).to receive(:read_guest_ip).and_return({"ip" => "127.0.0.1"})
+      expect(subject.ssh_info).to eq(ssh_info)
+    end
+  end
 end