diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 6458b82d7..7f14e98c1 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2747,6 +2747,205 @@ func TestContext2Apply_multiProvider(t *testing.T) { } } +func TestContext2Apply_multiProviderDestroy(t *testing.T) { + m := testModule(t, "apply-multi-provider-destroy") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + p2 := testProvider("do") + p2.ApplyFn = testApplyFn + p2.DiffFn = testDiffFn + + var state *State + + // First, create the instances + { + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + "vault": testProviderFuncFixed(p2), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + s, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + state = s + } + + // Destroy them + { + // Verify that aws_instance.bar is destroyed first + var checked bool + var called int32 + var lock sync.Mutex + applyFn := func( + info *InstanceInfo, + is *InstanceState, + id *InstanceDiff) (*InstanceState, error) { + lock.Lock() + defer lock.Unlock() + + if info.HumanId() == "aws_instance.bar" { + checked = true + + // Sleep to allow parallel execution + time.Sleep(50 * time.Millisecond) + + // Verify that called is 0 (dep not called) + if atomic.LoadInt32(&called) != 0 { + return nil, fmt.Errorf("nothing else should be called") + } + } + + atomic.AddInt32(&called, 1) + return testApplyFn(info, is, id) + } + + // Set the apply functions + p.ApplyFn = applyFn + p2.ApplyFn = applyFn + + ctx := testContext2(t, &ContextOpts{ + Destroy: true, + State: state, + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + "vault": testProviderFuncFixed(p2), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + s, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + if !checked { + t.Fatal("should be checked") + } + + state = s + } + + checkStateString(t, state, ``) +} + +// This is like the multiProviderDestroy test except it tests that +// dependent resources within a child module that inherit provider +// configuration are still destroyed first. +func TestContext2Apply_multiProviderDestroyChild(t *testing.T) { + m := testModule(t, "apply-multi-provider-destroy-child") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + p2 := testProvider("do") + p2.ApplyFn = testApplyFn + p2.DiffFn = testDiffFn + + var state *State + + // First, create the instances + { + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + "vault": testProviderFuncFixed(p2), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + s, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + state = s + } + + // Destroy them + { + // Verify that aws_instance.bar is destroyed first + var checked bool + var called int32 + var lock sync.Mutex + applyFn := func( + info *InstanceInfo, + is *InstanceState, + id *InstanceDiff) (*InstanceState, error) { + lock.Lock() + defer lock.Unlock() + + if info.HumanId() == "module.child.aws_instance.bar" { + checked = true + + // Sleep to allow parallel execution + time.Sleep(50 * time.Millisecond) + + // Verify that called is 0 (dep not called) + if atomic.LoadInt32(&called) != 0 { + return nil, fmt.Errorf("nothing else should be called") + } + } + + atomic.AddInt32(&called, 1) + return testApplyFn(info, is, id) + } + + // Set the apply functions + p.ApplyFn = applyFn + p2.ApplyFn = applyFn + + ctx := testContext2(t, &ContextOpts{ + Destroy: true, + State: state, + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + "vault": testProviderFuncFixed(p2), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + s, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + if !checked { + t.Fatal("should be checked") + } + + state = s + } + + checkStateString(t, state, ` + +module.child: + +`) +} + func TestContext2Apply_multiVar(t *testing.T) { m := testModule(t, "apply-multi-var") p := testProvider("aws") diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index e268204c4..75de53757 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -81,13 +81,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Attach the state &AttachStateTransformer{State: b.State}, - // Destruction ordering - &DestroyEdgeTransformer{Module: b.Module, State: b.State}, - GraphTransformIf( - func() bool { return !b.Destroy }, - &CBDEdgeTransformer{Module: b.Module, State: b.State}, - ), - // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Concrete: concreteProvider}, &ProviderTransformer{}, @@ -95,6 +88,13 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &ParentProviderTransformer{}, &AttachProviderConfigTransformer{Module: b.Module}, + // Destruction ordering + &DestroyEdgeTransformer{Module: b.Module, State: b.State}, + GraphTransformIf( + func() bool { return !b.Destroy }, + &CBDEdgeTransformer{Module: b.Module, State: b.State}, + ), + // Provisioner-related transformations GraphTransformIf( func() bool { return !b.Destroy }, diff --git a/terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf b/terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf new file mode 100644 index 000000000..ae1bc8ee4 --- /dev/null +++ b/terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "bar" { + foo = "bar" +} diff --git a/terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf b/terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf new file mode 100644 index 000000000..61e69e816 --- /dev/null +++ b/terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf @@ -0,0 +1,9 @@ +resource "vault_instance" "foo" {} + +provider "aws" { + addr = "${vault_instance.foo.id}" +} + +module "child" { + source = "./child" +} diff --git a/terraform/test-fixtures/apply-multi-provider-destroy/main.tf b/terraform/test-fixtures/apply-multi-provider-destroy/main.tf new file mode 100644 index 000000000..4a315a60e --- /dev/null +++ b/terraform/test-fixtures/apply-multi-provider-destroy/main.tf @@ -0,0 +1,9 @@ +resource "vault_instance" "foo" {} + +provider "aws" { + addr = "${vault_instance.foo.id}" +} + +resource "aws_instance" "bar" { + foo = "bar" +} diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 789586ef0..8c067892c 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -38,8 +38,8 @@ type GraphNodeCreator interface { // example: VPC with subnets, the VPC can't be deleted while there are // still subnets. type DestroyEdgeTransformer struct { - // Module and State are only needed to look up dependencies in - // any way possible. Either can be nil if not availabile. + // These are needed to properly build the graph of dependencies + // to determine what a destroy node depends on. Any of these can be nil. Module *module.Tree State *State } @@ -115,10 +115,22 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // Example: resource A is force new, then destroy A AND create A are // in the graph. BUT if resource A is just pure destroy, then only // destroy A is in the graph, and create A is not. + providerFn := func(a *NodeAbstractProvider) dag.Vertex { + return &NodeApplyableProvider{NodeAbstractProvider: a} + } steps := []GraphTransformer{ + // Add outputs and metadata &OutputTransformer{Module: t.Module}, &AttachResourceConfigTransformer{Module: t.Module}, &AttachStateTransformer{State: t.State}, + + // Add providers since they can affect destroy order as well + &MissingProviderTransformer{AllowAny: true, Concrete: providerFn}, + &ProviderTransformer{}, + &DisableProviderTransformer{}, + &ParentProviderTransformer{}, + &AttachProviderConfigTransformer{Module: t.Module}, + &ReferenceTransformer{}, } @@ -170,9 +182,13 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { refs = append(refs, raw.(dag.Vertex)) } + refNames := make([]string, len(refs)) + for i, ref := range refs { + refNames[i] = dag.VertexName(ref) + } log.Printf( - "[TRACE] DestroyEdgeTransformer: creation node %q references %v", - dag.VertexName(v), refs) + "[TRACE] DestroyEdgeTransformer: creation node %q references %s", + dag.VertexName(v), refNames) // If we have no references, then we won't need to do anything if len(refs) == 0 { diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index e4eba60c4..ac13bb9a5 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -114,6 +114,10 @@ type MissingProviderTransformer struct { // Providers is the list of providers we support. Providers []string + // AllowAny will not check that a provider is supported before adding + // it to the graph. + AllowAny bool + // Concrete, if set, overrides how the providers are made. Concrete ConcreteProviderNodeFunc } @@ -170,10 +174,12 @@ func (t *MissingProviderTransformer) Transform(g *Graph) error { ptype = p[:idx] } - if _, ok := supported[ptype]; !ok { - // If we don't support the provider type, skip it. - // Validation later will catch this as an error. - continue + if !t.AllowAny { + if _, ok := supported[ptype]; !ok { + // If we don't support the provider type, skip it. + // Validation later will catch this as an error. + continue + } } // Add the missing provider node to the graph