From e9f6c9c429a92ea0644ae3640fc31872d971128c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 20 Jan 2017 18:07:51 -0800 Subject: [PATCH] terraform: run destroy provisioners on destroy --- terraform/context_apply_test.go | 138 ++++++++++++++++++ terraform/eval_apply.go | 36 ++++- terraform/graph_builder_apply.go | 9 +- terraform/graph_builder_apply_test.go | 72 +++++++++ terraform/graph_test.go | 26 ++++ terraform/node_resource_apply.go | 1 + terraform/node_resource_destroy.go | 32 ++++ .../apply-provisioner-destroy/main.tf | 12 ++ .../graph-builder-apply-provisioner/main.tf | 3 + 9 files changed, 317 insertions(+), 12 deletions(-) create mode 100644 terraform/test-fixtures/apply-provisioner-destroy/main.tf create mode 100644 terraform/test-fixtures/graph-builder-apply-provisioner/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 77b0579fb..423ff1eee 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3998,6 +3998,144 @@ aws_instance.web: `) } +func TestContext2Apply_provisionerDestroy(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 { + val, ok := c.Config["foo"] + if !ok || val != "destroy" { + t.Fatalf("bad value for foo: %v %#v", val, c) + } + + return nil + } + + 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.Fatalf("err: %s", err) + } + + checkStateString(t, state, ``) + + // 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") + p := testProvider("aws") + pr := testProvisioner() + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + destroyCalled := false + pr.ApplyFn = func(rs *InstanceState, c *ResourceConfig) error { + expected := "create" + if rs.ID == "bar" { + destroyCalled = true + return nil + } + + val, ok := c.Config["foo"] + if !ok || val != expected { + t.Fatalf("bad value for foo: %v %#v", val, c) + } + + return nil + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Tainted: true, + }, + }, + }, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + State: state, + 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.Fatalf("err: %s", err) + } + + checkStateString(t, state, ` +aws_instance.foo: + ID = foo + foo = bar + type = aws_instance + `) + + // Verify apply was invoked + if !pr.ApplyCalled { + t.Fatalf("provisioner not invoked") + } + + if destroyCalled { + t.Fatal("destroy should not be called") + } +} + func TestContext2Apply_provisionerResourceRef(t *testing.T) { m := testModule(t, "apply-provisioner-resource-ref") p := testProvider("aws") diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index f8a42ed47..b554f50ce 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -140,18 +140,22 @@ type EvalApplyProvisioners struct { InterpResource *Resource CreateNew *bool Error *error + + // When is the type of provisioner to run at this point + When config.ProvisionerWhen } // TODO: test func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) { state := *n.State - if !*n.CreateNew { + if n.CreateNew != nil && !*n.CreateNew { // If we're not creating a new resource, then don't run provisioners return nil, nil } - if len(n.Resource.Provisioners) == 0 { + provs := n.filterProvisioners() + if len(provs) == 0 { // We have no provisioners, so don't do anything return nil, nil } @@ -176,7 +180,7 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) { // If there are no errors, then we append it to our output error // if we have one, otherwise we just output it. - err := n.apply(ctx) + err := n.apply(ctx, provs) if err != nil { // Provisioning failed, so mark the resource as tainted state.Tainted = true @@ -201,7 +205,29 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } -func (n *EvalApplyProvisioners) apply(ctx EvalContext) error { +// filterProvisioners filters the provisioners on the resource to only +// the provisioners specified by the "when" option. +func (n *EvalApplyProvisioners) filterProvisioners() []*config.Provisioner { + // Fast path the zero case + if n.Resource == nil { + return nil + } + + if len(n.Resource.Provisioners) == 0 { + return nil + } + + result := make([]*config.Provisioner, 0, len(n.Resource.Provisioners)) + for _, p := range n.Resource.Provisioners { + if p.When == n.When { + result = append(result, p) + } + } + + return result +} + +func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*config.Provisioner) error { state := *n.State // Store the original connection info, restore later @@ -210,7 +236,7 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext) error { state.Ephemeral.ConnInfo = origConnInfo }() - for _, prov := range n.Resource.Provisioners { + for _, prov := range provs { // Get the provisioner provisioner := ctx.Provisioner(prov.Type) diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 75de53757..4eeaab3e2 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -96,13 +96,8 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { ), // Provisioner-related transformations - GraphTransformIf( - func() bool { return !b.Destroy }, - GraphTransformMulti( - &MissingProvisionerTransformer{Provisioners: b.Provisioners}, - &ProvisionerTransformer{}, - ), - ), + &MissingProvisionerTransformer{Provisioners: b.Provisioners}, + &ProvisionerTransformer{}, // Add root variables &RootVariableTransformer{Module: b.Module}, diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index f6420154e..659f8e141 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -232,6 +232,78 @@ func TestApplyGraphBuilder_moduleDestroy(t *testing.T) { "module.A.null_resource.foo (destroy)") } +func TestApplyGraphBuilder_provisioner(t *testing.T) { + diff := &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + "null_resource.foo": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, + }, + } + + b := &ApplyGraphBuilder{ + Module: testModule(t, "graph-builder-apply-provisioner"), + Diff: diff, + Providers: []string{"null"}, + Provisioners: []string{"local"}, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + testGraphContains(t, g, "provisioner.local") + testGraphHappensBefore( + t, g, + "provisioner.local", + "null_resource.foo") +} + +func TestApplyGraphBuilder_provisionerDestroy(t *testing.T) { + diff := &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + "null_resource.foo": &InstanceDiff{ + Destroy: true, + }, + }, + }, + }, + } + + b := &ApplyGraphBuilder{ + Destroy: true, + Module: testModule(t, "graph-builder-apply-provisioner"), + Diff: diff, + Providers: []string{"null"}, + Provisioners: []string{"local"}, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + testGraphContains(t, g, "provisioner.local") + testGraphHappensBefore( + t, g, + "provisioner.local", + "null_resource.foo (destroy)") +} + const testApplyGraphBuilderStr = ` aws_instance.create provider.aws diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 3d5ae9aab..fc9fc25dd 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -88,6 +88,32 @@ func TestGraphWalk_panicWrap(t *testing.T) { } } +// testGraphContains is an assertion helper that tests that a node is +// contained in the graph. +func testGraphContains(t *testing.T, g *Graph, name string) { + for _, v := range g.Vertices() { + if dag.VertexName(v) == name { + return + } + } + + t.Fatalf( + "Expected %q in:\n\n%s", + name, g.String()) +} + +// testGraphnotContains is an assertion helper that tests that a node is +// NOT contained in the graph. +func testGraphNotContains(t *testing.T, g *Graph, name string) { + for _, v := range g.Vertices() { + if dag.VertexName(v) == name { + t.Fatalf( + "Expected %q to NOT be in:\n\n%s", + name, g.String()) + } + } +} + // testGraphHappensBefore is an assertion helper that tests that node // A (dag.VertexName value) happens before node B. func testGraphHappensBefore(t *testing.T, g *Graph, A, B string) { diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index a16683642..07b33a055 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -321,6 +321,7 @@ func (n *NodeApplyableResource) evalTreeManagedResource( InterpResource: resource, CreateNew: &createNew, Error: &err, + When: config.ProvisionerWhenCreate, }, &EvalIf{ If: func(ctx EvalContext) (bool, error) { diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 6b4a48b6b..df1e9f404 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -107,6 +107,17 @@ func (n *NodeDestroyResource) EvalTree() EvalNode { uniqueExtra: "destroy", } + // Build the resource for eval + addr := n.Addr + resource := &Resource{ + Name: addr.Name, + Type: addr.Type, + CountIndex: addr.Index, + } + if resource.CountIndex < 0 { + resource.CountIndex = 0 + } + // Get our state rs := n.ResourceState if rs == nil { @@ -160,6 +171,27 @@ func (n *NodeDestroyResource) EvalTree() EvalNode { &EvalRequireState{ State: &state, }, + + // Run destroy provisioners if not tainted + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if state != nil && state.Tainted { + return false, nil + } + + return true, nil + }, + + Then: &EvalApplyProvisioners{ + Info: info, + State: &state, + Resource: n.Config, + InterpResource: resource, + Error: &err, + When: config.ProvisionerWhenDestroy, + }, + }, + // Make sure we handle data sources properly. &EvalIf{ If: func(ctx EvalContext) (bool, error) { diff --git a/terraform/test-fixtures/apply-provisioner-destroy/main.tf b/terraform/test-fixtures/apply-provisioner-destroy/main.tf new file mode 100644 index 000000000..686a1b040 --- /dev/null +++ b/terraform/test-fixtures/apply-provisioner-destroy/main.tf @@ -0,0 +1,12 @@ +resource "aws_instance" "foo" { + foo = "bar" + + provisioner "shell" { + foo = "create" + } + + provisioner "shell" { + foo = "destroy" + when = "destroy" + } +} diff --git a/terraform/test-fixtures/graph-builder-apply-provisioner/main.tf b/terraform/test-fixtures/graph-builder-apply-provisioner/main.tf new file mode 100644 index 000000000..948622156 --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-provisioner/main.tf @@ -0,0 +1,3 @@ +resource "null_resource" "foo" { + provisioner "local" {} +}