From 7d9c252b3ab9030e1e6ae8afb5284294483118ce Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Tue, 16 Sep 2014 18:27:00 -0700 Subject: [PATCH 1/2] Clean VMX step should always remove floppy. When using the VMX builder its possible for the source machine to have a floppy configured which gets cloned to the new VM Packer spins up. When the new VM's Packer config doesn't have a floppy_files config entry, the Packer clean VMX step fails to remove the floppy disk from the new VM. This can cause build failures, for example with the vsphere post processor; generating errors like: * Post-processor failed: Failed: exit status 1 Error: File (/home/teamcity/tmp/buildTmp/packer941120499) could not be found. Opening the cloned VM's VMX file you can clearly see it has a floppy entry from the source machine's VMX file (exact same path) even though the Packer config contains no floppy_files entry. --- builder/vmware/common/step_clean_vmx.go | 16 +++++++--------- builder/vmware/common/step_clean_vmx_test.go | 1 - 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/builder/vmware/common/step_clean_vmx.go b/builder/vmware/common/step_clean_vmx.go index b55bcd549..9c677e3a8 100644 --- a/builder/vmware/common/step_clean_vmx.go +++ b/builder/vmware/common/step_clean_vmx.go @@ -32,17 +32,15 @@ func (s StepCleanVMX) Run(state multistep.StateBag) multistep.StepAction { return multistep.ActionHalt } - if _, ok := state.GetOk("floppy_path"); ok { - // Delete the floppy0 entries so the floppy is no longer mounted - ui.Message("Unmounting floppy from VMX...") - for k, _ := range vmxData { - if strings.HasPrefix(k, "floppy0.") { - log.Printf("Deleting key: %s", k) - delete(vmxData, k) - } + // Delete the floppy0 entries so the floppy is no longer mounted + ui.Message("Unmounting floppy from VMX...") + for k, _ := range vmxData { + if strings.HasPrefix(k, "floppy0.") { + log.Printf("Deleting key: %s", k) + delete(vmxData, k) } - vmxData["floppy0.present"] = "FALSE" } + vmxData["floppy0.present"] = "FALSE" if isoPathRaw, ok := state.GetOk("iso_path"); ok { isoPath := isoPathRaw.(string) diff --git a/builder/vmware/common/step_clean_vmx_test.go b/builder/vmware/common/step_clean_vmx_test.go index 59877027a..73a8abf45 100644 --- a/builder/vmware/common/step_clean_vmx_test.go +++ b/builder/vmware/common/step_clean_vmx_test.go @@ -39,7 +39,6 @@ func TestStepCleanVMX_floppyPath(t *testing.T) { t.Fatalf("err: %s", err) } - state.Put("floppy_path", "foo") state.Put("vmx_path", vmxPath) // Test the run From 5fd9651982770cc5d83e7f3d1f3580dd2c42e8ca Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Wed, 17 Sep 2014 11:06:56 -0700 Subject: [PATCH 2/2] GH 1508 - Ensure Packer VMX is updated and saved We need to ensure the VMWare process has exited before attempting to run VMX file cleanup steps, otherwise VMWare may overwrite our changes. While Packer does its best to ensure VMWare has exited, there's still a race condition on some OSs between VMWare flushing the VMX and Packer updating it. The workaround is to artifically wait 5 seconds. When using the VMX builder its possible for the source machine to have a floppy and/or CD-ROM mounted which gets cloned to the new VM Packer spins up, but have no Packer configuration for those devices. With this change we always attempt to remove the mounted devices regardless of the Packer configuration. --- builder/vmware/common/step_clean_vmx.go | 25 +++++++------------- builder/vmware/common/step_clean_vmx_test.go | 2 +- builder/vmware/common/step_shutdown.go | 10 +++++--- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/builder/vmware/common/step_clean_vmx.go b/builder/vmware/common/step_clean_vmx.go index 9c677e3a8..df7b42763 100644 --- a/builder/vmware/common/step_clean_vmx.go +++ b/builder/vmware/common/step_clean_vmx.go @@ -42,26 +42,17 @@ func (s StepCleanVMX) Run(state multistep.StateBag) multistep.StepAction { } vmxData["floppy0.present"] = "FALSE" - if isoPathRaw, ok := state.GetOk("iso_path"); ok { - isoPath := isoPathRaw.(string) + devRe := regexp.MustCompile(`^ide\d:\d\.`) + for k, v := range vmxData { + ide := devRe.FindString(k) + if ide == "" || v != "cdrom-image" { + continue + } ui.Message("Detaching ISO from CD-ROM device...") - devRe := regexp.MustCompile(`^ide\d:\d\.`) - for k, _ := range vmxData { - match := devRe.FindString(k) - if match == "" { - continue - } - filenameKey := match + "filename" - if filename, ok := vmxData[filenameKey]; ok { - if filename == isoPath { - // Change the CD-ROM device back to auto-detect to eject - vmxData[filenameKey] = "auto detect" - vmxData[match+"devicetype"] = "cdrom-raw" - } - } - } + vmxData[ide+"devicetype"] = "cdrom-raw" + vmxData[ide+"filename"] = "auto detect" } // Rewrite the VMX diff --git a/builder/vmware/common/step_clean_vmx_test.go b/builder/vmware/common/step_clean_vmx_test.go index 73a8abf45..ea30fb54a 100644 --- a/builder/vmware/common/step_clean_vmx_test.go +++ b/builder/vmware/common/step_clean_vmx_test.go @@ -88,7 +88,6 @@ func TestStepCleanVMX_isoPath(t *testing.T) { t.Fatalf("err: %s", err) } - state.Put("iso_path", "foo") state.Put("vmx_path", vmxPath) // Test the run @@ -135,6 +134,7 @@ floppy0.filetype = "file" ` const testVMXISOPath = ` +ide0:0.devicetype = "cdrom-image" ide0:0.filename = "foo" ide0:1.filename = "bar" foo = "bar" diff --git a/builder/vmware/common/step_shutdown.go b/builder/vmware/common/step_shutdown.go index 1b5b95f95..5aad45fd0 100644 --- a/builder/vmware/common/step_shutdown.go +++ b/builder/vmware/common/step_shutdown.go @@ -137,10 +137,14 @@ LockWaitLoop: } } - if runtime.GOOS == "windows" && !s.Testing { + if runtime.GOOS != "darwin" && !s.Testing { // Windows takes a while to yield control of the files when the - // process is exiting. We just sleep here. In the future, it'd be - // nice to find a better solution to this. + // process is exiting. Ubuntu will yield control of the files but + // VMWare may overwrite the VMX cleanup steps that run after this, + // so we wait to ensure VMWare has exited and flushed the VMX. + + // We just sleep here. In the future, it'd be nice to find a better + // solution to this. time.Sleep(5 * time.Second) }