Merge pull request #10659 from hashicorp/b-multi-provider-dep

terraform: destroy resources in dependent providers first

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, `<no 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, `
+<no state>
+module.child:
+  <no state>
+`)
+}
+
 func TestContext2Apply_multiVar(t *testing.T) {
 	m := testModule(t, "apply-multi-var")
 	p := testProvider("aws")

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 },

terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf

@@ -0,0 +1,3 @@
+resource "aws_instance" "bar" {
+    foo = "bar"
+}

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"
+}

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"
+}

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 {

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