From b1a583e3de94842a0363f0240ce705154820fc4f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 23 Sep 2014 14:15:40 -0700 Subject: [PATCH] terraform: plan with modules work --- terraform/context.go | 28 +++++++++---------- terraform/context_test.go | 23 +++++++++++++++ terraform/diff.go | 11 ++++++++ terraform/graph.go | 26 +++++++++-------- terraform/graph_test.go | 18 ++++++------ terraform/state.go | 11 ++++++++ terraform/terraform_test.go | 20 +++++++++++++ .../test-fixtures/plan-modules/child/main.tf | 3 ++ terraform/test-fixtures/plan-modules/main.tf | 11 ++++++++ 9 files changed, 114 insertions(+), 37 deletions(-) create mode 100644 terraform/test-fixtures/plan-modules/child/main.tf create mode 100644 terraform/test-fixtures/plan-modules/main.tf diff --git a/terraform/context.go b/terraform/context.go index 47d8772f4..9fa97ca86 100644 --- a/terraform/context.go +++ b/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 diff --git a/terraform/context_test.go b/terraform/context_test.go index 01dc00b69..e5d25590f 100644 --- a/terraform/context_test.go +++ b/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") diff --git a/terraform/diff.go b/terraform/diff.go index 29a7c1de0..ac1cad0ea 100644 --- a/terraform/diff.go +++ b/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. diff --git a/terraform/graph.go b/terraform/graph.go index 6c3746f47..056813e9c 100644 --- a/terraform/graph.go +++ b/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) { - // TODO: other modules - mod := s.RootModule() - +func graphEncodeDependencies(g *depgraph.Graph) { 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. } } diff --git a/terraform/graph_test.go b/terraform/graph_test.go index cc656d002..9b223d58b 100644 --- a/terraform/graph_test.go +++ b/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 := root.Resources["aws_instance.web"] + web := g.Noun("aws_instance.web").Meta.(*GraphNodeResource) 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 := root.Resources["aws_instance.web.0"] + web := g.Noun("aws_instance.web.0").Meta.(*GraphNodeResource) 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) } diff --git a/terraform/state.go b/terraform/state.go index c437b72be..a6988b195 100644 --- a/terraform/state.go +++ b/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. diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index fd5241896..7b6c93a36 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -491,6 +491,26 @@ 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: + + +` + const testTerraformPlanOrphanStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-modules/child/main.tf b/terraform/test-fixtures/plan-modules/child/main.tf new file mode 100644 index 000000000..98f5ee87e --- /dev/null +++ b/terraform/test-fixtures/plan-modules/child/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "foo" { + num = "2" +} diff --git a/terraform/test-fixtures/plan-modules/main.tf b/terraform/test-fixtures/plan-modules/main.tf new file mode 100644 index 000000000..dcdb236a1 --- /dev/null +++ b/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}" +}