From 2b60d0c6b67db1a6fa2d219941e8aa6966c34aa1 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 30 Sep 2015 15:50:21 -0700 Subject: [PATCH 1/3] Add repro test case for bug #2892 --- terraform/context_apply_test.go | 84 +++++++++++++++++++ .../child/main.tf | 5 ++ .../apply-destroy-cross-providers/main.tf | 6 ++ 3 files changed, 95 insertions(+) create mode 100644 terraform/test-fixtures/apply-destroy-cross-providers/child/main.tf create mode 100644 terraform/test-fixtures/apply-destroy-cross-providers/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 4b2113d632..1fd069db08 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -10,6 +10,8 @@ import ( "sync/atomic" "testing" "time" + + "github.com/hashicorp/terraform/config/module" ) func TestContext2Apply(t *testing.T) { @@ -298,6 +300,88 @@ func TestContext2Apply_destroyComputed(t *testing.T) { } } +// https://github.com/hashicorp/terraform/issues/2892 +func TestContext2Apply_destroyCrossProviders(t *testing.T) { + m := testModule(t, "apply-destroy-cross-providers") + + p_aws := testProvider("aws") + p_aws.ApplyFn = testApplyFn + p_aws.DiffFn = testDiffFn + + p_tf := testProvider("terraform") + p_tf.ApplyFn = testApplyFn + p_tf.DiffFn = testDiffFn + + providers := map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p_aws), + "terraform": testProviderFuncFixed(p_tf), + } + + // Bug only appears from time to time, + // so we run this test multiple times + // to check for the race-condition + for i := 0; i <= 10; i++ { + ctx := getContextForApply_destroyCrossProviders( + t, m, providers) + + if p, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } else { + t.Logf(p.String()) + } + + if _, err := ctx.Apply(); err != nil { + t.Fatalf("err: %s", err) + } + } +} + +func getContextForApply_destroyCrossProviders( + t *testing.T, + m *module.Tree, + providers map[string]ResourceProviderFactory) *Context { + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "terraform_remote_state.shared": &ResourceState{ + Type: "terraform_remote_state", + Primary: &InstanceState{ + ID: "remote-2652591293", + Attributes: map[string]string{ + "output.env_name": "test", + }, + }, + }, + }, + }, + &ModuleState{ + Path: []string{"root", "example"}, + Resources: map[string]*ResourceState{ + "aws_vpc.bar": &ResourceState{ + Type: "aws_vpc", + Primary: &InstanceState{ + ID: "vpc-aaabbb12", + Attributes: map[string]string{ + "value": "test", + }, + }, + }, + }, + }, + }, + } + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: providers, + State: state, + Destroy: true, + }) + + return ctx +} + func TestContext2Apply_minimal(t *testing.T) { m := testModule(t, "apply-minimal") p := testProvider("aws") diff --git a/terraform/test-fixtures/apply-destroy-cross-providers/child/main.tf b/terraform/test-fixtures/apply-destroy-cross-providers/child/main.tf new file mode 100644 index 0000000000..048b26dec8 --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-cross-providers/child/main.tf @@ -0,0 +1,5 @@ +variable "value" {} + +resource "aws_vpc" "bar" { + value = "${var.value}" +} diff --git a/terraform/test-fixtures/apply-destroy-cross-providers/main.tf b/terraform/test-fixtures/apply-destroy-cross-providers/main.tf new file mode 100644 index 0000000000..b0595b9e81 --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-cross-providers/main.tf @@ -0,0 +1,6 @@ +resource "terraform_remote_state" "shared" {} + +module "child" { + source = "./child" + value = "${terraform_remote_state.shared.output.env_name}" +} From 40b04154feb5d0a9c4fddeadc52b4d2999c41ae6 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Fri, 2 Oct 2015 11:18:33 -0700 Subject: [PATCH 2/3] Add operation walkDestroy --- terraform/context.go | 6 +++++- terraform/evaltree_provider.go | 4 ++-- terraform/graph_config_node_output.go | 2 +- terraform/graph_walk_operation.go | 1 + terraform/transform_deposed.go | 2 +- terraform/transform_orphan.go | 2 +- terraform/transform_output.go | 4 ++-- terraform/transform_provider.go | 2 +- terraform/transform_resource.go | 4 ++-- terraform/transform_tainted.go | 2 +- terraform/walkoperation_string.go | 4 ++-- 11 files changed, 19 insertions(+), 14 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index be01a492a1..0f567ddf24 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -292,7 +292,11 @@ func (c *Context) Apply() (*State, error) { } // Do the walk - _, err = c.walk(graph, walkApply) + if c.destroy { + _, err = c.walk(graph, walkDestroy) + } else { + _, err = c.walk(graph, walkApply) + } // Clean out any unused things c.state.prune() diff --git a/terraform/evaltree_provider.go b/terraform/evaltree_provider.go index 99e3ccb1e0..9ec6ea0c57 100644 --- a/terraform/evaltree_provider.go +++ b/terraform/evaltree_provider.go @@ -71,7 +71,7 @@ func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { // Apply stuff seq = append(seq, &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply}, + Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalGetProvider{ @@ -98,7 +98,7 @@ func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { // We configure on everything but validate, since validate may // not have access to all the variables. seq = append(seq, &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply}, + Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalConfigProvider{ diff --git a/terraform/graph_config_node_output.go b/terraform/graph_config_node_output.go index 5b2d95fdc7..d4f00451c7 100644 --- a/terraform/graph_config_node_output.go +++ b/terraform/graph_config_node_output.go @@ -44,7 +44,7 @@ func (n *GraphNodeConfigOutput) DependentOn() []string { // GraphNodeEvalable impl. func (n *GraphNodeConfigOutput) EvalTree() EvalNode { return &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply}, + Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalWriteOutput{ diff --git a/terraform/graph_walk_operation.go b/terraform/graph_walk_operation.go index c2143fbd83..f2de24134d 100644 --- a/terraform/graph_walk_operation.go +++ b/terraform/graph_walk_operation.go @@ -13,4 +13,5 @@ const ( walkPlanDestroy walkRefresh walkValidate + walkDestroy ) diff --git a/terraform/transform_deposed.go b/terraform/transform_deposed.go index 6ae1695f0c..fa3143c3ce 100644 --- a/terraform/transform_deposed.go +++ b/terraform/transform_deposed.go @@ -110,7 +110,7 @@ func (n *graphNodeDeposedResource) EvalTree() EvalNode { var diff *InstanceDiff var err error seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkApply}, + Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalGetProvider{ diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index bb381c823d..45ea050ba3 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -263,7 +263,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode { // Apply var err error seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkApply}, + Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalReadDiff{ diff --git a/terraform/transform_output.go b/terraform/transform_output.go index 5ea48a0165..d3e839ce1c 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -62,7 +62,7 @@ func (n *graphNodeOrphanOutput) Name() string { func (n *graphNodeOrphanOutput) EvalTree() EvalNode { return &EvalOpFilter{ - Ops: []walkOperation{walkApply, walkRefresh}, + Ops: []walkOperation{walkApply, walkDestroy, walkRefresh}, Node: &EvalDeleteOutput{ Name: n.OutputName, }, @@ -90,7 +90,7 @@ func (n *graphNodeOrphanOutputFlat) Name() string { func (n *graphNodeOrphanOutputFlat) EvalTree() EvalNode { return &EvalOpFilter{ - Ops: []walkOperation{walkApply, walkRefresh}, + Ops: []walkOperation{walkApply, walkDestroy, walkRefresh}, Node: &EvalDeleteOutput{ Name: n.OutputName, }, diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 8a6655182b..0ea226713c 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -255,7 +255,7 @@ func (n *graphNodeDisabledProvider) EvalTree() EvalNode { var resourceConfig *ResourceConfig return &EvalOpFilter{ - Ops: []walkOperation{walkInput, walkValidate, walkRefresh, walkPlan, walkApply}, + Ops: []walkOperation{walkInput, walkValidate, walkRefresh, walkPlan, walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalInterpolate{ diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 0b56721b07..a52b3ba724 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -369,7 +369,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { var createNew, tainted bool var createBeforeDestroyEnabled bool seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkApply}, + Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ // Get the saved diff for apply @@ -591,7 +591,7 @@ func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode { var state *InstanceState var err error return &EvalOpFilter{ - Ops: []walkOperation{walkApply}, + Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ // Get the saved diff for apply diff --git a/terraform/transform_tainted.go b/terraform/transform_tainted.go index fdc1ae6bc4..37e25df32a 100644 --- a/terraform/transform_tainted.go +++ b/terraform/transform_tainted.go @@ -114,7 +114,7 @@ func (n *graphNodeTaintedResource) EvalTree() EvalNode { // Apply var diff *InstanceDiff seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkApply}, + Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalGetProvider{ diff --git a/terraform/walkoperation_string.go b/terraform/walkoperation_string.go index 423793c3c8..1ce3661c49 100644 --- a/terraform/walkoperation_string.go +++ b/terraform/walkoperation_string.go @@ -4,9 +4,9 @@ package terraform import "fmt" -const _walkOperation_name = "walkInvalidwalkInputwalkApplywalkPlanwalkPlanDestroywalkRefreshwalkValidate" +const _walkOperation_name = "walkInvalidwalkInputwalkApplywalkPlanwalkPlanDestroywalkRefreshwalkValidatewalkDestroy" -var _walkOperation_index = [...]uint8{0, 11, 20, 29, 37, 52, 63, 75} +var _walkOperation_index = [...]uint8{0, 11, 20, 29, 37, 52, 63, 75, 86} func (i walkOperation) String() string { if i >= walkOperation(len(_walkOperation_index)-1) { From 3a05f0155397e5bb854c29a186486e758bf6085c Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Fri, 2 Oct 2015 11:20:09 -0700 Subject: [PATCH 3/3] Treat missing variables during destroy as unknown --- terraform/interpolate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/terraform/interpolate.go b/terraform/interpolate.go index d8e5288ec4..31c366eabc 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -342,7 +342,7 @@ func (i *Interpolater) computeResourceVariable( // TODO: test by creating a state and configuration that is referencing // a non-existent variable "foo.bar" where the state only has "foo" // and verify plan works, but apply doesn't. - if i.Operation == walkApply { + if i.Operation == walkApply || i.Operation == walkDestroy { goto MISSING } @@ -384,7 +384,7 @@ MISSING: // // For an input walk, computed values are okay to return because we're only // looking for missing variables to prompt the user for. - if i.Operation == walkRefresh || i.Operation == walkPlanDestroy || i.Operation == walkInput { + if i.Operation == walkRefresh || i.Operation == walkPlanDestroy || i.Operation == walkDestroy || i.Operation == walkInput { return config.UnknownVariableValue, nil } @@ -481,7 +481,7 @@ func (i *Interpolater) computeResourceMultiVariable( // // For an input walk, computed values are okay to return because we're only // looking for missing variables to prompt the user for. - if i.Operation == walkRefresh || i.Operation == walkPlanDestroy || i.Operation == walkInput { + if i.Operation == walkRefresh || i.Operation == walkPlanDestroy || i.Operation == walkDestroy || i.Operation == walkInput { return config.UnknownVariableValue, nil }