From 3fc2defb6d22fc7025501d836afa44c7b56d2814 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 16 Jun 2018 18:02:38 +0100 Subject: [PATCH 01/33] Refactor the export step for Hyper-V ISO and VMCX builders * Fixes a bug that caused the build to error if users did not explicitly set `skip_compaction:true` when setting `skip_export: true`. See #6392. * Improves the efficiency of the compaction and export process by reordering the compaction and export steps. * Further improves the efficiency of the compacting step through compacting the vmd* file directly rather than creating and then operating on a copy. * The changes mean the export process now stores the exported machine files and folders under a folder with name 'vm_name' in the output directory. Previously the exported machine files and folders were stored directly in the output directory. --- builder/hyperv/common/driver.go | 2 +- builder/hyperv/common/driver_mock.go | 12 ++-- builder/hyperv/common/driver_ps_4.go | 4 +- builder/hyperv/common/step_export_vm.go | 74 +++++++++---------------- common/powershell/hyperv/hyperv.go | 8 +-- 5 files changed, 38 insertions(+), 62 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index 535652923..2f79d8079 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -94,7 +94,7 @@ type Driver interface { ExportVirtualMachine(string, string) error - CompactDisks(string, string) error + CompactDisks(string) error CopyExportedVirtualMachine(string, string, string, string) error diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 3fcb46323..669e47dde 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -186,10 +186,9 @@ type DriverMock struct { ExportVirtualMachine_Path string ExportVirtualMachine_Err error - CompactDisks_Called bool - CompactDisks_ExpPath string - CompactDisks_VhdDir string - CompactDisks_Err error + CompactDisks_Called bool + CompactDisks_Path string + CompactDisks_Err error CopyExportedVirtualMachine_Called bool CopyExportedVirtualMachine_ExpPath string @@ -489,10 +488,9 @@ func (d *DriverMock) ExportVirtualMachine(vmName string, path string) error { return d.ExportVirtualMachine_Err } -func (d *DriverMock) CompactDisks(expPath string, vhdDir string) error { +func (d *DriverMock) CompactDisks(path string) error { d.CompactDisks_Called = true - d.CompactDisks_ExpPath = expPath - d.CompactDisks_VhdDir = vhdDir + d.CompactDisks_Path = path return d.CompactDisks_Err } diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index cadaa2ba5..2f75a1810 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -219,8 +219,8 @@ func (d *HypervPS4Driver) ExportVirtualMachine(vmName string, path string) error return hyperv.ExportVirtualMachine(vmName, path) } -func (d *HypervPS4Driver) CompactDisks(expPath string, vhdDir string) error { - return hyperv.CompactDisks(expPath, vhdDir) +func (d *HypervPS4Driver) CompactDisks(path string) error { + return hyperv.CompactDisks(path) } func (d *HypervPS4Driver) CopyExportedVirtualMachine(expPath string, outputPath string, vhdDir string, vmDir string) error { diff --git a/builder/hyperv/common/step_export_vm.go b/builder/hyperv/common/step_export_vm.go index 8161da31d..f582c30f6 100644 --- a/builder/hyperv/common/step_export_vm.go +++ b/builder/hyperv/common/step_export_vm.go @@ -3,18 +3,11 @@ package common import ( "context" "fmt" - "io/ioutil" - "path/filepath" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) -const ( - vhdDir string = "Virtual Hard Disks" - vmDir string = "Virtual Machines" -) - type StepExportVm struct { OutputDir string SkipCompaction bool @@ -24,63 +17,48 @@ type StepExportVm struct { func (s *StepExportVm) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { driver := state.Get("driver").(Driver) ui := state.Get("ui").(packer.Ui) - var err error - var errorMsg string - vmName := state.Get("vmName").(string) - tmpPath := state.Get("packerTempDir").(string) - outputPath := s.OutputDir - expPath := s.OutputDir - - // create temp path to export vm - errorMsg = "Error creating temp export path: %s" - vmExportPath, err := ioutil.TempDir(tmpPath, "export") - if err != nil { - err := fmt.Errorf(errorMsg, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt + // Get the VM name; Get the temp directory used to store the VMs files + // during the build process + var vmName, tmpPath string + if v, ok := state.GetOk("vmName"); ok { + vmName = v.(string) } - if !s.SkipExport { - ui.Say("Exporting vm...") - - err = driver.ExportVirtualMachine(vmName, vmExportPath) - if err != nil { - errorMsg = "Error exporting vm: %s" - err := fmt.Errorf(errorMsg, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } - // copy to output dir - expPath = filepath.Join(vmExportPath, vmName) + if v, ok := state.GetOk("packerTempDir"); ok { + tmpPath = v.(string) } + // Compact disks first so the export process has less to do if s.SkipCompaction { ui.Say("Skipping disk compaction...") } else { ui.Say("Compacting disks...") - err = driver.CompactDisks(expPath, vhdDir) + err := driver.CompactDisks(tmpPath) if err != nil { - errorMsg = "Error compacting disks: %s" - err := fmt.Errorf(errorMsg, err) + err := fmt.Errorf("Error compacting disks: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt } } - if !s.SkipExport { - ui.Say("Copying to output dir...") - err = driver.CopyExportedVirtualMachine(expPath, outputPath, vhdDir, vmDir) - if err != nil { - errorMsg = "Error exporting vm: %s" - err := fmt.Errorf(errorMsg, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } + if s.SkipExport { + ui.Say("Skipping export of virtual machine...") + return multistep.ActionContinue } + + ui.Say("Exporting virtual machine...") + // The export process exports the VM to a folder named 'vmName' under + // the output directory. This contains the usual 'Snapshots', 'Virtual + // Hard Disks' and 'Virtual Machines' directories. + err := driver.ExportVirtualMachine(vmName, s.OutputDir) + if err != nil { + err = fmt.Errorf("Error exporting vm: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + return multistep.ActionContinue } diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 343e02776..0a5505b19 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -665,16 +665,16 @@ if (Test-Path -Path ([IO.Path]::Combine($path, $vmName, 'Virtual Machines', '*.V return err } -func CompactDisks(expPath string, vhdDir string) error { +func CompactDisks(path string) error { var script = ` -param([string]$srcPath, [string]$vhdDirName) -Get-ChildItem "$srcPath/$vhdDirName" -Filter *.vhd* | %{ +param([string]$srcPath) +Get-ChildItem "$srcPath" -Filter *.vhd* | %{ Optimize-VHD -Path $_.FullName -Mode Full } ` var ps powershell.PowerShellCmd - err := ps.Run(script, expPath, vhdDir) + err := ps.Run(script, path) return err } From dc46848f89be30e7c44aaf8aa6cddba157e8f578 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 16 Jun 2018 18:26:39 +0100 Subject: [PATCH 02/33] Remove now redundant function to copy exported VM files --- builder/hyperv/common/driver.go | 2 -- builder/hyperv/common/driver_mock.go | 16 ---------------- builder/hyperv/common/driver_ps_4.go | 4 ---- common/powershell/hyperv/hyperv.go | 14 -------------- 4 files changed, 36 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index 2f79d8079..08e3f4eac 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -96,8 +96,6 @@ type Driver interface { CompactDisks(string) error - CopyExportedVirtualMachine(string, string, string, string) error - RestartVirtualMachine(string) error CreateDvdDrive(string, string, uint) (uint, uint, error) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 669e47dde..53c7f959b 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -190,13 +190,6 @@ type DriverMock struct { CompactDisks_Path string CompactDisks_Err error - CopyExportedVirtualMachine_Called bool - CopyExportedVirtualMachine_ExpPath string - CopyExportedVirtualMachine_OutputPath string - CopyExportedVirtualMachine_VhdDir string - CopyExportedVirtualMachine_VmDir string - CopyExportedVirtualMachine_Err error - RestartVirtualMachine_Called bool RestartVirtualMachine_VmName string RestartVirtualMachine_Err error @@ -494,15 +487,6 @@ func (d *DriverMock) CompactDisks(path string) error { return d.CompactDisks_Err } -func (d *DriverMock) CopyExportedVirtualMachine(expPath string, outputPath string, vhdDir string, vmDir string) error { - d.CopyExportedVirtualMachine_Called = true - d.CopyExportedVirtualMachine_ExpPath = expPath - d.CopyExportedVirtualMachine_OutputPath = outputPath - d.CopyExportedVirtualMachine_VhdDir = vhdDir - d.CopyExportedVirtualMachine_VmDir = vmDir - return d.CopyExportedVirtualMachine_Err -} - func (d *DriverMock) RestartVirtualMachine(vmName string) error { d.RestartVirtualMachine_Called = true d.RestartVirtualMachine_VmName = vmName diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index 2f75a1810..964f4b6a4 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -223,10 +223,6 @@ func (d *HypervPS4Driver) CompactDisks(path string) error { return hyperv.CompactDisks(path) } -func (d *HypervPS4Driver) CopyExportedVirtualMachine(expPath string, outputPath string, vhdDir string, vmDir string) error { - return hyperv.CopyExportedVirtualMachine(expPath, outputPath, vhdDir, vmDir) -} - func (d *HypervPS4Driver) RestartVirtualMachine(vmName string) error { return hyperv.RestartVirtualMachine(vmName) } diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 0a5505b19..d15bd6bd4 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -678,20 +678,6 @@ Get-ChildItem "$srcPath" -Filter *.vhd* | %{ return err } -func CopyExportedVirtualMachine(expPath string, outputPath string, vhdDir string, vmDir string) error { - - var script = ` -param([string]$srcPath, [string]$dstPath, [string]$vhdDirName, [string]$vmDir) -Move-Item -Path (Join-Path (Get-Item $srcPath).FullName "*.*") -Destination $dstPath -Move-Item -Path (Join-Path (Get-Item $srcPath).FullName $vhdDirName) -Destination $dstPath -Move-Item -Path (Join-Path (Get-Item $srcPath).FullName $vmDir) -Destination $dstPath -` - - var ps powershell.PowerShellCmd - err := ps.Run(script, expPath, outputPath, vhdDir, vmDir) - return err -} - func CreateVirtualSwitch(switchName string, switchType string) (bool, error) { var script = ` From 534fc4a4731156e6d6f89ef56e611f2e81531d8c Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 4 Jul 2018 13:03:32 +0100 Subject: [PATCH 03/33] Ensure the export directory structure matches that of previous versions Commit 3fc2defb6 altered the directory structure associated with an exported VM. The changes mean that the export process now stores the exported machine files and folders under a folder with name 'vm_name' in the output directory. This commit restores the previous behaviour whereby the exported machine files and folders were stored directly in the output directory. This allows us to keep the efficiency improvements introduced with 3fc2defb6 while maintaining backward compatibility. By default the Export-VM command creates three folders in the specified export directory - 'Virtual Hard Disks', 'Virtual Machines' and 'Snapshots'. When a machine with no associated snapshots is exported the 'Snapshots' directory is empty. Prior to 3fc2defb6 the Snapshots folder was not copied/incorporated into the output directory at all. This was a bug. This commit preserves the legacy behaviour by not including an empty Snapshots directory in the export. However, if there *are* Snapshots associated with the VM, they are now moved into the output directory along with the usual directories containing disks and VM metadata. This prevents warnings/errors on import due to missing snapshots. --- builder/hyperv/common/driver.go | 2 + builder/hyperv/common/driver_mock.go | 12 ++++++ builder/hyperv/common/driver_ps_4.go | 4 ++ builder/hyperv/common/step_export_vm.go | 16 +++++++ common/powershell/hyperv/hyperv.go | 56 +++++++++++++++++++++++++ 5 files changed, 90 insertions(+) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index 08e3f4eac..ef4e9c48d 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -94,6 +94,8 @@ type Driver interface { ExportVirtualMachine(string, string) error + PreserveLegacyExportBehaviour(string, string) error + CompactDisks(string) error RestartVirtualMachine(string) error diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 53c7f959b..28efa397d 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -186,6 +186,11 @@ type DriverMock struct { ExportVirtualMachine_Path string ExportVirtualMachine_Err error + PreserveLegacyExportBehaviour_Called bool + PreserveLegacyExportBehaviour_SrcPath string + PreserveLegacyExportBehaviour_DstPath string + PreserveLegacyExportBehaviour_Err error + CompactDisks_Called bool CompactDisks_Path string CompactDisks_Err error @@ -481,6 +486,13 @@ func (d *DriverMock) ExportVirtualMachine(vmName string, path string) error { return d.ExportVirtualMachine_Err } +func (d *DriverMock) PreserveLegacyExportBehaviour(srcPath string, dstPath string) error { + d.PreserveLegacyExportBehaviour_Called = true + d.PreserveLegacyExportBehaviour_SrcPath = srcPath + d.PreserveLegacyExportBehaviour_DstPath = dstPath + return d.PreserveLegacyExportBehaviour_Err +} + func (d *DriverMock) CompactDisks(path string) error { d.CompactDisks_Called = true d.CompactDisks_Path = path diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index 964f4b6a4..b013b2598 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -219,6 +219,10 @@ func (d *HypervPS4Driver) ExportVirtualMachine(vmName string, path string) error return hyperv.ExportVirtualMachine(vmName, path) } +func (d *HypervPS4Driver) PreserveLegacyExportBehaviour(srcPath string, dstPath string) error { + return hyperv.PreserveLegacyExportBehaviour(srcPath, dstPath) +} + func (d *HypervPS4Driver) CompactDisks(path string) error { return hyperv.CompactDisks(path) } diff --git a/builder/hyperv/common/step_export_vm.go b/builder/hyperv/common/step_export_vm.go index f582c30f6..0eb42a7c0 100644 --- a/builder/hyperv/common/step_export_vm.go +++ b/builder/hyperv/common/step_export_vm.go @@ -3,6 +3,7 @@ package common import ( "context" "fmt" + "path/filepath" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -59,6 +60,21 @@ func (s *StepExportVm) Run(_ context.Context, state multistep.StateBag) multiste return multistep.ActionHalt } + // Shuffle around the exported folders to maintain backwards + // compatibility. This moves the 'Snapshots', 'Virtual Hard Disks' and + // 'Virtual Machines' directories from / so + // they appear directly under . The empty '/' directory is removed when complete. + // The 'Snapshots' folder will not be moved into the output directory + // if it is empty. + exportPath := filepath.Join(s.OutputDir, vmName) + err = driver.PreserveLegacyExportBehaviour(exportPath, s.OutputDir) + if err != nil { + // No need to halt here; Just warn the user instead + err = fmt.Errorf("WARNING: Error restoring legacy export dir structure: %s", err) + ui.Error(err.Error()) + } + return multistep.ActionContinue } diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index d15bd6bd4..d51eef74c 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -665,6 +665,62 @@ if (Test-Path -Path ([IO.Path]::Combine($path, $vmName, 'Virtual Machines', '*.V return err } +func PreserveLegacyExportBehaviour(srcPath, dstPath string) error { + + var script = ` +param([string]$srcPath, [string]$dstPath) + +# Validate the paths returning an error if they are empty or don't exist +$srcPath, $dstPath | % { + if ($_) { + if (! (Test-Path $_)) { + [System.Console]::Error.WriteLine("Path $_ does not exist") + exit + } + } else { + [System.Console]::Error.WriteLine("A supplied path is empty") + exit + } +} + +# Export-VM should just create directories at the root of the export path +# but, just in case, move all files as well... +Move-Item -Path (Join-Path (Get-Item $srcPath).FullName "*.*") -Destination (Get-Item $dstPath).FullName + +# Move directories with content; Delete empty directories +$dirObj = Get-ChildItem $srcPath -Directory | % { + New-Object PSObject -Property @{ + FullName=$_.FullName; + HasContent=$(if ($_.GetFileSystemInfos().Count -gt 0) {$true} else {$false}) + } +} +foreach ($directory in $dirObj) { + if ($directory.HasContent) { + Move-Item -Path $directory.FullName -Destination (Get-Item $dstPath).FullName + } else { + Remove-Item -Path $directory.FullName + } +} + +# Only remove the source directory if it is now empty +if ( $((Get-Item $srcPath).GetFileSystemInfos().Count) -eq 0 ) { + Remove-Item -Path $srcPath +} else { + # 'Return' an error message to PowerShellCmd as the directory should + # always be empty at the end of the script. The check is here to stop + # the Remove-Item command from doing any damage if some unforeseen + # error has occured + [System.Console]::Error.WriteLine("Refusing to remove $srcPath as it is not empty") + exit +} +` + + var ps powershell.PowerShellCmd + err := ps.Run(script, srcPath, dstPath) + + return err +} + func CompactDisks(path string) error { var script = ` param([string]$srcPath) From c6b9d9ce90ee3117e80039ded77f852e7f8bff08 Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 5 Jul 2018 21:16:51 +0100 Subject: [PATCH 04/33] Add checks/error reporting to compaction process * Report compaction results * Failure to find any disks under the supplied path is treated as a 'soft' error and a warning message will be printed in place of the compaction result. Any other failure will cause the build to fail. --- builder/hyperv/common/driver.go | 2 +- builder/hyperv/common/driver_mock.go | 6 +++-- builder/hyperv/common/driver_ps_4.go | 2 +- builder/hyperv/common/step_export_vm.go | 8 +++++- common/powershell/hyperv/hyperv.go | 33 +++++++++++++++++++++---- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index ef4e9c48d..de3ba2651 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -96,7 +96,7 @@ type Driver interface { PreserveLegacyExportBehaviour(string, string) error - CompactDisks(string) error + CompactDisks(string) (string, error) RestartVirtualMachine(string) error diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 28efa397d..8e5a52cbd 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -193,6 +193,7 @@ type DriverMock struct { CompactDisks_Called bool CompactDisks_Path string + CompactDisks_Result string CompactDisks_Err error RestartVirtualMachine_Called bool @@ -493,10 +494,11 @@ func (d *DriverMock) PreserveLegacyExportBehaviour(srcPath string, dstPath strin return d.PreserveLegacyExportBehaviour_Err } -func (d *DriverMock) CompactDisks(path string) error { +func (d *DriverMock) CompactDisks(path string) (result string, err error) { d.CompactDisks_Called = true d.CompactDisks_Path = path - return d.CompactDisks_Err + d.CompactDisks_Result = "Mock compact result msg: mockdisk.vhdx. Disk size reduced by 20%" + return d.CompactDisks_Result, d.CompactDisks_Err } func (d *DriverMock) RestartVirtualMachine(vmName string) error { diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index b013b2598..acf399210 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -223,7 +223,7 @@ func (d *HypervPS4Driver) PreserveLegacyExportBehaviour(srcPath string, dstPath return hyperv.PreserveLegacyExportBehaviour(srcPath, dstPath) } -func (d *HypervPS4Driver) CompactDisks(path string) error { +func (d *HypervPS4Driver) CompactDisks(path string) (result string, err error) { return hyperv.CompactDisks(path) } diff --git a/builder/hyperv/common/step_export_vm.go b/builder/hyperv/common/step_export_vm.go index 0eb42a7c0..dfed94bf7 100644 --- a/builder/hyperv/common/step_export_vm.go +++ b/builder/hyperv/common/step_export_vm.go @@ -34,13 +34,19 @@ func (s *StepExportVm) Run(_ context.Context, state multistep.StateBag) multiste ui.Say("Skipping disk compaction...") } else { ui.Say("Compacting disks...") - err := driver.CompactDisks(tmpPath) + // CompactDisks searches for all VHD/VHDX files under the supplied + // path and runs the compacting process on each of them. If no disks + // are found under the supplied path this is treated as a 'soft' error + // and a warning message is printed. All other errors halt the build. + result, err := driver.CompactDisks(tmpPath) if err != nil { err := fmt.Errorf("Error compacting disks: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt } + // Report disk compaction results/warn if no disks were found + ui.Message(result) } if s.SkipExport { diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index d51eef74c..055d12706 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -721,17 +721,40 @@ if ( $((Get-Item $srcPath).GetFileSystemInfos().Count) -eq 0 ) { return err } -func CompactDisks(path string) error { +func CompactDisks(path string) (result string, err error) { var script = ` param([string]$srcPath) -Get-ChildItem "$srcPath" -Filter *.vhd* | %{ - Optimize-VHD -Path $_.FullName -Mode Full + +$disks = Get-ChildItem -Path $srcPath -Recurse -Filter *.vhd* -ErrorAction SilentlyContinue | % { $_.FullName } +# Failure to find any disks is treated as a 'soft' error. Simply print out +# a warning and exit +if ($disks.Length -eq 0) { + Write-Output "WARNING: No disks found under $srcPath" + exit +} + +foreach ($disk in $disks) { + Write-Output "Compacting disk: $(Split-Path $disk -leaf)" + + $sizeBefore = $disk.Length + Optimize-VHD -Path $disk -Mode Full + $sizeAfter = $disk.Length + + # Calculate the percentage change in disk size + if ($sizeAfter -gt 0) { # Protect against division by zero + $percentChange = ( ( $sizeAfter / $sizeBefore ) * 100 ) - 100 + switch($percentChange) { + {$_ -lt 0} {Write-Output "Disk size reduced by: $(([math]::Abs($_)).ToString("#.#"))%"} + {$_ -eq 0} {Write-Output "Disk size is unchanged"} + {$_ -gt 0} {Write-Output "WARNING: Disk size increased by: $($_.ToString("#.#"))%"} + } + } } ` var ps powershell.PowerShellCmd - err := ps.Run(script, path) - return err + result, err = ps.Output(script, path) + return } func CreateVirtualSwitch(switchName string, switchType string) (bool, error) { From da2df693013d1724d742e3dc3a92db9210504d1e Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 7 Jul 2018 12:27:09 +0100 Subject: [PATCH 05/33] Remove disk compaction from the export step --- builder/hyperv/common/step_export_vm.go | 42 ++++++------------------- builder/hyperv/iso/builder.go | 5 ++- builder/hyperv/vmcx/builder.go | 5 ++- 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/builder/hyperv/common/step_export_vm.go b/builder/hyperv/common/step_export_vm.go index dfed94bf7..89aee37f0 100644 --- a/builder/hyperv/common/step_export_vm.go +++ b/builder/hyperv/common/step_export_vm.go @@ -10,51 +10,27 @@ import ( ) type StepExportVm struct { - OutputDir string - SkipCompaction bool - SkipExport bool + OutputDir string + SkipExport bool } func (s *StepExportVm) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { driver := state.Get("driver").(Driver) ui := state.Get("ui").(packer.Ui) - // Get the VM name; Get the temp directory used to store the VMs files - // during the build process - var vmName, tmpPath string - if v, ok := state.GetOk("vmName"); ok { - vmName = v.(string) - } - if v, ok := state.GetOk("packerTempDir"); ok { - tmpPath = v.(string) - } - - // Compact disks first so the export process has less to do - if s.SkipCompaction { - ui.Say("Skipping disk compaction...") - } else { - ui.Say("Compacting disks...") - // CompactDisks searches for all VHD/VHDX files under the supplied - // path and runs the compacting process on each of them. If no disks - // are found under the supplied path this is treated as a 'soft' error - // and a warning message is printed. All other errors halt the build. - result, err := driver.CompactDisks(tmpPath) - if err != nil { - err := fmt.Errorf("Error compacting disks: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } - // Report disk compaction results/warn if no disks were found - ui.Message(result) - } - if s.SkipExport { ui.Say("Skipping export of virtual machine...") return multistep.ActionContinue } ui.Say("Exporting virtual machine...") + + // The VM name is needed for the export command + var vmName string + if v, ok := state.GetOk("vmName"); ok { + vmName = v.(string) + } + // The export process exports the VM to a folder named 'vmName' under // the output directory. This contains the usual 'Snapshots', 'Virtual // Hard Disks' and 'Virtual Machines' directories. diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index d50e5e26a..d01dd27d6 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -467,9 +467,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe Generation: b.config.Generation, }, &hypervcommon.StepExportVm{ - OutputDir: b.config.OutputDir, - SkipCompaction: b.config.SkipCompaction, - SkipExport: b.config.SkipExport, + OutputDir: b.config.OutputDir, + SkipExport: b.config.SkipExport, }, // the clean up actions for each step will be executed reverse order diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 987c41a12..7e7bc748a 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -476,9 +476,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe Generation: b.config.Generation, }, &hypervcommon.StepExportVm{ - OutputDir: b.config.OutputDir, - SkipCompaction: b.config.SkipCompaction, - SkipExport: b.config.SkipExport, + OutputDir: b.config.OutputDir, + SkipExport: b.config.SkipExport, }, // the clean up actions for each step will be executed reverse order From 09028c14a3dc396b0cbbbebfc6ffcb58d9ac0717 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 7 Jul 2018 12:42:22 +0100 Subject: [PATCH 06/33] Reintroduce the disk compaction process as a unique step --- builder/hyperv/common/step_compact_disk.go | 50 ++++++++++++++++++++++ builder/hyperv/iso/builder.go | 3 ++ builder/hyperv/vmcx/builder.go | 3 ++ 3 files changed, 56 insertions(+) create mode 100644 builder/hyperv/common/step_compact_disk.go diff --git a/builder/hyperv/common/step_compact_disk.go b/builder/hyperv/common/step_compact_disk.go new file mode 100644 index 000000000..628703c7a --- /dev/null +++ b/builder/hyperv/common/step_compact_disk.go @@ -0,0 +1,50 @@ +package common + +import ( + "context" + "fmt" + + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +type StepCompactDisk struct { + SkipCompaction bool +} + +// Run runs a compaction/optimisation process on attached VHD/VHDX disks +func (s *StepCompactDisk) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { + driver := state.Get("driver").(Driver) + ui := state.Get("ui").(packer.Ui) + + if s.SkipCompaction { + ui.Say("Skipping disk compaction...") + return multistep.ActionContinue + } + + // Get the tmp dir used to store the VMs files during the build process + var tmpPath string + if v, ok := state.GetOk("packerTempDir"); ok { + tmpPath = v.(string) + } + + ui.Say("Compacting disks...") + // CompactDisks searches for all VHD/VHDX files under the supplied + // path and runs the compacting process on each of them. If no disks + // are found under the supplied path this is treated as a 'soft' error + // and a warning message is printed. All other errors halt the build. + result, err := driver.CompactDisks(tmpPath) + if err != nil { + err := fmt.Errorf("Error compacting disks: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + // Report disk compaction results/warn if no disks were found + ui.Message(result) + + return multistep.ActionContinue +} + +// Cleanup does nothing +func (s *StepCompactDisk) Cleanup(state multistep.StateBag) {} diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index d01dd27d6..154161a83 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -466,6 +466,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &hypervcommon.StepUnmountFloppyDrive{ Generation: b.config.Generation, }, + &hypervcommon.StepCompactDisk{ + SkipCompaction: b.config.SkipCompaction, + }, &hypervcommon.StepExportVm{ OutputDir: b.config.OutputDir, SkipExport: b.config.SkipExport, diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 7e7bc748a..4a95a80dc 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -475,6 +475,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &hypervcommon.StepUnmountFloppyDrive{ Generation: b.config.Generation, }, + &hypervcommon.StepCompactDisk{ + SkipCompaction: b.config.SkipCompaction, + }, &hypervcommon.StepExportVm{ OutputDir: b.config.OutputDir, SkipExport: b.config.SkipExport, From 30a71da8e76db913c2fca187dfea3e17c5d93d78 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 7 Jul 2018 13:57:44 +0100 Subject: [PATCH 07/33] Add tests for disk compaction step --- .../hyperv/common/step_compact_disk_test.go | 63 +++++++++++++++++++ builder/hyperv/common/step_test.go | 19 ++++++ 2 files changed, 82 insertions(+) create mode 100644 builder/hyperv/common/step_compact_disk_test.go create mode 100644 builder/hyperv/common/step_test.go diff --git a/builder/hyperv/common/step_compact_disk_test.go b/builder/hyperv/common/step_compact_disk_test.go new file mode 100644 index 000000000..75f3aa19d --- /dev/null +++ b/builder/hyperv/common/step_compact_disk_test.go @@ -0,0 +1,63 @@ +package common + +import ( + "context" + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepCompactDisk_impl(t *testing.T) { + var _ multistep.Step = new(StepCompactDisk) +} + +func TestStepCompactDisk(t *testing.T) { + state := testState(t) + step := new(StepCompactDisk) + + // Set up the path to the tmp directory used to store VM files + tmpPath := "foopath" + state.Put("packerTempDir", tmpPath) + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + // Test the driver + if !driver.CompactDisks_Called { + t.Fatal("Should have called CompactDisks") + } + if driver.CompactDisks_Path != tmpPath { + t.Fatalf("Should call with correct path. Got: %s Wanted: %s", driver.CompactDisks_Path, tmpPath) + } +} + +func TestStepCompactDisk_skip(t *testing.T) { + state := testState(t) + step := new(StepCompactDisk) + step.SkipCompaction = true + + // Set up the path to the tmp directory used to store VM files + state.Put("packerTempDir", "foopath") + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatalf("Should NOT have error") + } + + // Test the driver + if driver.CompactDisks_Called { + t.Fatal("Should NOT have called CompactDisks") + } +} diff --git a/builder/hyperv/common/step_test.go b/builder/hyperv/common/step_test.go new file mode 100644 index 000000000..c8a12abb6 --- /dev/null +++ b/builder/hyperv/common/step_test.go @@ -0,0 +1,19 @@ +package common + +import ( + "bytes" + "testing" + + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +func testState(t *testing.T) multistep.StateBag { + state := new(multistep.BasicStateBag) + state.Put("driver", new(DriverMock)) + state.Put("ui", &packer.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + }) + return state +} From 35b4e87c423cf0c203b8d61be0e37c4ce47298e1 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 7 Jul 2018 17:35:15 +0100 Subject: [PATCH 08/33] Add tests for export VM step --- builder/hyperv/common/step_export_vm_test.go | 92 ++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 builder/hyperv/common/step_export_vm_test.go diff --git a/builder/hyperv/common/step_export_vm_test.go b/builder/hyperv/common/step_export_vm_test.go new file mode 100644 index 000000000..75b232b45 --- /dev/null +++ b/builder/hyperv/common/step_export_vm_test.go @@ -0,0 +1,92 @@ +package common + +import ( + "context" + "path/filepath" + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepExportVm_impl(t *testing.T) { + var _ multistep.Step = new(StepExportVm) +} + +func TestStepExportVm(t *testing.T) { + state := testState(t) + step := new(StepExportVm) + + // ExportVirtualMachine needs the VM name and a path to export to + vmName := "foo" + state.Put("vmName", vmName) + outputDir := "foopath" + step.OutputDir = outputDir + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + // Test the driver + if !driver.ExportVirtualMachine_Called { + t.Fatal("Should have called ExportVirtualMachine") + } + if driver.ExportVirtualMachine_Path != outputDir { + t.Fatalf("Should call with correct path. Got: %s Wanted: %s", + driver.ExportVirtualMachine_Path, outputDir) + } + if driver.ExportVirtualMachine_VmName != vmName { + t.Fatalf("Should call with correct vm name. Got: %s Wanted: %s", + driver.ExportVirtualMachine_VmName, vmName) + } + + if !driver.PreserveLegacyExportBehaviour_Called { + t.Fatal("Should have called PreserveLegacyExportBehaviour") + } + exportPath := filepath.Join(outputDir, vmName) + if driver.PreserveLegacyExportBehaviour_SrcPath != exportPath { + t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s", + driver.PreserveLegacyExportBehaviour_SrcPath, exportPath) + } + if driver.PreserveLegacyExportBehaviour_DstPath != outputDir { + t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s", + driver.PreserveLegacyExportBehaviour_DstPath, outputDir) + } + +} + +func TestStepExportVm_skip(t *testing.T) { + state := testState(t) + step := new(StepExportVm) + step.SkipExport = true + + // ExportVirtualMachine needs the VM name and a path to export to + vmName := "foo" + state.Put("vmName", vmName) + outputDir := "foopath" + step.OutputDir = outputDir + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatalf("Should NOT have error") + } + + // Test the driver + if driver.ExportVirtualMachine_Called { + t.Fatal("Should not have called ExportVirtualMachine") + } + + if driver.PreserveLegacyExportBehaviour_Called { + t.Fatal("Should NOT have called PreserveLegacyExportBehaviour") + } +} From d5d82c32b21aa5f524645ec6a8ab119623f88368 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 7 Jul 2018 22:52:02 +0100 Subject: [PATCH 09/33] Changes to the export process have made 'inline' build of disks redundant PR #5631 introduced code to build/create disks directly in the output directory if `skip_export` was set in an attempt to optimise the build process. These are no longer required. --- builder/hyperv/common/step_create_vm.go | 7 ------- builder/hyperv/iso/builder.go | 2 -- 2 files changed, 9 deletions(-) diff --git a/builder/hyperv/common/step_create_vm.go b/builder/hyperv/common/step_create_vm.go index 611b637b9..255eef558 100644 --- a/builder/hyperv/common/step_create_vm.go +++ b/builder/hyperv/common/step_create_vm.go @@ -32,8 +32,6 @@ type StepCreateVM struct { AdditionalDiskSize []uint DifferencingDisk bool MacAddress string - SkipExport bool - OutputDir string FixedVHD bool } @@ -59,11 +57,6 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste vhdPath := state.Get("packerVhdTempDir").(string) - // inline vhd path if export is skipped - if s.SkipExport { - vhdPath = filepath.Join(s.OutputDir, "Virtual Hard Disks") - } - // convert the MB to bytes ramSize := int64(s.RamSize * 1024 * 1024) diskSize := int64(s.DiskSize * 1024 * 1024) diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 154161a83..78425dc33 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -399,8 +399,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe EnableVirtualizationExtensions: b.config.EnableVirtualizationExtensions, AdditionalDiskSize: b.config.AdditionalDiskSize, DifferencingDisk: b.config.DifferencingDisk, - SkipExport: b.config.SkipExport, - OutputDir: b.config.OutputDir, MacAddress: b.config.MacAddress, FixedVHD: b.config.FixedVHD, }, From c2437ba592e11167054ffd2b391514e285346d03 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 11:16:02 +0100 Subject: [PATCH 10/33] Remove the option to place the VHD files in a separate directory The export process now exports the VM directly from the build directory into the output directory. There are no intermediate steps or copying of files involved. This means that there is no longer any benefit in having a separate directory to house the VHD files - see #5206 for the reasoning behind the introduction of this feature. If a user wishes to house the build files on a separate disk from the output directory (perhaps for performance reasons or due to disk space limitations) they can still do so through the use of `temp_path`. --- builder/hyperv/common/driver.go | 2 +- builder/hyperv/common/driver_mock.go | 4 +- builder/hyperv/common/driver_ps_4.go | 4 +- builder/hyperv/common/step_create_tempdir.go | 59 ++++++-------------- builder/hyperv/common/step_create_vm.go | 6 +- builder/hyperv/iso/builder.go | 8 +-- common/powershell/hyperv/hyperv.go | 14 ++--- 7 files changed, 32 insertions(+), 65 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index de3ba2651..c1bceed27 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -70,7 +70,7 @@ type Driver interface { DeleteVirtualSwitch(string) error - CreateVirtualMachine(string, string, string, string, int64, int64, int64, string, uint, bool, bool) error + CreateVirtualMachine(string, string, string, int64, int64, int64, string, uint, bool, bool) error AddVirtualMachineHardDrive(string, string, string, int64, int64, string) error diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 8e5a52cbd..35dc5ced1 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -124,7 +124,6 @@ type DriverMock struct { CreateVirtualMachine_VmName string CreateVirtualMachine_Path string CreateVirtualMachine_HarddrivePath string - CreateVirtualMachine_VhdPath string CreateVirtualMachine_Ram int64 CreateVirtualMachine_DiskSize int64 CreateVirtualMachine_DiskBlockSize int64 @@ -402,12 +401,11 @@ func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, v return d.AddVirtualMachineHardDrive_Err } -func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddrivePath string, vhdPath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { +func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { d.CreateVirtualMachine_Called = true d.CreateVirtualMachine_VmName = vmName d.CreateVirtualMachine_Path = path d.CreateVirtualMachine_HarddrivePath = harddrivePath - d.CreateVirtualMachine_VhdPath = vhdPath d.CreateVirtualMachine_Ram = ram d.CreateVirtualMachine_DiskSize = diskSize d.CreateVirtualMachine_DiskBlockSize = diskBlockSize diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index acf399210..ff9ef1407 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -179,8 +179,8 @@ func (d *HypervPS4Driver) AddVirtualMachineHardDrive(vmName string, vhdFile stri return hyperv.AddVirtualMachineHardDiskDrive(vmName, vhdFile, vhdName, vhdSizeBytes, diskBlockSize, controllerType) } -func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, harddrivePath string, vhdPath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { - return hyperv.CreateVirtualMachine(vmName, path, harddrivePath, vhdPath, ram, diskSize, diskBlockSize, switchName, generation, diffDisks, fixedVHD) +func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { + return hyperv.CreateVirtualMachine(vmName, path, harddrivePath, ram, diskSize, diskBlockSize, switchName, generation, diffDisks, fixedVHD) } func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { diff --git a/builder/hyperv/common/step_create_tempdir.go b/builder/hyperv/common/step_create_tempdir.go index b445a4702..6692afad4 100644 --- a/builder/hyperv/common/step_create_tempdir.go +++ b/builder/hyperv/common/step_create_tempdir.go @@ -11,14 +11,18 @@ import ( ) type StepCreateTempDir struct { - // The user-supplied root directories into which we create subdirectories. - TempPath string - VhdTempPath string - // The subdirectories with the randomly generated name. - dirPath string - vhdDirPath string + // User supplied directory under which we create the main build + // directory. The build directory is used to house the VM files and + // folders during the build. If unspecified the default temp directory + // for the OS is used + TempPath string + // The full path to the build directory. This is concatenation of + // TempPath plus a directory uniquely named for the build + dirPath string } +// Creates the main directory used to house the VMs files and folders +// during the build func (s *StepCreateTempDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packer.Ui) @@ -39,47 +43,20 @@ func (s *StepCreateTempDir) Run(_ context.Context, state multistep.StateBag) mul s.dirPath = packerTempDir state.Put("packerTempDir", packerTempDir) - if s.VhdTempPath == "" { - // Fall back to regular temp dir if no separate VHD temp dir set. - state.Put("packerVhdTempDir", packerTempDir) - } else { - packerVhdTempDir, err := ioutil.TempDir(s.VhdTempPath, "packerhv-vhd") - if err != nil { - err := fmt.Errorf("Error creating temporary VHD directory: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } - - s.vhdDirPath = packerVhdTempDir - state.Put("packerVhdTempDir", packerVhdTempDir) - } - - // ui.Say("packerTempDir = '" + packerTempDir + "'") - return multistep.ActionContinue } +// Cleanup removes the build directory func (s *StepCreateTempDir) Cleanup(state multistep.StateBag) { - ui := state.Get("ui").(packer.Ui) - - if s.dirPath != "" { - ui.Say("Deleting temporary directory...") - - err := os.RemoveAll(s.dirPath) - - if err != nil { - ui.Error(fmt.Sprintf("Error deleting temporary directory: %s", err)) - } + if s.dirPath == "" { + return } - if s.vhdDirPath != "" && s.dirPath != s.vhdDirPath { - ui.Say("Deleting temporary VHD directory...") + ui := state.Get("ui").(packer.Ui) + ui.Say("Deleting temporary directory...") - err := os.RemoveAll(s.vhdDirPath) - - if err != nil { - ui.Error(fmt.Sprintf("Error deleting temporary VHD directory: %s", err)) - } + err := os.RemoveAll(s.dirPath) + if err != nil { + ui.Error(fmt.Sprintf("Error deleting temporary directory: %s", err)) } } diff --git a/builder/hyperv/common/step_create_vm.go b/builder/hyperv/common/step_create_vm.go index 255eef558..9987c50b4 100644 --- a/builder/hyperv/common/step_create_vm.go +++ b/builder/hyperv/common/step_create_vm.go @@ -55,14 +55,12 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste log.Println("No existing virtual harddrive, not attaching.") } - vhdPath := state.Get("packerVhdTempDir").(string) - // convert the MB to bytes ramSize := int64(s.RamSize * 1024 * 1024) diskSize := int64(s.DiskSize * 1024 * 1024) diskBlockSize := int64(s.DiskBlockSize * 1024 * 1024) - err := driver.CreateVirtualMachine(s.VMName, path, harddrivePath, vhdPath, ramSize, diskSize, diskBlockSize, s.SwitchName, s.Generation, s.DifferencingDisk, s.FixedVHD) + err := driver.CreateVirtualMachine(s.VMName, path, harddrivePath, ramSize, diskSize, diskBlockSize, s.SwitchName, s.Generation, s.DifferencingDisk, s.FixedVHD) if err != nil { err := fmt.Errorf("Error creating virtual machine: %s", err) state.Put("error", err) @@ -121,7 +119,7 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste for index, size := range s.AdditionalDiskSize { diskSize := int64(size * 1024 * 1024) diskFile := fmt.Sprintf("%s-%d.vhdx", s.VMName, index) - err = driver.AddVirtualMachineHardDrive(s.VMName, vhdPath, diskFile, diskSize, diskBlockSize, "SCSI") + err = driver.AddVirtualMachineHardDrive(s.VMName, path, diskFile, diskSize, diskBlockSize, "SCSI") if err != nil { err := fmt.Errorf("Error creating and attaching additional disk drive: %s", err) state.Put("error", err) diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 78425dc33..e9ed68216 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -96,11 +96,6 @@ type Config struct { EnableVirtualizationExtensions bool `mapstructure:"enable_virtualization_extensions"` TempPath string `mapstructure:"temp_path"` - // A separate path can be used for storing the VM's disk image. The purpose is to enable - // reading and writing to take place on different physical disks (read from VHD temp path - // write to regular temp path while exporting the VM) to eliminate a single-disk bottleneck. - VhdTempPath string `mapstructure:"vhd_temp_path"` - Communicator string `mapstructure:"communicator"` AdditionalDiskSize []uint `mapstructure:"disk_additional_size"` @@ -356,8 +351,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe steps := []multistep.Step{ &hypervcommon.StepCreateTempDir{ - TempPath: b.config.TempPath, - VhdTempPath: b.config.VhdTempPath, + TempPath: b.config.TempPath, }, &hypervcommon.StepOutputDir{ Force: b.config.PackerForce, diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 055d12706..e21513c6d 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -198,13 +198,13 @@ Hyper-V\Set-VMFloppyDiskDrive -VMName $vmName -Path $null return err } -func CreateVirtualMachine(vmName string, path string, harddrivePath string, vhdRoot string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { +func CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { if generation == 2 { var script = ` -param([string]$vmName, [string]$path, [string]$harddrivePath, [string]$vhdRoot, [long]$memoryStartupBytes, [long]$newVHDSizeBytes, [long]$vhdBlockSizeBytes, [string]$switchName, [int]$generation, [string]$diffDisks) +param([string]$vmName, [string]$path, [string]$harddrivePath, [long]$memoryStartupBytes, [long]$newVHDSizeBytes, [long]$vhdBlockSizeBytes, [string]$switchName, [int]$generation, [string]$diffDisks) $vhdx = $vmName + '.vhdx' -$vhdPath = Join-Path -Path $vhdRoot -ChildPath $vhdx +$vhdPath = Join-Path -Path $path -ChildPath $vhdx if ($harddrivePath){ if($diffDisks -eq "true"){ New-VHD -Path $vhdPath -ParentPath $harddrivePath -Differencing -BlockSizeBytes $vhdBlockSizeBytes @@ -218,21 +218,21 @@ if ($harddrivePath){ } ` var ps powershell.PowerShellCmd - if err := ps.Run(script, vmName, path, harddrivePath, vhdRoot, strconv.FormatInt(ram, 10), strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), switchName, strconv.FormatInt(int64(generation), 10), strconv.FormatBool(diffDisks)); err != nil { + if err := ps.Run(script, vmName, path, harddrivePath, strconv.FormatInt(ram, 10), strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), switchName, strconv.FormatInt(int64(generation), 10), strconv.FormatBool(diffDisks)); err != nil { return err } return DisableAutomaticCheckpoints(vmName) } else { var script = ` -param([string]$vmName, [string]$path, [string]$harddrivePath, [string]$vhdRoot, [long]$memoryStartupBytes, [long]$newVHDSizeBytes, [long]$vhdBlockSizeBytes, [string]$switchName, [string]$diffDisks, [string]$fixedVHD) +param([string]$vmName, [string]$path, [string]$harddrivePath, [long]$memoryStartupBytes, [long]$newVHDSizeBytes, [long]$vhdBlockSizeBytes, [string]$switchName, [string]$diffDisks, [string]$fixedVHD) if($fixedVHD -eq "true"){ $vhdx = $vmName + '.vhd' } else{ $vhdx = $vmName + '.vhdx' } -$vhdPath = Join-Path -Path $vhdRoot -ChildPath $vhdx +$vhdPath = Join-Path -Path $path -ChildPath $vhdx if ($harddrivePath){ if($diffDisks -eq "true"){ New-VHD -Path $vhdPath -ParentPath $harddrivePath -Differencing -BlockSizeBytes $vhdBlockSizeBytes @@ -252,7 +252,7 @@ if ($harddrivePath){ } ` var ps powershell.PowerShellCmd - if err := ps.Run(script, vmName, path, harddrivePath, vhdRoot, strconv.FormatInt(ram, 10), strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), switchName, strconv.FormatBool(diffDisks), strconv.FormatBool(fixedVHD)); err != nil { + if err := ps.Run(script, vmName, path, harddrivePath, strconv.FormatInt(ram, 10), strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), switchName, strconv.FormatBool(diffDisks), strconv.FormatBool(fixedVHD)); err != nil { return err } From bd5692451b0987d4d28512ab4ecc10d12be2cdc3 Mon Sep 17 00:00:00 2001 From: DanHam Date: Mon, 9 Jul 2018 12:20:04 +0100 Subject: [PATCH 11/33] Remove deprecated `vhd_temp_path` option from documentation --- website/source/docs/builders/hyperv-iso.html.md.erb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/website/source/docs/builders/hyperv-iso.html.md.erb b/website/source/docs/builders/hyperv-iso.html.md.erb index 437076663..0eb136f8f 100644 --- a/website/source/docs/builders/hyperv-iso.html.md.erb +++ b/website/source/docs/builders/hyperv-iso.html.md.erb @@ -266,11 +266,6 @@ builder. option is outputing a disk that is in the format required for upload to Azure. -- `vhd_temp_path` (string) - A separate path to be used for storing the VM's - disk image. The purpose is to enable reading and writing to take place on - different physical disks (read from VHD temp path, write to regular temp - path while exporting the VM) to eliminate a single-disk bottleneck. - - `vlan_id` (string) - This is the VLAN of the virtual machine's network card for the new virtual machine. By default none is set. If none is set then VLANs are not set on the virtual machine's network card. From 232dd8f0a6d59893a89337b1d6d888c47a9a1659 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 11:57:15 +0100 Subject: [PATCH 12/33] Remove code to preserve legacy export dir structure from the export step Store the export path in the state bag in preparation for use in a later step --- builder/hyperv/common/step_export_vm.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/builder/hyperv/common/step_export_vm.go b/builder/hyperv/common/step_export_vm.go index 89aee37f0..6b543407d 100644 --- a/builder/hyperv/common/step_export_vm.go +++ b/builder/hyperv/common/step_export_vm.go @@ -42,20 +42,9 @@ func (s *StepExportVm) Run(_ context.Context, state multistep.StateBag) multiste return multistep.ActionHalt } - // Shuffle around the exported folders to maintain backwards - // compatibility. This moves the 'Snapshots', 'Virtual Hard Disks' and - // 'Virtual Machines' directories from / so - // they appear directly under . The empty '/' directory is removed when complete. - // The 'Snapshots' folder will not be moved into the output directory - // if it is empty. + // Store the path to the export directory for later steps exportPath := filepath.Join(s.OutputDir, vmName) - err = driver.PreserveLegacyExportBehaviour(exportPath, s.OutputDir) - if err != nil { - // No need to halt here; Just warn the user instead - err = fmt.Errorf("WARNING: Error restoring legacy export dir structure: %s", err) - ui.Error(err.Error()) - } + state.Put("export_path", exportPath) return multistep.ActionContinue } From ee7fa27ada36202b801dbe21109c4a8029b1e3f4 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 12:51:49 +0100 Subject: [PATCH 13/33] Fix up tests. Ensure export step stores the export path in state --- builder/hyperv/common/step_export_vm_test.go | 25 ++++++++------------ 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/builder/hyperv/common/step_export_vm_test.go b/builder/hyperv/common/step_export_vm_test.go index 75b232b45..81cc34af9 100644 --- a/builder/hyperv/common/step_export_vm_test.go +++ b/builder/hyperv/common/step_export_vm_test.go @@ -45,19 +45,13 @@ func TestStepExportVm(t *testing.T) { driver.ExportVirtualMachine_VmName, vmName) } - if !driver.PreserveLegacyExportBehaviour_Called { - t.Fatal("Should have called PreserveLegacyExportBehaviour") + // Test we stored the export path in the statebag and it is correct + expectedPath := filepath.Join(outputDir, vmName) + if exportPath, ok := state.GetOk("export_path"); !ok { + t.Fatal("Should set export_path") + } else if exportPath != expectedPath { + t.Fatalf("Bad path stored for export_path. Got: %#v Wanted: %#v", exportPath, expectedPath) } - exportPath := filepath.Join(outputDir, vmName) - if driver.PreserveLegacyExportBehaviour_SrcPath != exportPath { - t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s", - driver.PreserveLegacyExportBehaviour_SrcPath, exportPath) - } - if driver.PreserveLegacyExportBehaviour_DstPath != outputDir { - t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s", - driver.PreserveLegacyExportBehaviour_DstPath, outputDir) - } - } func TestStepExportVm_skip(t *testing.T) { @@ -83,10 +77,11 @@ func TestStepExportVm_skip(t *testing.T) { // Test the driver if driver.ExportVirtualMachine_Called { - t.Fatal("Should not have called ExportVirtualMachine") + t.Fatal("Should NOT have called ExportVirtualMachine") } - if driver.PreserveLegacyExportBehaviour_Called { - t.Fatal("Should NOT have called PreserveLegacyExportBehaviour") + // Should not store the export path in the statebag + if _, ok := state.GetOk("export_path"); ok { + t.Fatal("Should NOT have stored export_path in the statebag") } } From 32148168bd3bc1e559c078c6262bbf0ad7da32dd Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 14:12:32 +0100 Subject: [PATCH 14/33] Introduce a new step to collate build artifact at the end of the build The new step collects together all the required build artifacts and places them in the output directory. * Reintroduce/add the code removed from step export to preserve the legacy export directory structure when skip_export is unset/false * Add a place holder for a future function that will move just the VHD files from the build directory to the output directory when skip_export is true * Add tests for current functionality and placeholder tests for future functions --- .../hyperv/common/step_collate_artifacts.go | 57 +++++++++++++ .../common/step_collate_artifacts_test.go | 82 +++++++++++++++++++ builder/hyperv/iso/builder.go | 4 + builder/hyperv/vmcx/builder.go | 4 + 4 files changed, 147 insertions(+) create mode 100644 builder/hyperv/common/step_collate_artifacts.go create mode 100644 builder/hyperv/common/step_collate_artifacts_test.go diff --git a/builder/hyperv/common/step_collate_artifacts.go b/builder/hyperv/common/step_collate_artifacts.go new file mode 100644 index 000000000..4f506218b --- /dev/null +++ b/builder/hyperv/common/step_collate_artifacts.go @@ -0,0 +1,57 @@ +package common + +import ( + "context" + "fmt" + + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +type StepCollateArtifacts struct { + OutputDir string + SkipExport bool +} + +// Runs the step required to collate all build artifacts under the +// specified output directory +func (s *StepCollateArtifacts) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { + driver := state.Get("driver").(Driver) + ui := state.Get("ui").(packer.Ui) + + ui.Say("Collating build artifacts...") + + if s.SkipExport { + // If the user has chosen to skip a full export of the VM the only + // artifacts that they are interested in will be the VHDs + + // TODO: Grab the disks from the build directory and place them in + // a folder named 'Virtual Hard Disks' under the output directory + } else { + // Get the full path to the export directory from the statebag + var exportPath string + if v, ok := state.GetOk("export_path"); ok { + exportPath = v.(string) + } + // The export process exports the VM into a folder named 'vm name' + // under the output directory. However, to maintain backwards + // compatibility, we now need to shuffle around the exported folders + // so the 'Snapshots', 'Virtual Hard Disks' and 'Virtual Machines' + // directories appear *directly* under . + // The empty '/' directory is removed + // when complete. + // The 'Snapshots' folder will not be moved into the output + // directory if it is empty. + err := driver.PreserveLegacyExportBehaviour(exportPath, s.OutputDir) + if err != nil { + // No need to halt here; Just warn the user instead + err = fmt.Errorf("WARNING: Error restoring legacy export dir structure: %s", err) + ui.Error(err.Error()) + } + } + + return multistep.ActionContinue +} + +// Cleanup does nothing +func (s *StepCollateArtifacts) Cleanup(state multistep.StateBag) {} diff --git a/builder/hyperv/common/step_collate_artifacts_test.go b/builder/hyperv/common/step_collate_artifacts_test.go new file mode 100644 index 000000000..d138fb87d --- /dev/null +++ b/builder/hyperv/common/step_collate_artifacts_test.go @@ -0,0 +1,82 @@ +package common + +import ( + "context" + "path/filepath" + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepCollateArtifacts_impl(t *testing.T) { + var _ multistep.Step = new(StepCollateArtifacts) +} + +func TestStepCollateArtifacts_exportedArtifacts(t *testing.T) { + state := testState(t) + step := new(StepCollateArtifacts) + + step.OutputDir = "foopath" + vmName := "foo" + + // Uses export path from the state bag + exportPath := filepath.Join(step.OutputDir, vmName) + state.Put("export_path", exportPath) + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + // Test the driver + if !driver.PreserveLegacyExportBehaviour_Called { + t.Fatal("Should have called PreserveLegacyExportBehaviour") + } + if driver.PreserveLegacyExportBehaviour_SrcPath != exportPath { + t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s", + driver.PreserveLegacyExportBehaviour_SrcPath, exportPath) + } + if driver.PreserveLegacyExportBehaviour_DstPath != step.OutputDir { + t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s", + driver.PreserveLegacyExportBehaviour_DstPath, step.OutputDir) + } + + // TODO: Create MoveCreatedVHDsToOutput func etc + // if driver.MoveCreatedVHDsToOutput_Called { + // t.Fatal("Should NOT have called MoveCreatedVHDsToOutput") + // } +} + +func TestStepCollateArtifacts_skipExportedArtifacts(t *testing.T) { + state := testState(t) + step := new(StepCollateArtifacts) + + // TODO: Needs the path to the main output directory + // outputDir := "foopath" + // Export has been skipped + step.SkipExport = true + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + // TODO: Create MoveCreatedVHDsToOutput func etc + // if !driver.MoveCreatedVHDsToOutput_Called { + // t.Fatal("Should have called MoveCreatedVHDsToOutput") + // } + + if driver.PreserveLegacyExportBehaviour_Called { + t.Fatal("Should NOT have called PreserveLegacyExportBehaviour") + } +} diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index e9ed68216..f6627b8fe 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -465,6 +465,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe OutputDir: b.config.OutputDir, SkipExport: b.config.SkipExport, }, + &hypervcommon.StepCollateArtifacts{ + OutputDir: b.config.OutputDir, + SkipExport: b.config.SkipExport, + }, // the clean up actions for each step will be executed reverse order } diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 4a95a80dc..ada64faf0 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -482,6 +482,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe OutputDir: b.config.OutputDir, SkipExport: b.config.SkipExport, }, + &hypervcommon.StepCollateArtifacts{ + OutputDir: b.config.OutputDir, + SkipExport: b.config.SkipExport, + }, // the clean up actions for each step will be executed reverse order ) From 0a4ec13323d2baf8c6f337f4f5a4046ad3def59d Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 17:12:51 +0100 Subject: [PATCH 15/33] Tests for func to move VHDs to output dir when skip_export: true --- builder/hyperv/common/driver_mock.go | 12 +++++++ .../common/step_collate_artifacts_test.go | 32 ++++++++++++------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 35dc5ced1..e3ddada15 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -190,6 +190,11 @@ type DriverMock struct { PreserveLegacyExportBehaviour_DstPath string PreserveLegacyExportBehaviour_Err error + MoveCreatedVHDsToOutputDir_Called bool + MoveCreatedVHDsToOutputDir_SrcPath string + MoveCreatedVHDsToOutputDir_DstPath string + MoveCreatedVHDsToOutputDir_Err error + CompactDisks_Called bool CompactDisks_Path string CompactDisks_Result string @@ -492,6 +497,13 @@ func (d *DriverMock) PreserveLegacyExportBehaviour(srcPath string, dstPath strin return d.PreserveLegacyExportBehaviour_Err } +func (d *DriverMock) MoveCreatedVHDsToOutputDir(srcPath string, dstPath string) error { + d.MoveCreatedVHDsToOutputDir_Called = true + d.MoveCreatedVHDsToOutputDir_SrcPath = srcPath + d.MoveCreatedVHDsToOutputDir_DstPath = dstPath + return d.MoveCreatedVHDsToOutputDir_Err +} + func (d *DriverMock) CompactDisks(path string) (result string, err error) { d.CompactDisks_Called = true d.CompactDisks_Path = path diff --git a/builder/hyperv/common/step_collate_artifacts_test.go b/builder/hyperv/common/step_collate_artifacts_test.go index d138fb87d..4892e76e8 100644 --- a/builder/hyperv/common/step_collate_artifacts_test.go +++ b/builder/hyperv/common/step_collate_artifacts_test.go @@ -46,18 +46,20 @@ func TestStepCollateArtifacts_exportedArtifacts(t *testing.T) { driver.PreserveLegacyExportBehaviour_DstPath, step.OutputDir) } - // TODO: Create MoveCreatedVHDsToOutput func etc - // if driver.MoveCreatedVHDsToOutput_Called { - // t.Fatal("Should NOT have called MoveCreatedVHDsToOutput") - // } + // Should only be called when skip_export is true + if driver.MoveCreatedVHDsToOutputDir_Called { + t.Fatal("Should NOT have called MoveCreatedVHDsToOutputDir") + } } -func TestStepCollateArtifacts_skipExportedArtifacts(t *testing.T) { +func TestStepCollateArtifacts_skipExportArtifacts(t *testing.T) { state := testState(t) step := new(StepCollateArtifacts) - // TODO: Needs the path to the main output directory - // outputDir := "foopath" + // Needs the path to the main output directory and build directory + step.OutputDir = "foopath" + packerTempDir := "fooBuildPath" + state.Put("packerTempDir", packerTempDir) // Export has been skipped step.SkipExport = true @@ -71,10 +73,18 @@ func TestStepCollateArtifacts_skipExportedArtifacts(t *testing.T) { t.Fatal("Should NOT have error") } - // TODO: Create MoveCreatedVHDsToOutput func etc - // if !driver.MoveCreatedVHDsToOutput_Called { - // t.Fatal("Should have called MoveCreatedVHDsToOutput") - // } + // Test the driver + if !driver.MoveCreatedVHDsToOutputDir_Called { + t.Fatal("Should have called MoveCreatedVHDsToOutputDir") + } + if driver.MoveCreatedVHDsToOutputDir_SrcPath != packerTempDir { + t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s", + driver.MoveCreatedVHDsToOutputDir_SrcPath, packerTempDir) + } + if driver.MoveCreatedVHDsToOutputDir_DstPath != step.OutputDir { + t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s", + driver.MoveCreatedVHDsToOutputDir_DstPath, step.OutputDir) + } if driver.PreserveLegacyExportBehaviour_Called { t.Fatal("Should NOT have called PreserveLegacyExportBehaviour") From 181bb0ba23a4d7f532f3b06fa7444c1d80101c90 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 17:22:27 +0100 Subject: [PATCH 16/33] Add calling code and skeleton driver to make tests pass --- builder/hyperv/common/driver.go | 2 ++ builder/hyperv/common/driver_ps_4.go | 6 ++++++ builder/hyperv/common/step_collate_artifacts.go | 15 ++++++++++++--- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index c1bceed27..0001bd989 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -96,6 +96,8 @@ type Driver interface { PreserveLegacyExportBehaviour(string, string) error + MoveCreatedVHDsToOutputDir(string, string) error + CompactDisks(string) (string, error) RestartVirtualMachine(string) error diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index ff9ef1407..59eea7673 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -223,6 +223,12 @@ func (d *HypervPS4Driver) PreserveLegacyExportBehaviour(srcPath string, dstPath return hyperv.PreserveLegacyExportBehaviour(srcPath, dstPath) } +func (d *HypervPS4Driver) MoveCreatedVHDsToOutputDir(srcPath string, dstPath string) error { + // Not implemented yet + err := fmt.Errorf("Not implemented yet") + return err +} + func (d *HypervPS4Driver) CompactDisks(path string) (result string, err error) { return hyperv.CompactDisks(path) } diff --git a/builder/hyperv/common/step_collate_artifacts.go b/builder/hyperv/common/step_collate_artifacts.go index 4f506218b..5942c73ba 100644 --- a/builder/hyperv/common/step_collate_artifacts.go +++ b/builder/hyperv/common/step_collate_artifacts.go @@ -22,11 +22,20 @@ func (s *StepCollateArtifacts) Run(_ context.Context, state multistep.StateBag) ui.Say("Collating build artifacts...") if s.SkipExport { + // Get the path to the main build directory from the statebag + var packerTempDir string + if v, ok := state.GetOk("packerTempDir"); ok { + packerTempDir = v.(string) + } // If the user has chosen to skip a full export of the VM the only // artifacts that they are interested in will be the VHDs - - // TODO: Grab the disks from the build directory and place them in - // a folder named 'Virtual Hard Disks' under the output directory + err := driver.MoveCreatedVHDsToOutputDir(packerTempDir, s.OutputDir) + if err != nil { + err = fmt.Errorf("Error moving VHDs from build dir to output dir: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } } else { // Get the full path to the export directory from the statebag var exportPath string From d2390f464db52451b901954d4d2972025e37962c Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 17:48:23 +0100 Subject: [PATCH 17/33] Actually implement the function for the driver --- builder/hyperv/common/driver_ps_4.go | 4 +- .../hyperv/common/step_collate_artifacts.go | 5 +- common/powershell/hyperv/hyperv.go | 48 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index 59eea7673..c07473447 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -224,9 +224,7 @@ func (d *HypervPS4Driver) PreserveLegacyExportBehaviour(srcPath string, dstPath } func (d *HypervPS4Driver) MoveCreatedVHDsToOutputDir(srcPath string, dstPath string) error { - // Not implemented yet - err := fmt.Errorf("Not implemented yet") - return err + return hyperv.MoveCreatedVHDsToOutputDir(srcPath, dstPath) } func (d *HypervPS4Driver) CompactDisks(path string) (result string, err error) { diff --git a/builder/hyperv/common/step_collate_artifacts.go b/builder/hyperv/common/step_collate_artifacts.go index 5942c73ba..d98b42b38 100644 --- a/builder/hyperv/common/step_collate_artifacts.go +++ b/builder/hyperv/common/step_collate_artifacts.go @@ -28,7 +28,10 @@ func (s *StepCollateArtifacts) Run(_ context.Context, state multistep.StateBag) packerTempDir = v.(string) } // If the user has chosen to skip a full export of the VM the only - // artifacts that they are interested in will be the VHDs + // artifacts that they are interested in will be the VHDs. The + // called function searches for all disks under the given source + // directory and moves them to a 'Virtual Hard Disks' folder under + // the destination directory err := driver.MoveCreatedVHDsToOutputDir(packerTempDir, s.OutputDir) if err != nil { err = fmt.Errorf("Error moving VHDs from build dir to output dir: %s", err) diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index e21513c6d..8b267c8c5 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -721,6 +721,54 @@ if ( $((Get-Item $srcPath).GetFileSystemInfos().Count) -eq 0 ) { return err } +func MoveCreatedVHDsToOutputDir(srcPath, dstPath string) error { + + var script = ` +param([string]$srcPath, [string]$dstPath) + +# Validate the paths returning an error if the supplied path is empty +# or if the paths don't exist +$srcPath, $dstPath | % { + if ($_) { + if (! (Test-Path $_)) { + [System.Console]::Error.WriteLine("Path $_ does not exist") + exit + } + } else { + [System.Console]::Error.WriteLine("A supplied path is empty") + exit + } +} + +# Convert to absolute paths if required +$srcPathAbs = (Get-Item($srcPath)).FullName +$dstPathAbs = (Get-Item($dstPath)).FullName + +# Get the full path to all disks under the directory or exit if none are found +$disks = Get-ChildItem -Path $srcPathAbs -Recurse -Filter *.vhd* -ErrorAction SilentlyContinue | % { $_.FullName } +if ($disks.Length -eq 0) { + [System.Console]::Error.WriteLine("No disks found under $srcPathAbs") + exit +} + +# Set up directory for VHDs in the destination directory +$vhdDstDir = Join-Path -Path $dstPathAbs -ChildPath 'Virtual Hard Disks' +if (! (Test-Path $vhdDstDir)) { + New-Item -ItemType Directory -Force -Path $vhdDstDir +} + +# Move the disks +foreach ($disk in $disks) { + Move-Item -Path $disk -Destination $vhdDstDir +} +` + + var ps powershell.PowerShellCmd + err := ps.Run(script, srcPath, dstPath) + + return err +} + func CompactDisks(path string) (result string, err error) { var script = ` param([string]$srcPath) From b386e567db37fce7dfde29c14b2b0aaaaa8775cd Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 22:16:40 +0100 Subject: [PATCH 18/33] Change filename to better illustrate purpose of step --- .../common/{step_create_tempdir.go => step_create_build_dir.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename builder/hyperv/common/{step_create_tempdir.go => step_create_build_dir.go} (100%) diff --git a/builder/hyperv/common/step_create_tempdir.go b/builder/hyperv/common/step_create_build_dir.go similarity index 100% rename from builder/hyperv/common/step_create_tempdir.go rename to builder/hyperv/common/step_create_build_dir.go From 00276f2f64b8ba348da2e7af75f79b46a69470df Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 22:24:06 +0100 Subject: [PATCH 19/33] Change step name to better illustrate purpose of step --- builder/hyperv/common/step_create_build_dir.go | 18 +++++++++--------- builder/hyperv/iso/builder.go | 2 +- builder/hyperv/vmcx/builder.go | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/builder/hyperv/common/step_create_build_dir.go b/builder/hyperv/common/step_create_build_dir.go index 6692afad4..8641ddca4 100644 --- a/builder/hyperv/common/step_create_build_dir.go +++ b/builder/hyperv/common/step_create_build_dir.go @@ -10,23 +10,23 @@ import ( "github.com/hashicorp/packer/packer" ) -type StepCreateTempDir struct { +type StepCreateBuildDir struct { // User supplied directory under which we create the main build - // directory. The build directory is used to house the VM files and + // directory. The build directory is used to house the VM files and // folders during the build. If unspecified the default temp directory // for the OS is used TempPath string - // The full path to the build directory. This is concatenation of + // The full path to the build directory. This is the concatenation of // TempPath plus a directory uniquely named for the build dirPath string } // Creates the main directory used to house the VMs files and folders // during the build -func (s *StepCreateTempDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { +func (s *StepCreateBuildDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packer.Ui) - ui.Say("Creating temporary directory...") + ui.Say("Creating build directory...") if s.TempPath == "" { s.TempPath = os.TempDir() @@ -34,7 +34,7 @@ func (s *StepCreateTempDir) Run(_ context.Context, state multistep.StateBag) mul packerTempDir, err := ioutil.TempDir(s.TempPath, "packerhv") if err != nil { - err := fmt.Errorf("Error creating temporary directory: %s", err) + err := fmt.Errorf("Error creating build directory: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -47,16 +47,16 @@ func (s *StepCreateTempDir) Run(_ context.Context, state multistep.StateBag) mul } // Cleanup removes the build directory -func (s *StepCreateTempDir) Cleanup(state multistep.StateBag) { +func (s *StepCreateBuildDir) Cleanup(state multistep.StateBag) { if s.dirPath == "" { return } ui := state.Get("ui").(packer.Ui) - ui.Say("Deleting temporary directory...") + ui.Say("Deleting build directory...") err := os.RemoveAll(s.dirPath) if err != nil { - ui.Error(fmt.Sprintf("Error deleting temporary directory: %s", err)) + ui.Error(fmt.Sprintf("Error deleting build directory: %s", err)) } } diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index f6627b8fe..fd51c85c0 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -350,7 +350,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) steps := []multistep.Step{ - &hypervcommon.StepCreateTempDir{ + &hypervcommon.StepCreateBuildDir{ TempPath: b.config.TempPath, }, &hypervcommon.StepOutputDir{ diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index ada64faf0..7ae54a18d 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -362,7 +362,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) steps := []multistep.Step{ - &hypervcommon.StepCreateTempDir{}, + &hypervcommon.StepCreateBuildDir{}, &hypervcommon.StepOutputDir{ Force: b.config.PackerForce, Path: b.config.OutputDir, From ed5bebfa8c8bf310ef27491405e26613d029f4af Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 8 Jul 2018 23:27:37 +0100 Subject: [PATCH 20/33] Change variable/statebag key name to better convey purpose --- builder/hyperv/common/step_clone_vm.go | 2 +- .../hyperv/common/step_collate_artifacts.go | 8 ++++---- .../common/step_collate_artifacts_test.go | 8 ++++---- builder/hyperv/common/step_compact_disk.go | 10 +++++----- .../hyperv/common/step_compact_disk_test.go | 14 +++++++------- builder/hyperv/common/step_create_build_dir.go | 18 +++++++++++------- builder/hyperv/common/step_create_vm.go | 5 ++++- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/builder/hyperv/common/step_clone_vm.go b/builder/hyperv/common/step_clone_vm.go index ac28f9cea..6a3d434a8 100644 --- a/builder/hyperv/common/step_clone_vm.go +++ b/builder/hyperv/common/step_clone_vm.go @@ -37,7 +37,7 @@ func (s *StepCloneVM) Run(_ context.Context, state multistep.StateBag) multistep ui := state.Get("ui").(packer.Ui) ui.Say("Cloning virtual machine...") - path := state.Get("packerTempDir").(string) + path := state.Get("build_dir").(string) // Determine if we even have an existing virtual harddrive to attach harddrivePath := "" diff --git a/builder/hyperv/common/step_collate_artifacts.go b/builder/hyperv/common/step_collate_artifacts.go index d98b42b38..2e40a532f 100644 --- a/builder/hyperv/common/step_collate_artifacts.go +++ b/builder/hyperv/common/step_collate_artifacts.go @@ -23,16 +23,16 @@ func (s *StepCollateArtifacts) Run(_ context.Context, state multistep.StateBag) if s.SkipExport { // Get the path to the main build directory from the statebag - var packerTempDir string - if v, ok := state.GetOk("packerTempDir"); ok { - packerTempDir = v.(string) + var buildDir string + if v, ok := state.GetOk("build_dir"); ok { + buildDir = v.(string) } // If the user has chosen to skip a full export of the VM the only // artifacts that they are interested in will be the VHDs. The // called function searches for all disks under the given source // directory and moves them to a 'Virtual Hard Disks' folder under // the destination directory - err := driver.MoveCreatedVHDsToOutputDir(packerTempDir, s.OutputDir) + err := driver.MoveCreatedVHDsToOutputDir(buildDir, s.OutputDir) if err != nil { err = fmt.Errorf("Error moving VHDs from build dir to output dir: %s", err) state.Put("error", err) diff --git a/builder/hyperv/common/step_collate_artifacts_test.go b/builder/hyperv/common/step_collate_artifacts_test.go index 4892e76e8..d88ca59bc 100644 --- a/builder/hyperv/common/step_collate_artifacts_test.go +++ b/builder/hyperv/common/step_collate_artifacts_test.go @@ -58,8 +58,8 @@ func TestStepCollateArtifacts_skipExportArtifacts(t *testing.T) { // Needs the path to the main output directory and build directory step.OutputDir = "foopath" - packerTempDir := "fooBuildPath" - state.Put("packerTempDir", packerTempDir) + buildDir := "fooBuildPath" + state.Put("build_dir", buildDir) // Export has been skipped step.SkipExport = true @@ -77,9 +77,9 @@ func TestStepCollateArtifacts_skipExportArtifacts(t *testing.T) { if !driver.MoveCreatedVHDsToOutputDir_Called { t.Fatal("Should have called MoveCreatedVHDsToOutputDir") } - if driver.MoveCreatedVHDsToOutputDir_SrcPath != packerTempDir { + if driver.MoveCreatedVHDsToOutputDir_SrcPath != buildDir { t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s", - driver.MoveCreatedVHDsToOutputDir_SrcPath, packerTempDir) + driver.MoveCreatedVHDsToOutputDir_SrcPath, buildDir) } if driver.MoveCreatedVHDsToOutputDir_DstPath != step.OutputDir { t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s", diff --git a/builder/hyperv/common/step_compact_disk.go b/builder/hyperv/common/step_compact_disk.go index 628703c7a..ebaa9978e 100644 --- a/builder/hyperv/common/step_compact_disk.go +++ b/builder/hyperv/common/step_compact_disk.go @@ -22,10 +22,10 @@ func (s *StepCompactDisk) Run(_ context.Context, state multistep.StateBag) multi return multistep.ActionContinue } - // Get the tmp dir used to store the VMs files during the build process - var tmpPath string - if v, ok := state.GetOk("packerTempDir"); ok { - tmpPath = v.(string) + // Get the dir used to store the VMs files during the build process + var buildDir string + if v, ok := state.GetOk("build_dir"); ok { + buildDir = v.(string) } ui.Say("Compacting disks...") @@ -33,7 +33,7 @@ func (s *StepCompactDisk) Run(_ context.Context, state multistep.StateBag) multi // path and runs the compacting process on each of them. If no disks // are found under the supplied path this is treated as a 'soft' error // and a warning message is printed. All other errors halt the build. - result, err := driver.CompactDisks(tmpPath) + result, err := driver.CompactDisks(buildDir) if err != nil { err := fmt.Errorf("Error compacting disks: %s", err) state.Put("error", err) diff --git a/builder/hyperv/common/step_compact_disk_test.go b/builder/hyperv/common/step_compact_disk_test.go index 75f3aa19d..f8943d25f 100644 --- a/builder/hyperv/common/step_compact_disk_test.go +++ b/builder/hyperv/common/step_compact_disk_test.go @@ -15,9 +15,9 @@ func TestStepCompactDisk(t *testing.T) { state := testState(t) step := new(StepCompactDisk) - // Set up the path to the tmp directory used to store VM files - tmpPath := "foopath" - state.Put("packerTempDir", tmpPath) + // Set up the path to the build directory + buildDir := "foopath" + state.Put("build_dir", buildDir) driver := state.Get("driver").(*DriverMock) @@ -33,8 +33,8 @@ func TestStepCompactDisk(t *testing.T) { if !driver.CompactDisks_Called { t.Fatal("Should have called CompactDisks") } - if driver.CompactDisks_Path != tmpPath { - t.Fatalf("Should call with correct path. Got: %s Wanted: %s", driver.CompactDisks_Path, tmpPath) + if driver.CompactDisks_Path != buildDir { + t.Fatalf("Should call with correct path. Got: %s Wanted: %s", driver.CompactDisks_Path, buildDir) } } @@ -43,8 +43,8 @@ func TestStepCompactDisk_skip(t *testing.T) { step := new(StepCompactDisk) step.SkipCompaction = true - // Set up the path to the tmp directory used to store VM files - state.Put("packerTempDir", "foopath") + // Set up the path to the build directory + state.Put("build_dir", "foopath") driver := state.Get("driver").(*DriverMock) diff --git a/builder/hyperv/common/step_create_build_dir.go b/builder/hyperv/common/step_create_build_dir.go index 8641ddca4..405a3407c 100644 --- a/builder/hyperv/common/step_create_build_dir.go +++ b/builder/hyperv/common/step_create_build_dir.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io/ioutil" + "log" "os" "github.com/hashicorp/packer/helper/multistep" @@ -18,7 +19,7 @@ type StepCreateBuildDir struct { TempPath string // The full path to the build directory. This is the concatenation of // TempPath plus a directory uniquely named for the build - dirPath string + buildDir string } // Creates the main directory used to house the VMs files and folders @@ -32,30 +33,33 @@ func (s *StepCreateBuildDir) Run(_ context.Context, state multistep.StateBag) mu s.TempPath = os.TempDir() } - packerTempDir, err := ioutil.TempDir(s.TempPath, "packerhv") + var err error + s.buildDir, err = ioutil.TempDir(s.TempPath, "packerhv") if err != nil { - err := fmt.Errorf("Error creating build directory: %s", err) + err = fmt.Errorf("Error creating build directory: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt } - s.dirPath = packerTempDir - state.Put("packerTempDir", packerTempDir) + log.Printf("Created build directory: %s", s.buildDir) + + // Record the build directory location for later steps + state.Put("build_dir", s.buildDir) return multistep.ActionContinue } // Cleanup removes the build directory func (s *StepCreateBuildDir) Cleanup(state multistep.StateBag) { - if s.dirPath == "" { + if s.buildDir == "" { return } ui := state.Get("ui").(packer.Ui) ui.Say("Deleting build directory...") - err := os.RemoveAll(s.dirPath) + err := os.RemoveAll(s.buildDir) if err != nil { ui.Error(fmt.Sprintf("Error deleting build directory: %s", err)) } diff --git a/builder/hyperv/common/step_create_vm.go b/builder/hyperv/common/step_create_vm.go index 9987c50b4..1c655becf 100644 --- a/builder/hyperv/common/step_create_vm.go +++ b/builder/hyperv/common/step_create_vm.go @@ -40,7 +40,10 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste ui := state.Get("ui").(packer.Ui) ui.Say("Creating virtual machine...") - path := state.Get("packerTempDir").(string) + var path string + if v, ok := state.GetOk("build_dir"); ok { + path = v.(string) + } // Determine if we even have an existing virtual harddrive to attach harddrivePath := "" From f027585a932fd597cbda5e607d56d40a66a8e4a0 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 18 Jul 2018 00:34:01 +0100 Subject: [PATCH 21/33] Tests for step to create build directory --- .../common/step_create_build_dir_test.go | 113 ++++++++++++++++++ builder/hyperv/common/step_test.go | 9 ++ 2 files changed, 122 insertions(+) create mode 100644 builder/hyperv/common/step_create_build_dir_test.go diff --git a/builder/hyperv/common/step_create_build_dir_test.go b/builder/hyperv/common/step_create_build_dir_test.go new file mode 100644 index 000000000..3f0fe57cc --- /dev/null +++ b/builder/hyperv/common/step_create_build_dir_test.go @@ -0,0 +1,113 @@ +package common + +import ( + "context" + "os" + "path/filepath" + "regexp" + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepCreateBuildDir_imp(t *testing.T) { + var _ multistep.Step = new(StepCreateBuildDir) +} + +func TestStepCreateBuildDir_Defaults(t *testing.T) { + state := testState(t) + step := new(StepCreateBuildDir) + + // Default is for the user not to supply value for TempPath. When + // nothing is set the step should use the OS temp directory as the root + // for the build directory + step.TempPath = "" + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + if v, ok := state.GetOk("build_dir"); !ok { + t.Fatal("Should store path to build directory in statebag as 'build_dir'") + } else { + // On windows convert everything to forward slash separated paths + // This prevents the regexp interpreting backslashes as escape sequences + stateBuildDir := filepath.ToSlash(v.(string)) + expectedBuildDirRe := regexp.MustCompile( + filepath.ToSlash(filepath.Join(os.TempDir(), "packerhv") + `[[:digit:]]{9}$`)) + match := expectedBuildDirRe.MatchString(stateBuildDir) + if !match { + t.Fatalf("Got path that doesn't match expected format in 'build_dir': %s", stateBuildDir) + } + } + + // Test Cleanup + step.Cleanup(state) + if _, err := os.Stat(step.buildDir); err == nil { + t.Fatalf("Build directory should NOT exist after Cleanup: %s", step.buildDir) + } +} + +func TestStepCreateBuildDir_UserDefinedTempPath(t *testing.T) { + state := testState(t) + step := new(StepCreateBuildDir) + + // Create a directory we'll use as the user supplied temp_path + step.TempPath = genTestDirPath("userTempDir") + err := os.Mkdir(step.TempPath, 0755) // The directory must exist + if err != nil { + t.Fatal("Error creating test directory") + } + defer os.RemoveAll(step.TempPath) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + if v, ok := state.GetOk("build_dir"); !ok { + t.Fatal("Should store path to build directory in statebag as 'build_dir'") + } else { + // On windows convert everything to forward slash separated paths + // This prevents the regexp interpreting backslashes as escape sequences + stateBuildDir := filepath.ToSlash(v.(string)) + expectedBuildDirRe := regexp.MustCompile( + filepath.ToSlash(filepath.Join(step.TempPath, "packerhv") + `[[:digit:]]{9}$`)) + match := expectedBuildDirRe.MatchString(stateBuildDir) + if !match { + t.Fatalf("Got path that doesn't match expected format in 'build_dir': %s", stateBuildDir) + } + } + + // Test Cleanup + step.Cleanup(state) + if _, err := os.Stat(step.buildDir); err == nil { + t.Fatalf("Build directory should NOT exist after Cleanup: %s", step.buildDir) + } + if _, err := os.Stat(step.TempPath); err != nil { + t.Fatal("User supplied root for build directory should NOT be deleted by Cleanup") + } +} + +func TestStepCreateBuildDir_BadTempPath(t *testing.T) { + state := testState(t) + step := new(StepCreateBuildDir) + + // Bad + step.TempPath = genTestDirPath("iDontExist") + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionHalt { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); !ok { + t.Fatal("Should have error due to bad path") + } +} diff --git a/builder/hyperv/common/step_test.go b/builder/hyperv/common/step_test.go index c8a12abb6..e5f9f3998 100644 --- a/builder/hyperv/common/step_test.go +++ b/builder/hyperv/common/step_test.go @@ -2,8 +2,11 @@ package common import ( "bytes" + "os" + "path/filepath" "testing" + "github.com/hashicorp/packer/common/uuid" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -17,3 +20,9 @@ func testState(t *testing.T) multistep.StateBag { }) return state } + +// Generates an absolute path to a directory under OS temp with a name +// beginning with prefix and a UUID appended to the end +func genTestDirPath(prefix string) string { + return filepath.Join(os.TempDir(), prefix+"-"+uuid.TimeOrderedUUID()) +} From 8032c8151a4e472bc99056f786c7031faa2ad059 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 18 Jul 2018 01:52:23 +0100 Subject: [PATCH 22/33] Tests for step to create output directory --- builder/hyperv/common/step_output_dir_test.go | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 builder/hyperv/common/step_output_dir_test.go diff --git a/builder/hyperv/common/step_output_dir_test.go b/builder/hyperv/common/step_output_dir_test.go new file mode 100644 index 000000000..5933a5fe9 --- /dev/null +++ b/builder/hyperv/common/step_output_dir_test.go @@ -0,0 +1,146 @@ +package common + +import ( + "context" + "os" + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepOutputDir_imp(t *testing.T) { + var _ multistep.Step = new(StepOutputDir) +} + +func TestStepOutputDir_Default(t *testing.T) { + state := testState(t) + step := new(StepOutputDir) + + step.Path = genTestDirPath("packerHypervOutput") + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("Should NOT have error") + } + + // The directory should have been created + if _, err := os.Stat(step.Path); err != nil { + t.Fatal("Should have created output directory") + } + + // Remove the directory created due to test + err := os.RemoveAll(step.Path) + if err != nil { + t.Fatalf("Error encountered removing directory created by test: %s", err) + } +} + +func TestStepOutputDir_DirectoryAlreadyExistsNoForce(t *testing.T) { + state := testState(t) + step := new(StepOutputDir) + + step.Path = genTestDirPath("packerHypervOutput") + + // Create the directory so that we can test + err := os.Mkdir(step.Path, 0755) + if err != nil { + t.Fatal("Test failed to create directory for test of Cancel and Cleanup") + } + defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong + + step.Force = false // Default + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionHalt { + t.Fatalf("Should halt when directory exists and 'Force' is false. Bad action: %v", action) + } + if _, ok := state.GetOk("error"); !ok { + t.Fatal("Should error when directory exists and 'Force' is false") + } +} + +func TestStepOutputDir_DirectoryAlreadyExistsForce(t *testing.T) { + state := testState(t) + step := new(StepOutputDir) + + step.Path = genTestDirPath("packerHypervOutput") + + // Create the directory so that we can test + err := os.Mkdir(step.Path, 0755) + if err != nil { + t.Fatal("Test failed to create directory for test of Cancel and Cleanup") + } + defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong + + step.Force = true // User specified that existing directory and contents should be discarded + if action := step.Run(context.Background(), state); action != multistep.ActionContinue { + t.Fatalf("Should complete when directory exists and 'Force' is true. Bad action: %v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatalf("Should NOT error when directory exists and 'Force' is true: %s", err) + } +} + +func TestStepOutputDir_CleanupBuildCancelled(t *testing.T) { + state := testState(t) + step := new(StepOutputDir) + + step.Path = genTestDirPath("packerHypervOutput") + + // Create the directory so that we can test the cleanup + err := os.Mkdir(step.Path, 0755) + if err != nil { + t.Fatal("Test failed to create directory for test of Cancel and Cleanup") + } + defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong + + // 'Cancel' the build + state.Put(multistep.StateCancelled, true) + + // Ensure the directory isn't removed if the cleanup flag is false + step.cleanup = false + step.Cleanup(state) + if _, err := os.Stat(step.Path); err != nil { + t.Fatal("Output dir should NOT be removed if on 'Cancel' if cleanup flag is unset/false") + } + + // Ensure the directory is removed if the cleanup flag is true + step.cleanup = true + step.Cleanup(state) + if _, err := os.Stat(step.Path); err == nil { + t.Fatalf("Output directory should NOT exist after 'Cancel' and Cleanup: %s", step.Path) + } +} + +func TestStepOutputDir_CleanupBuildHalted(t *testing.T) { + state := testState(t) + step := new(StepOutputDir) + + step.Path = genTestDirPath("packerHypervOutput") + + // Create the directory so that we can test the cleanup + err := os.Mkdir(step.Path, 0755) + if err != nil { + t.Fatal("Test failed to create directory for test of Cancel and Cleanup") + } + defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong + + // 'Halt' the build and test the directory is removed + state.Put(multistep.StateHalted, true) + + // Ensure the directory isn't removed if the cleanup flag is false + step.cleanup = false + step.Cleanup(state) + if _, err := os.Stat(step.Path); err != nil { + t.Fatal("Output dir should NOT be removed if on 'Halt' if cleanup flag is unset/false") + } + + // Ensure the directory is removed if the cleanup flag is true + step.cleanup = true + step.Cleanup(state) + if _, err := os.Stat(step.Path); err == nil { + t.Fatalf("Output directory should NOT exist after 'Halt' and Cleanup: %s", step.Path) + } +} From ee0a2469ebac600141c1f03c72c48d37fe488537 Mon Sep 17 00:00:00 2001 From: DanHam Date: Mon, 9 Jul 2018 12:06:45 +0100 Subject: [PATCH 23/33] Add ability to specify 'temp_path' for the build directory to VMCX builder --- builder/hyperv/vmcx/builder.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 7ae54a18d..88ebef783 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -91,6 +91,7 @@ type Config struct { EnableSecureBoot bool `mapstructure:"enable_secure_boot"` SecureBootTemplate string `mapstructure:"secure_boot_template"` EnableVirtualizationExtensions bool `mapstructure:"enable_virtualization_extensions"` + TempPath string `mapstructure:"temp_path"` Communicator string `mapstructure:"communicator"` @@ -362,7 +363,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) steps := []multistep.Step{ - &hypervcommon.StepCreateBuildDir{}, + &hypervcommon.StepCreateBuildDir{ + TempPath: b.config.TempPath, + }, &hypervcommon.StepOutputDir{ Force: b.config.PackerForce, Path: b.config.OutputDir, From e42e03ef2ebf7e24cb132e681ac5f603d6bace37 Mon Sep 17 00:00:00 2001 From: DanHam Date: Mon, 9 Jul 2018 15:56:31 +0100 Subject: [PATCH 24/33] Update VMCX doc: Include `temp_path`; ISO & VMCX docs: Better explain opts --- .../docs/builders/hyperv-iso.html.md.erb | 34 ++++++++++++------- .../docs/builders/hyperv-vmcx.html.md.erb | 23 +++++++++---- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/website/source/docs/builders/hyperv-iso.html.md.erb b/website/source/docs/builders/hyperv-iso.html.md.erb index 0eb136f8f..6960a3148 100644 --- a/website/source/docs/builders/hyperv-iso.html.md.erb +++ b/website/source/docs/builders/hyperv-iso.html.md.erb @@ -201,12 +201,13 @@ builder. the default virtual network card. The MAC address must be a string with no delimiters, for example "0000deadbeef". -- `output_directory` (string) - This is the path to the directory where the - resulting virtual machine will be created. This may be relative or - absolute. If relative, the path is relative to the working directory when - `packer` is executed. This directory must not exist or be empty prior to - running the builder. By default this is "output-BUILDNAME" where - "BUILDNAME" is the name of the build. +- `output_directory` (string) - This setting specifies the directory that + artifacts from the build, such as the virtual machine files and disks, + will be output to. The path to the directory may be relative or + absolute. If relative, the path is relative to the working directory + `packer` is executed from. This directory must not exist or, if + created, must be empty prior to running the builder. By default this is + "output-BUILDNAME" where "BUILDNAME" is the name of the build. - `ram_size` (number) - The amount, in megabytes, of RAM to assign to the VM. By default, this is 1 GB. @@ -238,11 +239,11 @@ builder. - `skip_compaction` (boolean) - If `true` skip compacting the hard disk for the virtual machine when exporting. This defaults to `false`. -- `skip_export` (boolean) - If `true` Packer will skip the export of the - VM. If you are interested only in the VHD/VHDX files, you can enable - this option. This will create inline disks which improves the build - performance. There will not be any copying of source VHDs to the temp - directory. This defaults to `false`. +- `skip_export` (boolean) - If `true` Packer will skip the export of the VM. + If you are interested only in the VHD/VHDX files, you can enable this + option. The resulting VHD/VHDX file will be output to + `/Virtual Hard Disks`. By default this option is `false` + and Packer will export the VM to `output_directory`. - `switch_name` (string) - The name of the switch to connect the virtual machine to. By default, leaving this value unset will cause Packer to @@ -254,8 +255,15 @@ builder. set on the switch's network card. If this value is set it should match the VLAN specified in by `vlan_id`. -- `temp_path` (string) - This is the temporary path in which Packer will - create the virtual machine. By default the value is the system `%temp%`. +- `temp_path` (string) - The location under which Packer will create a + directory to house all the VM files and folders during the build. + By default `%TEMP%` is used which, for most systems, will evaluate to + `%USERPROFILE%/AppData/Local/Temp`. + + The build directory housed under `temp_path` will have a name similar + to `packerhv1234567`. The seven digit number at the end of the name is + automatically generated by Packer to ensure the directory name is + unique. - `use_fixed_vhd_format` (boolean) - If true, creates the boot disk on the virtual machine as a fixed VHD format disk. The default is `false`, which diff --git a/website/source/docs/builders/hyperv-vmcx.html.md.erb b/website/source/docs/builders/hyperv-vmcx.html.md.erb index e8ffe9071..037cebb07 100644 --- a/website/source/docs/builders/hyperv-vmcx.html.md.erb +++ b/website/source/docs/builders/hyperv-vmcx.html.md.erb @@ -202,12 +202,13 @@ builder. the default virtual network card. The MAC address must be a string with no delimiters, for example "0000deadbeef". -- `output_directory` (string) - This is the path to the directory where the - resulting virtual machine will be created. This may be relative or - absolute. If relative, the path is relative to the working directory when - `packer` is executed. This directory must not exist or be empty prior to - running the builder. By default this is "output-BUILDNAME" where - "BUILDNAME" is the name of the build. +- `output_directory` (string) - This setting specifies the directory that + artifacts from the build, such as the virtual machine files and disks, + will be output to. The path to the directory may be relative or + absolute. If relative, the path is relative to the working directory + `packer` is executed from. This directory must not exist or, if + created, must be empty prior to running the builder. By default this is + "output-BUILDNAME" where "BUILDNAME" is the name of the build. - `ram_size` (number) - The amount, in megabytes, of RAM to assign to the VM. By default, this is 1 GB. @@ -255,6 +256,16 @@ builder. set on the switch's network card. If this value is set it should match the VLAN specified in by `vlan_id`. +- `temp_path` (string) - The location under which Packer will create a + directory to house all the VM files and folders during the build. + By default `%TEMP%` is used which, for most systems, will evaluate to + `%USERPROFILE%/AppData/Local/Temp`. + + The build directory housed under `temp_path` will have a name similar + to `packerhv1234567`. The seven digit number at the end of the name is + automatically generated by Packer to ensure the directory name is + unique. + - `vlan_id` (string) - This is the VLAN of the virtual machine's network card for the new virtual machine. By default none is set. If none is set then VLANs are not set on the virtual machine's network card. From 29503e453ea8c72ff0441398e69f2ea3dfd9a34b Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 16 Jun 2018 19:29:03 +0100 Subject: [PATCH 25/33] Update Hyper-V docs with new export behaviour --- website/source/docs/builders/hyperv-vmcx.html.md.erb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/website/source/docs/builders/hyperv-vmcx.html.md.erb b/website/source/docs/builders/hyperv-vmcx.html.md.erb index 037cebb07..de1b22891 100644 --- a/website/source/docs/builders/hyperv-vmcx.html.md.erb +++ b/website/source/docs/builders/hyperv-vmcx.html.md.erb @@ -240,11 +240,11 @@ builder. - `skip_compaction` (boolean) - If `true` skip compacting the hard disk for the virtual machine when exporting. This defaults to `false`. -- `skip_export` (boolean) - If `true` Packer will skip the export of the - VM. If you are interested only in the VHD/VHDX files, you can enable - this option. This will create inline disks which improves the build - performance. There will not be any copying of source VHDs to the temp - directory. This defaults to `false`. +- `skip_export` (boolean) - If `true` Packer will skip the export of the VM. + If you are interested only in the VHD/VHDX files, you can enable this + option. The resulting VHD/VHDX file will be output to + `/Virtual Hard Disks`. By default this option is `false` + and Packer will export the VM to `output_directory`. - `switch_name` (string) - The name of the switch to connect the virtual machine to. By default, leaving this value unset will cause Packer to From 36bd2f5691b03674154d8db1de0db5cebfa11936 Mon Sep 17 00:00:00 2001 From: DanHam Date: Fri, 6 Jul 2018 01:45:40 +0100 Subject: [PATCH 26/33] Fix an error with an error. Add some comments. --- builder/hyperv/common/step_create_external_switch.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builder/hyperv/common/step_create_external_switch.go b/builder/hyperv/common/step_create_external_switch.go index 52626afce..483dc2c2c 100644 --- a/builder/hyperv/common/step_create_external_switch.go +++ b/builder/hyperv/common/step_create_external_switch.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/packer/packer" ) -// This step creates switch for VM. +// This step creates an external switch for the VM. // // Produces: // SwitchName string - The name of the Switch @@ -18,6 +18,9 @@ type StepCreateExternalSwitch struct { oldSwitchName string } +// Run runs the step required to create an external switch. Depending on +// the connectivity of the host machine, the external switch will allow the +// build VM to connect to the outside world. func (s *StepCreateExternalSwitch) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { driver := state.Get("driver").(Driver) ui := state.Get("ui").(packer.Ui) @@ -30,10 +33,12 @@ func (s *StepCreateExternalSwitch) Run(_ context.Context, state multistep.StateB packerExternalSwitchName := "paes_" + uuid.TimeOrderedUUID() + // CreateExternalVirtualSwitch checks for an existing external switch, + // creating one if required, and connects the VM to it err = driver.CreateExternalVirtualSwitch(vmName, packerExternalSwitchName) if err != nil { - err := fmt.Errorf("Error creating switch: %s", err) - state.Put(errorMsg, err) + err := fmt.Errorf(errorMsg, err) + state.Put("error", err) ui.Error(err.Error()) s.SwitchName = "" return multistep.ActionHalt From 2bb5a92755bef6fd980434bd17a7e8215d024df0 Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 17 Jul 2018 16:33:48 +0100 Subject: [PATCH 27/33] Fix error in comment due to copy/paste from VMware step --- builder/hyperv/common/step_shutdown.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/builder/hyperv/common/step_shutdown.go b/builder/hyperv/common/step_shutdown.go index b37716f60..ddfdf3388 100644 --- a/builder/hyperv/common/step_shutdown.go +++ b/builder/hyperv/common/step_shutdown.go @@ -17,10 +17,9 @@ import ( // // Uses: // communicator packer.Communicator -// dir OutputDir -// driver Driver -// ui packer.Ui -// vmx_path string +// driver Driver +// ui packer.Ui +// vmName string // // Produces: // From 674bad0ab4a61c6bcb85b450ee8677d011dacaa6 Mon Sep 17 00:00:00 2001 From: DanHam Date: Mon, 9 Jul 2018 17:20:38 +0100 Subject: [PATCH 28/33] Break very long lines for readability --- builder/hyperv/common/driver_mock.go | 17 +++-- builder/hyperv/common/driver_ps_4.go | 29 +++++--- builder/hyperv/common/step_clone_vm.go | 3 +- builder/hyperv/common/step_create_vm.go | 3 +- .../common/step_mount_guest_additions.go | 3 +- .../common/step_mount_secondary_dvd_images.go | 3 +- builder/hyperv/common/step_sleep.go | 3 +- .../hyperv/common/step_unmount_dvddrive.go | 6 +- .../common/step_unmount_guest_additions.go | 6 +- .../step_unmount_secondary_dvd_images.go | 6 +- builder/hyperv/iso/builder.go | 60 +++++++++++----- builder/hyperv/iso/builder_test.go | 15 ++-- builder/hyperv/vmcx/builder.go | 70 +++++++++++++------ builder/hyperv/vmcx/builder_test.go | 3 +- common/powershell/hyperv/hyperv.go | 40 ++++++++--- 15 files changed, 186 insertions(+), 81 deletions(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index e3ddada15..43a26b517 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -395,7 +395,8 @@ func (d *DriverMock) CreateVirtualSwitch(switchName string, switchType string) ( return d.CreateVirtualSwitch_Return, d.CreateVirtualSwitch_Err } -func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string, vhdSizeBytes int64, vhdDiskBlockSize int64, controllerType string) error { +func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string, + vhdSizeBytes int64, vhdDiskBlockSize int64, controllerType string) error { d.AddVirtualMachineHardDrive_Called = true d.AddVirtualMachineHardDrive_VmName = vmName d.AddVirtualMachineHardDrive_VhdFile = vhdFile @@ -406,7 +407,9 @@ func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, v return d.AddVirtualMachineHardDrive_Err } -func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { +func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddrivePath string, + ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, + diffDisks bool, fixedVHD bool) error { d.CreateVirtualMachine_Called = true d.CreateVirtualMachine_VmName = vmName d.CreateVirtualMachine_Path = path @@ -420,7 +423,9 @@ func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddriveP return d.CreateVirtualMachine_Err } -func (d *DriverMock) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { +func (d *DriverMock) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, + cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, + harddrivePath string, ram int64, switchName string) error { d.CloneVirtualMachine_Called = true d.CloneVirtualMachine_CloneFromVmxcPath = cloneFromVmxcPath d.CloneVirtualMachine_CloneFromVmName = cloneFromVmName @@ -525,7 +530,8 @@ func (d *DriverMock) CreateDvdDrive(vmName string, isoPath string, generation ui return d.CreateDvdDrive_ControllerNumber, d.CreateDvdDrive_ControllerLocation, d.CreateDvdDrive_Err } -func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber uint, controllerLocation uint) error { +func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber uint, + controllerLocation uint) error { d.MountDvdDrive_Called = true d.MountDvdDrive_VmName = vmName d.MountDvdDrive_Path = path @@ -534,7 +540,8 @@ func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber return d.MountDvdDrive_Err } -func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint) error { +func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, + generation uint) error { d.SetBootDvdDrive_Called = true d.SetBootDvdDrive_VmName = vmName d.SetBootDvdDrive_ControllerNumber = controllerNumber diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index c07473447..7e27ed5e4 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -175,16 +175,24 @@ func (d *HypervPS4Driver) CreateVirtualSwitch(switchName string, switchType stri return hyperv.CreateVirtualSwitch(switchName, switchType) } -func (d *HypervPS4Driver) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string, vhdSizeBytes int64, diskBlockSize int64, controllerType string) error { - return hyperv.AddVirtualMachineHardDiskDrive(vmName, vhdFile, vhdName, vhdSizeBytes, diskBlockSize, controllerType) +func (d *HypervPS4Driver) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string, + vhdSizeBytes int64, diskBlockSize int64, controllerType string) error { + return hyperv.AddVirtualMachineHardDiskDrive(vmName, vhdFile, vhdName, vhdSizeBytes, + diskBlockSize, controllerType) } -func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { - return hyperv.CreateVirtualMachine(vmName, path, harddrivePath, ram, diskSize, diskBlockSize, switchName, generation, diffDisks, fixedVHD) +func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, + diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, + fixedVHD bool) error { + return hyperv.CreateVirtualMachine(vmName, path, harddrivePath, ram, diskSize, diskBlockSize, switchName, + generation, diffDisks, fixedVHD) } -func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { - return hyperv.CloneVirtualMachine(cloneFromVmxcPath, cloneFromVmName, cloneFromSnapshotName, cloneAllSnapshots, vmName, path, harddrivePath, ram, switchName) +func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, + cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, + ram int64, switchName string) error { + return hyperv.CloneVirtualMachine(cloneFromVmxcPath, cloneFromVmName, cloneFromSnapshotName, + cloneAllSnapshots, vmName, path, harddrivePath, ram, switchName) } func (d *HypervPS4Driver) DeleteVirtualMachine(vmName string) error { @@ -211,7 +219,8 @@ func (d *HypervPS4Driver) SetVirtualMachineVirtualizationExtensions(vmName strin return hyperv.SetVirtualMachineVirtualizationExtensions(vmName, enable) } -func (d *HypervPS4Driver) EnableVirtualMachineIntegrationService(vmName string, integrationServiceName string) error { +func (d *HypervPS4Driver) EnableVirtualMachineIntegrationService(vmName string, + integrationServiceName string) error { return hyperv.EnableVirtualMachineIntegrationService(vmName, integrationServiceName) } @@ -239,11 +248,13 @@ func (d *HypervPS4Driver) CreateDvdDrive(vmName string, isoPath string, generati return hyperv.CreateDvdDrive(vmName, isoPath, generation) } -func (d *HypervPS4Driver) MountDvdDrive(vmName string, path string, controllerNumber uint, controllerLocation uint) error { +func (d *HypervPS4Driver) MountDvdDrive(vmName string, path string, controllerNumber uint, + controllerLocation uint) error { return hyperv.MountDvdDrive(vmName, path, controllerNumber, controllerLocation) } -func (d *HypervPS4Driver) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint) error { +func (d *HypervPS4Driver) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, + generation uint) error { return hyperv.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, generation) } diff --git a/builder/hyperv/common/step_clone_vm.go b/builder/hyperv/common/step_clone_vm.go index 6a3d434a8..514929669 100644 --- a/builder/hyperv/common/step_clone_vm.go +++ b/builder/hyperv/common/step_clone_vm.go @@ -55,7 +55,8 @@ func (s *StepCloneVM) Run(_ context.Context, state multistep.StateBag) multistep // convert the MB to bytes ramSize := int64(s.RamSize * 1024 * 1024) - err := driver.CloneVirtualMachine(s.CloneFromVMXCPath, s.CloneFromVMName, s.CloneFromSnapshotName, s.CloneAllSnapshots, s.VMName, path, harddrivePath, ramSize, s.SwitchName) + err := driver.CloneVirtualMachine(s.CloneFromVMXCPath, s.CloneFromVMName, s.CloneFromSnapshotName, + s.CloneAllSnapshots, s.VMName, path, harddrivePath, ramSize, s.SwitchName) if err != nil { err := fmt.Errorf("Error cloning virtual machine: %s", err) state.Put("error", err) diff --git a/builder/hyperv/common/step_create_vm.go b/builder/hyperv/common/step_create_vm.go index 1c655becf..b5b36ddaf 100644 --- a/builder/hyperv/common/step_create_vm.go +++ b/builder/hyperv/common/step_create_vm.go @@ -63,7 +63,8 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste diskSize := int64(s.DiskSize * 1024 * 1024) diskBlockSize := int64(s.DiskBlockSize * 1024 * 1024) - err := driver.CreateVirtualMachine(s.VMName, path, harddrivePath, ramSize, diskSize, diskBlockSize, s.SwitchName, s.Generation, s.DifferencingDisk, s.FixedVHD) + err := driver.CreateVirtualMachine(s.VMName, path, harddrivePath, ramSize, diskSize, diskBlockSize, + s.SwitchName, s.Generation, s.DifferencingDisk, s.FixedVHD) if err != nil { err := fmt.Errorf("Error creating virtual machine: %s", err) state.Put("error", err) diff --git a/builder/hyperv/common/step_mount_guest_additions.go b/builder/hyperv/common/step_mount_guest_additions.go index 71b6c3eec..d333c5baf 100644 --- a/builder/hyperv/common/step_mount_guest_additions.go +++ b/builder/hyperv/common/step_mount_guest_additions.go @@ -56,7 +56,8 @@ func (s *StepMountGuestAdditions) Run(_ context.Context, state multistep.StateBa return multistep.ActionHalt } - log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", s.GuestAdditionsPath, controllerNumber, controllerLocation)) + log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", s.GuestAdditionsPath, + controllerNumber, controllerLocation)) return multistep.ActionContinue } diff --git a/builder/hyperv/common/step_mount_secondary_dvd_images.go b/builder/hyperv/common/step_mount_secondary_dvd_images.go index c23e39fac..22563376b 100644 --- a/builder/hyperv/common/step_mount_secondary_dvd_images.go +++ b/builder/hyperv/common/step_mount_secondary_dvd_images.go @@ -58,7 +58,8 @@ func (s *StepMountSecondaryDvdImages) Run(_ context.Context, state multistep.Sta return multistep.ActionHalt } - log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", isoPath, controllerNumber, controllerLocation)) + log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", isoPath, controllerNumber, + controllerLocation)) } return multistep.ActionContinue diff --git a/builder/hyperv/common/step_sleep.go b/builder/hyperv/common/step_sleep.go index e65bcf0b1..e7ed01cca 100644 --- a/builder/hyperv/common/step_sleep.go +++ b/builder/hyperv/common/step_sleep.go @@ -18,7 +18,8 @@ func (s *StepSleep) Run(_ context.Context, state multistep.StateBag) multistep.S ui := state.Get("ui").(packer.Ui) if len(s.ActionName) > 0 { - ui.Say(s.ActionName + "! Waiting for " + fmt.Sprintf("%v", uint(s.Minutes)) + " minutes to let the action to complete...") + ui.Say(s.ActionName + "! Waiting for " + fmt.Sprintf("%v", uint(s.Minutes)) + + " minutes to let the action to complete...") } time.Sleep(time.Minute * s.Minutes) diff --git a/builder/hyperv/common/step_unmount_dvddrive.go b/builder/hyperv/common/step_unmount_dvddrive.go index 010e8b067..50298f6e1 100644 --- a/builder/hyperv/common/step_unmount_dvddrive.go +++ b/builder/hyperv/common/step_unmount_dvddrive.go @@ -27,7 +27,8 @@ func (s *StepUnmountDvdDrive) Run(_ context.Context, state multistep.StateBag) m dvdController := dvdControllerState.(DvdControllerProperties) if dvdController.Existing { - ui.Say(fmt.Sprintf("Unmounting os dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation)) + ui.Say(fmt.Sprintf("Unmounting os dvd drives controller %d location %d ...", + dvdController.ControllerNumber, dvdController.ControllerLocation)) err := driver.UnmountDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation) if err != nil { err := fmt.Errorf("Error unmounting os dvd drive: %s", err) @@ -36,7 +37,8 @@ func (s *StepUnmountDvdDrive) Run(_ context.Context, state multistep.StateBag) m return multistep.ActionHalt } } else { - ui.Say(fmt.Sprintf("Delete os dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation)) + ui.Say(fmt.Sprintf("Delete os dvd drives controller %d location %d ...", + dvdController.ControllerNumber, dvdController.ControllerLocation)) err := driver.DeleteDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation) if err != nil { err := fmt.Errorf("Error deleting os dvd drive: %s", err) diff --git a/builder/hyperv/common/step_unmount_guest_additions.go b/builder/hyperv/common/step_unmount_guest_additions.go index 06600ddaa..c7f78cf52 100644 --- a/builder/hyperv/common/step_unmount_guest_additions.go +++ b/builder/hyperv/common/step_unmount_guest_additions.go @@ -27,7 +27,8 @@ func (s *StepUnmountGuestAdditions) Run(_ context.Context, state multistep.State dvdController := dvdControllerState.(DvdControllerProperties) if dvdController.Existing { - ui.Say(fmt.Sprintf("Unmounting Integration Services dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation)) + ui.Say(fmt.Sprintf("Unmounting Integration Services dvd drives controller %d location %d ...", + dvdController.ControllerNumber, dvdController.ControllerLocation)) err := driver.UnmountDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation) if err != nil { err := fmt.Errorf("Error unmounting Integration Services dvd drive: %s", err) @@ -36,7 +37,8 @@ func (s *StepUnmountGuestAdditions) Run(_ context.Context, state multistep.State return multistep.ActionHalt } } else { - ui.Say(fmt.Sprintf("Delete Integration Services dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation)) + ui.Say(fmt.Sprintf("Delete Integration Services dvd drives controller %d location %d ...", + dvdController.ControllerNumber, dvdController.ControllerLocation)) err := driver.DeleteDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation) if err != nil { err := fmt.Errorf("Error deleting Integration Services dvd drive: %s", err) diff --git a/builder/hyperv/common/step_unmount_secondary_dvd_images.go b/builder/hyperv/common/step_unmount_secondary_dvd_images.go index 0f74d2789..06bd2e0db 100644 --- a/builder/hyperv/common/step_unmount_secondary_dvd_images.go +++ b/builder/hyperv/common/step_unmount_secondary_dvd_images.go @@ -28,7 +28,8 @@ func (s *StepUnmountSecondaryDvdImages) Run(_ context.Context, state multistep.S for _, dvdController := range dvdControllers { if dvdController.Existing { - ui.Say(fmt.Sprintf("Unmounting secondary dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation)) + ui.Say(fmt.Sprintf("Unmounting secondary dvd drives controller %d location %d ...", + dvdController.ControllerNumber, dvdController.ControllerLocation)) err := driver.UnmountDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation) if err != nil { err := fmt.Errorf("Error unmounting secondary dvd drive: %s", err) @@ -37,7 +38,8 @@ func (s *StepUnmountSecondaryDvdImages) Run(_ context.Context, state multistep.S return multistep.ActionHalt } } else { - ui.Say(fmt.Sprintf("Delete secondary dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation)) + ui.Say(fmt.Sprintf("Delete secondary dvd drives controller %d location %d ...", + dvdController.ControllerNumber, dvdController.ControllerLocation)) err := driver.DeleteDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation) if err != nil { err := fmt.Errorf("Error deleting secondary dvd drive: %s", err) diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index fd51c85c0..61aa70fc4 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -145,7 +145,9 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, b.config.SSHConfig.Prepare(&b.config.ctx)...) errs = packer.MultiErrorAppend(errs, b.config.ShutdownConfig.Prepare(&b.config.ctx)...) - if len(b.config.ISOConfig.ISOUrls) < 1 || (strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhd" && strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhdx") { + if len(b.config.ISOConfig.ISOUrls) < 1 || + (strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhd" && + strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhdx") { //We only create a new hard drive if an existing one to copy from does not exist err = b.checkDiskSize() if err != nil { @@ -249,25 +251,35 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.Generation < 2 && numberOfIsos > 2 { if b.config.GuestAdditionsMode == "attach" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, "+ + "so we can't support guest additions and these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } else { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, "+ + "so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) } } else if b.config.Generation > 1 && len(b.config.SecondaryDvdImages) > 16 { if b.config.GuestAdditionsMode == "attach" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available "+ + "for scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } else { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available "+ + "for scsi (limited to 16), so we can't support these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } } if b.config.EnableVirtualizationExtensions { hasVirtualMachineVirtualizationExtensions, err := powershell.HasVirtualMachineVirtualizationExtensions() if err != nil { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization extensions support: %s", err)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization "+ + "extensions support: %s", err)) } else { if !hasVirtualMachineVirtualizationExtensions { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support virtual machine virtualization extension. Please use Windows 10 or Windows Server 2016 or newer.")) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support "+ + "virtual machine virtualization extension. Please use Windows 10 or Windows Server "+ + "2016 or newer.")) } } } @@ -302,24 +314,29 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.EnableVirtualizationExtensions { if b.config.EnableDynamicMemory { - warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, dynamic memory should not be allowed.") + warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + + "dynamic memory should not be allowed.") warnings = appendWarnings(warnings, warning) } if !b.config.EnableMacSpoofing { - warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, mac spoofing should be allowed.") + warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + + "mac spoofing should be allowed.") warnings = appendWarnings(warnings, warning) } if b.config.RamSize < MinNestedVirtualizationRamSize { - warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start any nested VMs.") + warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + + "there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start " + + "any nested VMs.") warnings = appendWarnings(warnings, warning) } } if b.config.SwitchVlanId != "" { if b.config.SwitchVlanId != b.config.VlanId { - warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor vlan. The switch will not be able to see traffic from the VM.") + warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor " + + "vlan. The switch will not be able to see traffic from the VM.") warnings = appendWarnings(warnings, warning) } } @@ -524,11 +541,14 @@ func (b *Builder) checkDiskSize() error { log.Println(fmt.Sprintf("%s: %v", "DiskSize", b.config.DiskSize)) if b.config.DiskSize < MinDiskSize { - return fmt.Errorf("disk_size: Virtual machine requires disk space >= %v GB, but defined: %v", MinDiskSize, b.config.DiskSize/1024) + return fmt.Errorf("disk_size: Virtual machine requires disk space >= %v GB, but defined: %v", + MinDiskSize, b.config.DiskSize/1024) } else if b.config.DiskSize > MaxDiskSize && !b.config.FixedVHD { - return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v", MaxDiskSize, b.config.DiskSize/1024) + return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v", + MaxDiskSize, b.config.DiskSize/1024) } else if b.config.DiskSize > MaxVHDSize && b.config.FixedVHD { - return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v", MaxVHDSize/1024, b.config.DiskSize/1024) + return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v", + MaxVHDSize/1024, b.config.DiskSize/1024) } return nil @@ -542,9 +562,11 @@ func (b *Builder) checkDiskBlockSize() error { log.Println(fmt.Sprintf("%s: %v", "DiskBlockSize", b.config.DiskBlockSize)) if b.config.DiskBlockSize < MinDiskBlockSize { - return fmt.Errorf("disk_block_size: Virtual machine requires disk block size >= %v MB, but defined: %v", MinDiskBlockSize, b.config.DiskBlockSize) + return fmt.Errorf("disk_block_size: Virtual machine requires disk block size >= %v MB, but defined: %v", + MinDiskBlockSize, b.config.DiskBlockSize) } else if b.config.DiskBlockSize > MaxDiskBlockSize { - return fmt.Errorf("disk_block_size: Virtual machine requires disk block size <= %v MB, but defined: %v", MaxDiskBlockSize, b.config.DiskBlockSize) + return fmt.Errorf("disk_block_size: Virtual machine requires disk block size <= %v MB, but defined: %v", + MaxDiskBlockSize, b.config.DiskBlockSize) } return nil @@ -558,9 +580,11 @@ func (b *Builder) checkRamSize() error { log.Println(fmt.Sprintf("%s: %v", "RamSize", b.config.RamSize)) if b.config.RamSize < MinRamSize { - return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v", MinRamSize, b.config.RamSize) + return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v", + MinRamSize, b.config.RamSize) } else if b.config.RamSize > MaxRamSize { - return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v", MaxRamSize, b.config.RamSize) + return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v", + MaxRamSize, b.config.RamSize) } return nil diff --git a/builder/hyperv/iso/builder_test.go b/builder/hyperv/iso/builder_test.go index 4a63f3c67..44d73e8c8 100644 --- a/builder/hyperv/iso/builder_test.go +++ b/builder/hyperv/iso/builder_test.go @@ -104,7 +104,8 @@ func TestBuilderPrepare_DiskBlockSize(t *testing.T) { t.Fatalf("bad err: %s", err) } if b.config.DiskBlockSize != expected_default_block_size { - t.Fatalf("bad default block size with empty config: %d. Expected %d", b.config.DiskBlockSize, expected_default_block_size) + t.Fatalf("bad default block size with empty config: %d. Expected %d", b.config.DiskBlockSize, + expected_default_block_size) } test_sizes := []uint{0, 1, 32, 256, 512, 1 * 1024, 32 * 1024} @@ -117,7 +118,8 @@ func TestBuilderPrepare_DiskBlockSize(t *testing.T) { t.Fatalf("bad, should have no warns: %#v", warns) } if err == nil { - t.Fatalf("bad, should have error but didn't. disk_block_size=%d outside expected valid range [%d,%d]", test_size, expected_min_block_size, expected_max_block_size) + t.Fatalf("bad, should have error. disk_block_size=%d outside expected valid range [%d,%d]", + test_size, expected_min_block_size, expected_max_block_size) } } else { if len(warns) > 0 { @@ -128,11 +130,13 @@ func TestBuilderPrepare_DiskBlockSize(t *testing.T) { } if test_size == 0 { if b.config.DiskBlockSize != expected_default_block_size { - t.Fatalf("bad default block size with 0 value config: %d. Expected: %d", b.config.DiskBlockSize, expected_default_block_size) + t.Fatalf("bad default block size with 0 value config: %d. Expected: %d", + b.config.DiskBlockSize, expected_default_block_size) } } else { if b.config.DiskBlockSize != test_size { - t.Fatalf("bad block size with 0 value config: %d. Expected: %d", b.config.DiskBlockSize, expected_default_block_size) + t.Fatalf("bad block size with 0 value config: %d. Expected: %d", b.config.DiskBlockSize, + expected_default_block_size) } } } @@ -147,7 +151,8 @@ func TestBuilderPrepare_FixedVHDFormat(t *testing.T) { config["skip_compaction"] = true config["differencing_disk"] = false - //use_fixed_vhd_format should work with generation = 1, skip_compaction = true, and differencing_disk = false + // use_fixed_vhd_format should work with generation = 1, skip_compaction + // = true, and differencing_disk = false warns, err := b.Prepare(config) if len(warns) > 0 { t.Fatalf("bad: %#v", warns) diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 88ebef783..c4675c0cd 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -159,35 +159,44 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.CloneFromVMName == "" { if b.config.CloneFromVMXCPath == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vm_name must be specified if clone_from_vmxc_path is not specified.")) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vm_name must be specified if "+ + "clone_from_vmxc_path is not specified.")) } } else { virtualMachineExists, err := powershell.DoesVirtualMachineExist(b.config.CloneFromVMName) if err != nil { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to clone from exists: %s", err)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to clone "+ + "from exists: %s", err)) } else { if !virtualMachineExists { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine '%s' to clone from does not exist.", b.config.CloneFromVMName)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine '%s' to clone from does not "+ + "exist.", b.config.CloneFromVMName)) } else { b.config.Generation, err = powershell.GetVirtualMachineGeneration(b.config.CloneFromVMName) if err != nil { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine to clone from generation: %s", err)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine to clone "+ + "from generation: %s", err)) } if b.config.CloneFromSnapshotName != "" { - virtualMachineSnapshotExists, err := powershell.DoesVirtualMachineSnapshotExist(b.config.CloneFromVMName, b.config.CloneFromSnapshotName) + virtualMachineSnapshotExists, err := powershell.DoesVirtualMachineSnapshotExist( + b.config.CloneFromVMName, b.config.CloneFromSnapshotName) if err != nil { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine snapshot to clone from exists: %s", err)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine "+ + "snapshot to clone from exists: %s", err)) } else { if !virtualMachineSnapshotExists { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine snapshot '%s' on virtual machine '%s' to clone from does not exist.", b.config.CloneFromSnapshotName, b.config.CloneFromVMName)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine snapshot '%s' on "+ + "virtual machine '%s' to clone from does not exist.", + b.config.CloneFromSnapshotName, b.config.CloneFromVMName)) } } } virtualMachineOn, err := powershell.IsVirtualMachineOn(b.config.CloneFromVMName) if err != nil { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to clone is running: %s", err)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to "+ + "clone is running: %s", err)) } else { if virtualMachineOn { warning := fmt.Sprintf("Cloning from a virtual machine that is running.") @@ -200,7 +209,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.CloneFromVMXCPath == "" { if b.config.CloneFromVMName == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vmxc_path be specified if clone_from_vm_name must is not specified.")) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vmxc_path be specified if "+ + "clone_from_vm_name must is not specified.")) } } else { if _, err := os.Stat(b.config.CloneFromVMXCPath); os.IsNotExist(err) { @@ -277,25 +287,36 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.Generation < 2 && numberOfIsos > 2 { if b.config.GuestAdditionsMode == "attach" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so "+ + "we can't support guest additions and these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } else { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so "+ + "we can't support these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } } else if b.config.Generation > 1 && len(b.config.SecondaryDvdImages) > 16 { if b.config.GuestAdditionsMode == "attach" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for "+ + "scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } else { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", "))) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for "+ + "scsi (limited to 16), so we can't support these secondary dvds: %s", + strings.Join(b.config.SecondaryDvdImages, ", "))) } } if b.config.EnableVirtualizationExtensions { hasVirtualMachineVirtualizationExtensions, err := powershell.HasVirtualMachineVirtualizationExtensions() if err != nil { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization extensions support: %s", err)) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization "+ + "extensions support: %s", err)) } else { if !hasVirtualMachineVirtualizationExtensions { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support virtual machine virtualization extension. Please use Windows 10 or Windows Server 2016 or newer.")) + errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support "+ + "virtual machine virtualization extension. Please use Windows 10 or Windows Server 2016 "+ + "or newer.")) } } } @@ -315,24 +336,29 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { if b.config.EnableVirtualizationExtensions { if b.config.EnableDynamicMemory { - warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, dynamic memory should not be allowed.") + warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + + "dynamic memory should not be allowed.") warnings = appendWarnings(warnings, warning) } if !b.config.EnableMacSpoofing { - warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, mac spoofing should be allowed.") + warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + + "mac spoofing should be allowed.") warnings = appendWarnings(warnings, warning) } if b.config.RamSize < MinNestedVirtualizationRamSize { - warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start any nested VMs.") + warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + + "there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start " + + "any nested VMs.") warnings = appendWarnings(warnings, warning) } } if b.config.SwitchVlanId != "" { if b.config.SwitchVlanId != b.config.VlanId { - warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor vlan. The switch will not be able to see traffic from the VM.") + warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor " + + "vlan. The switch will not be able to see traffic from the VM.") warnings = appendWarnings(warnings, warning) } } @@ -544,9 +570,11 @@ func (b *Builder) checkRamSize() error { log.Println(fmt.Sprintf("%s: %v", "RamSize", b.config.RamSize)) if b.config.RamSize < MinRamSize { - return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v", MinRamSize, b.config.RamSize) + return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v", + MinRamSize, b.config.RamSize) } else if b.config.RamSize > MaxRamSize { - return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v", MaxRamSize, b.config.RamSize) + return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v", + MaxRamSize, b.config.RamSize) } return nil diff --git a/builder/hyperv/vmcx/builder_test.go b/builder/hyperv/vmcx/builder_test.go index 9a4a54c33..ce7729c51 100644 --- a/builder/hyperv/vmcx/builder_test.go +++ b/builder/hyperv/vmcx/builder_test.go @@ -162,7 +162,8 @@ func disabled_TestBuilderPrepare_CloneFromVmSettingUsedSoNoCloneFromVmxcPathRequ t.Fatal("should have error") } else { errorMessage := err.Error() - if errorMessage != "1 error(s) occurred:\n\n* Virtual machine 'test_machine_name_that_does_not_exist' to clone from does not exist." { + if errorMessage != "1 error(s) occurred:\n\n* Virtual machine 'test_machine_name_that_does_not_exist' "+ + "to clone from does not exist." { t.Fatalf("should not have error: %s", err) } } diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 8b267c8c5..32f1cb57a 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -111,7 +111,8 @@ Hyper-V\Set-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -Cont ` var ps powershell.PowerShellCmd - err := ps.Run(script, vmName, path, strconv.FormatInt(int64(controllerNumber), 10), strconv.FormatInt(int64(controllerLocation), 10)) + err := ps.Run(script, vmName, path, strconv.FormatInt(int64(controllerNumber), 10), + strconv.FormatInt(int64(controllerLocation), 10)) return err } @@ -124,7 +125,8 @@ Hyper-V\Set-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -Cont ` var ps powershell.PowerShellCmd - err := ps.Run(script, vmName, strconv.FormatInt(int64(controllerNumber), 10), strconv.FormatInt(int64(controllerLocation), 10)) + err := ps.Run(script, vmName, strconv.FormatInt(int64(controllerNumber), 10), + strconv.FormatInt(int64(controllerLocation), 10)) return err } @@ -146,7 +148,8 @@ if (!$vmDvdDrive) {throw 'unable to find dvd drive'} Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDvdDrive -ErrorAction SilentlyContinue ` var ps powershell.PowerShellCmd - err := ps.Run(script, vmName, strconv.FormatInt(int64(controllerNumber), 10), strconv.FormatInt(int64(controllerLocation), 10)) + err := ps.Run(script, vmName, strconv.FormatInt(int64(controllerNumber), 10), + strconv.FormatInt(int64(controllerLocation), 10)) return err } } @@ -160,7 +163,8 @@ Hyper-V\Remove-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -C ` var ps powershell.PowerShellCmd - err := ps.Run(script, vmName, strconv.FormatInt(int64(controllerNumber), 10), strconv.FormatInt(int64(controllerLocation), 10)) + err := ps.Run(script, vmName, strconv.FormatInt(int64(controllerNumber), 10), + strconv.FormatInt(int64(controllerLocation), 10)) return err } @@ -198,7 +202,9 @@ Hyper-V\Set-VMFloppyDiskDrive -VMName $vmName -Path $null return err } -func CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error { +func CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64, + diskSize int64, diskBlockSize int64, switchName string, generation uint, + diffDisks bool, fixedVHD bool) error { if generation == 2 { var script = ` @@ -218,7 +224,10 @@ if ($harddrivePath){ } ` var ps powershell.PowerShellCmd - if err := ps.Run(script, vmName, path, harddrivePath, strconv.FormatInt(ram, 10), strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), switchName, strconv.FormatInt(int64(generation), 10), strconv.FormatBool(diffDisks)); err != nil { + if err := ps.Run(script, vmName, path, harddrivePath, strconv.FormatInt(ram, 10), + strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), + switchName, strconv.FormatInt(int64(generation), 10), + strconv.FormatBool(diffDisks)); err != nil { return err } @@ -252,7 +261,9 @@ if ($harddrivePath){ } ` var ps powershell.PowerShellCmd - if err := ps.Run(script, vmName, path, harddrivePath, strconv.FormatInt(ram, 10), strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), switchName, strconv.FormatBool(diffDisks), strconv.FormatBool(fixedVHD)); err != nil { + if err := ps.Run(script, vmName, path, harddrivePath, strconv.FormatInt(ram, 10), + strconv.FormatInt(diskSize, 10), strconv.FormatInt(diskBlockSize, 10), + switchName, strconv.FormatBool(diffDisks), strconv.FormatBool(fixedVHD)); err != nil { return err } @@ -354,7 +365,9 @@ Hyper-V\Set-VMNetworkAdapter $vmName -staticmacaddress $mac return err } -func ImportVmxcVirtualMachine(importPath string, vmName string, harddrivePath string, ram int64, switchName string) error { +func ImportVmxcVirtualMachine(importPath string, vmName string, harddrivePath string, + ram int64, switchName string) error { + var script = ` param([string]$importPath, [string]$vmName, [string]$harddrivePath, [long]$memoryStartupBytes, [string]$switchName) @@ -409,9 +422,13 @@ if ($vm) { return err } -func CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { +func CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, + cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, + path string, harddrivePath string, ram int64, switchName string) error { + if cloneFromVmName != "" { - if err := ExportVmxcVirtualMachine(path, cloneFromVmName, cloneFromSnapshotName, cloneAllSnapshots); err != nil { + if err := ExportVmxcVirtualMachine(path, cloneFromVmName, + cloneFromSnapshotName, cloneAllSnapshots); err != nil { return err } } @@ -1018,7 +1035,8 @@ Hyper-V\Get-VMNetworkAdapter -VMName $vmName | Hyper-V\Connect-VMNetworkAdapter return err } -func AddVirtualMachineHardDiskDrive(vmName string, vhdRoot string, vhdName string, vhdSizeBytes int64, vhdBlockSize int64, controllerType string) error { +func AddVirtualMachineHardDiskDrive(vmName string, vhdRoot string, vhdName string, vhdSizeBytes int64, + vhdBlockSize int64, controllerType string) error { var script = ` param([string]$vmName,[string]$vhdRoot, [string]$vhdName, [string]$vhdSizeInBytes, [string]$vhdBlockSizeInByte, [string]$controllerType) From 28087cb9f73c6d804ba77331664a00084adceff1 Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 10 Jul 2018 17:31:34 +0100 Subject: [PATCH 29/33] Fixer and tests to remove deprecated 'vhd_temp_path' Hyper-V ISO setting --- fix/fixer.go | 2 + fix/fixer_hyperv_deprecations.go | 50 ++++++++++++++++++++++ fix/fixer_hyperv_deprecations_test.go | 60 +++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 fix/fixer_hyperv_deprecations.go create mode 100644 fix/fixer_hyperv_deprecations_test.go diff --git a/fix/fixer.go b/fix/fixer.go index ae8aac40d..cc8aa1258 100644 --- a/fix/fixer.go +++ b/fix/fixer.go @@ -36,6 +36,7 @@ func init() { "amazon-private-ip": new(FixerAmazonPrivateIP), "docker-email": new(FixerDockerEmail), "powershell-escapes": new(FixerPowerShellEscapes), + "hyperv-deprecations": new(FixerHypervDeprecations), } FixerOrder = []string{ @@ -55,5 +56,6 @@ func init() { "amazon-private-ip", "docker-email", "powershell-escapes", + "hyperv-deprecations", } } diff --git a/fix/fixer_hyperv_deprecations.go b/fix/fixer_hyperv_deprecations.go new file mode 100644 index 000000000..79a75d83d --- /dev/null +++ b/fix/fixer_hyperv_deprecations.go @@ -0,0 +1,50 @@ +package fix + +import ( + "github.com/mitchellh/mapstructure" +) + +// FixerHypervDeprecations removes the deprecated "vhd_temp_path" setting +// from Hyper-V ISO builder templates +type FixerHypervDeprecations struct{} + +func (FixerHypervDeprecations) Fix(input map[string]interface{}) (map[string]interface{}, error) { + // The type we'll decode into; we only care about builders + type template struct { + Builders []map[string]interface{} + } + + // Decode the input into our structure, if we can + var tpl template + if err := mapstructure.Decode(input, &tpl); err != nil { + return nil, err + } + + for _, builder := range tpl.Builders { + builderTypeRaw, ok := builder["type"] + if !ok { + continue + } + + builderType, ok := builderTypeRaw.(string) + if !ok { + continue + } + + if builderType != "hyperv-iso" { + continue + } + + _, ok = builder["vhd_temp_path"] + if ok { + delete(builder, "vhd_temp_path") + } + } + + input["builders"] = tpl.Builders + return input, nil +} + +func (FixerHypervDeprecations) Synopsis() string { + return `Removes the deprecated "vhd_temp_path" setting from Hyper-V ISO builder templates` +} diff --git a/fix/fixer_hyperv_deprecations_test.go b/fix/fixer_hyperv_deprecations_test.go new file mode 100644 index 000000000..8c90bff9b --- /dev/null +++ b/fix/fixer_hyperv_deprecations_test.go @@ -0,0 +1,60 @@ +package fix + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFixerHypervDeprecations_impl(t *testing.T) { + var _ Fixer = new(FixerHypervDeprecations) +} + +func TestFixerHypervDeprecations_Fix(t *testing.T) { + cases := []struct { + Input map[string]interface{} + Expected map[string]interface{} + }{ + // No vhd_temp_path field in template - noop + { + Input: map[string]interface{}{ + "type": "hyperv-iso", + }, + + Expected: map[string]interface{}{ + "type": "hyperv-iso", + }, + }, + + // Deprecated vhd_temp_path field in template should be deleted + { + Input: map[string]interface{}{ + "type": "hyperv-iso", + "vhd_temp_path": "foopath", + }, + + Expected: map[string]interface{}{ + "type": "hyperv-iso", + }, + }, + } + + for _, tc := range cases { + var f FixerHypervDeprecations + + input := map[string]interface{}{ + "builders": []map[string]interface{}{tc.Input}, + } + + expected := map[string]interface{}{ + "builders": []map[string]interface{}{tc.Expected}, + } + + output, err := f.Fix(input) + if err != nil { + t.Fatalf("err: %s", err) + } + + assert.Equal(t, expected, output, "Should be equal") + } +} From 3c5d7aec743bba81a921e072f151bc413798313a Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 11 Jul 2018 23:15:43 +0100 Subject: [PATCH 30/33] Ensure new fixer appears in 'packer fix' usage message --- command/fix.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/command/fix.go b/command/fix.go index d04cdfd0b..b8c1023c3 100644 --- a/command/fix.go +++ b/command/fix.go @@ -153,6 +153,8 @@ Fixes that are run: docker-email Removes "login_email" from the Docker builder powershell-escapes Removes PowerShell escapes from user env vars and elevated username and password strings + hyperv-deprecations Removes the deprecated "vhd_temp_path" setting from + Hyper-V ISO builder templates Options: From da21c257919ef4faf17bd3318a41e789a31160c5 Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 19 Jul 2018 18:53:42 +0100 Subject: [PATCH 31/33] Convert incorrect 'vmxc' -> 'vmcx' in codebase, docs and template opts grep -rli --exclude-dir={vendor,bin\*,\*vmware\*,\*pkg\*} vmxc . | \ xargs sed -i 's/\(vm\)\(x\)\(c\)/\1\3\2/ig' --- builder/hyperv/common/driver_mock.go | 6 +- builder/hyperv/common/driver_ps_4.go | 4 +- builder/hyperv/common/step_clone_vm.go | 4 +- builder/hyperv/vmcx/builder.go | 16 +++--- builder/hyperv/vmcx/builder_test.go | 56 +++++++++---------- common/powershell/hyperv/hyperv.go | 28 +++++----- .../docs/builders/hyperv-vmcx.html.md.erb | 4 +- 7 files changed, 59 insertions(+), 59 deletions(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 43a26b517..5bef1b3e4 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -134,7 +134,7 @@ type DriverMock struct { CreateVirtualMachine_Err error CloneVirtualMachine_Called bool - CloneVirtualMachine_CloneFromVmxcPath string + CloneVirtualMachine_CloneFromVmcxPath string CloneVirtualMachine_CloneFromVmName string CloneVirtualMachine_CloneFromSnapshotName string CloneVirtualMachine_CloneAllSnapshots bool @@ -423,11 +423,11 @@ func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddriveP return d.CreateVirtualMachine_Err } -func (d *DriverMock) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, +func (d *DriverMock) CloneVirtualMachine(cloneFromVmcxPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { d.CloneVirtualMachine_Called = true - d.CloneVirtualMachine_CloneFromVmxcPath = cloneFromVmxcPath + d.CloneVirtualMachine_CloneFromVmcxPath = cloneFromVmcxPath d.CloneVirtualMachine_CloneFromVmName = cloneFromVmName d.CloneVirtualMachine_CloneFromSnapshotName = cloneFromSnapshotName d.CloneVirtualMachine_CloneAllSnapshots = cloneAllSnapshots diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index 7e27ed5e4..b1803971f 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -188,10 +188,10 @@ func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, hardd generation, diffDisks, fixedVHD) } -func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, +func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmcxPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { - return hyperv.CloneVirtualMachine(cloneFromVmxcPath, cloneFromVmName, cloneFromSnapshotName, + return hyperv.CloneVirtualMachine(cloneFromVmcxPath, cloneFromVmName, cloneFromSnapshotName, cloneAllSnapshots, vmName, path, harddrivePath, ram, switchName) } diff --git a/builder/hyperv/common/step_clone_vm.go b/builder/hyperv/common/step_clone_vm.go index 514929669..f245cb4aa 100644 --- a/builder/hyperv/common/step_clone_vm.go +++ b/builder/hyperv/common/step_clone_vm.go @@ -16,7 +16,7 @@ import ( // Produces: // VMName string - The name of the VM type StepCloneVM struct { - CloneFromVMXCPath string + CloneFromVMCXPath string CloneFromVMName string CloneFromSnapshotName string CloneAllSnapshots bool @@ -55,7 +55,7 @@ func (s *StepCloneVM) Run(_ context.Context, state multistep.StateBag) multistep // convert the MB to bytes ramSize := int64(s.RamSize * 1024 * 1024) - err := driver.CloneVirtualMachine(s.CloneFromVMXCPath, s.CloneFromVMName, s.CloneFromSnapshotName, + err := driver.CloneVirtualMachine(s.CloneFromVMCXPath, s.CloneFromVMName, s.CloneFromSnapshotName, s.CloneAllSnapshots, s.VMName, path, harddrivePath, ramSize, s.SwitchName) if err != nil { err := fmt.Errorf("Error cloning virtual machine: %s", err) diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index c4675c0cd..d85c0aa22 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -62,7 +62,7 @@ type Config struct { GuestAdditionsPath string `mapstructure:"guest_additions_path"` // This is the path to a directory containing an exported virtual machine. - CloneFromVMXCPath string `mapstructure:"clone_from_vmxc_path"` + CloneFromVMCXPath string `mapstructure:"clone_from_vmcx_path"` // This is the name of the virtual machine to clone from. CloneFromVMName string `mapstructure:"clone_from_vm_name"` @@ -158,9 +158,9 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { b.config.Generation = 1 if b.config.CloneFromVMName == "" { - if b.config.CloneFromVMXCPath == "" { + if b.config.CloneFromVMCXPath == "" { errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vm_name must be specified if "+ - "clone_from_vmxc_path is not specified.")) + "clone_from_vmcx_path is not specified.")) } } else { virtualMachineExists, err := powershell.DoesVirtualMachineExist(b.config.CloneFromVMName) @@ -207,16 +207,16 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } } - if b.config.CloneFromVMXCPath == "" { + if b.config.CloneFromVMCXPath == "" { if b.config.CloneFromVMName == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vmxc_path be specified if "+ + errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vmcx_path be specified if "+ "clone_from_vm_name must is not specified.")) } } else { - if _, err := os.Stat(b.config.CloneFromVMXCPath); os.IsNotExist(err) { + if _, err := os.Stat(b.config.CloneFromVMCXPath); os.IsNotExist(err) { if err != nil { errs = packer.MultiErrorAppend( - errs, fmt.Errorf("CloneFromVMXCPath does not exist: %s", err)) + errs, fmt.Errorf("CloneFromVMCXPath does not exist: %s", err)) } } } @@ -426,7 +426,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe SwitchName: b.config.SwitchName, }, &hypervcommon.StepCloneVM{ - CloneFromVMXCPath: b.config.CloneFromVMXCPath, + CloneFromVMCXPath: b.config.CloneFromVMCXPath, CloneFromVMName: b.config.CloneFromVMName, CloneFromSnapshotName: b.config.CloneFromSnapshotName, CloneAllSnapshots: b.config.CloneAllSnapshots, diff --git a/builder/hyperv/vmcx/builder_test.go b/builder/hyperv/vmcx/builder_test.go index ce7729c51..3626a6434 100644 --- a/builder/hyperv/vmcx/builder_test.go +++ b/builder/hyperv/vmcx/builder_test.go @@ -23,7 +23,7 @@ func testConfig() map[string]interface{} { "ssh_username": "foo", "ram_size": 64, "guest_additions_mode": "none", - "clone_from_vmxc_path": "generated", + "clone_from_vmcx_path": "generated", packer.BuildNameConfigKey: "foo", } } @@ -40,13 +40,13 @@ func TestBuilderPrepare_Defaults(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td warns, err := b.Prepare(config) if len(warns) > 0 { @@ -65,13 +65,13 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td // Add a random key config["i_should_not_be_valid"] = true @@ -87,7 +87,7 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { func TestBuilderPrepare_CloneFromExistingMachineOrImportFromExportedMachineSettingsRequired(t *testing.T) { var b Builder config := testConfig() - delete(config, "clone_from_vmxc_path") + delete(config, "clone_from_vmcx_path") warns, err := b.Prepare(config) if len(warns) > 0 { @@ -102,7 +102,7 @@ func TestBuilderPrepare_ExportedMachinePathDoesNotExist(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) @@ -111,7 +111,7 @@ func TestBuilderPrepare_ExportedMachinePathDoesNotExist(t *testing.T) { //Delete the folder immediately os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td warns, err := b.Prepare(config) if len(warns) > 0 { @@ -126,7 +126,7 @@ func TestBuilderPrepare_ExportedMachinePathExists(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) @@ -135,7 +135,7 @@ func TestBuilderPrepare_ExportedMachinePathExists(t *testing.T) { //Only delete afterwards defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td warns, err := b.Prepare(config) if len(warns) > 0 { @@ -146,10 +146,10 @@ func TestBuilderPrepare_ExportedMachinePathExists(t *testing.T) { } } -func disabled_TestBuilderPrepare_CloneFromVmSettingUsedSoNoCloneFromVmxcPathRequired(t *testing.T) { +func disabled_TestBuilderPrepare_CloneFromVmSettingUsedSoNoCloneFromVmcxPathRequired(t *testing.T) { var b Builder config := testConfig() - delete(config, "clone_from_vmxc_path") + delete(config, "clone_from_vmcx_path") config["clone_from_vm_name"] = "test_machine_name_that_does_not_exist" @@ -173,13 +173,13 @@ func TestBuilderPrepare_ISOChecksum(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td // Test bad config["iso_checksum"] = "" @@ -211,13 +211,13 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td // Test bad config["iso_checksum_type"] = "" @@ -275,13 +275,13 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td delete(config, "iso_url") delete(config, "iso_urls") @@ -354,13 +354,13 @@ func TestBuilderPrepare_FloppyFiles(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td delete(config, "floppy_files") warns, err := b.Prepare(config) @@ -396,13 +396,13 @@ func TestBuilderPrepare_InvalidFloppies(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td config["floppy_files"] = []string{"nonexistent.bat", "nonexistent.ps1"} b = Builder{} @@ -421,13 +421,13 @@ func TestBuilderPrepare_CommConfig(t *testing.T) { { config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td config["communicator"] = "winrm" config["winrm_username"] = "username" @@ -458,13 +458,13 @@ func TestBuilderPrepare_CommConfig(t *testing.T) { { config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td config["communicator"] = "ssh" config["ssh_username"] = "username" @@ -496,13 +496,13 @@ func TestUserVariablesInBootCommand(t *testing.T) { var b Builder config := testConfig() - //Create vmxc folder + //Create vmcx folder td, err := ioutil.TempDir("", "packer") if err != nil { t.Fatalf("err: %s", err) } defer os.RemoveAll(td) - config["clone_from_vmxc_path"] = td + config["clone_from_vmcx_path"] = td config[packer.UserVariablesConfigKey] = map[string]string{"test-variable": "test"} config["boot_command"] = []string{"blah {{user `test-variable`}} blah"} diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 32f1cb57a..cc5ecef4a 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -286,7 +286,7 @@ if ((Get-Command Hyper-V\Set-Vm).Parameters["AutomaticCheckpointsEnabled"]) { return err } -func ExportVmxcVirtualMachine(exportPath string, vmName string, snapshotName string, allSnapshots bool) error { +func ExportVmcxVirtualMachine(exportPath string, vmName string, snapshotName string, allSnapshots bool) error { var script = ` param([string]$exportPath, [string]$vmName, [string]$snapshotName, [string]$allSnapshotsString) @@ -333,22 +333,22 @@ $result = Remove-Item -Path $WorkingPath return err } -func CopyVmxcVirtualMachine(exportPath string, cloneFromVmxcPath string) error { +func CopyVmcxVirtualMachine(exportPath string, cloneFromVmcxPath string) error { var script = ` -param([string]$exportPath, [string]$cloneFromVmxcPath) -if (!(Test-Path $cloneFromVmxcPath)){ - throw "Clone from vmxc directory: $cloneFromVmxcPath does not exist!" +param([string]$exportPath, [string]$cloneFromVmcxPath) +if (!(Test-Path $cloneFromVmcxPath)){ + throw "Clone from vmcx directory: $cloneFromVmcxPath does not exist!" } if (!(Test-Path $exportPath)){ New-Item -ItemType Directory -Force -Path $exportPath } -$cloneFromVmxcPath = Join-Path $cloneFromVmxcPath '\*' -Copy-Item $cloneFromVmxcPath $exportPath -Recurse -Force +$cloneFromVmcxPath = Join-Path $cloneFromVmcxPath '\*' +Copy-Item $cloneFromVmcxPath $exportPath -Recurse -Force ` var ps powershell.PowerShellCmd - err := ps.Run(script, exportPath, cloneFromVmxcPath) + err := ps.Run(script, exportPath, cloneFromVmcxPath) return err } @@ -365,7 +365,7 @@ Hyper-V\Set-VMNetworkAdapter $vmName -staticmacaddress $mac return err } -func ImportVmxcVirtualMachine(importPath string, vmName string, harddrivePath string, +func ImportVmcxVirtualMachine(importPath string, vmName string, harddrivePath string, ram int64, switchName string) error { var script = ` @@ -422,24 +422,24 @@ if ($vm) { return err } -func CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, +func CloneVirtualMachine(cloneFromVmcxPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error { if cloneFromVmName != "" { - if err := ExportVmxcVirtualMachine(path, cloneFromVmName, + if err := ExportVmcxVirtualMachine(path, cloneFromVmName, cloneFromSnapshotName, cloneAllSnapshots); err != nil { return err } } - if cloneFromVmxcPath != "" { - if err := CopyVmxcVirtualMachine(path, cloneFromVmxcPath); err != nil { + if cloneFromVmcxPath != "" { + if err := CopyVmcxVirtualMachine(path, cloneFromVmcxPath); err != nil { return err } } - if err := ImportVmxcVirtualMachine(path, vmName, harddrivePath, ram, switchName); err != nil { + if err := ImportVmcxVirtualMachine(path, vmName, harddrivePath, ram, switchName); err != nil { return err } diff --git a/website/source/docs/builders/hyperv-vmcx.html.md.erb b/website/source/docs/builders/hyperv-vmcx.html.md.erb index de1b22891..f509463c4 100644 --- a/website/source/docs/builders/hyperv-vmcx.html.md.erb +++ b/website/source/docs/builders/hyperv-vmcx.html.md.erb @@ -33,7 +33,7 @@ Import from folder: ``` json { "type": "hyperv-vmcx", - "clone_from_vmxc_path": "c:\\virtual machines\\ubuntu-12.04.5-server-amd64", + "clone_from_vmcx_path": "c:\\virtual machines\\ubuntu-12.04.5-server-amd64", "ssh_username": "packer", "ssh_password": "packer", "shutdown_command": "echo 'packer' | sudo -S shutdown -P now" @@ -71,7 +71,7 @@ builder. ### Required for virtual machine import: -- `clone_from_vmxc_path` (string) - The path to the exported virtual machine +- `clone_from_vmcx_path` (string) - The path to the exported virtual machine folder. ### Required for virtual machine clone: From c8f54d5291c95dc2032fa04a4987f8b063997576 Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 19 Jul 2018 20:10:20 +0100 Subject: [PATCH 32/33] Fixer and tests to convert 'clone_from_vmxc_path' -> 'clone_from_vmcx_path' --- command/fix.go | 2 + fix/fixer.go | 2 + fix/fixer_hyperv_vmxc_typo.go | 52 ++++++++++++++++++++++++ fix/fixer_hyperv_vmxc_typo_test.go | 64 ++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 fix/fixer_hyperv_vmxc_typo.go create mode 100644 fix/fixer_hyperv_vmxc_typo_test.go diff --git a/command/fix.go b/command/fix.go index b8c1023c3..ca096306c 100644 --- a/command/fix.go +++ b/command/fix.go @@ -155,6 +155,8 @@ Fixes that are run: elevated username and password strings hyperv-deprecations Removes the deprecated "vhd_temp_path" setting from Hyper-V ISO builder templates + hyperv-vmxc-typo Corrects a typo in the "clone_from_vmxc_path" + setting. Replaces with "clone_from_vmcx_path". Options: diff --git a/fix/fixer.go b/fix/fixer.go index cc8aa1258..c6278bda6 100644 --- a/fix/fixer.go +++ b/fix/fixer.go @@ -37,6 +37,7 @@ func init() { "docker-email": new(FixerDockerEmail), "powershell-escapes": new(FixerPowerShellEscapes), "hyperv-deprecations": new(FixerHypervDeprecations), + "hyperv-vmxc-typo": new(FixerHypervVmxcTypo), } FixerOrder = []string{ @@ -57,5 +58,6 @@ func init() { "docker-email", "powershell-escapes", "hyperv-deprecations", + "hyperv-vmxc-typo", } } diff --git a/fix/fixer_hyperv_vmxc_typo.go b/fix/fixer_hyperv_vmxc_typo.go new file mode 100644 index 000000000..fbc057c54 --- /dev/null +++ b/fix/fixer_hyperv_vmxc_typo.go @@ -0,0 +1,52 @@ +package fix + +import ( + "github.com/mitchellh/mapstructure" +) + +// FixerHypervVmxcTypo fixes the typo in "clone_from_vmxc_path" replacing +// it with "clone_from_vmcx_path" in Hyper-V VMCX builder templates +type FixerHypervVmxcTypo struct{} + +func (FixerHypervVmxcTypo) Fix(input map[string]interface{}) (map[string]interface{}, error) { + // The type we'll decode into; we only care about builders + type template struct { + Builders []map[string]interface{} + } + + // Decode the input into our structure, if we can + var tpl template + if err := mapstructure.Decode(input, &tpl); err != nil { + return nil, err + } + + for _, builder := range tpl.Builders { + builderTypeRaw, ok := builder["type"] + if !ok { + continue + } + + builderType, ok := builderTypeRaw.(string) + if !ok { + continue + } + + if builderType != "hyperv-vmcx" { + continue + } + + path, ok := builder["clone_from_vmxc_path"] + if ok { + delete(builder, "clone_from_vmxc_path") + builder["clone_from_vmcx_path"] = path + } + } + + input["builders"] = tpl.Builders + return input, nil +} + +func (FixerHypervVmxcTypo) Synopsis() string { + return `Fixes a typo replacing "clone_from_vmxc_path" with "clone_from_vmcx_path" ` + + `in Hyper-V VMCX builder templates` +} diff --git a/fix/fixer_hyperv_vmxc_typo_test.go b/fix/fixer_hyperv_vmxc_typo_test.go new file mode 100644 index 000000000..dbbea7072 --- /dev/null +++ b/fix/fixer_hyperv_vmxc_typo_test.go @@ -0,0 +1,64 @@ +package fix + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFixerHypervVmxcTypo_impl(t *testing.T) { + var _ Fixer = new(FixerHypervVmxcTypo) +} + +func TestFixerHypervVmxcTypo_Fix(t *testing.T) { + cases := []struct { + Input map[string]interface{} + Expected map[string]interface{} + }{ + // No "clone_from_vmxc_path" in template - noop + { + Input: map[string]interface{}{ + "type": "hyperv-vmcx", + "temp_path": "C:/some/temp/path", + }, + + Expected: map[string]interface{}{ + "type": "hyperv-vmcx", + "temp_path": "C:/some/temp/path", + }, + }, + + // "clone_from_vmxc_path" should be replaced with + // "clone_from_vmcx_path" in template + { + Input: map[string]interface{}{ + "type": "hyperv-vmcx", + "clone_from_vmxc_path": "C:/some/vmcx/path", + }, + + Expected: map[string]interface{}{ + "type": "hyperv-vmcx", + "clone_from_vmcx_path": "C:/some/vmcx/path", + }, + }, + } + + for _, tc := range cases { + var f FixerHypervVmxcTypo + + input := map[string]interface{}{ + "builders": []map[string]interface{}{tc.Input}, + } + + expected := map[string]interface{}{ + "builders": []map[string]interface{}{tc.Expected}, + } + + output, err := f.Fix(input) + if err != nil { + t.Fatalf("err: %s", err) + } + + assert.Equal(t, expected, output, "Should be equal") + } +} From 1a7804221e673407b7f87747b4d4efc312fa20b9 Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 19 Jul 2018 23:38:33 +0100 Subject: [PATCH 33/33] Minor fixes, changes and improvements to Hyper-V VMCX docs --- .../docs/builders/hyperv-vmcx.html.md.erb | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/website/source/docs/builders/hyperv-vmcx.html.md.erb b/website/source/docs/builders/hyperv-vmcx.html.md.erb index f509463c4..040f86217 100644 --- a/website/source/docs/builders/hyperv-vmcx.html.md.erb +++ b/website/source/docs/builders/hyperv-vmcx.html.md.erb @@ -22,18 +22,18 @@ boots it, provisions software within the OS, and then shuts it down. The result of the Hyper-V builder is a directory containing all the files necessary to run the virtual machine portably. -## Basic Example +## Basic Examples -Here is a basic example. This example is not functional. It will start the OS -installer but then fail because we don't provide the preseed file for Ubuntu -to self-install. Still, the example serves to show the basic configuration: +Here are some basic examples. Neither example would really do anything more +than producing a copy of the source virtual machine. However, the examples +could be used as a starting point for more advanced templates. Import from folder: ``` json { "type": "hyperv-vmcx", - "clone_from_vmcx_path": "c:\\virtual machines\\ubuntu-12.04.5-server-amd64", + "clone_from_vmcx_path": "c:/virtual machines/ubuntu-12.04.5-server-amd64", "ssh_username": "packer", "ssh_password": "packer", "shutdown_command": "echo 'packer' | sudo -S shutdown -P now" @@ -71,12 +71,14 @@ builder. ### Required for virtual machine import: -- `clone_from_vmcx_path` (string) - The path to the exported virtual machine - folder. +- `clone_from_vmcx_path` (string) - The path to a directory containing a + previously exported virtual machine. The exported machine will be used + as the source for new VM. + ### Required for virtual machine clone: -- `clone_from_vm_name` (string) - The name of the vm to clone from. Ideally +- `clone_from_vm_name` (string) - The name of the VM to clone from. Ideally the machine to clone from should be shutdown. ### Optional: @@ -94,11 +96,25 @@ builder. Packer to wait for 1 minute 30 seconds before typing the boot command. The default duration is "10s" (10 seconds). -- `clone_all_snapshots` (boolean) - If set to `true` all snapshots will be - cloned when the machine is cloned. +- `clone_all_snapshots` (boolean) - If set to `true` all snapshots + present in the source machine will be copied when the machine is + cloned. The final result of the build will be an exported virtual + machine that contains all the snapshots of the parent. -- `clone_from_snapshot_name` (string) - The name of the snapshot to clone - from. + If `clone_from_vmcx_path` is set, this setting has no effect and any + snapshots associated with the source VM will also be present in the + resultant VM. + + If `clone_from_snapshot_name` is configured, this setting has no + effect and the resultant machine will not contain any snapshots. + +- `clone_from_snapshot_name` (string) - The name of a snapshot in the + source machine to use as a starting point for the clone. If the value + given is an empty string, the last snapshot present in the source will + be chosen as the starting point for the new VM. + + This setting only has an effect if using `clone_from_vm_name` and is + ignored otherwise. - `cpu` (number) - The number of CPUs the virtual machine should use. If this isn't specified, the default is 1 CPU.