Do not add remotedisplay.vnc.ip to vmx data on ESXi

* The remotedisplay.vnc.ip vmx data key breaks ESXi, this commit prevents it
  from being automatically added during VNC configuration when using the ESX5
  driver.

* It can still be configured via the vmx_data section of the builder
  template

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

builder/vmware/common/step_configure_vnc.go

@@ -28,6 +28,9 @@ type StepConfigureVNC struct {
 
 type VNCAddressFinder interface {
 	VNCAddress(string, uint, uint) (string, uint, error)
+
+	// UpdateVMX, sets driver specific VNC values to VMX data.
+	UpdateVMX(vncAddress string, vncPort uint, vmxData map[string]string)
 }
 
 func (StepConfigureVNC) VNCAddress(vncBindAddress string, portMin, portMax uint) (string, uint, error) {
@@ -91,9 +94,7 @@ func (s *StepConfigureVNC) Run(state multistep.StateBag) multistep.StepAction {
 	log.Printf("Found available VNC port: %d", vncPort)
 
 	vmxData := ParseVMX(string(vmxBytes))
-	vmxData["remotedisplay.vnc.enabled"] = "TRUE"
+	vncFinder.UpdateVMX(vncBindAddress, vncPort, vmxData)
-	vmxData["remotedisplay.vnc.port"] = fmt.Sprintf("%d", vncPort)
-	vmxData["remotedisplay.vnc.ip"] = fmt.Sprintf("%s", vncBindAddress)
 
 	if err := WriteVMX(vmxPath, vmxData); err != nil {
 		err := fmt.Errorf("Error writing VMX data: %s", err)
@@ -108,5 +109,11 @@ func (s *StepConfigureVNC) Run(state multistep.StateBag) multistep.StepAction {
 	return multistep.ActionContinue
 }
 
+func (StepConfigureVNC) UpdateVMX(address string, port uint, data map[string]string) {
+	data["remotedisplay.vnc.enabled"] = "TRUE"
+	data["remotedisplay.vnc.port"] = fmt.Sprintf("%d", port)
+	data["remotedisplay.vnc.ip"] = address
+}
+
 func (StepConfigureVNC) Cleanup(multistep.StateBag) {
 }

builder/vmware/common/step_configure_vnc_test.go

@@ -0,0 +1,25 @@
+package common
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestStepConfigureVNC_implVNCAddressFinder(t *testing.T) {
+	var _ VNCAddressFinder = new(StepConfigureVNC)
+}
+
+func TestStepConfigureVNC_UpdateVMX(t *testing.T) {
+	var s StepConfigureVNC
+	data := make(map[string]string)
+	s.UpdateVMX("0.0.0.0", 5900, data)
+	if ip := data["remotedisplay.vnc.ip"]; ip != "0.0.0.0" {
+		t.Errorf("bad VMX data for key remotedisplay.vnc.ip: %v", ip)
+	}
+	if enabled := data["remotedisplay.vnc.enabled"]; enabled != "TRUE" {
+		t.Errorf("bad VMX data for key remotedisplay.vnc.enabled: %v", enabled)
+	}
+	if port := data["remotedisplay.vnc.port"]; port != fmt.Sprint(port) {
+		t.Errorf("bad VMX data for key remotedisplay.vnc.port: %v", port)
+	}
+}

builder/vmware/iso/driver_esx5.go

@@ -176,7 +176,7 @@ func (d *ESX5Driver) HostIP() (string, error) {
 	return host, err
 }
 
-func (d *ESX5Driver) VNCAddress(vncBindIP string, portMin, portMax uint) (string, uint, error) {
+func (d *ESX5Driver) VNCAddress(_ string, portMin, portMax uint) (string, uint, error) {
 	var vncPort uint
 
 	//Process ports ESXi is listening on to determine which are available
@@ -232,6 +232,13 @@ func (d *ESX5Driver) VNCAddress(vncBindIP string, portMin, portMax uint) (string
 	return d.Host, vncPort, nil
 }
 
+// UpdateVMX, adds the VNC port to the VMX data.
+func (ESX5Driver) UpdateVMX(_ string, port uint, data map[string]string) {
+	// Do not set remotedisplay.vnc.ip - this breaks ESXi.
+	data["remotedisplay.vnc.enabled"] = "TRUE"
+	data["remotedisplay.vnc.port"] = fmt.Sprintf("%d", port)
+}
+
 func (d *ESX5Driver) CommHost(state multistep.StateBag) (string, error) {
 	config := state.Get("config").(*Config)
 

builder/vmware/iso/driver_esx5_test.go

@@ -17,6 +17,10 @@ func TestESX5Driver_implOutputDir(t *testing.T) {
 	var _ vmwcommon.OutputDir = new(ESX5Driver)
 }
 
+func TestESX5Driver_implVNCAddressFinder(t *testing.T) {
+	var _ vmwcommon.VNCAddressFinder = new(ESX5Driver)
+}
+
 func TestESX5Driver_implRemoteDriver(t *testing.T) {
 	var _ RemoteDriver = new(ESX5Driver)
 }
@@ -76,3 +80,19 @@ func TestESX5Driver_CommHost(t *testing.T) {
 		t.Errorf("bad vm_address: %s", address.(string))
 	}
 }
+
+func TestESX5Driver_UpdateVMX(t *testing.T) {
+	var driver ESX5Driver
+	data := make(map[string]string)
+	driver.UpdateVMX("0.0.0.0", 5900, data)
+	if _, ok := data["remotedisplay.vnc.ip"]; ok {
+		// Do not add the remotedisplay.vnc.ip on ESXi
+		t.Fatal("invalid VMX data key: remotedisplay.vnc.ip")
+	}
+	if enabled := data["remotedisplay.vnc.enabled"]; enabled != "TRUE" {
+		t.Errorf("bad VMX data for key remotedisplay.vnc.enabled: %v", enabled)
+	}
+	if port := data["remotedisplay.vnc.port"]; port != fmt.Sprint(port) {
+		t.Errorf("bad VMX data for key remotedisplay.vnc.port: %v", port)
+	}
+}