provisioner/salt-masterless: switch replace to that prior replace works

/cc @markpeek

CHANGELOG.md

@@ -1,3 +1,18 @@
+## 0.3.1 (August 12, 2013)
+
+IMPROVEMENTS:
+
+* provisioner/shell: New setting `start_retry_timeout` which is the timeout
+  for the provisioner to attempt to _start_ the remote process. This allows
+  the shell provisioner to work properly with reboots. [GH-260]
+
+BUG FIXES:
+
+* core: Remote command output containing '\r' now looks much better
+  within the Packer output.
+* builder/vmware: Fix issue with finding driver files. [GH-279]
+* provisioner/salt-masterless: Uploads work properly from Windows. [GH-276]
+
 ## 0.3.0 (August 12, 2013)
 
 BACKWARDS INCOMPATIBILITIES:

builder/vmware/driver_workstation9_windows.go

@@ -27,7 +27,7 @@ func workstationFindVdiskManager() (string, error) {
 }
 
 func workstationFindVMware() (string, error) {
-	path, _ := exec.LookPath("vmware.exe")
+	path, err := exec.LookPath("vmware.exe")
 	if err == nil {
 		return path, nil
 	}
@@ -36,7 +36,7 @@ func workstationFindVMware() (string, error) {
 }
 
 func workstationFindVmrun() (string, error) {
-	path, _ := exec.LookPath("vmrun.exe")
+	path, err := exec.LookPath("vmrun.exe")
 	if err == nil {
 		return path, nil
 	}
@@ -45,7 +45,7 @@ func workstationFindVmrun() (string, error) {
 }
 
 func workstationToolsIsoPath(flavor string) string {
-	return findFile(flavor+".iso", workstationProgramFilePaths()), nil
+	return findFile(flavor+".iso", workstationProgramFilePaths())
 }
 
 func workstationDhcpLeasesPath(device string) string {
@@ -56,11 +56,11 @@ func workstationDhcpLeasesPath(device string) string {
 		return path
 	}
 
-	return findFile("vmnetdhcp.leases", workstationDataFilePaths()), nil
+	return findFile("vmnetdhcp.leases", workstationDataFilePaths())
 }
 
 func workstationVmnetnatConfPath() string {
-	return findFile("vmnetnat.conf", workstationDataFilePaths()), nil
+	return findFile("vmnetnat.conf", workstationDataFilePaths())
 }
 
 // See http://blog.natefinch.com/2012/11/go-win-stuff.html
@@ -141,7 +141,7 @@ func findFile(file string, paths []string) string {
 		path = normalizePath(path)
 		log.Printf("Searching for file '%s'", path)
 
-		if _, err := os.Stat(path); err != nil {
+		if _, err := os.Stat(path); err == nil {
 			log.Printf("Found file '%s'", path)
 			return path
 		}
@@ -184,7 +184,7 @@ func workstationProgramFilePaths() []string {
 // workstationDataFilePaths returns a list of paths that are eligible
 // to contain data files we may want such as vmnet NAT configuration files.
 func workstationDataFilePaths() []string {
-	leasesPath, err := workstationVmnetDhcpLeasesPathFromRegistry()
+	leasesPath, err := workstationDhcpLeasesPathRegistry()
 	if err != nil {
 		log.Printf("Error getting DHCP leases path: %s", err)
 	}
@@ -198,7 +198,7 @@ func workstationDataFilePaths() []string {
 		paths = append(paths, os.Getenv("VMWARE_DATA"))
 	}
 
-	if path != "" {
+	if leasesPath != "" {
 		paths = append(paths, leasesPath)
 	}
 

communicator/ssh/communicator.go

@@ -205,9 +205,14 @@ func (c *comm) Download(string, io.Writer) error {
 	panic("not implemented yet")
 }
 
-func (c *comm) newSession() (*ssh.Session, error) {
+func (c *comm) newSession() (session *ssh.Session, err error) {
 	log.Println("opening new ssh session")
-	session, err := c.client.NewSession()
+	if c.client == nil {
+		err = errors.New("client not available")
+	} else {
+		session, err = c.client.NewSession()
+	}
+
 	if err != nil {
 		log.Printf("ssh session open error: '%s', attempting reconnect", err)
 		if err := c.reconnect(); err != nil {
@@ -225,6 +230,10 @@ func (c *comm) reconnect() (err error) {
 		c.conn.Close()
 	}
 
+	// Set the conn and client to nil since we'll recreate it
+	c.conn = nil
+	c.client = nil
+
 	log.Printf("reconnecting to TCP connection for SSH")
 	c.conn, err = c.config.Connection()
 	if err != nil {

packer/communicator.go

@@ -72,6 +72,14 @@ func (r *RemoteCmd) StartWithUi(c Communicator, ui Ui) error {
 	defer stdout_w.Close()
 	defer stderr_w.Close()
 
+	// Retain the original stdout/stderr that we can replace back in.
+	originalStdout := r.Stdout
+	originalStderr := r.Stderr
+	defer func() {
+		r.Stdout = originalStdout
+		r.Stderr = originalStderr
+	}()
+
 	// Set the writers for the output so that we get it streamed to us
 	if r.Stdout == nil {
 		r.Stdout = stdout_w
@@ -108,9 +116,9 @@ OutputLoop:
 	for {
 		select {
 		case output := <-stderrCh:
-			ui.Message(strings.TrimSpace(output))
+			ui.Message(r.cleanOutputLine(output))
 		case output := <-stdoutCh:
-			ui.Message(strings.TrimSpace(output))
+			ui.Message(r.cleanOutputLine(output))
 		case <-exitCh:
 			break OutputLoop
 		}
@@ -156,3 +164,19 @@ func (r *RemoteCmd) Wait() {
 
 	<-r.exitCh
 }
+
+// cleanOutputLine cleans up a line so that '\r' don't muck up the
+// UI output when we're reading from a remote command.
+func (r *RemoteCmd) cleanOutputLine(line string) string {
+	// Trim surrounding whitespace
+	line = strings.TrimSpace(line)
+
+	// Trim up to the first carriage return, since that text would be
+	// lost anyways.
+	idx := strings.LastIndex(line, "\r")
+	if idx > -1 {
+		line = line[idx+1:]
+	}
+
+	return line
+}

packer/version.go

@@ -10,7 +10,7 @@ import (
 var GitCommit string
 
 // The version of packer.
-const Version = "0.3.0"
+const Version = "0.3.1"
 
 // Any pre-release marker for the version. If this is "" (empty string),
 // then it means that it is a final release. Otherwise, this is the

provisioner/salt-masterless/provisioner.go

@@ -137,6 +137,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 func UploadLocalDirectory(localDir string, remoteDir string, comm packer.Communicator, ui packer.Ui) (err error) {
 	visitPath := func(localPath string, f os.FileInfo, err error) (err2 error) {
 		localRelPath := strings.Replace(localPath, localDir, "", 1)
+		localRelPath = strings.Replace(localRelPath, "\\", "/", -1)
 		remotePath := fmt.Sprintf("%s%s", remoteDir, localRelPath)
 		if f.IsDir() && f.Name() == ".git" {
 			return filepath.SkipDir

provisioner/shell/provisioner.go

@@ -12,6 +12,7 @@ import (
 	"log"
 	"os"
 	"strings"
+	"time"
 )
 
 const DefaultRemotePath = "/tmp/script.sh"
@@ -45,7 +46,13 @@ type config struct {
 	// can be used to inject the environment_vars into the environment.
 	ExecuteCommand string `mapstructure:"execute_command"`
 
-	tpl *common.Template
+	// The timeout for retrying to start the process. Until this timeout
+	// is reached, if the provisioner can't start a process, it retries.
+	// This can be set high to allow for reboots.
+	RawStartRetryTimeout string `mapstructure:"start_retry_timeout"`
+
+	startRetryTimeout time.Duration
+	tpl               *common.Template
 }
 
 type Provisioner struct {
@@ -84,6 +91,10 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 		p.config.InlineShebang = "/bin/sh"
 	}
 
+	if p.config.RawStartRetryTimeout == "" {
+		p.config.RawStartRetryTimeout = "5m"
+	}
+
 	if p.config.RemotePath == "" {
 		p.config.RemotePath = DefaultRemotePath
 	}
@@ -106,9 +117,10 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 	}
 
 	templates := map[string]*string{
-		"inline_shebang": &p.config.InlineShebang,
-		"script":         &p.config.Script,
-		"remote_path":    &p.config.RemotePath,
+		"inline_shebang":      &p.config.InlineShebang,
+		"script":              &p.config.Script,
+		"start_retry_timeout": &p.config.RawStartRetryTimeout,
+		"remote_path":         &p.config.RemotePath,
 	}
 
 	for n, ptr := range templates {
@@ -161,6 +173,14 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 		}
 	}
 
+	if p.config.RawStartRetryTimeout != "" {
+		p.config.startRetryTimeout, err = time.ParseDuration(p.config.RawStartRetryTimeout)
+		if err != nil {
+			errs = packer.MultiErrorAppend(
+				errs, fmt.Errorf("Failed parsing start_retry_timeout: %s", err))
+		}
+	}
+
 	if errs != nil && len(errs.Errors) > 0 {
 		return errs
 	}
@@ -238,9 +258,26 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		}
 
 		cmd := &packer.RemoteCmd{Command: command}
+		startTimeout := time.After(p.config.startRetryTimeout)
 		log.Printf("Executing command: %s", cmd.Command)
-		if err := cmd.StartWithUi(comm, ui); err != nil {
-			return fmt.Errorf("Failed executing command: %s", err)
+		for {
+			if err := cmd.StartWithUi(comm, ui); err == nil {
+				break
+			}
+
+			// Create an error and log it
+			err = fmt.Errorf("Error executing command: %s", err)
+			log.Printf(err.Error())
+
+			// Check if we timed out, otherwise we retry. It is safe to
+			// retry since the only error case above is if the command
+			// failed to START.
+			select {
+			case <-startTimeout:
+				return err
+			default:
+				time.Sleep(2 * time.Second)
+			}
 		}
 
 		if cmd.ExitStatus != 0 {

website/source/docs/provisioners/shell.html.markdown

@@ -67,6 +67,12 @@ Optional parameters:
   in the machine. This defaults to "/tmp/script.sh". This value must be
   a writable location and any parent directories must already exist.
 
+* `start_retry_timeout` (string) - The amount of time to attempt to
+  _start_ the remote process. By default this is "5m" or 5 minutes. This
+  setting exists in order to deal with times when SSH may restart, such as
+  a system reboot. Set this to a higher value if reboots take a longer
+  amount of time.
+
 ## Execute Command Example
 
 To many new users, the `execute_command` is puzzling. However, it provides