diff --git a/terraform/context.go b/terraform/context.go index 6336b2bcd..ddd4966ff 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1259,35 +1259,14 @@ func (c *walkContext) genericWalkResource( rc.interpolate(c, rn.Resource) // Expand the node to the actual resources - ns, err := rn.Expand() + g, err := rn.Expand() if err != nil { return err } - // Go through all the nouns and run them in parallel, collecting - // any errors. - var l sync.Mutex - var wg sync.WaitGroup - errs := make([]error, 0, len(ns)) - for _, n := range ns { - wg.Add(1) - - go func(n *depgraph.Noun) { - defer wg.Done() - if err := fn(n); err != nil { - l.Lock() - defer l.Unlock() - errs = append(errs, err) - } - }(n) - } - - // Wait for the subgraph - wg.Wait() - - // If there are errors, then we should return them - if len(errs) > 0 { - return &multierror.Error{Errors: errs} + // Walk the graph with our function + if err := g.Walk(fn); err != nil { + return err } return nil diff --git a/terraform/graph.go b/terraform/graph.go index e94ffb098..d7da7d098 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -1016,12 +1016,7 @@ func graphAddConfigProviderConfigs(g *depgraph.Graph, c *config.Config) { func graphAddRoot(g *depgraph.Graph) { root := &depgraph.Noun{Name: GraphRootNode} for _, n := range g.Nouns { - switch m := n.Meta.(type) { - case *GraphNodeResource: - // If the resource is part of a group, we don't need to make a dep - if m.Index != -1 { - continue - } + switch n.Meta.(type) { case *GraphNodeResourceProvider: // ResourceProviders don't need to be in the root deps because // they're always pointed to by some resource. @@ -1554,6 +1549,27 @@ func graphMapResourceProvisioners(g *depgraph.Graph, return nil } +// grpahRemoveInvalidDeps goes through the graph and removes dependencies +// that no longer exist. +func graphRemoveInvalidDeps(g *depgraph.Graph) { + check := make(map[*depgraph.Noun]struct{}) + for _, n := range g.Nouns { + check[n] = struct{}{} + } + for _, n := range g.Nouns { + deps := n.Deps + num := len(deps) + for i := 0; i < num; i++ { + if _, ok := check[deps[i].Target]; !ok { + deps[i], deps[num-1] = deps[num-1], nil + i-- + num-- + } + } + n.Deps = deps[:num] + } +} + // MergeConfig merges all the configurations in the proper order // to result in the final configuration to use to configure this // provider. @@ -1599,7 +1615,7 @@ func (p *graphSharedProvider) MergeConfig( } // Expand will expand this node into a subgraph if Expand is set. -func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { +func (n *GraphNodeResource) Expand() (*depgraph.Graph, error) { // Expand the count out, which should be interpolated at this point. count, err := n.Config.Count() if err != nil { @@ -1627,7 +1643,7 @@ func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { // If we're just expanding the apply, then filter those out and // return them now. if n.ExpandMode == ResourceExpandApply { - return n.finalizeNouns(g, false), nil + return n.finalizeGraph(g, false) } if n.State != nil { @@ -1637,7 +1653,7 @@ func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { graphAddTainted(g, n.State) } - return n.finalizeNouns(g, true), nil + return n.finalizeGraph(g, true) } // expand expands this resource and adds the resources to the graph. It @@ -1759,8 +1775,8 @@ func (n *GraphNodeResource) copyResource(id string) *Resource { return &resource } -func (n *GraphNodeResource) finalizeNouns( - g *depgraph.Graph, destroy bool) []*depgraph.Noun { +func (n *GraphNodeResource) finalizeGraph( + g *depgraph.Graph, destroy bool) (*depgraph.Graph, error) { result := make([]*depgraph.Noun, 0, len(g.Nouns)) for _, n := range g.Nouns { rn, ok := n.Meta.(*GraphNodeResource) @@ -1802,7 +1818,22 @@ func (n *GraphNodeResource) finalizeNouns( result = append(result, n) } } - return result + + // Set the nouns to be only those we care about + g.Nouns = result + + // Remove the dependencies that don't exist + graphRemoveInvalidDeps(g) + + // Build the root so that we have a single valid root + graphAddRoot(g) + + // Validate + if err := g.Validate(); err != nil { + return nil, err + } + + return g, nil } // matchingPrefixes takes a resource type and a set of resource diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 17ced74d2..237c5a73f 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -980,6 +980,98 @@ func TestGraph_orphanDependenciesModules(t *testing.T) { } } +func TestGraphNodeResourceExpand(t *testing.T) { + m := testModule(t, "graph-resource-expand") + + g, err := Graph(&GraphOpts{Module: m}) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Get the resource we care about expanding + n := g.Noun("aws_instance.web") + if n == nil { + t.Fatal("could not find") + } + rn := n.Meta.(*GraphNodeResource) + + g, err = rn.Expand() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphResourceExpandStr) + if actual != expected { + t.Fatalf("bad:\n\nactual:\n%s\n\nexpected:\n%s", actual, expected) + } +} + +/* +func TestGraphNodeResourceExpand_tainted(t *testing.T) { + m := testModule(t, "graph-resource-expand") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + + Resources: map[string]*ResourceState{ + "aws_instance.web.0": &ResourceState{ + Type: "aws_instance", + Tainted: []*InstanceState{ + &InstanceState{ + ID: "bar", + }, + }, + }, + }, + }, + }, + } + + g, err := Graph(&GraphOpts{Module: m, State: state}) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Get the resource we care about expanding + n := g.Noun("aws_instance.web") + if n == nil { + t.Fatal("could not find") + } + rn := n.Meta.(*GraphNodeResource) + + g, err = rn.Expand() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphResourceExpandTaintStr) + if actual != expected { + t.Fatalf("bad:\n\nactual:\n%s\n\nexpected:\n%s", actual, expected) + } + + // Check the destruction noun + n = g.Noun("aws_instance.web (destroy)") + if n == nil { + t.Fatal("could not find") + } + rn = n.Meta.(*GraphNodeResource) + + g, err = rn.Expand() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual = strings.TrimSpace(g.String()) + expected = strings.TrimSpace(testTerraformGraphResourceExpandStr) + if actual != expected { + t.Fatalf("bad:\n\nactual:\n%s\n\nexpected:\n%s", actual, expected) + } +} +*/ + const testTerraformGraphStr = ` root: root aws_instance.web @@ -1277,3 +1369,25 @@ root root -> aws_security_group.firewall root -> module.consul ` + +const testTerraformGraphResourceExpandStr = ` +root: root +aws_instance.web.0 +aws_instance.web.1 +aws_instance.web.2 +root + root -> aws_instance.web.0 + root -> aws_instance.web.1 + root -> aws_instance.web.2 +` + +const testTerraformGraphResourceExpandTaintStr = ` +root: root +aws_instance.web.0 +aws_instance.web.1 +aws_instance.web.2 +root + root -> aws_instance.web.0 + root -> aws_instance.web.1 + root -> aws_instance.web.2 +` diff --git a/terraform/test-fixtures/graph-resource-expand/main.tf b/terraform/test-fixtures/graph-resource-expand/main.tf new file mode 100644 index 000000000..b00b04eff --- /dev/null +++ b/terraform/test-fixtures/graph-resource-expand/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "web" { + count = 3 +}