fix: quote the ISO path, bound the wait for a guest IP, and explain bad host output

lib/kitchen/driver/powershell.rb

@@ -193,7 +193,16 @@ def execute_command(cmd, options = {})
         raise "Failed: #{sh.stderr}" if sh.exit_status != 0
 
         stdout = sanitize_stdout(sh.stdout)
-        JSON.parse(stdout) if stdout.length > 2
+        return if stdout.length <= 2
+
+        begin
+          JSON.parse(stdout)
+        rescue JSON::ParserError => e
+          # A bare JSON::ParserError names an offset in a string the user never
+          # sees, which is useless for working out what the host actually said.
+          raise "Expected JSON from the Hyper-V host but could not parse its " \
+                "output (#{e.message}). The host returned:\n#{stdout}"
+        end
       end
 
       # Strip the interactive prompt lines PowerShell interleaves with output,
@@ -372,11 +381,14 @@ def vm_default_switch_ps
 
       # Script that attaches the configured ISO to the VM's DVD drive.
       #
+      # The path is quoted: unquoted, PowerShell splits an ISO path containing
+      # a space into two arguments and binds only the first to -Path.
+      #
       # @return [String] PowerShell source
       # @api private
       def mount_vm_iso
         <<-MOUNTISO
-          mount-vmiso -id "#{@state[:id]}" -Path #{config[:iso_path]}
+          mount-vmiso -id "#{@state[:id]}" -Path "#{config[:iso_path]}"
         MOUNTISO
       end
 

spec/kitchen/driver/powershell_spec.rb

@@ -152,6 +152,13 @@ def with_arch(value)
         .to raise_error(/Hyper-V is not installed/)
     end
 
+    it "reports what the host actually said when the output is not JSON" do
+      connection.stub_script(/Get-Thing/, stdout: "WARNING: the RPC server is unavailable")
+
+      expect { generate(:run_ps, "Get-Thing") }
+        .to raise_error(/Expected JSON.*RPC server is unavailable/m)
+    end
+
     context "with dry_run enabled" do
       let(:driver_config) { { dry_run: true } }
 
@@ -324,7 +331,13 @@ def with_arch(value)
     it "mounts an ISO by id" do
       driver.send(:config)[:iso_path] = 'C:\iso\tools.iso'
 
-      expect(generate(:mount_vm_iso)).to include(%{mount-vmiso -id "vm-0001" -Path C:\\iso\\tools.iso})
+      expect(generate(:mount_vm_iso)).to include(%{mount-vmiso -id "vm-0001" -Path "C:\\iso\\tools.iso"})
+    end
+
+    it "quotes the ISO path so a path containing a space stays one argument" do
+      driver.send(:config)[:iso_path] = 'C:\ISO Files\tools.iso'
+
+      expect(generate(:mount_vm_iso)).to include(%{-Path "C:\\ISO Files\\tools.iso"})
     end
 
     describe "#copy_vm_file_ps" do

spec/powershell/hyperv.Tests.ps1

@@ -380,6 +380,50 @@ Describe 'Get-VmDetail' {
         $detail.Id | Should -Be 'vm-1'
         $detail.IpAddress | Should -Be '192.168.1.50'
     }
+
+    It 'asks for the address once when it is already available' {
+        Mock Get-VM { [pscustomobject]@{ Name = 'kitchen'; ID = 'vm-1' } }
+        Mock Get-VmIP { '192.168.1.50' }
+
+        $null = Get-VmDetail -Id 'vm-1'
+
+        # Get-VmIP sleeps 10 seconds per call, so asking twice cost every
+        # create an extra 10 seconds for an address it already had.
+        Should -Invoke Get-VmIP -Exactly -Times 1
+    }
+
+    It 'keeps waiting while the guest has no address yet' {
+        Mock Get-VM { [pscustomobject]@{ Name = 'kitchen'; ID = 'vm-1' } }
+        Mock Start-Sleep
+        $script:calls = 0
+        Mock Get-VmIP {
+            $script:calls++
+            if ($script:calls -ge 3) { '192.168.1.50' }
+        }
+
+        $detail = Get-VmDetail -Id 'vm-1'
+
+        $detail.IpAddress | Should -Be '192.168.1.50'
+        Should -Invoke Start-Sleep -Exactly -Times 2
+    }
+
+    It 'gives up rather than hanging forever when no address ever arrives' {
+        Mock Get-VM { [pscustomobject]@{ Name = 'kitchen'; ID = 'vm-1' } }
+        Mock Get-VmIP { $null }
+        Mock Start-Sleep
+
+        { Get-VmDetail -Id 'vm-1' -TimeoutSeconds 0 } |
+            Should -Throw -ExpectedMessage '*Timed out after 0 seconds*'
+    }
+
+    It 'names the virtual machine in the timeout message' {
+        Mock Get-VM { [pscustomobject]@{ Name = 'kitchen'; ID = 'vm-1' } }
+        Mock Get-VmIP { $null }
+        Mock Start-Sleep
+
+        { Get-VmDetail -Id 'vm-1' -TimeoutSeconds 0 } |
+            Should -Throw -ExpectedMessage "*'kitchen'*"
+    }
 }
 
 Describe 'Get-VmStatus' {

support/hyperv.ps1

@@ -211,20 +211,37 @@ Function Set-VMNetworkConfiguration {
 
 function Get-VmDetail {
     [cmdletbinding()]
-    param($Id)
+    param(
+        $Id,
+
+        # Longest to wait for the guest to report an IPv4 address. Without a
+        # bound, a VM whose network never comes up leaves `kitchen create`
+        # hanging silently for as long as the user is willing to wait.
+        [int]$TimeoutSeconds = 600
+    )
 
     Get-VM -Id $Id |
         ForEach-Object {
         $vm = $_
-        do {
+        $deadline = (Get-Date).AddSeconds($TimeoutSeconds)
+
+        # Keep the address the wait found rather than asking again: Get-VmIP
+        # sleeps 10 seconds per call, so a second call cost every create an
+        # extra 10 seconds and could return a different adapter's address.
+        $ipAddress = Get-VmIP $vm
+
+        while (-not $ipAddress) {
+            if ((Get-Date) -gt $deadline) {
+                throw "Timed out after $TimeoutSeconds seconds waiting for virtual machine '$($vm.Name)' to report an IP address. Check that the VM booted and that its network adapter is connected to a switch with DHCP."
+            }
             Start-Sleep -Seconds 1
+            $ipAddress = Get-VmIP $vm
         }
-        while (-not (Get-VmIP $vm))
 
         [pscustomobject]@{
             Name = $vm.name
             Id = $vm.ID
-            IpAddress = (Get-VmIP $vm)
+            IpAddress = $ipAddress
         }
     }
 }