v0.3.4

Add sleep tip about ssh

.travis.yml

@@ -5,7 +5,9 @@ go:
     - tip
 
 install: make deps
-script: make test
+script:
+    - go test ./...
+    - go test -race ./...
 
 notifications:
     flowdock:

CHANGELOG.md

@@ -1,3 +1,16 @@
+## 0.3.4 (August 21, 2013)
+
+IMPROVEMENTS:
+
+* post-processor/vagrant: the file being compressed will be shown
+  in the UI [GH-314]
+
+BUG FIXES:
+
+* core: Avoid panics when double-interrupting Packer.
+* provisioner/shell: Retry shell script uploads, making reboots more
+  robust if they happen to fail in this stage. [GH-282]
+
 ## 0.3.3 (August 19, 2013)
 
 FEATURES:

communicator/ssh/communicator_test.go

@@ -1,3 +1,5 @@
+// +build !race
+
 package ssh
 
 import (

packer/communicator.go

@@ -33,9 +33,11 @@ type RemoteCmd struct {
 	// Once Exited is true, this will contain the exit code of the process.
 	ExitStatus int
 
-	// Internal locks and such used for safely setting some shared variables
-	l      sync.Mutex
+	// Internal fields
 	exitCh chan struct{}
+
+	// This thing is a mutex, lock when making modifications concurrently
+	sync.Mutex
 }
 
 // A Communicator is the interface used to communicate with the machine
@@ -76,6 +78,9 @@ func (r *RemoteCmd) StartWithUi(c Communicator, ui Ui) error {
 	originalStdout := r.Stdout
 	originalStderr := r.Stderr
 	defer func() {
+		r.Lock()
+		defer r.Unlock()
+
 		r.Stdout = originalStdout
 		r.Stderr = originalStderr
 	}()
@@ -141,8 +146,8 @@ OutputLoop:
 // should be called by communicators who are running a remote command in
 // order to set that the command is done.
 func (r *RemoteCmd) SetExited(status int) {
-	r.l.Lock()
-	defer r.l.Unlock()
+	r.Lock()
+	defer r.Unlock()
 
 	if r.exitCh == nil {
 		r.exitCh = make(chan struct{})
@@ -156,11 +161,11 @@ func (r *RemoteCmd) SetExited(status int) {
 // Wait waits for the remote command to complete.
 func (r *RemoteCmd) Wait() {
 	// Make sure our condition variable is initialized.
-	r.l.Lock()
+	r.Lock()
 	if r.exitCh == nil {
 		r.exitCh = make(chan struct{})
 	}
-	r.l.Unlock()
+	r.Unlock()
 
 	<-r.exitCh
 }

packer/communicator_test.go

@@ -15,6 +15,9 @@ type TestCommunicator struct {
 
 func (c *TestCommunicator) Start(rc *RemoteCmd) error {
 	go func() {
+		rc.Lock()
+		defer rc.Unlock()
+
 		if rc.Stdout != nil && c.Stdout != nil {
 			io.Copy(rc.Stdout, c.Stdout)
 		}

packer/plugin/builder.go

@@ -40,7 +40,7 @@ func (b *cmdBuilder) Cancel() {
 func (c *cmdBuilder) checkExit(p interface{}, cb func()) {
 	if c.client.Exited() && cb != nil {
 		cb()
-	} else if p != nil {
+	} else if p != nil && !Killed {
 		log.Panic(p)
 	}
 }

packer/plugin/client.go

@@ -2,7 +2,6 @@ package plugin
 
 import (
 	"bufio"
-	"bytes"
 	"errors"
 	"fmt"
 	"github.com/mitchellh/packer/packer"
@@ -20,6 +19,10 @@ import (
 	"unicode"
 )
 
+// If this is true, then the "unexpected EOF" panic will not be
+// raised throughout the clients.
+var Killed = false
+
 // This is a slice of the "managed" clients which are cleaned up when
 // calling Cleanup
 var managedClients = make([]*Client, 0, 5)
@@ -69,6 +72,9 @@ type ClientConfig struct {
 //
 // This must only be called _once_.
 func CleanupClients() {
+	// Set the killed to true so that we don't get unexpected panics
+	Killed = true
+
 	// Kill all the managed clients in parallel and use a WaitGroup
 	// to wait for them all to finish up.
 	var wg sync.WaitGroup
@@ -116,6 +122,8 @@ func NewClient(config *ClientConfig) (c *Client) {
 
 // Tells whether or not the underlying process has exited.
 func (c *Client) Exited() bool {
+	c.l.Lock()
+	defer c.l.Unlock()
 	return c.exited
 }
 
@@ -215,7 +223,7 @@ func (c *Client) Start() (address string, err error) {
 		fmt.Sprintf("PACKER_PLUGIN_MAX_PORT=%d", c.config.MaxPort),
 	}
 
-	stdout := new(bytes.Buffer)
+	stdout_r, stdout_w := io.Pipe()
 	stderr_r, stderr_w := io.Pipe()
 
 	cmd := c.config.Cmd
@@ -223,7 +231,7 @@ func (c *Client) Start() (address string, err error) {
 	cmd.Env = append(cmd.Env, env...)
 	cmd.Stdin = os.Stdin
 	cmd.Stderr = stderr_w
-	cmd.Stdout = stdout
+	cmd.Stdout = stdout_w
 
 	log.Printf("Starting plugin: %s %#v", cmd.Path, cmd.Args)
 	err = cmd.Start()
@@ -245,10 +253,12 @@ func (c *Client) Start() (address string, err error) {
 	}()
 
 	// Start goroutine to wait for process to exit
+	exitCh := make(chan struct{})
 	go func() {
-		// Make sure we close the write end of our stderr listener so
-		// that the log goroutine ends properly.
+		// Make sure we close the write end of our stderr/stdout so
+		// that the readers send EOF properly.
 		defer stderr_w.Close()
+		defer stdout_w.Close()
 
 		// Wait for the command to end.
 		cmd.Wait()
@@ -258,46 +268,60 @@ func (c *Client) Start() (address string, err error) {
 		os.Stderr.Sync()
 
 		// Mark that we exited
+		close(exitCh)
+
+		// Set that we exited, which takes a lock
+		c.l.Lock()
+		defer c.l.Unlock()
 		c.exited = true
 	}()
 
 	// Start goroutine that logs the stderr
 	go c.logStderr(stderr_r)
 
+	// Start a goroutine that is going to be reading the lines
+	// out of stdout
+	linesCh := make(chan []byte)
+	go func() {
+		defer close(linesCh)
+
+		buf := bufio.NewReader(stdout_r)
+		for {
+			line, err := buf.ReadBytes('\n')
+			if line != nil {
+				linesCh <- line
+			}
+
+			if err == io.EOF {
+				return
+			}
+		}
+	}()
+
+	// Make sure after we exit we read the lines from stdout forever
+	// so they dont' block since it is an io.Pipe
+	defer func() {
+		go func() {
+			for _ = range linesCh {
+			}
+		}()
+	}()
+
 	// Some channels for the next step
 	timeout := time.After(c.config.StartTimeout)
 
 	// Start looking for the address
 	log.Printf("Waiting for RPC address for: %s", cmd.Path)
-	for done := false; !done; {
-		select {
-		case <-timeout:
-			err = errors.New("timeout while waiting for plugin to start")
-			done = true
-		default:
-		}
-
-		if err == nil && c.Exited() {
-			err = errors.New("plugin exited before we could connect")
-			done = true
-		}
-
-		if line, lerr := stdout.ReadBytes('\n'); lerr == nil {
-			// Trim the address and reset the err since we were able
-			// to read some sort of address.
-			c.address = strings.TrimSpace(string(line))
-			address = c.address
-			err = nil
-			break
-		}
-
-		// If error is nil from previously, return now
-		if err != nil {
-			return
-		}
-
-		// Wait a bit
-		time.Sleep(10 * time.Millisecond)
+	select {
+	case <-timeout:
+		err = errors.New("timeout while waiting for plugin to start")
+	case <-exitCh:
+		err = errors.New("plugin exited before we could connect")
+	case line := <-linesCh:
+		// Trim the address and reset the err since we were able
+		// to read some sort of address.
+		c.address = strings.TrimSpace(string(line))
+		address = c.address
 	}
 
 	return

packer/plugin/command.go

@@ -45,7 +45,7 @@ func (c *cmdCommand) Synopsis() (result string) {
 func (c *cmdCommand) checkExit(p interface{}, cb func()) {
 	if c.client.Exited() {
 		cb()
-	} else if p != nil {
+	} else if p != nil && !Killed {
 		log.Panic(p)
 	}
 }

packer/plugin/hook.go

@@ -22,7 +22,7 @@ func (c *cmdHook) Run(name string, ui packer.Ui, comm packer.Communicator, data
 func (c *cmdHook) checkExit(p interface{}, cb func()) {
 	if c.client.Exited() {
 		cb()
-	} else if p != nil {
+	} else if p != nil && !Killed {
 		log.Panic(p)
 	}
 }

packer/plugin/post_processor.go

@@ -31,7 +31,7 @@ func (c *cmdPostProcessor) PostProcess(ui packer.Ui, a packer.Artifact) (packer.
 func (c *cmdPostProcessor) checkExit(p interface{}, cb func()) {
 	if c.client.Exited() {
 		cb()
-	} else if p != nil {
+	} else if p != nil && !Killed {
 		log.Panic(p)
 	}
 }

packer/plugin/provisioner.go

@@ -31,7 +31,7 @@ func (c *cmdProvisioner) Provision(ui packer.Ui, comm packer.Communicator) error
 func (c *cmdProvisioner) checkExit(p interface{}, cb func()) {
 	if c.client.Exited() {
 		cb()
-	} else if p != nil {
+	} else if p != nil && !Killed {
 		log.Panic(p)
 	}
 }

packer/rpc/communicator.go

@@ -78,7 +78,8 @@ func (c *communicator) Start(cmd *packer.RemoteCmd) (err error) {
 
 		conn, err := responseL.Accept()
 		if err != nil {
-			log.Panic(err)
+			cmd.SetExited(123)
+			return
 		}
 
 		defer conn.Close()
@@ -87,7 +88,8 @@ func (c *communicator) Start(cmd *packer.RemoteCmd) (err error) {
 
 		var finished CommandFinished
 		if err := decoder.Decode(&finished); err != nil {
-			log.Panic(err)
+			cmd.SetExited(123)
+			return
 		}
 
 		cmd.SetExited(finished.ExitStatus)

packer/rpc/communicator_test.go

@@ -99,7 +99,10 @@ func TestCommunicatorRPC(t *testing.T) {
 	c.startCmd.SetExited(42)
 
 	for i := 0; i < 5; i++ {
-		if cmd.Exited {
+		cmd.Lock()
+		exited := cmd.Exited
+		cmd.Unlock()
+		if exited {
 			assert.Equal(cmd.ExitStatus, 42, "should have proper exit status")
 			break
 		}

packer/version.go

@@ -10,7 +10,7 @@ import (
 var GitCommit string
 
 // The version of packer.
-const Version = "0.3.3"
+const Version = "0.3.4"
 
 // 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

post-processor/vagrant/aws.go

@@ -134,7 +134,7 @@ func (p *AWSBoxPostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact
 	}
 
 	// Compress the directory to the given output path
-	if err := DirToBox(outputPath, dir); err != nil {
+	if err := DirToBox(outputPath, dir, ui); err != nil {
 		err = fmt.Errorf("error creating box: %s", err)
 		return nil, false, err
 	}

post-processor/vagrant/util.go

@@ -4,6 +4,8 @@ import (
 	"archive/tar"
 	"compress/gzip"
 	"encoding/json"
+	"fmt"
+	"github.com/mitchellh/packer/packer"
 	"io"
 	"log"
 	"os"
@@ -42,7 +44,7 @@ func CopyContents(dst, src string) error {
 // DirToBox takes the directory and compresses it into a Vagrant-compatible
 // box. This function does not perform checks to verify that dir is
 // actually a proper box. This is an expected precondition.
-func DirToBox(dst, dir string) error {
+func DirToBox(dst, dir string, ui packer.Ui) error {
 	log.Printf("Turning dir into box: %s => %s", dir, dst)
 	dstF, err := os.Create(dst)
 	if err != nil {
@@ -90,6 +92,10 @@ func DirToBox(dst, dir string) error {
 			return err
 		}
 
+		if ui != nil {
+			ui.Message(fmt.Sprintf("Compressing: %s", header.Name))
+		}
+
 		if err := tarWriter.WriteHeader(header); err != nil {
 			return err
 		}

post-processor/vagrant/virtualbox.go

@@ -143,7 +143,7 @@ func (p *VBoxBoxPostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifac
 
 	// Compress the directory to the given output path
 	ui.Message(fmt.Sprintf("Compressing box..."))
-	if err := DirToBox(outputPath, dir); err != nil {
+	if err := DirToBox(outputPath, dir, ui); err != nil {
 		return nil, false, err
 	}
 

post-processor/vagrant/vmware.go

@@ -119,7 +119,7 @@ func (p *VMwareBoxPostProcessor) PostProcess(ui packer.Ui, artifact packer.Artif
 
 	// Compress the directory to the given output path
 	ui.Message(fmt.Sprintf("Compressing box..."))
-	if err := DirToBox(outputPath, dir); err != nil {
+	if err := DirToBox(outputPath, dir, ui); err != nil {
 		return nil, false, err
 	}
 

provisioner/shell/provisioner.go

@@ -237,7 +237,9 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		defer f.Close()
 
 		log.Printf("Uploading %s => %s", path, p.config.RemotePath)
-		err = comm.Upload(p.config.RemotePath, f)
+		err = p.retryable(func() error {
+			return comm.Upload(p.config.RemotePath, f)
+		})
 		if err != nil {
 			return fmt.Errorf("Error uploading shell script: %s", err)
 		}
@@ -258,26 +260,13 @@ 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)
-		for {
-			if err := cmd.StartWithUi(comm, ui); err == nil {
-				break
-			}
+		err = p.retryable(func() error {
+			return cmd.StartWithUi(comm, ui)
+		})
 
-			// 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 err != nil {
+			return err
 		}
 
 		if cmd.ExitStatus != 0 {
@@ -287,3 +276,29 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 
 	return nil
 }
+
+// retryable will retry the given function over and over until a
+// non-error is returned.
+func (p *Provisioner) retryable(f func() error) error {
+	startTimeout := time.After(p.config.startRetryTimeout)
+	for {
+		var err error
+		if err = f(); err == nil {
+			return nil
+		}
+
+		// Create an error and log it
+		err = fmt.Errorf("Retryable error: %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)
+		}
+	}
+}

scripts/build.sh

@@ -20,9 +20,19 @@ cd $DIR
 GIT_COMMIT=$(git rev-parse HEAD)
 GIT_DIRTY=$(test -n "`git status --porcelain`" && echo "+CHANGES" || true)
 
+# If we're building a race-enabled build, then set that up.
+if [ ! -z $PACKER_RACE ]; then
+    echo -e "${OK_COLOR}--> Building with race detection enabled${NO_COLOR}"
+    PACKER_RACE="-race"
+fi
+
+echo -e "${OK_COLOR}--> Installing dependencies to speed up builds...${NO_COLOR}"
+go get ./...
+
 # Compile the main Packer app
 echo -e "${OK_COLOR}--> Compiling Packer${NO_COLOR}"
 go build \
+    ${PACKER_RACE} \
     -ldflags "-X github.com/mitchellh/packer/packer.GitCommit ${GIT_COMMIT}${GIT_DIRTY}" \
     -v \
     -o bin/packer .
@@ -32,6 +42,7 @@ for PLUGIN in $(find ./plugin -mindepth 1 -maxdepth 1 -type d); do
     PLUGIN_NAME=$(basename ${PLUGIN})
     echo -e "${OK_COLOR}--> Compiling Plugin: ${PLUGIN_NAME}${NO_COLOR}"
     go build \
+        ${PACKER_RACE} \
         -ldflags "-X github.com/mitchellh/packer/packer.GitCommit ${GIT_COMMIT}${GIT_DIRTY}" \
         -v \
         -o bin/packer-${PLUGIN_NAME} ${PLUGIN}

signal.go

@@ -26,7 +26,8 @@ func setupSignalHandlers(env packer.Environment) {
 
 		env.Ui().Error("Interrupt signal received twice. Forcefully exiting now.")
 
-		// Force kill all the plugins
+		// Force kill all the plugins, but mark that we're killing them
+		// first so that we don't get panics everywhere.
 		plugin.CleanupClients()
 		os.Exit(1)
 	}()

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

@@ -127,3 +127,36 @@ reboot so that SSH will eventually be killed automatically:
 reboot
 sleep 60
 ```
+
+## Troubleshooting
+
+*My shell script doesn't work correctly on Ubuntu*
+
+* On Ubuntu the /bin/sh shell is
+[dash](http://en.wikipedia.org/wiki/Debian_Almquist_shell). If your script has
+[bash](http://en.wikipedia.org/wiki/Bash_(Unix_shell\)) specific commands in it
+then put `#!/bin/bash` at the top of your script. Differences
+between dash and bash can be found on the [DashAsBinSh](https://wiki.ubuntu.com/DashAsBinSh) Ubuntu wiki page.
+
+*My shell works when I login but fails with the shell provisioner*
+
+* See the above tip. More than likely your login shell is using /bin/bash
+while the provisioner is using /bin/sh.
+
+*How do I tell what my shell script is doing?*
+
+* Adding a `-x` flag to the shebang at the top of the script (`#!/bin/sh -x`)
+will echo the script statements as it is executing.
+
+*My builds don't always work the same*
+
+* Some distributions start the SSH daemon before other core services which
+can create race conditions. Your first provisoner can tell the machine to
+wait until it completely boots.
+
+<pre class="prettyprint">
+    {
+       "type": "script",
+       "inline": [ "sleep 10" ]
+    }
+</pre>