From ff36378c4eb9f79d083f64aaa40013273c5c4a0d Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 17 Jul 2014 15:32:19 -0700
Subject: [PATCH] terraform: partial state should be saved on apply error

---
 terraform/context.go        | 17 +++++++++++------
 terraform/context_test.go   | 13 ++++++-------
 terraform/terraform_test.go |  2 ++
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/terraform/context.go b/terraform/context.go
index fe2168f37..54d870e19 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -440,6 +440,8 @@ func (c *Context) releaseRun(ch chan<- struct{}) {
 
 func (c *Context) applyWalkFn() depgraph.WalkFunc {
 	cb := func(r *Resource) error {
+		var err error
+
 		diff := r.Diff
 		if diff.Empty() {
 			log.Printf("[DEBUG] %s: Diff is empty. Will not apply.", r.Id)
@@ -452,7 +454,6 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 				return err
 			}
 
-			var err error
 			diff, err = r.Provider.Diff(r.State, r.Config)
 			if err != nil {
 				return err
@@ -486,9 +487,11 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 
 		// With the completed diff, apply!
 		log.Printf("[DEBUG] %s: Executing Apply", r.Id)
-		rs, err := r.Provider.Apply(r.State, diff)
-		if err != nil {
-			return err
+		rs, applyerr := r.Provider.Apply(r.State, diff)
+
+		var errs []error
+		if applyerr != nil {
+			errs = append(errs, applyerr)
 		}
 
 		// Make sure the result is instantiated
@@ -508,7 +511,6 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 			rs.Attributes["id"] = rs.ID
 		}
 
-		var errs []error
 		for ak, av := range rs.Attributes {
 			// If the value is the unknown variable value, then it is an error.
 			// In this case we record the error and remove it from the state
@@ -522,7 +524,10 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 		// Invoke any provisioners we have defined. This is only done
 		// if the resource was created, as updates or deletes do not
 		// invoke provisioners.
-		if r.State.ID == "" && len(r.Provisioners) > 0 {
+		//
+		// Additionally, we need to be careful to not run this if there
+		// was an error during the provider apply.
+		if applyerr == nil && r.State.ID == "" && len(r.Provisioners) > 0 {
 			rs, err = c.applyProvisioners(r, rs)
 			if err != nil {
 				errs = append(errs, err)
diff --git a/terraform/context_test.go b/terraform/context_test.go
index cfe7727a9..40143f36d 100644
--- a/terraform/context_test.go
+++ b/terraform/context_test.go
@@ -738,7 +738,10 @@ func TestContextApply_error(t *testing.T) {
 
 	p.ApplyFn = func(*ResourceState, *ResourceDiff) (*ResourceState, error) {
 		if errored {
-			return nil, fmt.Errorf("error")
+			state := &ResourceState{
+				ID: "bar",
+			}
+			return state, fmt.Errorf("error")
 		}
 		errored = true
 
@@ -768,10 +771,6 @@ func TestContextApply_error(t *testing.T) {
 		t.Fatal("should have error")
 	}
 
-	if len(state.Resources) != 1 {
-		t.Fatalf("bad: %#v", state.Resources)
-	}
-
 	actual := strings.TrimSpace(state.String())
 	expected := strings.TrimSpace(testTerraformApplyErrorStr)
 	if actual != expected {
@@ -800,9 +799,9 @@ func TestContextApply_errorPartial(t *testing.T) {
 		State: s,
 	})
 
-	p.ApplyFn = func(*ResourceState, *ResourceDiff) (*ResourceState, error) {
+	p.ApplyFn = func(s *ResourceState, d *ResourceDiff) (*ResourceState, error) {
 		if errored {
-			return nil, fmt.Errorf("error")
+			return s, fmt.Errorf("error")
 		}
 		errored = true
 
diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go
index 203741ed7..635148d93 100644
--- a/terraform/terraform_test.go
+++ b/terraform/terraform_test.go
@@ -135,6 +135,8 @@ const testTerraformApplyDestroyStr = `
 `
 
 const testTerraformApplyErrorStr = `
+aws_instance.bar:
+  ID = bar
 aws_instance.foo:
   ID = foo
   num = 2