From 6a7e3668a20d5149073b8f461e2e8c3a7dca0768 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 7 Jul 2014 21:20:48 -0700
Subject: [PATCH] command: apply saves state even if error occurs

---
 command/apply.go                          | 45 +++++++--------
 command/apply_test.go                     | 70 +++++++++++++++++++++++
 command/test-fixtures/apply-error/main.tf |  7 +++
 terraform/context.go                      |  1 +
 4 files changed, 98 insertions(+), 25 deletions(-)
 create mode 100644 command/test-fixtures/apply-error/main.tf

diff --git a/command/apply.go b/command/apply.go
index 3f072e436..448258dec 100644
--- a/command/apply.go
+++ b/command/apply.go
@@ -60,20 +60,15 @@ func (c *ApplyCommand) Run(args []string) int {
 		return 1
 	}
 
-	errCh := make(chan error)
-	stateCh := make(chan *terraform.State)
+	var state *terraform.State
+	var applyErr error
+	doneCh := make(chan struct{})
 	go func() {
-		state, err := ctx.Apply()
-		if err != nil {
-			errCh <- err
-			return
-		}
-
-		stateCh <- state
+		defer close(doneCh)
+		state, applyErr = ctx.Apply()
 	}()
 
 	err = nil
-	var state *terraform.State
 	select {
 	case <-c.ShutdownCh:
 		c.Ui.Output("Interrupt received. Gracefully shutting down...")
@@ -88,26 +83,26 @@ func (c *ApplyCommand) Run(args []string) int {
 				"Two interrupts received. Exiting immediately. Note that data\n" +
 					"loss may have occurred.")
 			return 1
-		case state = <-stateCh:
-		case err = <-errCh:
+		case <-doneCh:
 		}
-	case state = <-stateCh:
-	case err = <-errCh:
+	case <-doneCh:
 	}
 
-	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error applying plan: %s", err))
-		return 1
+	if state != nil {
+		// Write state out to the file
+		f, err := os.Create(stateOutPath)
+		if err == nil {
+			err = terraform.WriteState(state, f)
+			f.Close()
+		}
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Failed to save state: %s", err))
+			return 1
+		}
 	}
 
-	// Write state out to the file
-	f, err := os.Create(stateOutPath)
-	if err == nil {
-		err = terraform.WriteState(state, f)
-		f.Close()
-	}
-	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to save state: %s", err))
+	if applyErr != nil {
+		c.Ui.Error(fmt.Sprintf("Error applying plan: %s", applyErr))
 		return 1
 	}
 
diff --git a/command/apply_test.go b/command/apply_test.go
index 8f89f8221..750e533f5 100644
--- a/command/apply_test.go
+++ b/command/apply_test.go
@@ -1,8 +1,10 @@
 package command
 
 import (
+	"fmt"
 	"os"
 	"reflect"
+	"sync"
 	"testing"
 	"time"
 
@@ -67,6 +69,74 @@ func TestApply_configInvalid(t *testing.T) {
 	}
 }
 
+func TestApply_error(t *testing.T) {
+	statePath := testTempFile(t)
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		ContextOpts: testCtxConfig(p),
+		Ui:          ui,
+	}
+
+	var lock sync.Mutex
+	errored := false
+	p.ApplyFn = func(
+		s *terraform.ResourceState,
+		d *terraform.ResourceDiff) (*terraform.ResourceState, error) {
+		lock.Lock()
+		defer lock.Unlock()
+
+		if !errored {
+			errored = true
+			return nil, fmt.Errorf("error")
+		}
+
+		return &terraform.ResourceState{ID: "foo"}, nil
+	}
+	p.DiffFn = func(
+		*terraform.ResourceState,
+		*terraform.ResourceConfig) (*terraform.ResourceDiff, error) {
+		return &terraform.ResourceDiff{
+			Attributes: map[string]*terraform.ResourceAttrDiff{
+				"ami": &terraform.ResourceAttrDiff{
+					New: "bar",
+				},
+			},
+		}, nil
+	}
+
+	args := []string{
+		"-init",
+		statePath,
+		testFixturePath("apply-error"),
+	}
+	if code := c.Run(args); code != 1 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	if _, err := os.Stat(statePath); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	f, err := os.Open(statePath)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer f.Close()
+
+	state, err := terraform.ReadState(f)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if state == nil {
+		t.Fatal("state should not be nil")
+	}
+	if len(state.Resources) == 0 {
+		t.Fatal("no resources in state")
+	}
+}
+
 func TestApply_plan(t *testing.T) {
 	planPath := testPlanFile(t, &terraform.Plan{
 		Config: new(config.Config),
diff --git a/command/test-fixtures/apply-error/main.tf b/command/test-fixtures/apply-error/main.tf
new file mode 100644
index 000000000..9b4b65a8f
--- /dev/null
+++ b/command/test-fixtures/apply-error/main.tf
@@ -0,0 +1,7 @@
+resource "test_instance" "foo" {
+    ami = "bar"
+}
+
+resource "test_instance" "bar" {
+    error = "true"
+}
diff --git a/terraform/context.go b/terraform/context.go
index 318778fd4..9727aadfe 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -738,6 +738,7 @@ func (c *Context) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc {
 		// Call the callack
 		log.Printf("[INFO] Walking: %s", rn.Resource.Id)
 		if err := cb(rn.Resource); err != nil {
+			log.Printf("[ERROR] Error walking '%s': %s", rn.Resource.Id, err)
 			return err
 		}