terraform: ResourceProvisioner can't return a state anymore

terraform/context.go

@@ -555,8 +555,7 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 		// 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 := c.applyProvisioners(r, rs); err != nil {
-			if err != nil {
 				errs = append(errs, err)
 			}
 		}
@@ -591,9 +590,7 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 
 // applyProvisioners is used to run any provisioners a resource has
 // defined after the resource creation has already completed.
-func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceState, error) {
+func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) error {
-	var err error
-
 	// Store the original connection info, restore later
 	origConnInfo := rs.ConnInfo
 	defer func() {
@@ -604,13 +601,13 @@ func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceSt
 		// Interpolate since we may have variables that depend on the
 		// local resource.
 		if err := prov.Config.interpolate(c); err != nil {
-			return rs, err
+			return err
 		}
 
 		// Interpolate the conn info, since it may contain variables
 		connInfo := NewResourceConfig(prov.ConnInfo)
 		if err := connInfo.interpolate(c); err != nil {
-			return rs, err
+			return err
 		}
 
 		// Merge the connection information
@@ -643,12 +640,12 @@ func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceSt
 		rs.ConnInfo = overlay
 
 		// Invoke the Provisioner
-		rs, err = prov.Provisioner.Apply(rs, prov.Config)
+		if err := prov.Provisioner.Apply(rs, prov.Config); err != nil {
-		if err != nil {
+			return err
-			return rs, err
 		}
 	}
-	return rs, nil
+
+	return nil
 }
 
 func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc {

terraform/context_test.go

@@ -414,12 +414,13 @@ func TestContextApply_Provisioner_compute(t *testing.T) {
 	pr := testProvisioner()
 	p.ApplyFn = testApplyFn
 	p.DiffFn = testDiffFn
-	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) (*ResourceState, error) {
+	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error {
 		val, ok := c.Config["foo"]
 		if !ok || val != "computed_dynamical" {
 			t.Fatalf("bad value for foo: %v %#v", val, c)
 		}
-		return rs, nil
+
+		return nil
 	}
 	ctx := testContext(t, &ContextOpts{
 		Config: c,
@@ -455,6 +456,44 @@ func TestContextApply_Provisioner_compute(t *testing.T) {
 	}
 }
 
+func TestContextApply_provisionerFail(t *testing.T) {
+	t.Skip()
+
+	c := testConfig(t, "apply-provisioner-fail")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	ctx := testContext(t, &ContextOpts{
+		Config: c,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+		Variables: map[string]string{
+			"value": "1",
+		},
+	})
+
+	if _, err := ctx.Plan(nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyProvisionerFailStr)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
 func TestContextApply_outputDiffVars(t *testing.T) {
 	c := testConfig(t, "apply-good")
 	p := testProvider("aws")
@@ -527,7 +566,7 @@ func TestContextApply_Provisioner_ConnInfo(t *testing.T) {
 	}
 	p.DiffFn = testDiffFn
 
-	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) (*ResourceState, error) {
+	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error {
 		conn := rs.ConnInfo
 		if conn["type"] != "telnet" {
 			t.Fatalf("Bad: %#v", conn)
@@ -544,7 +583,8 @@ func TestContextApply_Provisioner_ConnInfo(t *testing.T) {
 		if conn["pass"] != "test" {
 			t.Fatalf("Bad: %#v", conn)
 		}
-		return rs, nil
+
+		return nil
 	}
 
 	ctx := testContext(t, &ContextOpts{

terraform/resource_provisioner.go

@@ -20,7 +20,7 @@ type ResourceProvisioner interface {
 	// resource state along with an error. Instead of a diff, the ResourceConfig
 	// is provided since provisioners only run after a resource has been
 	// newly created.
-	Apply(*ResourceState, *ResourceConfig) (*ResourceState, error)
+	Apply(*ResourceState, *ResourceConfig) error
 }
 
 // ResourceProvisionerFactory is a function type that creates a new instance

terraform/resource_provisioner_mock.go

@@ -9,8 +9,7 @@ type MockResourceProvisioner struct {
 	ApplyCalled      bool
 	ApplyState       *ResourceState
 	ApplyConfig      *ResourceConfig
-	ApplyFn          func(*ResourceState, *ResourceConfig) (*ResourceState, error)
+	ApplyFn          func(*ResourceState, *ResourceConfig) error
-	ApplyReturn      *ResourceState
 	ApplyReturnError error
 
 	ValidateCalled       bool
@@ -29,12 +28,12 @@ func (p *MockResourceProvisioner) Validate(c *ResourceConfig) ([]string, []error
 	return p.ValidateReturnWarns, p.ValidateReturnErrors
 }
 
-func (p *MockResourceProvisioner) Apply(state *ResourceState, c *ResourceConfig) (*ResourceState, error) {
+func (p *MockResourceProvisioner) Apply(state *ResourceState, c *ResourceConfig) error {
 	p.ApplyCalled = true
 	p.ApplyState = state
 	p.ApplyConfig = c
 	if p.ApplyFn != nil {
 		return p.ApplyFn(state, c)
 	}
-	return p.ApplyReturn, p.ApplyReturnError
+	return p.ApplyReturnError
 }

terraform/terraform_test.go

@@ -130,6 +130,16 @@ aws_instance.foo:
   type = aws_instance
 `
 
+const testTerraformApplyProvisionerFailStr = `
+aws_instance.bar:
+  ID = foo
+aws_instance.foo:
+  ID = foo
+  dynamical = computed_dynamical
+  num = 2
+  type = aws_instance
+`
+
 const testTerraformApplyDestroyStr = `
 <no state>
 `

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

@@ -0,0 +1,7 @@
+resource "aws_instance" "foo" {
+    num = "2"
+}
+
+resource "aws_instance" "bar" {
+    provisioner "shell" {}
+}