From 2a679edd2529bc52cda70d8538d32014276941bd Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Tue, 17 May 2016 07:56:24 -0700 Subject: [PATCH] core: Fix destroy on nested module vars for count Building on b10564a, adding tweaks that allow the module var count search to act recursively, ensuring that a sitaution where something like var.top gets passed to module middle, as var.middle, and then to module bottom, as var.bottom, which is then used in a resource count. --- terraform/context_apply_test.go | 86 +++++++++++++++++++ terraform/graph_config_node_variable.go | 40 +++++++-- .../child/child2/main.tf | 5 ++ .../child/main.tf | 8 ++ .../main.tf | 9 ++ 5 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/child2/main.tf create mode 100644 terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/main.tf create mode 100644 terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 4be476982..9193c3b93 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2706,6 +2706,92 @@ module.child: } } +func TestContext2Apply_destroyWithModuleVariableAndCountNested(t *testing.T) { + m := testModule(t, "apply-destroy-mod-var-and-count-nested") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + var state *State + var err error + { + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + // First plan and apply a create operation + if _, err := ctx.Plan(); err != nil { + t.Fatalf("plan err: %s", err) + } + + state, err = ctx.Apply() + if err != nil { + t.Fatalf("apply err: %s", err) + } + } + + h := new(HookRecordApplyOrder) + h.Active = true + + { + ctx := testContext2(t, &ContextOpts{ + Destroy: true, + Module: m, + State: state, + Hooks: []Hook{h}, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + // First plan and apply a create operation + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("destroy plan err: %s", err) + } + + var buf bytes.Buffer + if err := WritePlan(plan, &buf); err != nil { + t.Fatalf("plan write err: %s", err) + } + + planFromFile, err := ReadPlan(&buf) + if err != nil { + t.Fatalf("plan read err: %s", err) + } + + ctx, err = planFromFile.Context(&ContextOpts{ + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + state, err = ctx.Apply() + if err != nil { + t.Fatalf("destroy apply err: %s", err) + } + } + + //Test that things were destroyed + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(` + +module.child: + +module.child.child2: + + `) + if actual != expected { + t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual) + } +} + func TestContext2Apply_destroyOutputs(t *testing.T) { m := testModule(t, "apply-destroy-outputs") h := new(HookRecordApplyOrder) diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index 0c8a3ded0..1f17471e8 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -62,13 +62,16 @@ func (n *GraphNodeConfigVariable) VariableName() string { func (n *GraphNodeConfigVariable) DestroyEdgeInclude(v dag.Vertex) bool { // Only include this variable in a destroy edge if the source vertex // "v" has a count dependency on this variable. + log.Printf("[DEBUG] DestroyEdgeInclude: Checking: %s", dag.VertexName(v)) cv, ok := v.(GraphNodeCountDependent) if !ok { + log.Printf("[DEBUG] DestroyEdgeInclude: Not GraphNodeCountDependent: %s", dag.VertexName(v)) return false } for _, d := range cv.CountDependentOn() { for _, d2 := range n.DependableName() { + log.Printf("[DEBUG] DestroyEdgeInclude: d = %s : d2 = %s", d, d2) if d == d2 { return true } @@ -97,14 +100,10 @@ func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool { // If we're destroying, we have no need of variables unless they are depended // on by the count of a resource. if modDiff != nil && modDiff.Destroy { - for _, v := range opts.Graph.UpEdges(opts.Vertex).List() { - // Here we borrow the implementation of DestroyEdgeInclude, whose logic - // and semantics are exactly what we want here. - if n.DestroyEdgeInclude(v) { - log.Printf("[DEBUG] Variable has destroy edge from %s, not a noop", - dag.VertexName(v)) - return false - } + if n.hasDestroyEdgeInPath(opts, nil) { + log.Printf("[DEBUG] Variable has destroy edge from %s, not a noop", + dag.VertexName(opts.Vertex)) + return false } log.Printf("[DEBUG] Variable has no included destroy edges: noop!") return true @@ -124,6 +123,31 @@ func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool { return true } +// hasDestroyEdgeInPath recursively walks for a destroy edge, ensuring that +// a variable both has no immediate destroy edges or any in its full module +// path, ensuring that links do not get severed in the middle. +func (n *GraphNodeConfigVariable) hasDestroyEdgeInPath(opts *NoopOpts, vertex dag.Vertex) bool { + if vertex == nil { + vertex = opts.Vertex + } + log.Printf("[DEBUG] hasDestroyEdgeInPath: Looking for destroy edge: %s - %T", dag.VertexName(vertex), vertex) + for _, v := range opts.Graph.UpEdges(vertex).List() { + if len(opts.Graph.UpEdges(v).List()) > 1 { + if n.hasDestroyEdgeInPath(opts, v) == true { + return true + } + } + // Here we borrow the implementation of DestroyEdgeInclude, whose logic + // and semantics are exactly what we want here. + if cv, ok := vertex.(*GraphNodeConfigVariableFlat); ok { + if cv.DestroyEdgeInclude(v) { + return true + } + } + } + return false +} + // GraphNodeProxy impl. func (n *GraphNodeConfigVariable) Proxy() bool { return true diff --git a/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/child2/main.tf b/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/child2/main.tf new file mode 100644 index 000000000..6a4f91d5e --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/child2/main.tf @@ -0,0 +1,5 @@ +variable "mod_count_child2" { } + +resource "aws_instance" "foo" { + count = "${var.mod_count_child2}" +} diff --git a/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/main.tf b/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/main.tf new file mode 100644 index 000000000..28b526795 --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/main.tf @@ -0,0 +1,8 @@ +variable "mod_count_child" { } + +module "child2" { + source = "./child2" + mod_count_child2 = "${var.mod_count_child}" +} + +resource "aws_instance" "foo" { } diff --git a/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/main.tf b/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/main.tf new file mode 100644 index 000000000..676ddc79b --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/main.tf @@ -0,0 +1,9 @@ +variable "mod_count_root" { + type = "string" + default = "3" +} + +module "child" { + source = "./child" + mod_count_child = "${var.mod_count_root}" +}