builder/vmware: make things more Go-like

This commit currently breaks the builder though, since the ISo is now
uploaded back into ESX.

builder/vmware/builder.go

@@ -138,6 +138,18 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		b.config.OutputDir = fmt.Sprintf("output-%s", b.config.PackerBuildName)
 	}
 
+	if b.config.RemoteUser == "" {
+		b.config.RemoteUser = "root"
+	}
+
+	if b.config.RemoteDatastore == "" {
+		b.config.RemoteDatastore = "datastore1"
+	}
+
+	if b.config.RemotePort == 0 {
+		b.config.RemotePort = 22
+	}
+
 	if b.config.SSHPort == 0 {
 		b.config.SSHPort = 22
 	}
@@ -165,6 +177,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		"shutdown_timeout":    &b.config.RawShutdownTimeout,
 		"ssh_wait_timeout":    &b.config.RawSSHWaitTimeout,
 		"vmx_template_path":   &b.config.VMXTemplatePath,
+		"remote_type":         &b.config.RemoteType,
 		"remote_host":         &b.config.RemoteHost,
 		"remote_datastore":    &b.config.RemoteDatastore,
 		"remote_user":         &b.config.RemoteUser,
@@ -338,6 +351,14 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 			errs, fmt.Errorf("vnc_port_min must be less than vnc_port_max"))
 	}
 
+	// Remote configuration validation
+	if b.config.RemoteType != "" {
+		if b.config.RemoteHost == "" {
+			errs = packer.MultiErrorAppend(errs,
+				fmt.Errorf("remote_host must be specified"))
+		}
+	}
+
 	// Warnings
 	if b.config.ShutdownCommand == "" {
 		warnings = append(warnings,
@@ -353,20 +374,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 }
 
 func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
-	// Initialize the driver that will handle our interaction with VMware
+	driver, err := NewDriver(&b.config)
-	var driver Driver
-	var err error
-	var sshAddressFunc func(multistep.StateBag) (string, error) = sshAddress
-	var downloadFunc func(*common.DownloadConfig, multistep.StateBag) (string, error, bool)
-
-	if b.config.RemoteType == "" {
-		driver, err = NewDriver()
-	} else {
-		driver, err = NewRemoteDriver(&b.config)
-		sshAddressFunc = driver.(RemoteDriver).SSHAddress()
-		downloadFunc = driver.(RemoteDriver).Download()
-	}
-
 	if err != nil {
 		return nil, fmt.Errorf("Failed creating VMware driver: %s", err)
 	}
@@ -382,7 +390,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Description:  "ISO",
 			ResultKey:    "iso_path",
 			Url:          b.config.ISOUrls,
-			Download:     downloadFunc,
 		},
 		&stepPrepareOutputDir{},
 		&common.StepCreateFloppy{
@@ -395,7 +402,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&stepRun{},
 		&stepTypeBootCommand{},
 		&common.StepConnectSSH{
-			SSHAddress:     sshAddressFunc,
+			SSHAddress:     driver.SSHAddress,
 			SSHConfig:      sshConfig,
 			SSHWaitTimeout: b.config.sshWaitTimeout,
 			NoPty:          b.config.SSHSkipRequestPty,

builder/vmware/driver.go

@@ -3,6 +3,7 @@ package vmware
 import (
 	"bytes"
 	"fmt"
+	"github.com/mitchellh/multistep"
 	"log"
 	"os/exec"
 	"runtime"
@@ -20,6 +21,10 @@ type Driver interface {
 	// Checks if the VMX file at the given path is running.
 	IsRunning(string) (bool, error)
 
+	// SSHAddress returns the SSH address for the VM that is being
+	// managed by this driver.
+	SSHAddress(multistep.StateBag) (string, error)
+
 	// Start starts a VM specified by the path to the VMX given.
 	Start(string, bool) error
 
@@ -41,9 +46,20 @@ type Driver interface {
 
 // NewDriver returns a new driver implementation for this operating
 // system, or an error if the driver couldn't be initialized.
-func NewDriver() (Driver, error) {
+func NewDriver(config *config) (Driver, error) {
 	drivers := []Driver{}
 
+	if config.RemoteType != "" {
+		drivers = []Driver{
+			&ESX5Driver{
+				Host:      config.RemoteHost,
+				Port:      config.RemotePort,
+				Username:  config.RemoteUser,
+				Password:  config.RemotePassword,
+				Datastore: config.RemoteDatastore,
+			},
+		}
+	} else {
 		switch runtime.GOOS {
 		case "darwin":
 			drivers = []Driver{
@@ -63,6 +79,7 @@ func NewDriver() (Driver, error) {
 		default:
 			return nil, fmt.Errorf("can't find driver for OS: %s", runtime.GOOS)
 		}
+	}
 
 	errs := ""
 	for _, driver := range drivers {

builder/vmware/driver_esx5.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"github.com/mitchellh/multistep"
-	"github.com/mitchellh/packer/common"
 	"github.com/mitchellh/packer/communicator/ssh"
 	"github.com/mitchellh/packer/packer"
 	"io"
@@ -21,8 +20,13 @@ import (
 )
 
 type ESX5Driver struct {
+	Host      string
+	Port      uint
+	Username  string
+	Password  string
+	Datastore string
+
 	comm packer.Communicator
-	config *config
 }
 
 func (d *ESX5Driver) CompactDisk(diskPathLocal string) error {
@@ -76,12 +80,10 @@ func (d *ESX5Driver) Verify() error {
 		d.connect,
 		d.checkSystemVersion,
 		d.checkGuestIPHackEnabled,
-		d.checkOutputFolder,
 	}
 
 	for _, check := range checks {
-		err := check()
+		if err := check(); err != nil {
-		if err != nil {
 			return err
 		}
 	}
@@ -90,7 +92,7 @@ func (d *ESX5Driver) Verify() error {
 }
 
 func (d *ESX5Driver) HostIP() (string, error) {
-	ip := net.ParseIP(d.config.RemoteHost)
+	ip := net.ParseIP(d.Host)
 	interfaces, err := net.Interfaces()
 	if err != nil {
 		return "", err
@@ -117,7 +119,7 @@ func (d *ESX5Driver) VNCAddress(portMin, portMax uint) (string, uint) {
 	var vncPort uint
 	// TODO(dougm) use esxcli network ip connection list
 	for port := portMin; port <= portMax; port++ {
-		address := fmt.Sprintf("%s:%d", d.config.RemoteHost, port)
+		address := fmt.Sprintf("%s:%d", d.Host, port)
 		log.Printf("Trying address: %s...", address)
 		l, err := net.DialTimeout("tcp", address, 1*time.Second)
 
@@ -135,23 +137,79 @@ func (d *ESX5Driver) VNCAddress(portMin, portMax uint) (string, uint) {
 		}
 	}
 
-	return d.config.RemoteHost, vncPort
+	return d.Host, vncPort
 }
 
-func (d *ESX5Driver) SSHAddress() func(multistep.StateBag) (string, error) {
+func (d *ESX5Driver) SSHAddress(state multistep.StateBag) (string, error) {
-	return d.sshAddress
+	config := state.Get("config").(*config)
+
+	if address, ok := state.GetOk("vm_address"); ok {
+		return address.(string), nil
 	}
 
-func (d *ESX5Driver) Download() func(*common.DownloadConfig, multistep.StateBag) (string, error, bool) {
+	r, err := d.esxcli("network", "vm", "list")
-	return d.download
-}
-
-func (d *ESX5Driver) FileExists(path string) bool {
-	err := d.sh("test", "-e", d.datastorePath(path))
 	if err != nil {
-		return false
+		return "", err
 	}
-	return true
+
+	record, err := r.find("Name", config.VMName)
+	if err != nil {
+		return "", err
+	}
+	wid := record["WorldID"]
+	if wid == "" {
+		return "", errors.New("VM WorldID not found")
+	}
+
+	r, err = d.esxcli("network", "vm", "port", "list", "-w", wid)
+	if err != nil {
+		return "", err
+	}
+
+	record, err = r.read()
+	if err != nil {
+		return "", err
+	}
+
+	if record["IPAddress"] == "0.0.0.0" {
+		return "", errors.New("VM network port found, but no IP address")
+	}
+
+	address := fmt.Sprintf("%s:%d", record["IPAddress"], config.SSHPort)
+	state.Put("vm_address", address)
+	return address, nil
+}
+
+/*
+func (d *ESX5Driver) Download(*common.DownloadConfig, multistep.StateBag) (string, error, bool) {
+	config := state.Get("config").(*config)
+
+	cacheRoot, _ := filepath.Abs(".")
+	targetFile, err := filepath.Rel(cacheRoot, dconfig.TargetPath)
+	if err != nil {
+		return "", err, false
+	}
+
+	if err := d.MkdirAll(filepath.Dir(targetFile)); err != nil {
+		return "", err, false
+	}
+
+	path := d.datastorePath(targetFile)
+	if d.verifyChecksum(config.ISOChecksumType, config.ISOChecksum, path) {
+		log.Println("Initial checksum matched, no download needed.")
+		return path, nil, true
+	}
+
+	// TODO(dougm) progress and handle interrupt
+	err = d.sh("wget", dconfig.Url, "-O", path)
+
+	return path, err, true
+}
+*/
+
+func (d *ESX5Driver) DirExists(path string) (bool, error) {
+	err := d.sh("test", "-e", d.datastorePath(path))
+	return err == nil, err
 }
 
 func (d *ESX5Driver) MkdirAll(path string) error {
@@ -162,32 +220,24 @@ func (d *ESX5Driver) RemoveAll(path string) error {
 	return d.sh("rm", "-rf", d.datastorePath(path))
 }
 
-func (d *ESX5Driver) DirType() string {
-	return "datastore"
-}
-
 func (d *ESX5Driver) datastorePath(path string) string {
-	return filepath.Join("/vmfs/volumes", d.config.RemoteDatastore, path)
+	return filepath.Join("/vmfs/volumes", d.Datastore, path)
 }
 
 func (d *ESX5Driver) connect() error {
-	if d.config.RemoteHost == "" {
+	address := fmt.Sprintf("%s:%d", d.Host, d.Port)
-		return errors.New("A remote_host must be specified.")
+
-	}
-	if d.config.RemotePort == 0 {
-		d.config.RemotePort = 22
-	}
-	address := fmt.Sprintf("%s:%d", d.config.RemoteHost, d.config.RemotePort)
 	auth := []gossh.ClientAuth{
-		gossh.ClientAuthPassword(ssh.Password(d.config.RemotePassword)),
+		gossh.ClientAuthPassword(ssh.Password(d.Password)),
 		gossh.ClientAuthKeyboardInteractive(
-			ssh.PasswordKeyboardInteractive(d.config.RemotePassword)),
+			ssh.PasswordKeyboardInteractive(d.Password)),
 	}
+
 	// TODO(dougm) KeyPath support
 	sshConfig := &ssh.Config{
 		Connection: ssh.ConnectFunc("tcp", address),
 		SSHConfig: &gossh.ClientConfig{
-			User: d.config.RemoteUser,
+			User: d.Username,
 			Auth: auth,
 		},
 		NoPty: true,
@@ -204,7 +254,6 @@ func (d *ESX5Driver) connect() error {
 
 func (d *ESX5Driver) checkSystemVersion() error {
 	r, err := d.esxcli("system", "version", "get")
-
 	if err != nil {
 		return err
 	}
@@ -214,20 +263,18 @@ func (d *ESX5Driver) checkSystemVersion() error {
 		return err
 	}
 
-	log.Printf("Connected to %s %s %s", record["Product"], record["Version"], record["Build"])
+	log.Printf("Connected to %s %s %s", record["Product"],
-
+		record["Version"], record["Build"])
 	return nil
 }
 
 func (d *ESX5Driver) checkGuestIPHackEnabled() error {
 	r, err := d.esxcli("system", "settings", "advanced", "list", "-o", "/Net/GuestIPHack")
-
 	if err != nil {
 		return err
 	}
 
 	record, err := r.read()
-
 	if err != nil {
 		return err
 	}
@@ -240,42 +287,6 @@ func (d *ESX5Driver) checkGuestIPHackEnabled() error {
 	return nil
 }
 
-func (d *ESX5Driver) checkOutputFolder() error {
-	if d.config.RemoteDatastore == "" {
-		d.config.RemoteDatastore = "datastore1"
-	}
-	if !d.config.PackerForce && d.FileExists(d.config.OutputDir) {
-		return fmt.Errorf("Output folder '%s' already exists. It must not exist.",
-			d.config.OutputDir)
-	}
-	return nil
-}
-
-func (d *ESX5Driver) download(config *common.DownloadConfig, state multistep.StateBag) (string, error, bool) {
-	cacheRoot, _ := filepath.Abs(".")
-	targetFile, err := filepath.Rel(cacheRoot, config.TargetPath)
-
-	if err != nil {
-		return "", err, false
-	}
-
-	path := d.datastorePath(targetFile)
-
-	err = d.MkdirAll(filepath.Dir(targetFile))
-	if err != nil {
-		return "", err, false
-	}
-
-	if d.verifyChecksum(d.config.ISOChecksumType, d.config.ISOChecksum, path) {
-		log.Println("Initial checksum matched, no download needed.")
-		return path, nil, true
-	}
-	// TODO(dougm) progress and handle interrupt
-	err = d.sh("wget", config.Url, "-O", path)
-
-	return path, err, true
-}
-
 func (d *ESX5Driver) upload(src, dst string) error {
 	f, err := os.Open(src)
 	if err != nil {
@@ -378,41 +389,3 @@ func (r *esxcliReader) find(key, val string) (map[string]string, error) {
 		}
 	}
 }
-
-func (d *ESX5Driver) sshAddress(state multistep.StateBag) (string, error) {
-	if address, ok := state.GetOk("vm_address"); ok {
-		return address.(string), nil
-	}
-
-	r, err := d.esxcli("network", "vm", "list")
-	if err != nil {
-		return "", err
-	}
-
-	record, err := r.find("Name", d.config.VMName)
-	if err != nil {
-		return "", err
-	}
-	wid := record["WorldID"]
-	if wid == "" {
-		return "", errors.New("VM WorldID not found")
-	}
-
-	r, err = d.esxcli("network", "vm", "port", "list", "-w", wid)
-	if err != nil {
-		return "", err
-	}
-
-	record, err = r.read()
-	if err != nil {
-		return "", err
-	}
-
-	if record["IPAddress"] == "0.0.0.0" {
-		return "", errors.New("VM network port found, but no IP address")
-	}
-
-	address := fmt.Sprintf("%s:%d", record["IPAddress"], d.config.SSHPort)
-	state.Put("vm_address", address)
-	return address, nil
-}

builder/vmware/driver_fusion5.go

@@ -2,6 +2,7 @@ package vmware
 
 import (
 	"fmt"
+	"github.com/mitchellh/multistep"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -58,6 +59,10 @@ func (d *Fusion5Driver) IsRunning(vmxPath string) (bool, error) {
 	return false, nil
 }
 
+func (d *Fusion5Driver) SSHAddress(state multistep.StateBag) (string, error) {
+	return sshAddress(state)
+}
+
 func (d *Fusion5Driver) Start(vmxPath string, headless bool) error {
 	guiArgument := "gui"
 	if headless == true {

builder/vmware/driver_player5.go

@@ -2,6 +2,7 @@ package vmware
 
 import (
 	"fmt"
+	"github.com/mitchellh/multistep"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -86,6 +87,10 @@ func (d *Player5LinuxDriver) IsRunning(vmxPath string) (bool, error) {
 	return false, nil
 }
 
+func (d *Player5LinuxDriver) SSHAddress(state multistep.StateBag) (string, error) {
+	return sshAddress(state)
+}
+
 func (d *Player5LinuxDriver) Start(vmxPath string, headless bool) error {
 	guiArgument := "gui"
 	if headless {

builder/vmware/driver_workstation9.go

@@ -2,6 +2,7 @@ package vmware
 
 import (
 	"fmt"
+	"github.com/mitchellh/multistep"
 	"log"
 	"os"
 	"os/exec"
@@ -61,6 +62,10 @@ func (d *Workstation9Driver) IsRunning(vmxPath string) (bool, error) {
 	return false, nil
 }
 
+func (d *Workstation9Driver) SSHAddress(state multistep.StateBag) (string, error) {
+	return sshAddress(state)
+}
+
 func (d *Workstation9Driver) Start(vmxPath string, headless bool) error {
 	guiArgument := "gui"
 	if headless {

builder/vmware/remote_driver.go

@@ -1,28 +0,0 @@
-package vmware
-
-import (
-	"fmt"
-	"github.com/mitchellh/multistep"
-	"github.com/mitchellh/packer/common"
-)
-
-type RemoteDriver interface {
-	Driver
-	SSHAddress() func(multistep.StateBag) (string, error)
-	Download() func(*common.DownloadConfig, multistep.StateBag) (string, error, bool)
-}
-
-func NewRemoteDriver(config *config) (Driver, error) {
-	var driver Driver
-
-	switch config.RemoteType {
-	case "esx5":
-		driver = &ESX5Driver{
-			config: config,
-		}
-	default:
-		return nil, fmt.Errorf("Unknown product type: '%s'", config.RemoteType)
-	}
-
-	return driver, driver.Verify()
-}

builder/vmware/step_prepare_output_dir.go

@@ -1,49 +1,27 @@
 package vmware
 
 import (
-	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 	"log"
-	"os"
 	"time"
 )
 
-type OutputDir interface {
-	FileExists(path string) bool
-	MkdirAll(path string) error
-	RemoveAll(path string) error
-	DirType() string
-}
-
-type localOutputDir struct{}
-
-func (localOutputDir) FileExists(path string) bool {
-	_, err := os.Stat(path)
-	return err == nil
-}
-
-func (localOutputDir) MkdirAll(path string) error {
-	return os.MkdirAll(path, 0755)
-}
-
-func (localOutputDir) RemoveAll(path string) error {
-	return os.RemoveAll(path)
-}
-
-func (localOutputDir) DirType() string {
-	return "local"
-}
-
 type stepPrepareOutputDir struct{}
 
 func (s *stepPrepareOutputDir) Run(state multistep.StateBag) multistep.StepAction {
 	config := state.Get("config").(*config)
 	ui := state.Get("ui").(packer.Ui)
 
-	for _, dir := range s.outputDirs(state) {
+	dir := s.outputDir(state)
-		if dir.FileExists(config.OutputDir) && config.PackerForce {
+	exists, err := dir.DirExists(config.OutputDir)
-			ui.Say(fmt.Sprintf("Deleting previous %s output directory...", dir.DirType()))
+	if err != nil {
+		state.Put("error", err)
+		return multistep.ActionHalt
+	}
+
+	if exists && config.PackerForce {
+		ui.Say("Deleting previous output directory...")
 		dir.RemoveAll(config.OutputDir)
 	}
 
@@ -51,7 +29,6 @@ func (s *stepPrepareOutputDir) Run(state multistep.StateBag) multistep.StepActio
 		state.Put("error", err)
 		return multistep.ActionHalt
 	}
-	}
 
 	return multistep.ActionContinue
 }
@@ -64,8 +41,8 @@ func (s *stepPrepareOutputDir) Cleanup(state multistep.StateBag) {
 		config := state.Get("config").(*config)
 		ui := state.Get("ui").(packer.Ui)
 
-		for _, dir := range s.outputDirs(state) {
+		dir := s.outputDir(state)
-			ui.Say(fmt.Sprintf("Deleting %s output directory...", dir.DirType()))
+		ui.Say("Deleting output directory...")
 		for i := 0; i < 5; i++ {
 			err := dir.RemoveAll(config.OutputDir)
 			if err == nil {
@@ -77,17 +54,16 @@ func (s *stepPrepareOutputDir) Cleanup(state multistep.StateBag) {
 		}
 	}
 }
-}
 
-func (s *stepPrepareOutputDir) outputDirs(state multistep.StateBag) []OutputDir {
+func (s *stepPrepareOutputDir) outputDir(state multistep.StateBag) (dir OutputDir) {
 	driver := state.Get("driver").(Driver)
-	dirs := []OutputDir{
+
-		localOutputDir{},
+	switch d := driver.(type) {
+	case OutputDir:
+		dir = d
+	default:
+		dir = new(localOutputDir)
 	}
 
-	if dir, ok := driver.(OutputDir); ok {
+	return
-		dirs = append(dirs, dir)
-	}
-
-	return dirs
 }

common/step_download.go

@@ -35,8 +35,6 @@ type StepDownload struct {
 
 	// A list of URLs to attempt to download this thing.
 	Url []string
-
-	Download func(*DownloadConfig, multistep.StateBag) (string, error, bool)
 }
 
 func (s *StepDownload) Run(state multistep.StateBag) multistep.StepAction {
@@ -55,11 +53,6 @@ func (s *StepDownload) Run(state multistep.StateBag) multistep.StepAction {
 
 	ui.Say(fmt.Sprintf("Downloading or copying %s", s.Description))
 
-	downloadFunc := s.Download
-	if downloadFunc == nil {
-		downloadFunc = s.download
-	}
-
 	var finalPath string
 	for _, url := range s.Url {
 		ui.Message(fmt.Sprintf("Downloading or copying: %s", url))
@@ -79,7 +72,7 @@ func (s *StepDownload) Run(state multistep.StateBag) multistep.StepAction {
 			Checksum:   checksum,
 		}
 
-		path, err, retry := downloadFunc(config, state)
+		path, err, retry := s.download(config, state)
 		if err != nil {
 			ui.Message(fmt.Sprintf("Error downloading: %s", err))
 		}