terraform: plan with modules work

terraform/context.go

@@ -134,9 +134,6 @@ func (c *Context) Apply() (*State, error) {
 	}
 	c.state.init()
 
-	// Initialize the state with all the resources
-	graphInitState(c.state, g)
-
 	// Walk
 	log.Printf("[INFO] Apply walk starting")
 	wc := c.walkContext(rootModulePath)
@@ -217,9 +214,6 @@ func (c *Context) Plan(opts *PlanOpts) (*Plan, error) {
 			c.state = old
 		}()
 
-		// Initialize the state with all the resources
-		graphInitState(c.state, g)
-
 		walkFn = c.walkContext(rootModulePath).planWalkFn(p)
 	}
 
@@ -255,11 +249,6 @@ func (c *Context) Refresh() (*State, error) {
 		return c.state, err
 	}
 
-	if c.state != nil {
-		// Initialize the state with all the resources
-		graphInitState(c.state, g)
-	}
-
 	// Walk the graph
 	err = g.Walk(c.walkContext(rootModulePath).refreshWalkFn())
 
@@ -723,11 +712,15 @@ func (c *walkContext) planWalkFn(result *Plan) depgraph.WalkFunc {
 			}
 		}
 
-		l.Lock()
 		if !diff.Empty() {
-			result.Diff.RootModule().Resources[r.Id] = diff
+			l.Lock()
+			md := result.Diff.ModuleByPath(c.Path)
+			if md == nil {
+				md = result.Diff.AddModule(c.Path)
+			}
+			md.Resources[r.Id] = diff
+			l.Unlock()
 		}
-		l.Unlock()
 
 		for _, h := range c.Context.hooks {
 			handleHook(h.PostDiff(r.Id, diff))
@@ -957,9 +950,14 @@ func (c *walkContext) persistState(r *Resource) {
 	// exist because we call graphInitState before anything that could
 	// potentially call this.
 	module := c.Context.state.ModuleByPath(c.Path)
+	if module == nil {
+		module = c.Context.state.AddModule(c.Path)
+	}
 	rs := module.Resources[r.Id]
 	if rs == nil {
-		panic(fmt.Sprintf("nil ResourceState for ID: %s", r.Id))
+		rs = &ResourceState{Type: r.Info.Type}
+		rs.init()
+		module.Resources[r.Id] = rs
 	}
 
 	// Assign the instance state to the proper location

terraform/context_test.go

@@ -1428,6 +1428,29 @@ func TestContextPlan_minimal(t *testing.T) {
 	}
 }
 
+func TestContextPlan_modules(t *testing.T) {
+	m := testModule(t, "plan-modules")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	plan, err := ctx.Plan(nil)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(testTerraformPlanModulesStr)
+	if actual != expected {
+		t.Fatalf("bad:\n%s", actual)
+	}
+}
+
 func TestContextPlan_nil(t *testing.T) {
 	m := testModule(t, "plan-nil")
 	p := testProvider("aws")

terraform/diff.go

@@ -16,6 +16,17 @@ type Diff struct {
 	Modules []*ModuleDiff
 }
 
+// AddModule adds the module with the given path to the diff.
+//
+// This should be the preferred method to add module diffs since it
+// allows us to optimize lookups later as well as control sorting.
+func (d *Diff) AddModule(path []string) *ModuleDiff {
+	m := &ModuleDiff{Path: path}
+	m.init()
+	d.Modules = append(d.Modules, m)
+	return m
+}
+
 // ModuleByPath is used to lookup the module diff for the given path.
 // This should be the prefered lookup mechanism as it allows for future
 // lookup optimizations.

terraform/graph.go

@@ -41,6 +41,10 @@ type GraphOpts struct {
 	// State, if present, will make the ResourceState available on each
 	// resource node. Additionally, any orphans will be added automatically
 	// to the graph.
+	//
+	// Note: the state will be modified so it is initialized with basic
+	// empty states for all modules/resources in this graph. If you call prune
+	// later, these will be removed, but the graph adds important metadata.
 	State *State
 
 	// Providers is a mapping of prefixes to a resource provider. If given,
@@ -75,6 +79,7 @@ type GraphNodeModule struct {
 type GraphNodeResource struct {
 	Index              int
 	Config             *config.Resource
+	Dependencies       []string
 	Resource           *Resource
 	ResourceProviderID string
 }
@@ -208,6 +213,9 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) {
 		}
 	}
 
+	// Encode the dependencies
+	graphEncodeDependencies(g)
+
 	// Validate
 	if err := g.Validate(); err != nil {
 		return nil, err
@@ -221,16 +229,13 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) {
 	return g, nil
 }
 
-// graphInitState is used to initialize a State with a ResourceState
+// graphEncodeDependencies is used to initialize a State with a ResourceState
 // for every resource.
 //
 // This method is very important to call because it will properly setup
 // the ResourceState dependency information with data from the graph. This
 // allows orphaned resources to be destroyed in the proper order.
-func graphInitState(s *State, g *depgraph.Graph) {
+func graphEncodeDependencies(g *depgraph.Graph) {
-	// TODO: other modules
-	mod := s.RootModule()
-
 	for _, n := range g.Nouns {
 		// Ignore any non-resource nodes
 		rn, ok := n.Meta.(*GraphNodeResource)
@@ -238,12 +243,6 @@ func graphInitState(s *State, g *depgraph.Graph) {
 			continue
 		}
 		r := rn.Resource
-		rs := mod.Resources[r.Id]
-		if rs == nil {
-			rs = new(ResourceState)
-			rs.init()
-			mod.Resources[r.Id] = rs
-		}
 
 		// Update the dependencies
 		var inject []string
@@ -265,7 +264,7 @@ func graphInitState(s *State, g *depgraph.Graph) {
 		}
 
 		// Update the dependencies
-		rs.Dependencies = inject
+		rn.Dependencies = inject
 	}
 }
 
@@ -348,6 +347,9 @@ func graphAddConfigResources(
 						// from count == 1 to count > 1
 						state = mod.Resources[r.Id()]
 					}
+
+					// TODO(mitchellh): If one of the above works, delete
+					// the old style and just copy it to the new style.
 				}
 			}
 

terraform/graph_test.go

@@ -562,7 +562,7 @@ func TestGraphAddDiff_destroy_counts(t *testing.T) {
 	}
 }
 
-func TestGraphInitState(t *testing.T) {
+func TestGraphEncodeDependencies(t *testing.T) {
 	m := testModule(t, "graph-basic")
 	state := &State{
 		Modules: []*ModuleState{
@@ -592,21 +592,20 @@ func TestGraphInitState(t *testing.T) {
 	}
 
 	// This should encode the dependency information into the state
-	graphInitState(state, g)
+	graphEncodeDependencies(g)
 
-	root := state.RootModule()
+	web := g.Noun("aws_instance.web").Meta.(*GraphNodeResource)
-	web := root.Resources["aws_instance.web"]
 	if len(web.Dependencies) != 1 || web.Dependencies[0] != "aws_security_group.firewall" {
 		t.Fatalf("bad: %#v", web)
 	}
 
-	weblb := root.Resources["aws_load_balancer.weblb"]
+	weblb := g.Noun("aws_load_balancer.weblb").Meta.(*GraphNodeResource)
 	if len(weblb.Dependencies) != 1 || weblb.Dependencies[0] != "aws_instance.web" {
 		t.Fatalf("bad: %#v", weblb)
 	}
 }
 
-func TestGraphInitState_Count(t *testing.T) {
+func TestGraphEncodeDependencies_count(t *testing.T) {
 	m := testModule(t, "graph-count")
 	state := &State{
 		Modules: []*ModuleState{
@@ -636,15 +635,14 @@ func TestGraphInitState_Count(t *testing.T) {
 	}
 
 	// This should encode the dependency information into the state
-	graphInitState(state, g)
+	graphEncodeDependencies(g)
 
-	root := state.RootModule()
+	web := g.Noun("aws_instance.web.0").Meta.(*GraphNodeResource)
-	web := root.Resources["aws_instance.web.0"]
 	if len(web.Dependencies) != 0 {
 		t.Fatalf("bad: %#v", web)
 	}
 
-	weblb := root.Resources["aws_load_balancer.weblb"]
+	weblb := g.Noun("aws_load_balancer.weblb").Meta.(*GraphNodeResource)
 	if len(weblb.Dependencies) != 3 {
 		t.Fatalf("bad: %#v", weblb)
 	}

terraform/state.go

@@ -38,6 +38,17 @@ type State struct {
 	Modules []*ModuleState `json:"modules"`
 }
 
+// AddModule adds the module with the given path to the state.
+//
+// This should be the preferred method to add module states since it
+// allows us to optimize lookups later as well as control sorting.
+func (s *State) AddModule(path []string) *ModuleState{
+	m := &ModuleState{Path: path}
+	m.init()
+	s.Modules = append(s.Modules, m)
+	return m
+}
+
 // ModuleByPath is used to lookup the module state for the given path.
 // This should be the prefered lookup mechanism as it allows for future
 // lookup optimizations.

terraform/terraform_test.go

@@ -491,6 +491,26 @@ STATE:
 <no state>
 `
 
+const testTerraformPlanModulesStr = `
+DIFF:
+
+CREATE: aws_instance.bar
+  foo:  "" => "2"
+  type: "" => "aws_instance"
+CREATE: aws_instance.foo
+  num:  "" => "2"
+  type: "" => "aws_instance"
+
+module.child:
+  CREATE: aws_instance.foo
+    num:  "" => "2"
+    type: "" => "aws_instance"
+
+STATE:
+
+<no state>
+`
+
 const testTerraformPlanOrphanStr = `
 DIFF:
 

terraform/test-fixtures/plan-modules/child/main.tf

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

terraform/test-fixtures/plan-modules/main.tf

@@ -0,0 +1,11 @@
+module "child" {
+    source = "./child"
+}
+
+resource "aws_instance" "foo" {
+    num = "2"
+}
+
+resource "aws_instance" "bar" {
+    foo = "${aws_instance.foo.num}"
+}