diff --git a/builder/vmware/common/step_configure_vmx.go b/builder/vmware/common/step_configure_vmx.go index 102ac52ee..c3f63b2b2 100644 --- a/builder/vmware/common/step_configure_vmx.go +++ b/builder/vmware/common/step_configure_vmx.go @@ -16,6 +16,9 @@ import ( // // Uses: // vmx_path string +// +// Produces: +// display_name string - Value of the displayName key set in the VMX file type StepConfigureVMX struct { CustomData map[string]string SkipFloppy bool @@ -73,6 +76,19 @@ func (s *StepConfigureVMX) Run(_ context.Context, state multistep.StateBag) mult return multistep.ActionHalt } + // If the build is taking place on a remote ESX server, the displayName + // will be needed for discovery of the VM's IP address and for export + // of the VM. The displayName key should always be set in the VMX file, + // so error if we don't find it + if displayName, ok := vmxData["displayname"]; !ok { // Packer converts key names to lowercase! + err := fmt.Errorf("Error: Could not get value of displayName from VMX data") + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } else { + state.Put("display_name", displayName) + } + return multistep.ActionContinue } diff --git a/builder/vmware/common/step_configure_vmx_test.go b/builder/vmware/common/step_configure_vmx_test.go index c5e242a51..d06ae51b9 100644 --- a/builder/vmware/common/step_configure_vmx_test.go +++ b/builder/vmware/common/step_configure_vmx_test.go @@ -14,6 +14,9 @@ func testVMXFile(t *testing.T) string { if err != nil { t.Fatalf("err: %s", err) } + + // displayName must always be set + err = WriteVMX(tf.Name(), map[string]string{"displayName": "PackerBuild"}) tf.Close() return tf.Name() @@ -132,12 +135,29 @@ func TestStepConfigureVMX_generatedAddresses(t *testing.T) { vmxPath := testVMXFile(t) defer os.Remove(vmxPath) - err := WriteVMX(vmxPath, map[string]string{ - "foo": "bar", - "ethernet0.generatedAddress": "foo", - "ethernet1.generatedAddress": "foo", - "ethernet1.generatedAddressOffset": "foo", - }) + additionalTestVmxData := []struct { + Key string + Value string + }{ + {"foo", "bar"}, + {"ethernet0.generatedaddress", "foo"}, + {"ethernet1.generatedaddress", "foo"}, + {"ethernet1.generatedaddressoffset", "foo"}, + } + + // Get any existing VMX data from the VMX file + vmxData, err := ReadVMX(vmxPath) + if err != nil { + t.Fatalf("err %s", err) + } + + // Add the additional key/value pairs we need for this test to the existing VMX data + for _, data := range additionalTestVmxData { + vmxData[data.Key] = data.Value + } + + // Recreate the VMX file so it includes all the data needed for this test + err = WriteVMX(vmxPath, vmxData) if err != nil { t.Fatalf("err: %s", err) } @@ -157,7 +177,7 @@ func TestStepConfigureVMX_generatedAddresses(t *testing.T) { if err != nil { t.Fatalf("err: %s", err) } - vmxData := ParseVMX(string(vmxContents)) + vmxData = ParseVMX(string(vmxContents)) cases := []struct { Key string @@ -180,5 +200,71 @@ func TestStepConfigureVMX_generatedAddresses(t *testing.T) { } } } +} + +// Should fail if the displayName key is not found in the VMX +func TestStepConfigureVMX_displayNameMissing(t *testing.T) { + state := testState(t) + step := new(StepConfigureVMX) + + // testVMXFile adds displayName key/value pair to the VMX + vmxPath := testVMXFile(t) + defer os.Remove(vmxPath) + + // Bad: Delete displayName from the VMX/Create an empty VMX file + err := WriteVMX(vmxPath, map[string]string{}) + if err != nil { + t.Fatalf("err: %s", err) + } + + state.Put("vmx_path", vmxPath) + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionHalt { + t.Fatalf("bad action: %#v. Should halt when displayName key is missing from VMX", action) + } + if _, ok := state.GetOk("error"); !ok { + t.Fatal("should store error in state when displayName key is missing from VMX") + } +} + +// Should store the value of displayName in the statebag +func TestStepConfigureVMX_displayNameStore(t *testing.T) { + state := testState(t) + step := new(StepConfigureVMX) + + // testVMXFile adds displayName key/value pair to the VMX + vmxPath := testVMXFile(t) + defer os.Remove(vmxPath) + + state.Put("vmx_path", vmxPath) + + // 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 value of displayName must be stored in the statebag + if _, ok := state.GetOk("display_name"); !ok { + t.Fatalf("displayName should be stored in the statebag as 'display_name'") + } +} + +func TestStepConfigureVMX_vmxPathBad(t *testing.T) { + state := testState(t) + step := new(StepConfigureVMX) + + state.Put("vmx_path", "some_bad_path") + + // Test the run + if action := step.Run(context.Background(), state); action != multistep.ActionHalt { + t.Fatalf("bad action: %#v. Should halt when vmxPath is bad", action) + } + if _, ok := state.GetOk("error"); !ok { + t.Fatal("should store error in state when vmxPath is bad") + } } diff --git a/builder/vmware/iso/driver_esx5.go b/builder/vmware/iso/driver_esx5.go index b3eb8c078..bfeee8d24 100644 --- a/builder/vmware/iso/driver_esx5.go +++ b/builder/vmware/iso/driver_esx5.go @@ -389,7 +389,13 @@ func (d *ESX5Driver) CommHost(state multistep.StateBag) (string, error) { return "", err } - record, err := r.find("Name", config.VMName) + // The value in the Name field returned by 'esxcli network vm list' + // corresponds directly to the value of displayName set in the VMX file + var displayName string + if v, ok := state.GetOk("display_name"); ok { + displayName = v.(string) + } + record, err := r.find("Name", displayName) if err != nil { return "", err } diff --git a/builder/vmware/iso/step_export.go b/builder/vmware/iso/step_export.go index 50b1efd9f..c39443c8f 100644 --- a/builder/vmware/iso/step_export.go +++ b/builder/vmware/iso/step_export.go @@ -14,13 +14,17 @@ import ( "github.com/hashicorp/packer/packer" ) +// This step exports a VM built on ESXi using ovftool +// +// Uses: +// display_name string type StepExport struct { Format string SkipExport bool OutputDir string } -func (s *StepExport) generateArgs(c *Config, hidePassword bool) []string { +func (s *StepExport) generateArgs(c *Config, displayName string, hidePassword bool) []string { password := url.QueryEscape(c.RemotePassword) if hidePassword { password = "****" @@ -29,7 +33,7 @@ func (s *StepExport) generateArgs(c *Config, hidePassword bool) []string { "--noSSLVerify=true", "--skipManifestCheck", "-tt=" + s.Format, - "vi://" + c.RemoteUser + ":" + password + "@" + c.RemoteHost + "/" + c.VMName, + "vi://" + c.RemoteUser + ":" + password + "@" + c.RemoteHost + "/" + displayName, s.OutputDir, } return append(c.OVFToolOptions, args...) @@ -72,9 +76,13 @@ func (s *StepExport) Run(_ context.Context, state multistep.StateBag) multistep. } ui.Say("Exporting virtual machine...") - ui.Message(fmt.Sprintf("Executing: %s %s", ovftool, strings.Join(s.generateArgs(c, true), " "))) + var displayName string + if v, ok := state.GetOk("display_name"); ok { + displayName = v.(string) + } + ui.Message(fmt.Sprintf("Executing: %s %s", ovftool, strings.Join(s.generateArgs(c, displayName, true), " "))) var out bytes.Buffer - cmd := exec.Command(ovftool, s.generateArgs(c, false)...) + cmd := exec.Command(ovftool, s.generateArgs(c, displayName, false)...) cmd.Stdout = &out if err := cmd.Run(); err != nil { err := fmt.Errorf("Error exporting virtual machine: %s\n%s\n", err, out.String())