terraform: on destroy prov failure, don't taint and preserve state

terraform/context_apply_test.go

@@ -4058,6 +4058,65 @@ func TestContext2Apply_provisionerDestroy(t *testing.T) {
 	}
 }
 
+// Verify that on destroy provisioner failure, nothing happens to the instance
+func TestContext2Apply_provisionerDestroyFail(t *testing.T) {
+	m := testModule(t, "apply-provisioner-destroy")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	pr.ApplyFn = func(rs *InstanceState, c *ResourceConfig) error {
+		return fmt.Errorf("provisioner error")
+	}
+
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "bar",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Module:  m,
+		State:   state,
+		Destroy: true,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err == nil {
+		t.Fatal("should error")
+	}
+
+	checkStateString(t, state, `
+aws_instance.foo:
+  ID = bar
+	`)
+
+	// Verify apply was invoked
+	if !pr.ApplyCalled {
+		t.Fatalf("provisioner not invoked")
+	}
+}
+
 // Verify destroy provisioners are not run for tainted instances.
 func TestContext2Apply_provisionerDestroyTainted(t *testing.T) {
 	m := testModule(t, "apply-provisioner-destroy")

terraform/eval_apply.go

@@ -160,9 +160,13 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, nil
 	}
 
+	// taint tells us whether to enable tainting.
+	taint := n.When == config.ProvisionerWhenCreate
+
 	if n.Error != nil && *n.Error != nil {
-		// We're already errored creating, so mark as tainted and continue
+		if taint {
 			state.Tainted = true
+		}
 
 		// We're already tainted, so just return out
 		return nil, nil
@@ -182,8 +186,9 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) {
 	// if we have one, otherwise we just output it.
 	err := n.apply(ctx, provs)
 	if err != nil {
-		// Provisioning failed, so mark the resource as tainted
+		if taint {
 			state.Tainted = true
+		}
 
 		if n.Error != nil {
 			*n.Error = multierror.Append(*n.Error, err)

terraform/node_resource_destroy.go

@@ -192,6 +192,20 @@ func (n *NodeDestroyResource) EvalTree() EvalNode {
 					},
 				},
 
+				// If we have a provisioning error, then we just call
+				// the post-apply hook now.
+				&EvalIf{
+					If: func(ctx EvalContext) (bool, error) {
+						return err != nil, nil
+					},
+
+					Then: &EvalApplyPost{
+						Info:  info,
+						State: &state,
+						Error: &err,
+					},
+				},
+
 				// Make sure we handle data sources properly.
 				&EvalIf{
 					If: func(ctx EvalContext) (bool, error) {