Pass exitCode by reference

Arguments to defer statements are evaluated when the defer statement is
evaluated, so pass exitCode by reference instead.

Fixes issue #346

command/agent/remote_exec.go

@@ -137,7 +137,7 @@ func (a *Agent) handleRemoteExec(msg *UserEvent) {
 
 	// Ensure we write out an exit code
 	exitCode := 0
-	defer a.remoteExecWriteExitCode(&event, exitCode)
+	defer a.remoteExecWriteExitCode(&event, &exitCode)
 
 	// Check if this is a script, we may need to spill to disk
 	var script string
@@ -190,7 +190,7 @@ func (a *Agent) handleRemoteExec(msg *UserEvent) {
 		err := cmd.Wait()
 		writer.Flush()
 		close(writer.BufCh)
-		if err != nil {
+		if err == nil {
 			exitCh <- 0
 			return
 		}
@@ -287,8 +287,8 @@ func (a *Agent) remoteExecWriteOutput(event *remoteExecEvent, num int, output []
 }
 
 // remoteExecWriteExitCode is used to write an exit code
-func (a *Agent) remoteExecWriteExitCode(event *remoteExecEvent, exitCode int) bool {
+func (a *Agent) remoteExecWriteExitCode(event *remoteExecEvent, exitCode *int) bool {
-	val := []byte(strconv.FormatInt(int64(exitCode), 10))
+	val := []byte(strconv.FormatInt(int64(*exitCode), 10))
 	if err := a.remoteExecWriteKey(event, remoteExecExitSuffix, val); err != nil {
 		a.logger.Printf("[ERR] agent: failed to write exit code for remote exec job: %v", err)
 		return false

command/agent/remote_exec_test.go

@@ -140,7 +140,8 @@ func TestRemoteExecWrites(t *testing.T) {
 		t.Fatalf("bad")
 	}
 
-	if !agent.remoteExecWriteExitCode(event, 1) {
+	exitCode := 1
+	if !agent.remoteExecWriteExitCode(event, &exitCode) {
 		t.Fatalf("bad")
 	}
 
@@ -227,6 +228,64 @@ func TestHandleRemoteExec(t *testing.T) {
 	}
 }
 
+func TestHandleRemoteExecFailed(t *testing.T) {
+	dir, agent := makeAgent(t, nextConfig())
+	defer os.RemoveAll(dir)
+	defer agent.Shutdown()
+	testutil.WaitForLeader(t, agent.RPC, "dc1")
+
+	event := &remoteExecEvent{
+		Prefix:  "_rexec",
+		Session: makeRexecSession(t, agent),
+	}
+	defer destroySession(t, agent, event.Session)
+
+	spec := &remoteExecSpec{
+		Command: "echo failing;exit 2",
+		Wait:    time.Second,
+	}
+	buf, err := json.Marshal(spec)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	key := "_rexec/" + event.Session + "/job"
+	setKV(t, agent, key, buf)
+
+	buf, err = json.Marshal(event)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	msg := &UserEvent{
+		ID:      generateUUID(),
+		Payload: buf,
+	}
+
+	// Handle the event...
+	agent.handleRemoteExec(msg)
+
+	// Verify we have an ack
+	key = "_rexec/" + event.Session + "/" + agent.config.NodeName + "/ack"
+	d := getKV(t, agent, key)
+	if d == nil || d.Session != event.Session {
+		t.Fatalf("bad ack: %#v", d)
+	}
+
+	// Verify we have output
+	key = "_rexec/" + event.Session + "/" + agent.config.NodeName + "/out/00000"
+	d = getKV(t, agent, key)
+	if d == nil || d.Session != event.Session ||
+		!bytes.Contains(d.Value, []byte("failing")) {
+		t.Fatalf("bad output: %#v", d)
+	}
+
+	// Verify we have an exit code
+	key = "_rexec/" + event.Session + "/" + agent.config.NodeName + "/exit"
+	d = getKV(t, agent, key)
+	if d == nil || d.Session != event.Session || string(d.Value) != "2" {
+		t.Fatalf("bad output: %#v", d)
+	}
+}
+
 func makeRexecSession(t *testing.T, agent *Agent) string {
 	args := structs.SessionRequest{
 		Datacenter: agent.config.Datacenter,