From 808036bf602e30ecfeaabe770b085ca943f05bf1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 09:56:39 -0700 Subject: [PATCH] terraform: ResourceProvisioner can't return a state anymore --- terraform/context.go | 19 ++++---- terraform/context_test.go | 48 +++++++++++++++++-- terraform/resource_provisioner.go | 2 +- terraform/resource_provisioner_mock.go | 7 ++- terraform/terraform_test.go | 10 ++++ .../apply-provisioner-fail/main.tf | 7 +++ 6 files changed, 73 insertions(+), 20 deletions(-) create mode 100644 terraform/test-fixtures/apply-provisioner-fail/main.tf diff --git a/terraform/context.go b/terraform/context.go index 7674714ac0..cedc20b06a 100644 --- a/terraform/context.go +++ b/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 != nil { + if err := c.applyProvisioners(r, rs); 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) { - var err error - +func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) 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 != nil { - return rs, err + if err := prov.Provisioner.Apply(rs, prov.Config); err != nil { + return err } } - return rs, nil + + return nil } func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc { diff --git a/terraform/context_test.go b/terraform/context_test.go index 9d161ea6bf..f206a5080b 100644 --- a/terraform/context_test.go +++ b/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{ diff --git a/terraform/resource_provisioner.go b/terraform/resource_provisioner.go index 6c0c03f10e..967e0e037f 100644 --- a/terraform/resource_provisioner.go +++ b/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 diff --git a/terraform/resource_provisioner_mock.go b/terraform/resource_provisioner_mock.go index 2f9a70b9fa..a6e62a593d 100644 --- a/terraform/resource_provisioner_mock.go +++ b/terraform/resource_provisioner_mock.go @@ -9,8 +9,7 @@ type MockResourceProvisioner struct { ApplyCalled bool ApplyState *ResourceState ApplyConfig *ResourceConfig - ApplyFn func(*ResourceState, *ResourceConfig) (*ResourceState, error) - ApplyReturn *ResourceState + ApplyFn func(*ResourceState, *ResourceConfig) error 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 } diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 24e780f9e1..464d43a208 100644 --- a/terraform/terraform_test.go +++ b/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 = ` ` diff --git a/terraform/test-fixtures/apply-provisioner-fail/main.tf b/terraform/test-fixtures/apply-provisioner-fail/main.tf new file mode 100644 index 0000000000..d27208d4fa --- /dev/null +++ b/terraform/test-fixtures/apply-provisioner-fail/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "foo" { + num = "2" +} + +resource "aws_instance" "bar" { + provisioner "shell" {} +}