Implement dag.GraphNodeDotter (temporarily)

To maintain the same output, the Graph.Dot implementation needs to be
aware of GraphNodeDotter. Copy the interface into the dag package, and
make the Dot marshaler aware of which nodes implemented the interface.
This way we can remove most of the remaining dot code from terraform.

dag/dot.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+
+	"github.com/hashicorp/terraform/dot"
 )
 
 // DotOpts are the options for generating a dot formatted Graph.
@@ -23,6 +25,18 @@ type DotOpts struct {
 	cluster bool
 }
 
+// GraphNodeDotter can be implemented by a node to cause it to be included
+// in the dot graph. The Dot method will be called which is expected to
+// return a representation of this node.
+// TODO remove the dot package dependency
+type GraphNodeDotter interface {
+	// Dot is called to return the dot formatting for the node.
+	// The first parameter is the title of the node.
+	// The second parameter includes user-specified options that affect the dot
+	// graph. See GraphDotOpts below for details.
+	DotNode(string, *DotOpts) *dot.Node
+}
+
 // Returns the DOT representation of this Graph.
 func (g *marshalGraph) Dot(opts *DotOpts) []byte {
 	if opts == nil {
@@ -123,7 +137,15 @@ func (g *marshalGraph) writeBody(opts *DotOpts, w *indentWriter) {
 		w.WriteString(as + "\n")
 	}
 
+	// list of Vertices that aren't to be included in the dot output
+	skip := map[string]bool{}
+
 	for _, v := range g.Vertices {
+		if !v.graphNodeDotter {
+			skip[v.ID] = true
+			continue
+		}
+
 		w.Write(v.dot(g))
 	}
 
@@ -141,6 +163,11 @@ func (g *marshalGraph) writeBody(opts *DotOpts, w *indentWriter) {
 				}
 				src := c[i]
 				tgt := c[j]
+
+				if skip[src.ID] || skip[tgt.ID] {
+					continue
+				}
+
 				e := &marshalEdge{
 					Name:   fmt.Sprintf("%s|%s", src.Name, tgt.Name),
 					Source: src.ID,

dag/marshal.go

@@ -9,13 +9,28 @@ import (
 
 // the marshal* structs are for serialization of the graph data.
 type marshalGraph struct {
-	ID        string             `json:",omitempty"`
-	Name      string             `json:",omitempty"`
-	Attrs     map[string]string  `json:",omitempty"`
-	Vertices  []*marshalVertex   `json:",omitempty"`
-	Edges     []*marshalEdge     `json:",omitempty"`
-	Subgraphs []*marshalGraph    `json:",omitempty"`
-	Cycles    [][]*marshalVertex `json:",omitempty"`
+	// Each marshal structure require a unique ID so that it can be references
+	// by other structures.
+	ID string `json:",omitempty"`
+
+	// Human readable name for this graph.
+	Name string `json:",omitempty"`
+
+	// Arbitrary attributes that can be added to the output.
+	Attrs map[string]string `json:",omitempty"`
+
+	// List of graph vertices, sorted by ID.
+	Vertices []*marshalVertex `json:",omitempty"`
+
+	// List of edges, sorted by Source ID.
+	Edges []*marshalEdge `json:",omitempty"`
+
+	// Any number of subgraphs. A subgraph itself is considered a vertex, and
+	// may be referenced by either end of an edge.
+	Subgraphs []*marshalGraph `json:",omitempty"`
+
+	// Any lists of vertices that are included in cycles.
+	Cycles [][]*marshalVertex `json:",omitempty"`
 }
 
 func (g *marshalGraph) vertexByID(id string) *marshalVertex {
@@ -28,11 +43,21 @@ func (g *marshalGraph) vertexByID(id string) *marshalVertex {
 }
 
 type marshalVertex struct {
-	ID    string
-	Name  string            `json:",omitempty"`
+	// Unique ID, used to reference this vertex from other structures.
+	ID string
+
+	// Human readable name
+	Name string `json:",omitempty"`
+
 	Attrs map[string]string `json:",omitempty"`
+
+	// This is to help transition from the old Dot interfaces. We record if the
+	// node was a GraphNodeDotter here, so know if it should be included in the
+	// dot output
+	graphNodeDotter bool
 }
 
+// vertices is a sort.Interface implementation for sorting vertices by ID
 type vertices []*marshalVertex
 
 func (v vertices) Less(i, j int) bool { return v[i].Name < v[j].Name }
@@ -40,18 +65,24 @@ func (v vertices) Len() int           { return len(v) }
 func (v vertices) Swap(i, j int)      { v[i], v[j] = v[j], v[i] }
 
 type marshalEdge struct {
-	Name   string
+	// Human readable name
+	Name string
+
+	// Source and Target Vertices by ID
 	Source string
 	Target string
-	Attrs  map[string]string `json:",omitempty"`
+
+	Attrs map[string]string `json:",omitempty"`
 }
 
+// edges is a sort.Interface implementation for sorting edges by Source ID
 type edges []*marshalEdge
 
 func (e edges) Less(i, j int) bool { return e[i].Name < e[j].Name }
 func (e edges) Len() int           { return len(e) }
 func (e edges) Swap(i, j int)      { e[i], e[j] = e[j], e[i] }
 
+// build a marshalGraph structure from a *Graph
 func newMarshalGraph(name string, g *Graph) *marshalGraph {
 	dg := &marshalGraph{
 		Name:  name,
@@ -59,6 +90,16 @@ func newMarshalGraph(name string, g *Graph) *marshalGraph {
 	}
 
 	for _, v := range g.Vertices() {
+		// We only care about nodes that yield non-empty Dot strings.
+		dn, isDotter := v.(GraphNodeDotter)
+		dotOpts := &DotOpts{
+			Verbose:    true,
+			DrawCycles: true,
+		}
+		if isDotter && dn.DotNode("fake", dotOpts) == nil {
+			isDotter = false
+		}
+
 		id := marshalVertexID(v)
 		if sg, ok := marshalSubgrapher(v); ok {
 
@@ -68,9 +109,10 @@ func newMarshalGraph(name string, g *Graph) *marshalGraph {
 		}
 
 		dv := &marshalVertex{
-			ID:    id,
-			Name:  VertexName(v),
-			Attrs: make(map[string]string),
+			ID:              id,
+			Name:            VertexName(v),
+			Attrs:           make(map[string]string),
+			graphNodeDotter: isDotter,
 		}
 
 		dg.Vertices = append(dg.Vertices, dv)
@@ -124,9 +166,11 @@ func marshalVertexID(v Vertex) string {
 		}
 	}
 
+	// fallback to a name, which we hope is unique.
+	return VertexName(v)
+
 	// we could try harder by attempting to read the arbitrary value from the
 	// interface, but we shouldn't get here from terraform right now.
-	panic("unhashable value in graph")
 }
 
 // check for a Subgrapher, and return the underlying *Graph.

terraform/graph_debug.go

@@ -7,7 +7,7 @@ import (
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/hashicorp/terraform/dag"
-	"github.com/hashicorp/terraform/dot"
+	"github.com/mitchellh/copystructure"
 )
 
 // The NodeDebug method outputs debug information to annotate the graphs
@@ -20,14 +20,14 @@ type GraphNodeDebugOrigin interface {
 	DotOrigin() bool
 }
 type DebugGraph struct {
-	// TODO: can we combine this and dot.Graph into a generalized graph representation?
 	sync.Mutex
 	Name string
 
 	ord int
 	buf bytes.Buffer
 
-	Dot     *dot.Graph
+	Graph *Graph
+
 	dotOpts *dag.DotOpts
 }
 
@@ -38,14 +38,11 @@ type DebugGraph struct {
 func NewDebugGraph(name string, g *Graph, opts *dag.DotOpts) (*DebugGraph, error) {
 	dg := &DebugGraph{
 		Name:    name,
+		Graph:   g,
 		dotOpts: opts,
 	}
 
-	err := dg.build(g)
-	if err != nil {
-		dbug.WriteFile(dg.Name, []byte(err.Error()))
-		return nil, err
-	}
+	dbug.WriteFile(dg.Name, g.Dot(opts))
 	return dg, nil
 }
 
@@ -84,7 +81,7 @@ func (dg *DebugGraph) DotBytes() []byte {
 	}
 	dg.Lock()
 	defer dg.Unlock()
-	return dg.Dot.Bytes()
+	return dg.Graph.Dot(dg.dotOpts)
 }
 
 func (dg *DebugGraph) DebugNode(v interface{}) {
@@ -98,198 +95,17 @@ func (dg *DebugGraph) DebugNode(v interface{}) {
 	ord := dg.ord
 	dg.ord++
 
-	name := graphDotNodeName("root", v)
-
-	var node *dot.Node
-	// TODO: recursive
-	for _, sg := range dg.Dot.Subgraphs {
-		node, _ = sg.GetNode(name)
-		if node != nil {
-			break
-		}
-	}
+	name := dag.VertexName(v)
+	vCopy, _ := copystructure.Config{Lock: true}.Copy(v)
 
 	// record as much of the node data structure as we can
-	spew.Fdump(&dg.buf, v)
+	spew.Fdump(&dg.buf, vCopy)
 
-	// for now, record the order of visits in the node label
-	if node != nil {
-		node.Attrs["label"] = fmt.Sprintf("%s %d", node.Attrs["label"], ord)
-	}
+	dg.buf.WriteString(fmt.Sprintf("%d visited %s\n", ord, name))
 
 	// if the node provides debug output, insert it into the graph, and log it
 	if nd, ok := v.(GraphNodeDebugger); ok {
 		out := nd.NodeDebug()
-		if node != nil {
-			node.Attrs["comment"] = out
-			dg.buf.WriteString(fmt.Sprintf("NodeDebug (%s):'%s'\n", name, out))
-		}
+		dg.buf.WriteString(fmt.Sprintf("NodeDebug (%s):'%s'\n", name, out))
 	}
 }
-
-//  takes a Terraform Graph and build the internal debug graph
-func (dg *DebugGraph) build(g *Graph) error {
-	if dg == nil {
-		return nil
-	}
-	dg.Lock()
-	defer dg.Unlock()
-
-	dg.Dot = dot.NewGraph(map[string]string{
-		"compound": "true",
-		"newrank":  "true",
-	})
-	dg.Dot.Directed = true
-
-	if dg.dotOpts == nil {
-		dg.dotOpts = &dag.DotOpts{
-			DrawCycles: true,
-			MaxDepth:   -1,
-			Verbose:    true,
-		}
-	}
-
-	err := dg.buildSubgraph("root", g, 0)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func (dg *DebugGraph) buildSubgraph(modName string, g *Graph, modDepth int) error {
-	// Respect user-specified module depth
-	if dg.dotOpts.MaxDepth >= 0 && modDepth > dg.dotOpts.MaxDepth {
-		return nil
-	}
-
-	// Begin module subgraph
-	var sg *dot.Subgraph
-	if modDepth == 0 {
-		sg = dg.Dot.AddSubgraph(modName)
-	} else {
-		sg = dg.Dot.AddSubgraph(modName)
-		sg.Cluster = true
-		sg.AddAttr("label", modName)
-	}
-
-	origins, err := graphDotFindOrigins(g)
-	if err != nil {
-		return err
-	}
-
-	drawableVertices := make(map[dag.Vertex]struct{})
-	toDraw := make([]dag.Vertex, 0, len(g.Vertices()))
-	subgraphVertices := make(map[dag.Vertex]*Graph)
-
-	for _, v := range g.Vertices() {
-		if sn, ok := v.(GraphNodeSubgraph); ok {
-			subgraphVertices[v] = sn.Subgraph().(*Graph)
-		}
-	}
-
-	walk := func(v dag.Vertex, depth int) error {
-		// We only care about nodes that yield non-empty Dot strings.
-		if dn, ok := v.(GraphNodeDotter); !ok {
-			return nil
-		} else if dn.DotNode("fake", dg.dotOpts) == nil {
-			return nil
-		}
-
-		drawableVertices[v] = struct{}{}
-		toDraw = append(toDraw, v)
-
-		if sn, ok := v.(GraphNodeSubgraph); ok {
-			subgraphVertices[v] = sn.Subgraph().(*Graph)
-		}
-		return nil
-	}
-
-	if err := g.ReverseDepthFirstWalk(origins, walk); err != nil {
-		return err
-	}
-
-	for _, v := range toDraw {
-		dn := v.(GraphNodeDotter)
-		nodeName := graphDotNodeName(modName, v)
-		sg.AddNode(dn.DotNode(nodeName, dg.dotOpts))
-
-		// Draw all the edges from this vertex to other nodes
-		targets := dag.AsVertexList(g.DownEdges(v))
-		for _, t := range targets {
-			target := t.(dag.Vertex)
-			// Only want edges where both sides are drawable.
-			if _, ok := drawableVertices[target]; !ok {
-				continue
-			}
-
-			if err := sg.AddEdgeBetween(
-				graphDotNodeName(modName, v),
-				graphDotNodeName(modName, target),
-				map[string]string{}); err != nil {
-				return err
-			}
-		}
-	}
-
-	// Recurse into any subgraphs
-	for _, v := range toDraw {
-		subgraph, ok := subgraphVertices[v]
-		if !ok {
-			continue
-		}
-
-		err := dg.buildSubgraph(dag.VertexName(v), subgraph, modDepth+1)
-		if err != nil {
-			return err
-		}
-	}
-
-	if dg.dotOpts.DrawCycles {
-		colors := []string{"red", "green", "blue"}
-		for ci, cycle := range g.Cycles() {
-			for i, c := range cycle {
-				// Catch the last wrapping edge of the cycle
-				if i+1 >= len(cycle) {
-					i = -1
-				}
-				edgeAttrs := map[string]string{
-					"color":    colors[ci%len(colors)],
-					"penwidth": "2.0",
-				}
-
-				if err := sg.AddEdgeBetween(
-					graphDotNodeName(modName, c),
-					graphDotNodeName(modName, cycle[i+1]),
-					edgeAttrs); err != nil {
-					return err
-				}
-
-			}
-		}
-	}
-
-	return nil
-}
-
-func graphDotNodeName(modName, v dag.Vertex) string {
-	return fmt.Sprintf("[%s] %s", modName, dag.VertexName(v))
-}
-
-func graphDotFindOrigins(g *Graph) ([]dag.Vertex, error) {
-	var origin []dag.Vertex
-
-	for _, v := range g.Vertices() {
-		if dr, ok := v.(GraphNodeDebugOrigin); ok {
-			if dr.DotOrigin() {
-				origin = append(origin, v)
-			}
-		}
-	}
-
-	if len(origin) == 0 {
-		return nil, fmt.Errorf("No DOT origin nodes found.\nGraph: %s", g.String())
-	}
-
-	return origin, nil
-}

terraform/graph_dot.go

@@ -19,9 +19,5 @@ type GraphNodeDotter interface {
 // GraphDot returns the dot formatting of a visual representation of
 // the given Terraform graph.
 func GraphDot(g *Graph, opts *dag.DotOpts) (string, error) {
-	dg, err := NewDebugGraph("root", g, opts)
-	if err != nil {
-		return "", err
-	}
-	return dg.Dot.String(), nil
+	return string(g.Dot(opts)), nil
 }