diff --git a/terraform/context.go b/terraform/context.go index 7546e9dae..75c922d98 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -929,6 +929,13 @@ func (c *walkContext) planDestroyWalkFn() depgraph.WalkFunc { walkFn = func(n *depgraph.Noun) error { switch m := n.Meta.(type) { case *GraphNodeModule: + // Set the destroy bool on the module + md := result.Diff.ModuleByPath(m.Path) + if md == nil { + md = result.Diff.AddModule(m.Path) + } + md.Destroy = true + // Build another walkContext for this module and walk it. wc := c.Context.walkContext(c.Operation, m.Path) diff --git a/terraform/diff.go b/terraform/diff.go index 3770a1d47..c5e821cdf 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -114,6 +114,7 @@ func (d *Diff) init() { type ModuleDiff struct { Path []string Resources map[string]*InstanceDiff + Destroy bool // Set only by the destroy plan } func (d *ModuleDiff) init() { @@ -192,6 +193,10 @@ func (d *ModuleDiff) IsRoot() bool { func (d *ModuleDiff) String() string { var buf bytes.Buffer + if d.Destroy { + buf.WriteString("DESTROY MODULE\n") + } + names := make([]string, 0, len(d.Resources)) for name, _ := range d.Resources { names = append(names, name) diff --git a/terraform/graph.go b/terraform/graph.go index ff1e32ffb..1adcfbf3a 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -80,6 +80,8 @@ type GraphNodeModule struct { Config *config.Module Path []string Graph *depgraph.Graph + State *ModuleState + Flags ResourceFlag } // GraphNodeResource is a node type in the graph that represents a resource @@ -246,6 +248,9 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { // Add the orphan dependencies graphAddOrphanDeps(g, modState) + // Add the orphan module dependencies + graphAddOrphanModuleDeps(g, modState) + // Add the provider dependencies graphAddResourceProviderDeps(g) @@ -269,7 +274,7 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { // If we have a diff, then make sure to add that in if modDiff != nil { - if err := graphAddDiff(g, modDiff); err != nil { + if err := graphAddDiff(g, opts.Diff, modDiff); err != nil { return nil, err } } @@ -298,45 +303,70 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { // allows orphaned resources to be destroyed in the proper order. func graphEncodeDependencies(g *depgraph.Graph) { for _, n := range g.Nouns { - // Ignore any non-resource nodes - rn, ok := n.Meta.(*GraphNodeResource) - if !ok { - continue - } - r := rn.Resource + switch rn := n.Meta.(type) { + case *GraphNodeResource: + // If we are using create-before-destroy, there + // are some special depedencies injected on the + // deposed node that would cause a circular depedency + // chain if persisted. We must only handle the new node, + // node the deposed node. + r := rn.Resource + if r.Flags&FlagDeposed != 0 { + continue + } - // If we are using create-before-destroy, there - // are some special depedencies injected on the - // deposed node that would cause a circular depedency - // chain if persisted. We must only handle the new node, - // node the deposed node. - if r.Flags&FlagDeposed != 0 { - continue - } + // Update the dependencies + var inject []string + for _, dep := range n.Deps { + switch target := dep.Target.Meta.(type) { + case *GraphNodeModule: + inject = append(inject, dep.Target.Name) - // Update the dependencies - var inject []string - for _, dep := range n.Deps { - switch target := dep.Target.Meta.(type) { - case *GraphNodeModule: - inject = append(inject, dep.Target.Name) + case *GraphNodeResource: + if target.Resource.Id == r.Id { + continue + } + inject = append(inject, target.Resource.Id) - case *GraphNodeResource: - if target.Resource.Id == r.Id { - continue + case *GraphNodeResourceProvider: + // Do nothing + + default: + panic(fmt.Sprintf("Unknown graph node: %#v", dep.Target)) } - inject = append(inject, target.Resource.Id) + } - case *GraphNodeResourceProvider: - // Do nothing + // Update the dependencies + r.Dependencies = inject - default: - panic(fmt.Sprintf("Unknown graph node: %#v", dep.Target)) + case *GraphNodeModule: + // Update the dependencies + var inject []string + for _, dep := range n.Deps { + switch target := dep.Target.Meta.(type) { + case *GraphNodeModule: + if dep.Target.Name == n.Name { + continue + } + inject = append(inject, dep.Target.Name) + + case *GraphNodeResource: + inject = append(inject, target.Resource.Id) + + case *GraphNodeResourceProvider: + // Do nothing + + default: + panic(fmt.Sprintf("Unknown graph node: %#v", dep.Target)) + } + + } + + // Update the dependencies + if rn.State != nil { + rn.State.Dependencies = inject } } - - // Update the dependencies - r.Dependencies = inject } } @@ -357,6 +387,14 @@ func graphAddConfigModules( if n, err := graphModuleNoun(m.Name, m, g, opts); err != nil { return err } else { + // Attach the module state if any + if opts.State != nil { + module := n.Meta.(*GraphNodeModule) + module.State = opts.State.ModuleByPath(module.Path) + if module.State == nil { + module.State = opts.State.AddModule(module.Path) + } + } nounsList = append(nounsList, n) } } @@ -506,10 +544,22 @@ func graphAddConfigResources( // destroying the VPC's subnets first, whereas creating a VPC requires // doing it before the subnets are created. This function handles inserting // these nodes for you. -func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { +func graphAddDiff(g *depgraph.Graph, gDiff *Diff, d *ModuleDiff) error { var nlist []*depgraph.Noun + var modules []*depgraph.Noun injected := make(map[*depgraph.Dependency]struct{}) for _, n := range g.Nouns { + // A module is being destroyed if all it's resources are being + // destroyed (via a destroy plan) or if it is orphaned. Only in + // those cases do we need to handle depedency inversion. + if mod, ok := n.Meta.(*GraphNodeModule); ok { + md := gDiff.ModuleByPath(mod.Path) + if mod.Flags&FlagOrphan != 0 || (md != nil && md.Destroy) { + modules = append(modules, n) + } + continue + } + rn, ok := n.Meta.(*GraphNodeResource) if !ok { continue @@ -655,78 +705,81 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { rn.Resource.Diff = rd } - // Go through each noun and make sure we calculate all the dependencies - // properly. - for _, n := range nlist { - deps := n.Deps - num := len(deps) - for i := 0; i < num; i++ { - dep := deps[i] + // Go through each resource and module and make sure we + // calculate all the dependencies properly. + invertDeps := [][]*depgraph.Noun{nlist, modules} + for _, list := range invertDeps { + for _, n := range list { + deps := n.Deps + num := len(deps) + for i := 0; i < num; i++ { + dep := deps[i] - // Check if this dependency was just injected, otherwise - // we will incorrectly flip the depedency twice. - if _, ok := injected[dep]; ok { - continue - } + // Check if this dependency was just injected, otherwise + // we will incorrectly flip the depedency twice. + if _, ok := injected[dep]; ok { + continue + } - switch target := dep.Target.Meta.(type) { - case *GraphNodeResource: - // If the other node is also being deleted, - // we must be deleted first. E.g. if A -> B, - // then when we create, B is created first then A. - // On teardown, A is destroyed first, then B. - // Thus we must flip our depedency and instead inject - // it on B. - for _, n2 := range nlist { - rn2 := n2.Meta.(*GraphNodeResource) - if target.Resource.Id == rn2.Resource.Id { - newDep := &depgraph.Dependency{ - Name: n.Name, - Source: n2, - Target: n, + switch target := dep.Target.Meta.(type) { + case *GraphNodeResource: + // If the other node is also being deleted, + // we must be deleted first. E.g. if A -> B, + // then when we create, B is created first then A. + // On teardown, A is destroyed first, then B. + // Thus we must flip our depedency and instead inject + // it on B. + for _, n2 := range nlist { + rn2 := n2.Meta.(*GraphNodeResource) + if target.Resource.Id == rn2.Resource.Id { + newDep := &depgraph.Dependency{ + Name: n.Name, + Source: n2, + Target: n, + } + injected[newDep] = struct{}{} + n2.Deps = append(n2.Deps, newDep) + break } - injected[newDep] = struct{}{} - n2.Deps = append(n2.Deps, newDep) - break } + + // Drop the dependency. We may have created + // an inverse depedency if the dependent resource + // is also being deleted, but this dependence is + // no longer required. + deps[i], deps[num-1] = deps[num-1], nil + num-- + i-- + + case *GraphNodeModule: + // We invert any module dependencies so we're destroyed + // first, before any modules are applied. + newDep := &depgraph.Dependency{ + Name: n.Name, + Source: dep.Target, + Target: n, + } + dep.Target.Deps = append(dep.Target.Deps, newDep) + + // Drop the dependency. We may have created + // an inverse depedency if the dependent resource + // is also being deleted, but this dependence is + // no longer required. + deps[i], deps[num-1] = deps[num-1], nil + num-- + i-- + case *GraphNodeResourceProvider: + // Keep these around, but fix up the source to be ourselves + // rather than the old node. + newDep := *dep + newDep.Source = n + deps[i] = &newDep + default: + panic(fmt.Errorf("Unhandled depedency type: %#v", dep.Target.Meta)) } - - // Drop the dependency. We may have created - // an inverse depedency if the dependent resource - // is also being deleted, but this dependence is - // no longer required. - deps[i], deps[num-1] = deps[num-1], nil - num-- - i-- - - case *GraphNodeModule: - // We invert any module dependencies so we're destroyed - // first, before any modules are applied. - newDep := &depgraph.Dependency{ - Name: n.Name, - Source: dep.Target, - Target: n, - } - dep.Target.Deps = append(dep.Target.Deps, newDep) - - // Drop the dependency. We may have created - // an inverse depedency if the dependent resource - // is also being deleted, but this dependence is - // no longer required. - deps[i], deps[num-1] = deps[num-1], nil - num-- - i-- - case *GraphNodeResourceProvider: - // Keep these around, but fix up the source to be ourselves - // rather than the old node. - newDep := *dep - newDep.Source = n - deps[i] = &newDep - default: - panic(fmt.Errorf("Unhandled depedency type: %#v", dep.Target.Meta)) } + n.Deps = deps[:num] } - n.Deps = deps[:num] } // Add the nouns to the graph @@ -857,6 +910,10 @@ func graphAddModuleOrphans( if n, err := graphModuleNoun(k, nil, g, opts); err != nil { return err } else { + // Mark this module as being an orphan + module := n.Meta.(*GraphNodeModule) + module.Flags |= FlagOrphan + module.State = m nounsList = append(nounsList, n) } } @@ -916,6 +973,56 @@ func graphAddOrphanDeps(g *depgraph.Graph, mod *ModuleState) { } } +// graphAddOrphanModuleDeps adds the dependencies to the orphan +// modules based on their explicit Dependencies state. +func graphAddOrphanModuleDeps(g *depgraph.Graph, mod *ModuleState) { + for _, n := range g.Nouns { + module, ok := n.Meta.(*GraphNodeModule) + if !ok { + continue + } + if module.Flags&FlagOrphan == 0 { + continue + } + + // If we have no dependencies, then just continue + if len(module.State.Dependencies) == 0 { + continue + } + + for _, n2 := range g.Nouns { + // Don't ever depend on ourselves + if n2.Meta == n.Meta { + continue + } + + var compareName string + switch rn2 := n2.Meta.(type) { + case *GraphNodeModule: + compareName = n2.Name + case *GraphNodeResource: + compareName = rn2.Resource.Id + } + if compareName == "" { + continue + } + + for _, depName := range module.State.Dependencies { + if !strings.HasPrefix(depName, compareName) { + continue + } + dep := &depgraph.Dependency{ + Name: depName, + Source: n, + Target: n2, + } + n.Deps = append(n.Deps, dep) + break + } + } + } +} + // graphAddOrphans adds the orphans to the graph. func graphAddOrphans(g *depgraph.Graph, c *config.Config, mod *ModuleState) { meta := g.Meta.(*GraphMeta) diff --git a/terraform/graph_test.go b/terraform/graph_test.go index a9b8194e5..4f87e245b 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -731,6 +731,61 @@ func TestGraphAddDiff_module(t *testing.T) { } } +func TestGraphAddDiff_module_depends(t *testing.T) { + m := testModule(t, "graph-diff-module-dep") + diff := &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: rootModulePath, + Resources: map[string]*InstanceDiff{ + "aws_instance.foo": &InstanceDiff{ + Destroy: true, + }, + }, + }, + &ModuleDiff{ + Path: []string{"root", "child"}, + Destroy: true, + Resources: map[string]*InstanceDiff{ + "aws_instance.foo": &InstanceDiff{ + Destroy: true, + }, + }, + }, + }, + } + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: []string{"root", "orphan"}, + Resources: map[string]*ResourceState{ + "aws_instance.dead": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "dead", + }, + }, + }, + Dependencies: []string{ + "aws_instance.foo", + "module.child", + }, + }, + }, + } + + g, err := Graph(&GraphOpts{Module: m, Diff: diff, State: state}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphDiffModuleDependsStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { m := testModule(t, "graph-diff-create-before") diff := &Diff{ @@ -909,7 +964,7 @@ func TestGraphEncodeDependencies_count(t *testing.T) { func TestGraphEncodeDependencies_module(t *testing.T) { m := testModule(t, "graph-modules") - g, err := Graph(&GraphOpts{Module: m}) + g, err := Graph(&GraphOpts{Module: m, State: &State{}}) if err != nil { t.Fatalf("err: %s", err) } @@ -928,6 +983,15 @@ func TestGraphEncodeDependencies_module(t *testing.T) { if web.Dependencies[1] != "module.consul" { t.Fatalf("bad: %#v", web) } + + mod := g.Noun("module.consul").Meta.(*GraphNodeModule) + deps := mod.State.Dependencies + if len(deps) != 1 { + t.Fatalf("Bad: %#v", deps) + } + if deps[0] != "aws_security_group.firewall" { + t.Fatalf("Bad: %#v", deps) + } } func TestGraph_orphan_dependencies(t *testing.T) { @@ -1012,6 +1076,57 @@ func TestGraph_orphanDependenciesModules(t *testing.T) { } } +func TestGraph_orphanModules_Dependencies(t *testing.T) { + m := testModule(t, "graph-modules") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + Dependencies: []string{ + "module.consul", + }, + }, + }, + }, + + // Add an orphan module + &ModuleState{ + Path: []string{"root", "orphan"}, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + }, + }, + }, + Dependencies: []string{ + "aws_instance.foo", + "aws_instance.web", + }, + }, + }, + } + + g, err := Graph(&GraphOpts{Module: m, State: state}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphOrphanedModuleDepsStr) + if actual != expected { + t.Fatalf("bad:\n\nactual:\n%s\n\nexpected:\n%s", actual, expected) + } +} + func TestGraphNodeResourceExpand(t *testing.T) { m := testModule(t, "graph-resource-expand") @@ -1214,6 +1329,22 @@ root root -> module.child ` +const testTerraformGraphDiffModuleDependsStr = ` +root: root +aws_instance.foo + aws_instance.foo -> aws_instance.foo (destroy) +aws_instance.foo (destroy) + aws_instance.foo (destroy) -> module.child + aws_instance.foo (destroy) -> module.orphan +module.child + module.child -> module.orphan +module.orphan +root + root -> aws_instance.foo + root -> module.child + root -> module.orphan +` + const testTerraformGraphModulesStr = ` root: root aws_instance.web @@ -1382,6 +1513,33 @@ root root -> module.consul ` +const testTerraformGraphOrphanedModuleDepsStr = ` +root: root +aws_instance.foo + aws_instance.foo -> module.consul + aws_instance.foo -> provider.aws +aws_instance.web + aws_instance.web -> aws_security_group.firewall + aws_instance.web -> module.consul + aws_instance.web -> provider.aws +aws_security_group.firewall + aws_security_group.firewall -> provider.aws +module.consul + module.consul -> aws_security_group.firewall + module.consul -> provider.aws +module.orphan + module.orphan -> aws_instance.foo + module.orphan -> aws_instance.web + module.orphan -> provider.aws +provider.aws +root + root -> aws_instance.foo + root -> aws_instance.web + root -> aws_security_group.firewall + root -> module.consul + root -> module.orphan +` + const testTerraformGraphResourceExpandStr = ` root: root aws_instance.web.0 diff --git a/terraform/state.go b/terraform/state.go index f13ecf109..529f33983 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -185,6 +185,20 @@ type ModuleState struct { // N instances underneath, although a user only needs to think // about the 1:1 case. Resources map[string]*ResourceState `json:"resources"` + + // Dependencies are a list of things that this module relies on + // existing to remain intact. For example: an module may depend + // on a VPC ID given by an aws_vpc resource. + // + // Terraform uses this information to build valid destruction + // orders and to warn the user if they're destroying a module that + // another resource depends on. + // + // Things can be put into this list that may not be managed by + // Terraform. If Terraform doesn't find a matching ID in the + // overall state, then it assumes it isn't managed and doesn't + // worry about it. + Dependencies []string `json:"depends_on,omitempty"` } // IsRoot says whether or not this module diff is for the root module. diff --git a/terraform/state_test.go b/terraform/state_test.go index 9ee251745..57e1308c1 100644 --- a/terraform/state_test.go +++ b/terraform/state_test.go @@ -180,6 +180,9 @@ func TestReadWriteState(t *testing.T) { Modules: []*ModuleState{ &ModuleState{ Path: rootModulePath, + Dependencies: []string{ + "aws_instance.bar", + }, Resources: map[string]*ResourceState{ "foo": &ResourceState{ Primary: &InstanceState{ diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 72568d267..e9bc6ca7f 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -762,6 +762,7 @@ DIFF: DESTROY: aws_instance.foo module.child: + DESTROY MODULE DESTROY: aws_instance.foo STATE: diff --git a/terraform/test-fixtures/graph-diff-module-dep/child/main.tf b/terraform/test-fixtures/graph-diff-module-dep/child/main.tf new file mode 100644 index 000000000..84d1de905 --- /dev/null +++ b/terraform/test-fixtures/graph-diff-module-dep/child/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "foo" {} + +output "bar" { + value = "baz" +} diff --git a/terraform/test-fixtures/graph-diff-module-dep/main.tf b/terraform/test-fixtures/graph-diff-module-dep/main.tf new file mode 100644 index 000000000..2f61386b2 --- /dev/null +++ b/terraform/test-fixtures/graph-diff-module-dep/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" {} + +module "child" { + source = "./child" + in = "${aws_instance.foo.id}" +} + +