terraform: taint resources who error on create with provisioners

[GH-434]

CHANGELOG.md

@@ -7,6 +7,8 @@ BUG FIXES:
   * core: Fix a hang that can occur with enough resources. [GH-410]
   * core: Config validation will not error if the field is being
       computed so the value is still unknown.
+  * core: If a resource fails to create and has provisioners, it is
+      marked as tainted. [GH-434]
   * providers/aws: Refresh of launch configs and autoscale groups load
       the correct data and don't incorrectly recreate themselves. [GH-425]
   * providers/aws: Fix case where ELB would incorrectly plan to modify

terraform/context.go

@@ -743,19 +743,26 @@ func (c *walkContext) applyWalkFn() depgraph.WalkFunc {
 		// Additionally, we need to be careful to not run this if there
 		// was an error during the provider apply.
 		tainted := false
-		if applyerr == nil && createNew && len(r.Provisioners) > 0 {
-			for _, h := range c.Context.hooks {
-				handleHook(h.PreProvisionResource(r.Info, is))
-			}
+		if createNew && len(r.Provisioners) > 0 {
+			if applyerr == nil {
+				// If the apply succeeded, we have to run the provisioners
+				for _, h := range c.Context.hooks {
+					handleHook(h.PreProvisionResource(r.Info, is))
+				}
 
-			if err := c.applyProvisioners(r, is); err != nil {
-				errs = append(errs, err)
+				if err := c.applyProvisioners(r, is); err != nil {
+					errs = append(errs, err)
+					tainted = true
+				}
+
+				for _, h := range c.Context.hooks {
+					handleHook(h.PostProvisionResource(r.Info, is))
+				}
+			} else {
+				// If we failed to create properly and we have provisioners,
+				// then we have to mark ourselves as tainted to try again.
 				tainted = true
 			}
-
-			for _, h := range c.Context.hooks {
-				handleHook(h.PostProvisionResource(r.Info, is))
-			}
 		}
 
 		// If we're tainted then we need to update some flags

terraform/context_test.go

@@ -1304,6 +1304,46 @@ func TestContextApply_Provisioner_compute(t *testing.T) {
 	}
 }
 
+func TestContextApply_provisionerCreateFail(t *testing.T) {
+	m := testModule(t, "apply-provisioner-fail-create")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.DiffFn = testDiffFn
+
+	p.ApplyFn = func(
+		info *InstanceInfo,
+		is *InstanceState,
+		id *InstanceDiff) (*InstanceState, error) {
+		is.ID = "foo"
+		return is, fmt.Errorf("error")
+	}
+
+	ctx := testContext(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+	})
+
+	if _, err := ctx.Plan(nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err == nil {
+		t.Fatal("should error")
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyProvisionerFailCreateStr)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
 func TestContextApply_provisionerFail(t *testing.T) {
 	m := testModule(t, "apply-provisioner-fail")
 	p := testProvider("aws")

terraform/terraform_test.go

@@ -275,6 +275,12 @@ aws_instance.foo:
   type = aws_instance
 `
 
+const testTerraformApplyProvisionerFailCreateStr = `
+aws_instance.bar: (1 tainted)
+  ID = <not created>
+  Tainted ID 1 = foo
+`
+
 const testTerraformApplyProvisionerFailCreateBeforeDestroyStr = `
 aws_instance.bar: (1 tainted)
   ID = bar

terraform/test-fixtures/apply-provisioner-fail-create/main.tf

@@ -0,0 +1,3 @@
+resource "aws_instance" "bar" {
+    provisioner "shell" {}
+}