fix: quote the ISO path, bound the wait for a guest IP, and explain bad host output - #169
Merged
Merged
Conversation
…ad host output
Three bugs found while sweeping the driver.
An ISO path containing a space never mounted. mount_vm_iso interpolated
config[:iso_path] into the generated PowerShell unquoted, so
`C:\ISO Files\tools.iso` reached the host as two arguments and only
`C:\ISO` bound to -Path. Every other path the driver emits is quoted; this
one was not.
Get-VmDetail waited forever for an IP address. Its `do { Start-Sleep 1 }
while (-not (Get-VmIP $vm))` loop had no bound, so a VM that booted but
never got an address left `kitchen create` hanging silently until the user
gave up. It now stops after ten minutes and says which VM never answered
and what to check. The same change stops it asking Get-VmIP for the address
a second time after the wait already found one -- Get-VmIP sleeps ten
seconds per call, so every successful create paid an extra ten seconds for
an address it was holding, and the second call could return a different
adapter's address than the one that ended the wait.
Non-JSON output from the host produced an unusable error. execute_command
handed whatever came back straight to JSON.parse, so a PowerShell warning
or a partial transport read surfaced as "unexpected token at ..." naming an
offset into a string the user never sees. It now raises with the host's
actual output included.
Signed-off-by: Tim Smith <tim@mondoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three bugs found while sweeping the driver and
support/hyperv.ps1. Each one is independent, but they are all small and each comes with the test that would have caught it, so they are together rather than in three near-empty PRs.An ISO path containing a space never mounted
mount_vm_isointerpolatedconfig[:iso_path]into the generated PowerShell unquoted:PowerShell splits that into two arguments, binds
C:\ISOto-Path, and then fails on the strayFiles\tools.iso. Every other path the driver emits is quoted —New-DifferencingDisk,New-VHD,Resize-VHD,New-KitchenVM— so this was simply a miss.C:\ISO Files\...andC:\Program Files\...are entirely normal places to keep install media.Get-VmDetailwaited forever for an IP addressNo bound, no output. A VM that boots but never gets an address — no DHCP on the switch, adapter not connected, guest network stack down — leaves
kitchen createhanging silently until the user gives up and kills it. There was a@todoonvm_details_psnoting exactly this.It now stops after ten minutes and says which VM never answered and what to check:
The bound is a
-TimeoutSecondsparameter, so it stays adjustable from PowerShell.The same change stops the function calling
Get-VmIPa second time after the wait already found an address.Get-VmIPopens withStart-Sleep -Seconds 10, so every successful create paid an extra ten seconds for an address it was already holding — and because the second call re-reads$vm.networkadapters.ipaddresses, it could return a different adapter's address than the one that ended the wait.Non-JSON output from the host produced an unusable error
execute_commandhanded whatever came back straight toJSON.parse. When the host wrote something that was not JSON — a PowerShell warning, a truncated transport read — the user got:or worse, an offset into a string they never see. It now raises with the host's actual output included, which is the only thing that makes the failure diagnosable.
Verification