diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b21b26e0..bcf9e9f55 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/terraform/context.go b/terraform/context.go index ea4f4f39a..3565fbe66 100644 --- a/terraform/context.go +++ b/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 diff --git a/terraform/context_test.go b/terraform/context_test.go index 4e526455c..1ef4a1a2c 100644 --- a/terraform/context_test.go +++ b/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") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 51e1324cf..f056a8c5f 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -275,6 +275,12 @@ aws_instance.foo: type = aws_instance ` +const testTerraformApplyProvisionerFailCreateStr = ` +aws_instance.bar: (1 tainted) + ID = + Tainted ID 1 = foo +` + const testTerraformApplyProvisionerFailCreateBeforeDestroyStr = ` aws_instance.bar: (1 tainted) ID = bar diff --git a/terraform/test-fixtures/apply-provisioner-fail-create/main.tf b/terraform/test-fixtures/apply-provisioner-fail-create/main.tf new file mode 100644 index 000000000..c1dcd222c --- /dev/null +++ b/terraform/test-fixtures/apply-provisioner-fail-create/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "bar" { + provisioner "shell" {} +}