From b0eafeb2124eb60336dda4f81167f43716a60956 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 13 May 2015 12:10:02 -0500 Subject: [PATCH] core: fix deadlock w/ CBD + modules fixes #1947 Root cause was a bad edge being made by the CBD transform going from the flattened destroy node to the unflattened create node, which was no longer in the graph. The destroy node therefore had a dependency that could never be satisfied, which locked up the walk. --- terraform/context_test.go | 50 +++++++++++++++++++ terraform/graph_config_node_resource.go | 8 +++ .../plan-module-deadlock/child/main.tf | 4 ++ .../plan-module-deadlock/main.tf | 3 ++ 4 files changed, 65 insertions(+) create mode 100644 terraform/test-fixtures/plan-module-deadlock/child/main.tf create mode 100644 terraform/test-fixtures/plan-module-deadlock/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 15bb62778..2ae5f8ba0 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -139,6 +139,56 @@ func TestContext2Plan_moduleCycle(t *testing.T) { } } +func TestContext2Plan_moduleDeadlock(t *testing.T) { + m := testModule(t, "plan-module-deadlock") + p := testProvider("aws") + p.DiffFn = testDiffFn + timeout := make(chan bool, 1) + done := make(chan bool, 1) + go func() { + time.Sleep(3 * time.Second) + timeout <- true + }() + go func() { + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + plan, err := ctx.Plan() + done <- true + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(` +DIFF: + +module.child: + CREATE: aws_instance.foo.0 + CREATE: aws_instance.foo.1 + CREATE: aws_instance.foo.2 + +STATE: + + + `) + if actual != expected { + t.Fatalf("expected:\n%sgot:\n%s", expected, actual) + } + }() + + select { + case <-timeout: + t.Fatalf("timed out! probably deadlock") + case <-done: + // ok + } +} + func TestContext2Plan_moduleInput(t *testing.T) { m := testModule(t, "plan-module-input") p := testProvider("aws") diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 30667cb43..bf1803b00 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -311,6 +311,7 @@ func (n *GraphNodeConfigResourceFlat) DestroyNode(mode GraphNodeDestroyMode) Gra return &graphNodeResourceDestroyFlat{ graphNodeResourceDestroy: node, PathValue: n.PathValue, + FlatCreateNode: n, } } @@ -318,6 +319,9 @@ type graphNodeResourceDestroyFlat struct { *graphNodeResourceDestroy PathValue []string + + // Needs to be able to properly yield back a flattened create node to prevent + FlatCreateNode *GraphNodeConfigResourceFlat } func (n *graphNodeResourceDestroyFlat) Name() string { @@ -329,6 +333,10 @@ func (n *graphNodeResourceDestroyFlat) Path() []string { return n.PathValue } +func (n *graphNodeResourceDestroyFlat) CreateNode() dag.Vertex { + return n.FlatCreateNode +} + // graphNodeResourceDestroy represents the logical destruction of a // resource. This node doesn't mean it will be destroyed for sure, but // instead that if a destroy were to happen, it must happen at this point. diff --git a/terraform/test-fixtures/plan-module-deadlock/child/main.tf b/terraform/test-fixtures/plan-module-deadlock/child/main.tf new file mode 100644 index 000000000..ccee7bfb4 --- /dev/null +++ b/terraform/test-fixtures/plan-module-deadlock/child/main.tf @@ -0,0 +1,4 @@ +resource "aws_instance" "foo" { + count = "${length("abc")}" + lifecycle { create_before_destroy = true } +} diff --git a/terraform/test-fixtures/plan-module-deadlock/main.tf b/terraform/test-fixtures/plan-module-deadlock/main.tf new file mode 100644 index 000000000..1f95749fa --- /dev/null +++ b/terraform/test-fixtures/plan-module-deadlock/main.tf @@ -0,0 +1,3 @@ +module "child" { + source = "./child" +}