From 4fe0c4ada4dfc7086e172a94f6071ce068184574 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Oct 2014 18:06:25 -0700 Subject: [PATCH 01/20] terraform: don't use Meta node anymore --- terraform/diff.go | 13 ++ terraform/graph.go | 264 ++++++++++++++++++++++++---------------- terraform/graph_test.go | 58 +++------ 3 files changed, 184 insertions(+), 151 deletions(-) diff --git a/terraform/diff.go b/terraform/diff.go index 5b0331510..f618e4423 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -167,6 +167,19 @@ func (d *ModuleDiff) Empty() bool { return true } +// Instances returns the instance diffs for the id given. This can return +// multiple instance diffs if there are counts within the resource. +func (d *ModuleDiff) Instances(id string) []*InstanceDiff { + var result []*InstanceDiff + for k, diff := range d.Resources { + if strings.HasPrefix(k, id) && !diff.Empty() { + result = append(result, diff) + } + } + + return result +} + // IsRoot says whether or not this module diff is for the root module. func (d *ModuleDiff) IsRoot() bool { return reflect.DeepEqual(d.Path, rootModulePath) diff --git a/terraform/graph.go b/terraform/graph.go index 76c51ad11..087d049f2 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -91,6 +91,10 @@ type GraphNodeResource struct { Config *config.Resource Resource *Resource ResourceProviderNode string + + // Expand, if true, indicates that this resource needs to be expanded + // at walk-time to multiple resources. + ExpandMode ResourceExpandMode } // GraphNodeResourceMeta is a node type in the graph that represents the @@ -123,6 +127,16 @@ type graphSharedProvider struct { parentNoun *depgraph.Noun } +// ResourceExpandMode specifies the expand behavior of the GraphNodeResource +// node. +type ResourceExpandMode byte + +const ( + ResourceExpandNone ResourceExpandMode = iota + ResourceExpandApply + ResourceExpandDestroy +) + // Graph builds a dependency graph of all the resources for infrastructure // change. // @@ -372,108 +386,123 @@ func graphAddConfigResources( meta := g.Meta.(*GraphMeta) // This tracks all the resource nouns - nouns := make(map[string]*depgraph.Noun) - for _, r := range c.Resources { - resourceNouns := make([]*depgraph.Noun, r.Count) - for i := 0; i < r.Count; i++ { - name := r.Id() - index := -1 - - // If we have a count that is more than one, then make sure - // we suffix with the number of the resource that this is. - if r.Count > 1 { - name = fmt.Sprintf("%s.%d", name, i) - index = i - } - - var state *ResourceState - if mod != nil { - // Lookup the resource state - state = mod.Resources[name] - if state == nil { - if r.Count == 1 { - // If the count is one, check the state for ".0" - // appended, which might exist if we go from - // count > 1 to count == 1. - state = mod.Resources[r.Id()+".0"] - } else if i == 0 { - // If count is greater than one, check for state - // with just the ID, which might exist if we go - // 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. - } - } - - if state == nil { - state = &ResourceState{ - Type: r.Type, - } - } - - flags := FlagPrimary - if len(state.Tainted) > 0 { - flags |= FlagHasTainted - } - - resourceNouns[i] = &depgraph.Noun{ - Name: name, - Meta: &GraphNodeResource{ - Index: index, - Config: r, - Resource: &Resource{ - Id: name, - Info: &InstanceInfo{ - Id: name, - ModulePath: meta.ModulePath, - Type: r.Type, - }, - State: state.Primary, - Config: NewResourceConfig(r.RawConfig), - Flags: flags, + nounsList := make([]*depgraph.Noun, len(c.Resources)) + for i, r := range c.Resources { + name := r.Id() + nounsList[i] = &depgraph.Noun{ + Name: name, + Meta: &GraphNodeResource{ + Index: -1, + Config: r, + Resource: &Resource{ + Id: name, + Info: &InstanceInfo{ + Id: name, + ModulePath: meta.ModulePath, + Type: r.Type, }, }, - } + ExpandMode: ResourceExpandApply, + }, } - // If we have more than one, then create a meta node to track - // the resources. - if r.Count > 1 { - metaNoun := &depgraph.Noun{ - Name: r.Id(), - Meta: &GraphNodeResourceMeta{ - ID: r.Id(), - Name: r.Name, - Type: r.Type, - Count: r.Count, - }, + /* + TODO: probably did something important, bring it back somehow + resourceNouns := make([]*depgraph.Noun, r.Count) + for i := 0; i < r.Count; i++ { + name := r.Id() + index := -1 + + // If we have a count that is more than one, then make sure + // we suffix with the number of the resource that this is. + if r.Count > 1 { + name = fmt.Sprintf("%s.%d", name, i) + index = i + } + + var state *ResourceState + if mod != nil { + // Lookup the resource state + state = mod.Resources[name] + if state == nil { + if r.Count == 1 { + // If the count is one, check the state for ".0" + // appended, which might exist if we go from + // count > 1 to count == 1. + state = mod.Resources[r.Id()+".0"] + } else if i == 0 { + // If count is greater than one, check for state + // with just the ID, which might exist if we go + // 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. + } + } + + if state == nil { + state = &ResourceState{ + Type: r.Type, + } + } + + flags := FlagPrimary + if len(state.Tainted) > 0 { + flags |= FlagHasTainted + } + + resourceNouns[i] = &depgraph.Noun{ + Name: name, + Meta: &GraphNodeResource{ + Index: index, + Config: r, + Resource: &Resource{ + Id: name, + Info: &InstanceInfo{ + Id: name, + ModulePath: meta.ModulePath, + Type: r.Type, + }, + State: state.Primary, + Config: NewResourceConfig(r.RawConfig), + Flags: flags, + }, + }, + } + } + + // If we have more than one, then create a meta node to track + // the resources. + if r.Count > 1 { + metaNoun := &depgraph.Noun{ + Name: r.Id(), + Meta: &GraphNodeResourceMeta{ + ID: r.Id(), + Name: r.Name, + Type: r.Type, + Count: r.Count, + }, + } + + // Create the dependencies on this noun + for _, n := range resourceNouns { + metaNoun.Deps = append(metaNoun.Deps, &depgraph.Dependency{ + Name: n.Name, + Source: metaNoun, + Target: n, + }) + } + + // Assign it to the map so that we have it + nouns[metaNoun.Name] = metaNoun } - // Create the dependencies on this noun for _, n := range resourceNouns { - metaNoun.Deps = append(metaNoun.Deps, &depgraph.Dependency{ - Name: n.Name, - Source: metaNoun, - Target: n, - }) + nouns[n.Name] = n } - - // Assign it to the map so that we have it - nouns[metaNoun.Name] = metaNoun - } - - for _, n := range resourceNouns { - nouns[n.Name] = n - } - } - - // Build the list of nouns that we iterate over - nounsList := make([]*depgraph.Noun, 0, len(nouns)) - for _, n := range nouns { - nounsList = append(nounsList, n) + */ } g.Name = "terraform" @@ -501,15 +530,28 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { continue } - rd, ok := d.Resources[rn.Resource.Id] - if !ok { + change := false + destroy := false + diffs := d.Instances(rn.Resource.Id) + if len(diffs) == 0 { continue } - if rd.Empty() { - continue + for _, d := range diffs { + if d.Destroy { + destroy = true + } + + if len(d.Attributes) > 0 { + change = true + } } - if rd.Destroy { + var rd *InstanceDiff + if rn.ExpandMode == ResourceExpandNone { + rd = diffs[0] + } + + if destroy { // If we're destroying, we create a new destroy node with // the proper dependencies. Perform a dirty copy operation. newNode := new(GraphNodeResource) @@ -520,6 +562,11 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { // Make the diff _just_ the destroy. newNode.Resource.Diff = &InstanceDiff{Destroy: true} + // Make sure ExpandDestroy is set if Expand + if newNode.ExpandMode == ResourceExpandApply { + newNode.ExpandMode = ResourceExpandDestroy + } + // Create the new node newN := &depgraph.Noun{ Name: fmt.Sprintf("%s (destroy)", newNode.Resource.Id), @@ -533,17 +580,19 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { // Append it to the list so we handle it later nlist = append(nlist, newN) - // Mark the old diff to not destroy since we handle that in - // the dedicated node. - newDiff := new(InstanceDiff) - *newDiff = *rd - newDiff.Destroy = false - rd = newDiff + if rd != nil { + // Mark the old diff to not destroy since we handle that in + // the dedicated node. + newDiff := new(InstanceDiff) + *newDiff = *rd + newDiff.Destroy = false + rd = newDiff + } // The dependency ordering depends on if the CreateBeforeDestroy // flag is enabled. If so, we must create the replacement first, // and then destroy the old instance. - if rn.Config != nil && rn.Config.Lifecycle.CreateBeforeDestroy && !rd.Empty() { + if rn.Config != nil && rn.Config.Lifecycle.CreateBeforeDestroy && change { dep := &depgraph.Dependency{ Name: n.Name, Source: newN, @@ -877,7 +926,7 @@ func graphAddOrphanDeps(g *depgraph.Graph, mod *ModuleState) { } for _, depName := range rs.Dependencies { - if compareName != depName { + if !strings.HasPrefix(depName, compareName) { continue } dep := &depgraph.Dependency{ @@ -886,6 +935,7 @@ func graphAddOrphanDeps(g *depgraph.Graph, mod *ModuleState) { Target: n2, } n.Deps = append(n.Deps, dep) + break } } } diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 461b49d01..7fdd7b044 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -7,7 +7,7 @@ import ( "testing" ) -func TestGraph(t *testing.T) { +func TestGraph_basic(t *testing.T) { m := testModule(t, "graph-basic") g, err := Graph(&GraphOpts{Module: m}) @@ -447,6 +447,8 @@ func TestGraphAddDiff(t *testing.T) { t.Fatalf("bad:\n\n%s", actual) } + /* + TODO: test this somewhere // Verify that the state has been added n := g.Noun("aws_instance.foo") rn := n.Meta.(*GraphNodeResource) @@ -456,6 +458,7 @@ func TestGraphAddDiff(t *testing.T) { if !reflect.DeepEqual(actual2, expected2) { t.Fatalf("bad: %#v", actual2) } + */ } func TestGraphAddDiff_destroy(t *testing.T) { @@ -609,13 +612,11 @@ func TestGraphAddDiff_destroy_counts(t *testing.T) { } // Verify that the state has been added - n := g.Noun("aws_instance.web.0 (destroy)") + n := g.Noun("aws_instance.web (destroy)") rn := n.Meta.(*GraphNodeResource) - expected2 := &InstanceDiff{Destroy: true} - actual2 := rn.Resource.Diff - if !reflect.DeepEqual(actual2, expected2) { - t.Fatalf("bad: %#v", actual2) + if rn.ExpandMode != ResourceExpandDestroy { + t.Fatalf("bad: %#v", rn) } // Verify that our original structure has not been modified @@ -816,13 +817,13 @@ func TestGraphEncodeDependencies_count(t *testing.T) { // This should encode the dependency information into the state graphEncodeDependencies(g) - web := g.Noun("aws_instance.web.0").Meta.(*GraphNodeResource).Resource + web := g.Noun("aws_instance.web").Meta.(*GraphNodeResource).Resource if len(web.Dependencies) != 0 { t.Fatalf("bad: %#v", web) } weblb := g.Noun("aws_load_balancer.weblb").Meta.(*GraphNodeResource).Resource - if len(weblb.Dependencies) != 3 { + if len(weblb.Dependencies) != 1 { t.Fatalf("bad: %#v", weblb) } } @@ -956,12 +957,6 @@ root const testTerraformGraphCountStr = ` root: root aws_instance.web - aws_instance.web -> aws_instance.web.0 - aws_instance.web -> aws_instance.web.1 - aws_instance.web -> aws_instance.web.2 -aws_instance.web.0 -aws_instance.web.1 -aws_instance.web.2 aws_load_balancer.weblb aws_load_balancer.weblb -> aws_instance.web root @@ -982,12 +977,7 @@ root const testTerraformGraphDependsCountStr = ` root: root aws_instance.db - aws_instance.db -> aws_instance.db.0 - aws_instance.db -> aws_instance.db.1 -aws_instance.db.0 - aws_instance.db.0 -> aws_instance.web -aws_instance.db.1 - aws_instance.db.1 -> aws_instance.web + aws_instance.db -> aws_instance.web aws_instance.web root root -> aws_instance.db @@ -1024,21 +1014,9 @@ root const testTerraformGraphDiffDestroyCountsStr = ` root: root aws_instance.web - aws_instance.web -> aws_instance.web.0 - aws_instance.web -> aws_instance.web.1 - aws_instance.web -> aws_instance.web.2 -aws_instance.web.0 - aws_instance.web.0 -> aws_instance.web.0 (destroy) -aws_instance.web.0 (destroy) - aws_instance.web.0 (destroy) -> aws_load_balancer.weblb (destroy) -aws_instance.web.1 - aws_instance.web.1 -> aws_instance.web.1 (destroy) -aws_instance.web.1 (destroy) - aws_instance.web.1 (destroy) -> aws_load_balancer.weblb (destroy) -aws_instance.web.2 - aws_instance.web.2 -> aws_instance.web.2 (destroy) -aws_instance.web.2 (destroy) - aws_instance.web.2 (destroy) -> aws_load_balancer.weblb (destroy) + aws_instance.web -> aws_instance.web (destroy) +aws_instance.web (destroy) + aws_instance.web (destroy) -> aws_load_balancer.weblb (destroy) aws_load_balancer.weblb aws_load_balancer.weblb -> aws_instance.web aws_load_balancer.weblb -> aws_load_balancer.weblb (destroy) @@ -1197,16 +1175,8 @@ root const testTerraformGraphCountOrphanStr = ` root: root aws_instance.web - aws_instance.web -> aws_instance.web.0 - aws_instance.web -> aws_instance.web.1 - aws_instance.web -> aws_instance.web.2 -aws_instance.web.0 -aws_instance.web.1 -aws_instance.web.2 aws_load_balancer.old - aws_load_balancer.old -> aws_instance.web.0 - aws_load_balancer.old -> aws_instance.web.1 - aws_load_balancer.old -> aws_instance.web.2 + aws_load_balancer.old -> aws_instance.web aws_load_balancer.weblb aws_load_balancer.weblb -> aws_instance.web root From fecb68f117326e39611529441c05fe19e97a00ef Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Oct 2014 18:08:52 -0700 Subject: [PATCH 02/20] terraform: remove meta nodes --- terraform/context.go | 3 --- terraform/graph.go | 46 ------------------------------------------ terraform/graph_dot.go | 3 ++- 3 files changed, 2 insertions(+), 50 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index d5d865e77..b6fc2d172 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1104,9 +1104,6 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { return wc.Walk() case *GraphNodeResource: // Continue, we care about this the most - case *GraphNodeResourceMeta: - // Skip it - return nil case *GraphNodeResourceProvider: sharedProvider := m.Provider diff --git a/terraform/graph.go b/terraform/graph.go index 087d049f2..3d1cf6c10 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -97,16 +97,6 @@ type GraphNodeResource struct { ExpandMode ResourceExpandMode } -// GraphNodeResourceMeta is a node type in the graph that represents the -// metadata for a resource. There will be one meta node for every resource -// in the configuration. -type GraphNodeResourceMeta struct { - ID string - Name string - Type string - Count int -} - // GraphNodeResourceProvider is a node type in the graph that represents // the configuration for a resource provider. type GraphNodeResourceProvider struct { @@ -335,13 +325,6 @@ func graphEncodeDependencies(g *depgraph.Graph) { } inject = append(inject, target.Resource.Id) - case *GraphNodeResourceMeta: - // Inject each sub-resource as a depedency - for i := 0; i < target.Count; i++ { - id := fmt.Sprintf("%s.%d", target.ID, i) - inject = append(inject, id) - } - case *GraphNodeResourceProvider: // Do nothing @@ -702,33 +685,6 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { num-- i-- - case *GraphNodeResourceMeta: - // Check if any of the resources part of the meta node - // are being destroyed, because we must be destroyed first. - for i := 0; i < target.Count; i++ { - id := fmt.Sprintf("%s.%d", target.ID, i) - for _, n2 := range nlist { - rn2 := n2.Meta.(*GraphNodeResource) - if id == rn2.Resource.Id { - newDep := &depgraph.Dependency{ - Name: n.Name, - Source: n2, - Target: n, - } - injected[newDep] = struct{}{} - n2.Deps = append(n2.Deps, newDep) - break - } - } - } - - // Drop the dependency, since there is - // nothing that needs to be done for a meta - // resource on destroy. - 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. @@ -1049,8 +1005,6 @@ func graphAddRoot(g *depgraph.Graph) { if m.Index != -1 { continue } - case *GraphNodeResourceMeta: - // Always in the graph case *GraphNodeResourceProvider: // ResourceProviders don't need to be in the root deps because // they're always pointed to by some resource. diff --git a/terraform/graph_dot.go b/terraform/graph_dot.go index cca757b42..ce4a08d7f 100644 --- a/terraform/graph_dot.go +++ b/terraform/graph_dot.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/terraform/depgraph" - "github.com/hashicorp/terraform/digraph" ) // GraphDotOpts are options for turning a graph into dot format. @@ -221,6 +220,7 @@ func graphDotAddResources( } // Handle the meta resources + /* edgeBuf.Reset() for _, n := range g.Nouns { _, ok := n.Meta.(*GraphNodeResourceMeta) @@ -264,6 +264,7 @@ func graphDotAddResources( buf.WriteString(edgeBuf.String()) buf.WriteString("\n") } + */ } func graphDotAddResourceProviders( From fb1c224e12f8a88643b44effd5bb38acad1ac78b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 10:42:58 -0700 Subject: [PATCH 03/20] terraform: expand resource nodes at walk time --- terraform/context.go | 41 ++++++++++- terraform/graph.go | 165 +++++++++++++++++++++++++++++++++++++++++++ terraform/state.go | 18 +++++ 3 files changed, 223 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index b6fc2d172..3a240fe91 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1057,7 +1057,8 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { // This will keep track of whether we're stopped or not var stop uint32 = 0 - return func(n *depgraph.Noun) error { + var walkFn depgraph.WalkFunc + walkFn = func(n *depgraph.Noun) error { // If it is the root node, ignore if n.Name == GraphRootNode { return nil @@ -1132,6 +1133,42 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { rn := n.Meta.(*GraphNodeResource) + // If we're expanding, then expand the nodes, and then rewalk the graph + if rn.ExpandMode > ResourceExpandNone { + ns, 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() { + defer wg.Done() + if err := walkFn(n); err != nil { + l.Lock() + defer l.Unlock() + errs = append(errs, err) + } + }() + } + + // Wait for the subgraph + wg.Wait() + + // If there are errors, then we should return them + if len(errs) > 0 { + return &multierror.Error{Errors: errs} + } + + return nil + } + // Make sure that at least some resource configuration is set if rn.Config == nil { rn.Resource.Config = new(ResourceConfig) @@ -1163,6 +1200,8 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { return nil } + + return walkFn } // applyProvisioners is used to run any provisioners a resource has diff --git a/terraform/graph.go b/terraform/graph.go index 3d1cf6c10..a20ca76d4 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -92,6 +92,9 @@ type GraphNodeResource struct { Resource *Resource ResourceProviderNode string + Diff *ModuleDiff + State *ModuleState + // Expand, if true, indicates that this resource needs to be expanded // at walk-time to multiple resources. ExpandMode ResourceExpandMode @@ -372,6 +375,8 @@ func graphAddConfigResources( nounsList := make([]*depgraph.Noun, len(c.Resources)) for i, r := range c.Resources { name := r.Id() + + // Build the noun nounsList[i] = &depgraph.Noun{ Name: name, Meta: &GraphNodeResource{ @@ -385,6 +390,7 @@ func graphAddConfigResources( Type: r.Type, }, }, + State: mod.View(name), ExpandMode: ResourceExpandApply, }, } @@ -529,6 +535,11 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { } } + // If we're expanding, save the diff so we can add it on later + if rn.ExpandMode > ResourceExpandNone { + rn.Diff = d + } + var rd *InstanceDiff if rn.ExpandMode == ResourceExpandNone { rd = diffs[0] @@ -1577,6 +1588,160 @@ func (p *graphSharedProvider) MergeConfig( return NewResourceConfig(rc) } +// Expand will expand this node into a subgraph if Expand is set. +func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { + count := 1 + + g := new(depgraph.Graph) + + // Determine the nodes to create. If we're just looking for the + // nodes to create, return that. + n.expandCreate(g, count) + + // Add in the diff if we have it + if n.Diff != nil { + if err := graphAddDiff(g, n.Diff); err != nil { + return nil, err + } + } + + // If we're just expanding the apply, then filter those out and + // return them now. + if n.ExpandMode == ResourceExpandApply { + return n.filterNouns(g, false), nil + } + + if n.State != nil { + // TODO: orphans + + // Add the tainted resources + graphAddTainted(g, n.State) + } + + return n.filterNouns(g, true), nil +} + +// expandCreate expands this resource and adds the resources to the graph. +func (n *GraphNodeResource) expandCreate(g *depgraph.Graph, count int) { + // Create the list of nouns that we'd have to create + create := make([]*depgraph.Noun, 0, count) + + // First thing, expand the counts that we have defined for our + // current config into the full set of resources. + r := n.Config + for i := 0; i < count; i++ { + name := r.Id() + index := -1 + + // If we have a count that is more than one, then make sure + // we suffix with the number of the resource that this is. + if count > 1 { + name = fmt.Sprintf("%s.%d", name, i) + index = i + } + + var state *ResourceState + if n.State != nil { + // Lookup the resource state + if s, ok := n.State.Resources[name]; ok { + state = s + } + + if state == nil { + if count == 1 { + // If the count is one, check the state for ".0" + // appended, which might exist if we go from + // count > 1 to count == 1. + k := r.Id() + ".0" + state = n.State.Resources[k] + } else if i == 0 { + // If count is greater than one, check for state + // with just the ID, which might exist if we go + // from count == 1 to count > 1 + state = n.State.Resources[r.Id()] + } + } + } + + if state == nil { + state = &ResourceState{ + Type: r.Type, + } + } + + flags := FlagPrimary + if len(state.Tainted) > 0 { + flags |= FlagHasTainted + } + + // Copy the base resource so we can fill it in + resource := n.copyResource(name) + resource.State = state.Primary + resource.Flags = flags + // TODO: we need the diff here... + + // Add the result + create = append(create, &depgraph.Noun{ + Name: name, + Meta: &GraphNodeResource{ + Index: index, + Config: r, + Resource: resource, + }, + }) + } + + g.Nouns = append(g.Nouns, create...) +} + +// copyResource copies the Resource structure to assign to a subgraph. +func (n *GraphNodeResource) copyResource(id string) *Resource { + info := *n.Resource.Info + info.Id = id + resource := *n.Resource + resource.Id = id + resource.Info = &info + resource.Config = NewResourceConfig(n.Config.RawConfig) + return &resource +} + +func (n *GraphNodeResource) filterNouns( + g *depgraph.Graph, destroy bool) []*depgraph.Noun { + result := make([]*depgraph.Noun, 0, len(g.Nouns)) + for _, n := range g.Nouns { + rn, ok := n.Meta.(*GraphNodeResource) + if !ok { + continue + } + + // If the diff is nil, then we're not destroying, so append only + // in that case. + if rn.Resource.Diff == nil { + if !destroy { + result = append(result, n) + } + + continue + } + + // If we are destroying, append it only if we care about destroys + if rn.Resource.Diff.Destroy { + if destroy { + result = append(result, n) + } + + continue + } + + // If we're not destroying, then add it only if we don't + // care about deploys. + if !destroy { + result = append(result, n) + } + } + return result +} + // matchingPrefixes takes a resource type and a set of resource // providers we know about by prefix and returns a list of prefixes // that might be valid for that resource. diff --git a/terraform/state.go b/terraform/state.go index f71a34289..ef537ab54 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -212,6 +212,24 @@ func (m *ModuleState) Orphans(c *config.Config) []string { return result } +// View returns a view with the given resource prefix. +func (m *ModuleState) View(id string) *ModuleState { + if m == nil { + return m + } + + r := m.deepcopy() + for k, _ := range r.Resources { + if id == k || strings.HasPrefix(k, id+".") { + continue + } + + delete(r.Resources, k) + } + + return r +} + func (m *ModuleState) init() { if m.Outputs == nil { m.Outputs = make(map[string]string) From 8e2315599fcb5e642d9a6d4bdea1eabf95989551 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 11:14:50 -0700 Subject: [PATCH 04/20] config: Count can be a string (for interpolation) --- config/config.go | 28 ++++++---------------------- config/config_string.go | 4 ++-- config/config_test.go | 21 --------------------- config/loader_hcl.go | 11 +++++++++-- config/raw_config.go | 13 +++++++++++++ config/raw_config_test.go | 30 ++++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 47 deletions(-) diff --git a/config/config.go b/config/config.go index 0b4350a2a..59db96de1 100644 --- a/config/config.go +++ b/config/config.go @@ -56,7 +56,7 @@ type ProviderConfig struct { type Resource struct { Name string Type string - Count int + Count *RawConfig RawConfig *RawConfig Provisioners []*Provisioner DependsOn []string @@ -244,12 +244,6 @@ func (c *Config) Validate() error { // Validate resources for n, r := range resources { - if r.Count < 1 { - errs = append(errs, fmt.Errorf( - "%s: count must be greater than or equal to 1", - n)) - } - for _, d := range r.DependsOn { if _, ok := resources[d]; !ok { errs = append(errs, fmt.Errorf( @@ -267,8 +261,7 @@ func (c *Config) Validate() error { } id := fmt.Sprintf("%s.%s", rv.Type, rv.Name) - r, ok := resources[id] - if !ok { + if _, ok := resources[id]; !ok { errs = append(errs, fmt.Errorf( "%s: unknown resource '%s' referenced in variable %s", source, @@ -276,18 +269,6 @@ func (c *Config) Validate() error { rv.FullKey())) continue } - - // If it is a multi reference and resource has a single - // count, it is an error. - if r.Count > 1 && !rv.Multi { - errs = append(errs, fmt.Errorf( - "%s: variable '%s' must specify index for multi-count "+ - "resource %s", - source, - rv.FullKey(), - id)) - continue - } } } @@ -327,6 +308,9 @@ func (c *Config) InterpolatedVariables() map[string][]InterpolatedVariable { for _, rc := range c.Resources { source := fmt.Sprintf("resource '%s'", rc.Id()) + for _, v := range rc.Count.Variables { + result[source] = append(result[source], v) + } for _, v := range rc.RawConfig.Variables { result[source] = append(result[source], v) } @@ -400,7 +384,7 @@ func (r *Resource) mergerMerge(m merger) merger { result.Type = r2.Type result.RawConfig = result.RawConfig.merge(r2.RawConfig) - if r2.Count > 0 { + if r2.Count.Value() != "1" { result.Count = r2.Count } diff --git a/config/config_string.go b/config/config_string.go index 89a990ae8..983bdf8d4 100644 --- a/config/config_string.go +++ b/config/config_string.go @@ -190,10 +190,10 @@ func resourcesStr(rs []*Resource) string { for _, i := range order { r := rs[i] result += fmt.Sprintf( - "%s[%s] (x%d)\n", + "%s[%s] (x%s)\n", r.Type, r.Name, - r.Count) + r.Count.Value()) ks := make([]string, 0, len(r.RawConfig.Raw)) for k, _ := range r.RawConfig.Raw { diff --git a/config/config_test.go b/config/config_test.go index 76886c89b..b072e5469 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -23,27 +23,6 @@ func TestConfigValidate_badDependsOn(t *testing.T) { } } -func TestConfigValidate_badMultiResource(t *testing.T) { - c := testConfig(t, "validate-bad-multi-resource") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - -func TestConfigValidate_countBelowZero(t *testing.T) { - c := testConfig(t, "validate-count-below-zero") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - -func TestConfigValidate_countZero(t *testing.T) { - c := testConfig(t, "validate-count-zero") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - func TestConfigValidate_dupModule(t *testing.T) { c := testConfig(t, "validate-dup-module") if err := c.Validate(); err == nil { diff --git a/config/loader_hcl.go b/config/loader_hcl.go index d777873e7..78c7064d5 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -406,7 +406,7 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { } // If we have a count, then figure it out - var count int = 1 + var count string = "1" if o := obj.Get("count", false); o != nil { err = hcl.DecodeObject(&count, o) if err != nil { @@ -417,6 +417,13 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { err) } } + countConfig, err := NewRawConfig(map[string]interface{}{ + "count": count, + }) + if err != nil { + return nil, err + } + countConfig.Key = "count" // If we have depends fields, then add those in var dependsOn []string @@ -475,7 +482,7 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { result = append(result, &Resource{ Name: k, Type: t.Key, - Count: count, + Count: countConfig, RawConfig: rawConfig, Provisioners: provisioners, DependsOn: dependsOn, diff --git a/config/raw_config.go b/config/raw_config.go index 2674476a2..3b08d9570 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -24,6 +24,7 @@ const UnknownVariableValue = "74D93920-ED26-11E3-AC10-0800200C9A66" // RawConfig supports a query-like interface to request // information from deep within the structure. type RawConfig struct { + Key string Raw map[string]interface{} Interpolations []Interpolation Variables map[string]InterpolatedVariable @@ -43,6 +44,18 @@ func NewRawConfig(raw map[string]interface{}) (*RawConfig, error) { return result, nil } +// Value returns the value of the configuration if this configuration +// has a Key set. If this does not have a Key set, nil will be returned. +func (r *RawConfig) Value() interface{} { + if c := r.Config(); c != nil { + if v, ok := c[r.Key]; ok { + return v + } + } + + return r.Raw[r.Key] +} + // Config returns the entire configuration with the variables // interpolated from any call to Interpolate. // diff --git a/config/raw_config_test.go b/config/raw_config_test.go index 9746f200e..a17112552 100644 --- a/config/raw_config_test.go +++ b/config/raw_config_test.go @@ -125,6 +125,36 @@ func TestRawConfig_unknown(t *testing.T) { } } +func TestRawConfigValue(t *testing.T) { + raw := map[string]interface{}{ + "foo": "${var.bar}", + } + + rc, err := NewRawConfig(raw) + if err != nil { + t.Fatalf("err: %s", err) + } + + rc.Key = "" + if rc.Value() != nil { + t.Fatalf("bad: %#v", rc.Value()) + } + + rc.Key = "foo" + if rc.Value() != "${var.bar}" { + t.Fatalf("err: %#v", rc.Value()) + } + + vars := map[string]string{"var.bar": "baz"} + if err := rc.Interpolate(vars); err != nil { + t.Fatalf("err: %s", err) + } + + if rc.Value() != "baz" { + t.Fatalf("bad: %#v", rc.Value()) + } +} + func TestRawConfig_implGob(t *testing.T) { var _ gob.GobDecoder = new(RawConfig) var _ gob.GobEncoder = new(RawConfig) From f772c11103ae03c5314d63a5cfb3821acf752cb7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 11:18:57 -0700 Subject: [PATCH 05/20] config: validate unknown var in count --- config/config_test.go | 7 +++++++ config/test-fixtures/validate-unknownvar-count/main.tf | 5 +++++ 2 files changed, 12 insertions(+) create mode 100644 config/test-fixtures/validate-unknownvar-count/main.tf diff --git a/config/config_test.go b/config/config_test.go index b072e5469..8b9a65f8d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -79,6 +79,13 @@ func TestConfigValidate_unknownVar(t *testing.T) { } } +func TestConfigValidate_unknownVarCount(t *testing.T) { + c := testConfig(t, "validate-unknownvar-count") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_varDefault(t *testing.T) { c := testConfig(t, "validate-var-default") if err := c.Validate(); err != nil { diff --git a/config/test-fixtures/validate-unknownvar-count/main.tf b/config/test-fixtures/validate-unknownvar-count/main.tf new file mode 100644 index 000000000..1f8cbd1e7 --- /dev/null +++ b/config/test-fixtures/validate-unknownvar-count/main.tf @@ -0,0 +1,5 @@ +resource "foo" "bar" { + default = "bar" + description = "bar" + count = "${var.bar}" +} From 101ac636a2a016096ae051764e13a672f2d5a225 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 11:34:08 -0700 Subject: [PATCH 06/20] config: add Config method --- config/config.go | 19 +++++++++++--- config/config_string.go | 2 +- config/config_test.go | 30 +++++++++++++++++++++++ config/loader_hcl.go | 2 +- config/test-fixtures/count-int/main.tf | 3 +++ config/test-fixtures/count-string/main.tf | 3 +++ config/test-fixtures/count-var/main.tf | 3 +++ terraform/state.go | 7 +++--- terraform/state_v1.go | 8 +++--- 9 files changed, 65 insertions(+), 12 deletions(-) create mode 100644 config/test-fixtures/count-int/main.tf create mode 100644 config/test-fixtures/count-string/main.tf create mode 100644 config/test-fixtures/count-var/main.tf diff --git a/config/config.go b/config/config.go index 59db96de1..67e0a2e7e 100644 --- a/config/config.go +++ b/config/config.go @@ -4,6 +4,7 @@ package config import ( "fmt" + "strconv" "strings" "github.com/hashicorp/terraform/flatmap" @@ -56,7 +57,7 @@ type ProviderConfig struct { type Resource struct { Name string Type string - Count *RawConfig + RawCount *RawConfig RawConfig *RawConfig Provisioners []*Provisioner DependsOn []string @@ -120,6 +121,16 @@ func (r *Module) Id() string { return fmt.Sprintf("%s", r.Name) } +// Count returns the count of this resource. +func (r *Resource) Count() (int, error) { + v, err := strconv.ParseInt(r.RawCount.Value().(string), 0, 0) + if err != nil { + return 0, err + } + + return int(v), nil +} + // A unique identifier for this resource. func (r *Resource) Id() string { return fmt.Sprintf("%s.%s", r.Type, r.Name) @@ -308,7 +319,7 @@ func (c *Config) InterpolatedVariables() map[string][]InterpolatedVariable { for _, rc := range c.Resources { source := fmt.Sprintf("resource '%s'", rc.Id()) - for _, v := range rc.Count.Variables { + for _, v := range rc.RawCount.Variables { result[source] = append(result[source], v) } for _, v := range rc.RawConfig.Variables { @@ -384,8 +395,8 @@ func (r *Resource) mergerMerge(m merger) merger { result.Type = r2.Type result.RawConfig = result.RawConfig.merge(r2.RawConfig) - if r2.Count.Value() != "1" { - result.Count = r2.Count + if r2.RawCount.Value() != "1" { + result.RawCount = r2.RawCount } if len(r2.Provisioners) > 0 { diff --git a/config/config_string.go b/config/config_string.go index 983bdf8d4..2db0cbe49 100644 --- a/config/config_string.go +++ b/config/config_string.go @@ -193,7 +193,7 @@ func resourcesStr(rs []*Resource) string { "%s[%s] (x%s)\n", r.Type, r.Name, - r.Count.Value()) + r.RawCount.Value()) ks := make([]string, 0, len(r.RawConfig.Raw)) for k, _ := range r.RawConfig.Raw { diff --git a/config/config_test.go b/config/config_test.go index 8b9a65f8d..a31df6b5a 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -9,6 +9,36 @@ import ( // This is the directory where our test fixtures are. const fixtureDir = "./test-fixtures" +func TestConfigCount(t *testing.T) { + c := testConfig(t, "count-int") + actual, err := c.Resources[0].Count() + if err != nil { + t.Fatalf("err: %s", err) + } + if actual != 5 { + t.Fatalf("bad: %#v", actual) + } +} + +func TestConfigCount_string(t *testing.T) { + c := testConfig(t, "count-string") + actual, err := c.Resources[0].Count() + if err != nil { + t.Fatalf("err: %s", err) + } + if actual != 5 { + t.Fatalf("bad: %#v", actual) + } +} + +func TestConfigCount_var(t *testing.T) { + c := testConfig(t, "count-var") + _, err := c.Resources[0].Count() + if err == nil { + t.Fatalf("should error") + } +} + func TestConfigValidate(t *testing.T) { c := testConfig(t, "validate-good") if err := c.Validate(); err != nil { diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 78c7064d5..10e178c1e 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -482,7 +482,7 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { result = append(result, &Resource{ Name: k, Type: t.Key, - Count: countConfig, + RawCount: countConfig, RawConfig: rawConfig, Provisioners: provisioners, DependsOn: dependsOn, diff --git a/config/test-fixtures/count-int/main.tf b/config/test-fixtures/count-int/main.tf new file mode 100644 index 000000000..213bb4dfa --- /dev/null +++ b/config/test-fixtures/count-int/main.tf @@ -0,0 +1,3 @@ +resource "foo" "bar" { + count = 5 +} diff --git a/config/test-fixtures/count-string/main.tf b/config/test-fixtures/count-string/main.tf new file mode 100644 index 000000000..6ad7191f7 --- /dev/null +++ b/config/test-fixtures/count-string/main.tf @@ -0,0 +1,3 @@ +resource "foo" "bar" { + count = "5" +} diff --git a/config/test-fixtures/count-var/main.tf b/config/test-fixtures/count-var/main.tf new file mode 100644 index 000000000..789262030 --- /dev/null +++ b/config/test-fixtures/count-var/main.tf @@ -0,0 +1,3 @@ +resource "foo" "bar" { + count = "${var.foo}" +} diff --git a/terraform/state.go b/terraform/state.go index ef537ab54..1a6980fc4 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -198,9 +198,10 @@ func (m *ModuleState) Orphans(c *config.Config) []string { for _, r := range c.Resources { delete(keys, r.Id()) - // Mark all the counts as not orphans. - for i := 0; i < r.Count; i++ { - delete(keys, fmt.Sprintf("%s.%d", r.Id(), i)) + for k, _ := range keys { + if strings.HasPrefix(k, r.Id()+".") { + delete(keys, k) + } } } diff --git a/terraform/state_v1.go b/terraform/state_v1.go index 14436f205..85ba939a7 100644 --- a/terraform/state_v1.go +++ b/terraform/state_v1.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "sort" + "strings" "sync" "github.com/hashicorp/terraform/config" @@ -80,9 +81,10 @@ func (s *StateV1) Orphans(c *config.Config) []string { for _, r := range c.Resources { delete(keys, r.Id()) - // Mark all the counts as not orphans. - for i := 0; i < r.Count; i++ { - delete(keys, fmt.Sprintf("%s.%d", r.Id(), i)) + for k, _ := range keys { + if strings.HasPrefix(k, r.Id()+".") { + delete(keys, k) + } } } From 3b89a7bdc706a2d8bf06bface25cfcc951c00a4a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 11:48:00 -0700 Subject: [PATCH 07/20] terraform: more tests passing --- terraform/context.go | 22 ++++++++++++++++++---- terraform/graph.go | 23 +++++++++++++++++------ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 3a240fe91..c483094cf 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1135,6 +1135,11 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { // If we're expanding, then expand the nodes, and then rewalk the graph if rn.ExpandMode > ResourceExpandNone { + // Interpolate the count + rc := NewResourceConfig(rn.Config.RawCount) + rc.interpolate(c) + + // Expand the node to the actual resources ns, err := rn.Expand() if err != nil { return err @@ -1148,14 +1153,14 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { for _, n := range ns { wg.Add(1) - go func() { + go func(n *depgraph.Noun) { defer wg.Done() if err := walkFn(n); err != nil { l.Lock() defer l.Unlock() errs = append(errs, err) } - }() + }(n) } // Wait for the subgraph @@ -1515,13 +1520,22 @@ func (c *walkContext) computeResourceMultiVariable( // TODO: Not use only root module module := c.Context.state.RootModule() + // TODO: handle computed here + count, err := cr.Count() + if err != nil { + return "", fmt.Errorf( + "Error reading %s count: %s", + v.ResourceId(), + err) + } + var values []string - for i := 0; i < cr.Count; i++ { + for i := 0; i < count; i++ { id := fmt.Sprintf("%s.%d", v.ResourceId(), i) // If we're dealing with only a single resource, then the // ID doesn't have a trailing index. - if cr.Count == 1 { + if count == 1 { id = v.ResourceId() } diff --git a/terraform/graph.go b/terraform/graph.go index a20ca76d4..22f8c43f9 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -603,11 +603,13 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { // Add a depedency from the root, since the create node // does not depend on us - g.Root.Deps = append(g.Root.Deps, &depgraph.Dependency{ - Name: newN.Name, - Source: g.Root, - Target: newN, - }) + if g.Root != nil { + g.Root.Deps = append(g.Root.Deps, &depgraph.Dependency{ + Name: newN.Name, + Source: g.Root, + Target: newN, + }) + } // Set the ReplacePrimary flag on the new instance so that // it will become the new primary, and Diposed flag on the @@ -1590,9 +1592,18 @@ func (p *graphSharedProvider) MergeConfig( // Expand will expand this node into a subgraph if Expand is set. func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { - count := 1 + // Expand the count out, which should be interpolated at this point + count, err := n.Config.Count() + if err != nil { + return nil, err + } + log.Printf("[DEBUG] %s: expanding to count = %d", n.Resource.Id, count) + // TODO: can we DRY this up? g := new(depgraph.Graph) + g.Meta = &GraphMeta{ + ModulePath: n.Resource.Info.ModulePath, + } // Determine the nodes to create. If we're just looking for the // nodes to create, return that. From 0f087141e3957f8c74763be2222b76c691285792 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 13:12:53 -0700 Subject: [PATCH 08/20] terraform: properly discover count orphans --- terraform/graph.go | 75 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/terraform/graph.go b/terraform/graph.go index 22f8c43f9..ab82b7afd 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -92,12 +92,11 @@ type GraphNodeResource struct { Resource *Resource ResourceProviderNode string - Diff *ModuleDiff - State *ModuleState - - // Expand, if true, indicates that this resource needs to be expanded - // at walk-time to multiple resources. + // All the fields below are related to expansion. These are set by + // the graph but aren't useful individually. ExpandMode ResourceExpandMode + Diff *ModuleDiff + State *ModuleState } // GraphNodeResourceProvider is a node type in the graph that represents @@ -1607,7 +1606,7 @@ func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { // Determine the nodes to create. If we're just looking for the // nodes to create, return that. - n.expandCreate(g, count) + n.expand(g, count) // Add in the diff if we have it if n.Diff != nil { @@ -1619,7 +1618,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.filterNouns(g, false), nil + return n.finalizeNouns(g, false), nil } if n.State != nil { @@ -1629,16 +1628,27 @@ func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { graphAddTainted(g, n.State) } - return n.filterNouns(g, true), nil + return n.finalizeNouns(g, true), nil } -// expandCreate expands this resource and adds the resources to the graph. -func (n *GraphNodeResource) expandCreate(g *depgraph.Graph, count int) { - // Create the list of nouns that we'd have to create - create := make([]*depgraph.Noun, 0, count) +// expand expands this resource and adds the resources to the graph. It +// adds both create and destroy resources. +func (n *GraphNodeResource) expand(g *depgraph.Graph, count int) { + // Create the list of nouns + result := make([]*depgraph.Noun, 0, count) + + // Build the key set so we know what is removed + var keys map[string]struct{} + if n.State != nil { + keys = make(map[string]struct{}) + for k, _ := range n.State.Resources { + keys[k] = struct{}{} + } + } // First thing, expand the counts that we have defined for our - // current config into the full set of resources. + // current config into the full set of resources that are being + // created. r := n.Config for i := 0; i < count; i++ { name := r.Id() @@ -1656,6 +1666,7 @@ func (n *GraphNodeResource) expandCreate(g *depgraph.Graph, count int) { // Lookup the resource state if s, ok := n.State.Resources[name]; ok { state = s + delete(keys, name) } if state == nil { @@ -1665,11 +1676,13 @@ func (n *GraphNodeResource) expandCreate(g *depgraph.Graph, count int) { // count > 1 to count == 1. k := r.Id() + ".0" state = n.State.Resources[k] + delete(keys, k) } else if i == 0 { // If count is greater than one, check for state // with just the ID, which might exist if we go // from count == 1 to count > 1 state = n.State.Resources[r.Id()] + delete(keys, r.Id()) } } } @@ -1689,10 +1702,9 @@ func (n *GraphNodeResource) expandCreate(g *depgraph.Graph, count int) { resource := n.copyResource(name) resource.State = state.Primary resource.Flags = flags - // TODO: we need the diff here... // Add the result - create = append(create, &depgraph.Noun{ + result = append(result, &depgraph.Noun{ Name: name, Meta: &GraphNodeResource{ Index: index, @@ -1702,7 +1714,27 @@ func (n *GraphNodeResource) expandCreate(g *depgraph.Graph, count int) { }) } - g.Nouns = append(g.Nouns, create...) + // Go over the leftover keys which are orphans (decreasing counts) + for k, _ := range keys { + rs := n.State.Resources[k] + + resource := n.copyResource(k) + resource.Config = NewResourceConfig(nil) + resource.State = rs.Primary + resource.Flags = FlagOrphan + + noun := &depgraph.Noun{ + Name: k, + Meta: &GraphNodeResource{ + Index: -1, + Resource: resource, + }, + } + + result = append(result, noun) + } + + g.Nouns = append(g.Nouns, result...) } // copyResource copies the Resource structure to assign to a subgraph. @@ -1716,7 +1748,7 @@ func (n *GraphNodeResource) copyResource(id string) *Resource { return &resource } -func (n *GraphNodeResource) filterNouns( +func (n *GraphNodeResource) finalizeNouns( g *depgraph.Graph, destroy bool) []*depgraph.Noun { result := make([]*depgraph.Noun, 0, len(g.Nouns)) for _, n := range g.Nouns { @@ -1744,6 +1776,15 @@ func (n *GraphNodeResource) filterNouns( continue } + // If this is an oprhan, we only care about it if we're destroying. + if rn.Resource.Flags&FlagOrphan != 0 { + if destroy { + result = append(result, n) + } + + continue + } + // If we're not destroying, then add it only if we don't // care about deploys. if !destroy { From 79520a19c4aaaac523dcaa48d97b0446b9a4a507 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 13:23:16 -0700 Subject: [PATCH 09/20] terraform: walk the resource properly for destroy --- terraform/context.go | 90 +++++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index c483094cf..34505e3cf 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -869,7 +869,8 @@ func (c *walkContext) planDestroyWalkFn() depgraph.WalkFunc { result := c.Meta.(*Plan) result.init() - return func(n *depgraph.Noun) error { + var walkFn depgraph.WalkFunc + walkFn = func(n *depgraph.Noun) error { switch m := n.Meta.(type) { case *GraphNodeModule: // Build another walkContext for this module and walk it. @@ -883,7 +884,13 @@ func (c *walkContext) planDestroyWalkFn() depgraph.WalkFunc { return wc.Walk() case *GraphNodeResource: + // If we're expanding, then expand the nodes, and then rewalk the graph + if m.ExpandMode > ResourceExpandNone { + return c.genericWalkResource(m, walkFn) + } + r := m.Resource + if r.State != nil && r.State.ID != "" { log.Printf("[DEBUG] %s: Making for destroy", r.Id) @@ -901,6 +908,8 @@ func (c *walkContext) planDestroyWalkFn() depgraph.WalkFunc { return nil } + + return walkFn } func (c *walkContext) refreshWalkFn() depgraph.WalkFunc { @@ -1135,43 +1144,7 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { // If we're expanding, then expand the nodes, and then rewalk the graph if rn.ExpandMode > ResourceExpandNone { - // Interpolate the count - rc := NewResourceConfig(rn.Config.RawCount) - rc.interpolate(c) - - // Expand the node to the actual resources - ns, 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 := walkFn(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} - } - - return nil + return c.genericWalkResource(rn, walkFn) } // Make sure that at least some resource configuration is set @@ -1209,6 +1182,47 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { return walkFn } +func (c *walkContext) genericWalkResource( + rn *GraphNodeResource, fn depgraph.WalkFunc) error { + // Interpolate the count + rc := NewResourceConfig(rn.Config.RawCount) + rc.interpolate(c) + + // Expand the node to the actual resources + ns, 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} + } + + return nil +} + // applyProvisioners is used to run any provisioners a resource has // defined after the resource creation has already completed. func (c *walkContext) applyProvisioners(r *Resource, is *InstanceState) error { From 941e27b9f3d09a1e77984b5b79c3cfd38ad90b67 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 13:24:38 -0700 Subject: [PATCH 10/20] terraform: Validate expands properly --- terraform/context.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 34505e3cf..a8894a73f 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -956,7 +956,8 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { meta.Children = make(map[string]*walkValidateMeta) } - return func(n *depgraph.Noun) error { + var walkFn depgraph.WalkFunc + walkFn = func(n *depgraph.Noun) error { // If it is the root node, ignore if n.Name == GraphRootNode { return nil @@ -988,6 +989,11 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { panic("resource should never be nil") } + // If we're expanding, then expand the nodes, and then rewalk the graph + if rn.ExpandMode > ResourceExpandNone { + return c.genericWalkResource(rn, walkFn) + } + // If it doesn't have a provider, that is a different problem if rn.Resource.Provider == nil { return nil @@ -1060,6 +1066,8 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { return nil } + + return walkFn } func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { From fa05b165ad488060669cb2902b6cd28d29341494 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 13:42:36 -0700 Subject: [PATCH 11/20] config: fix gob encode/decode for raw config and keys --- config/raw_config.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/config/raw_config.go b/config/raw_config.go index 3b08d9570..f3b144800 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -153,11 +153,15 @@ func (r *RawConfig) UnknownKeys() []string { // See GobEncode func (r *RawConfig) GobDecode(b []byte) error { - err := gob.NewDecoder(bytes.NewReader(b)).Decode(&r.Raw) + var data gobRawConfig + err := gob.NewDecoder(bytes.NewReader(b)).Decode(&data) if err != nil { return err } + r.Key = data.Key + r.Raw = data.Raw + return r.init() } @@ -166,10 +170,20 @@ func (r *RawConfig) GobDecode(b []byte) error { // tree of interpolated variables is recomputed on decode, since it is // referentially transparent. func (r *RawConfig) GobEncode() ([]byte, error) { + data := gobRawConfig{ + Key: r.Key, + Raw: r.Raw, + } + var buf bytes.Buffer - if err := gob.NewEncoder(&buf).Encode(r.Raw); err != nil { + if err := gob.NewEncoder(&buf).Encode(data); err != nil { return nil, err } return buf.Bytes(), nil } + +type gobRawConfig struct { + Key string + Raw map[string]interface{} +} From 039531e9cae1120db41868926619066aaf38060a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 13:54:04 -0700 Subject: [PATCH 12/20] terraform: dependencies in the graph from count properly show up --- terraform/graph.go | 6 +++- terraform/graph_test.go | 28 +++++++++++++++++++ .../graph-count-var-resource/main.tf | 9 ++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 terraform/test-fixtures/graph-count-var-resource/main.tf diff --git a/terraform/graph.go b/terraform/graph.go index ab82b7afd..26e5e6b58 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -1046,8 +1046,12 @@ func graphAddVariableDeps(g *depgraph.Graph) { case *GraphNodeResource: if m.Config != nil { + // Handle the count variables + vars := m.Config.RawCount.Variables + nounAddVariableDeps(g, n, vars, false) + // Handle the resource variables - vars := m.Config.RawConfig.Variables + vars = m.Config.RawConfig.Variables nounAddVariableDeps(g, n, vars, false) } diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 7fdd7b044..1eaff78f4 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -43,6 +43,21 @@ func TestGraph_count(t *testing.T) { } } +func TestGraph_varResource(t *testing.T) { + m := testModule(t, "graph-count-var-resource") + + g, err := Graph(&GraphOpts{Module: m}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphCountVarResourceStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestGraph_cycle(t *testing.T) { m := testModule(t, "graph-cycle") @@ -964,6 +979,19 @@ root root -> aws_load_balancer.weblb ` +const testTerraformGraphCountVarResourceStr = ` +root: root +aws_instance.foo +aws_instance.web + aws_instance.web -> aws_instance.foo +aws_load_balancer.weblb + aws_load_balancer.weblb -> aws_instance.web +root + root -> aws_instance.foo + root -> aws_instance.web + root -> aws_load_balancer.weblb +` + const testTerraformGraphDependsStr = ` root: root aws_instance.db diff --git a/terraform/test-fixtures/graph-count-var-resource/main.tf b/terraform/test-fixtures/graph-count-var-resource/main.tf new file mode 100644 index 000000000..9c7407fa5 --- /dev/null +++ b/terraform/test-fixtures/graph-count-var-resource/main.tf @@ -0,0 +1,9 @@ +resource "aws_instance" "foo" {} + +resource "aws_instance" "web" { + count = "${aws_instance.foo.bar}" +} + +resource "aws_load_balancer" "weblb" { + members = "${aws_instance.web.*.id}" +} From ced4125037028ff90194b24ffaf3b1d873427382 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 15:47:00 -0700 Subject: [PATCH 13/20] teraform: test that count can be a variable --- terraform/context_test.go | 26 +++++++++++++++++++ terraform/terraform_test.go | 21 +++++++++++++++ .../test-fixtures/plan-count-var/main.tf | 10 +++++++ 3 files changed, 57 insertions(+) create mode 100644 terraform/test-fixtures/plan-count-var/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 6ebd9ac0b..1313a6f27 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2428,6 +2428,32 @@ func TestContextPlan_count(t *testing.T) { } } +func TestContextPlan_countVar(t *testing.T) { + m := testModule(t, "plan-count-var") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Variables: map[string]string{ + "count": "3", + }, + }) + + plan, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanCountVarStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_countDecreaseToOne(t *testing.T) { m := testModule(t, "plan-count-dec") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index cdcb6ea20..350a2354d 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -477,6 +477,27 @@ STATE: ` +const testTerraformPlanCountVarStr = ` +DIFF: + +CREATE: aws_instance.bar + foo: "" => "foo,foo,foo" + type: "" => "aws_instance" +CREATE: aws_instance.foo.0 + foo: "" => "foo" + type: "" => "aws_instance" +CREATE: aws_instance.foo.1 + foo: "" => "foo" + type: "" => "aws_instance" +CREATE: aws_instance.foo.2 + foo: "" => "foo" + type: "" => "aws_instance" + +STATE: + + +` + const testTerraformPlanCountDecreaseStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-count-var/main.tf b/terraform/test-fixtures/plan-count-var/main.tf new file mode 100644 index 000000000..f64b9655b --- /dev/null +++ b/terraform/test-fixtures/plan-count-var/main.tf @@ -0,0 +1,10 @@ +variable "count" {} + +resource "aws_instance" "foo" { + count = "${var.count}" + foo = "foo" +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.*.foo}" +} From 53d05cb81fdca8ebcc83d9ed9434f05472942474 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 16:21:17 -0700 Subject: [PATCH 14/20] terraform: counts can't be computed --- terraform/context_test.go | 17 +++++++++++++++++ terraform/graph.go | 10 +++++++++- .../test-fixtures/plan-count-computed/main.tf | 8 ++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 terraform/test-fixtures/plan-count-computed/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 1313a6f27..b4ea97824 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2428,6 +2428,23 @@ func TestContextPlan_count(t *testing.T) { } } +func TestContextPlan_countComputed(t *testing.T) { + m := testModule(t, "plan-count-computed") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + _, err := ctx.Plan(nil) + if err == nil { + t.Fatal("should error") + } +} + func TestContextPlan_countVar(t *testing.T) { m := testModule(t, "plan-count-var") p := testProvider("aws") diff --git a/terraform/graph.go b/terraform/graph.go index 26e5e6b58..ae34d6452 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -1595,7 +1595,15 @@ func (p *graphSharedProvider) MergeConfig( // Expand will expand this node into a subgraph if Expand is set. func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { - // Expand the count out, which should be interpolated at this point + // If the count configuration is empty then it means that the + // count is computed. In this case, we set the count to one + // but set a flag telling upstream that we're computing. + if len(n.Config.RawConfig.Config()) == 0 { + return nil, fmt.Errorf( + "%s: computed count attribute not allowed", n.Resource.Id) + } + + // Expand the count out, which should be interpolated at this point. count, err := n.Config.Count() if err != nil { return nil, err diff --git a/terraform/test-fixtures/plan-count-computed/main.tf b/terraform/test-fixtures/plan-count-computed/main.tf new file mode 100644 index 000000000..8a029236b --- /dev/null +++ b/terraform/test-fixtures/plan-count-computed/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + num = "2" + compute = "foo" +} + +resource "aws_instance" "bar" { + count = "${aws_instance.foo.foo}" +} From 50906781682a690e5db78a41deb063554d408559 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 16:30:46 -0700 Subject: [PATCH 15/20] config: validate that only proper variables can be in the count --- config/config.go | 20 ++++++++++++++++++ config/config_test.go | 21 +++++++++++++++++++ .../validate-count-module-var/main.tf | 7 +++++++ .../validate-count-resource-var/main.tf | 5 +++++ .../validate-count-user-var/main.tf | 5 +++++ 5 files changed, 58 insertions(+) create mode 100644 config/test-fixtures/validate-count-module-var/main.tf create mode 100644 config/test-fixtures/validate-count-resource-var/main.tf create mode 100644 config/test-fixtures/validate-count-user-var/main.tf diff --git a/config/config.go b/config/config.go index 67e0a2e7e..ef8be866d 100644 --- a/config/config.go +++ b/config/config.go @@ -255,6 +255,26 @@ func (c *Config) Validate() error { // Validate resources for n, r := range resources { + // Verify count variables + for _, v := range r.RawCount.Variables { + switch v.(type) { + case *ModuleVariable: + errs = append(errs, fmt.Errorf( + "%s: resource count can't reference module variable: %s", + n, + v.FullKey())) + case *ResourceVariable: + errs = append(errs, fmt.Errorf( + "%s: resource count can't reference resource variable: %s", + n, + v.FullKey())) + case *UserVariable: + // Good + default: + panic("Unknown type in count var: " + n) + } + } + for _, d := range r.DependsOn { if _, ok := resources[d]; !ok { errs = append(errs, fmt.Errorf( diff --git a/config/config_test.go b/config/config_test.go index a31df6b5a..636af0b58 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -53,6 +53,27 @@ func TestConfigValidate_badDependsOn(t *testing.T) { } } +func TestConfigValidate_countModuleVar(t *testing.T) { + c := testConfig(t, "validate-count-module-var") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + +func TestConfigValidate_countResourceVar(t *testing.T) { + c := testConfig(t, "validate-count-resource-var") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + +func TestConfigValidate_countUserVar(t *testing.T) { + c := testConfig(t, "validate-count-user-var") + if err := c.Validate(); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestConfigValidate_dupModule(t *testing.T) { c := testConfig(t, "validate-dup-module") if err := c.Validate(); err == nil { diff --git a/config/test-fixtures/validate-count-module-var/main.tf b/config/test-fixtures/validate-count-module-var/main.tf new file mode 100644 index 000000000..16d901e3a --- /dev/null +++ b/config/test-fixtures/validate-count-module-var/main.tf @@ -0,0 +1,7 @@ +module "foo" { + source = "./bar" +} + +resource "aws_instance" "web" { + count = "${module.foo.bar}" +} diff --git a/config/test-fixtures/validate-count-resource-var/main.tf b/config/test-fixtures/validate-count-resource-var/main.tf new file mode 100644 index 000000000..e352a916c --- /dev/null +++ b/config/test-fixtures/validate-count-resource-var/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "foo" {} + +resource "aws_instance" "web" { + count = "${aws_instance.foo.bar}" +} diff --git a/config/test-fixtures/validate-count-user-var/main.tf b/config/test-fixtures/validate-count-user-var/main.tf new file mode 100644 index 000000000..7b27c2202 --- /dev/null +++ b/config/test-fixtures/validate-count-user-var/main.tf @@ -0,0 +1,5 @@ +variable "foo" {} + +resource "aws_instance" "web" { + count = "${var.foo}" +} From 581d1dee8c8b5d0a3cc481d818316ae989499a92 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 16:32:11 -0700 Subject: [PATCH 16/20] terraform: remove jank computed check for count --- terraform/graph.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/terraform/graph.go b/terraform/graph.go index ae34d6452..49cf68edb 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -1595,14 +1595,6 @@ func (p *graphSharedProvider) MergeConfig( // Expand will expand this node into a subgraph if Expand is set. func (n *GraphNodeResource) Expand() ([]*depgraph.Noun, error) { - // If the count configuration is empty then it means that the - // count is computed. In this case, we set the count to one - // but set a flag telling upstream that we're computing. - if len(n.Config.RawConfig.Config()) == 0 { - return nil, fmt.Errorf( - "%s: computed count attribute not allowed", n.Resource.Id) - } - // Expand the count out, which should be interpolated at this point. count, err := n.Config.Count() if err != nil { From dd1430302250df25311c12428abbb9df88a1ea35 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 16:51:20 -0700 Subject: [PATCH 17/20] config: validate that count is an int --- config/config.go | 12 +++++++ config/config_test.go | 14 ++++++++ config/raw_config.go | 36 ++++++++++--------- .../test-fixtures/validate-count-int/main.tf | 5 +++ .../validate-count-not-int/main.tf | 5 +++ 5 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 config/test-fixtures/validate-count-int/main.tf create mode 100644 config/test-fixtures/validate-count-not-int/main.tf diff --git a/config/config.go b/config/config.go index ef8be866d..4d6ea1a85 100644 --- a/config/config.go +++ b/config/config.go @@ -275,6 +275,18 @@ func (c *Config) Validate() error { } } + // Interpolate with a fixed number to verify that its a number + r.RawCount.interpolate(func(Interpolation) (string, error) { + return "5", nil + }) + _, err := strconv.ParseInt(r.RawCount.Value().(string), 0, 0) + if err != nil { + errs = append(errs, fmt.Errorf( + "%s: resource count must be an integer", + n)) + } + r.RawCount.init() + for _, d := range r.DependsOn { if _, ok := resources[d]; !ok { errs = append(errs, fmt.Errorf( diff --git a/config/config_test.go b/config/config_test.go index 636af0b58..76c12d4a0 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -53,6 +53,13 @@ func TestConfigValidate_badDependsOn(t *testing.T) { } } +func TestConfigValidate_countInt(t *testing.T) { + c := testConfig(t, "validate-count-int") + if err := c.Validate(); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestConfigValidate_countModuleVar(t *testing.T) { c := testConfig(t, "validate-count-module-var") if err := c.Validate(); err == nil { @@ -60,6 +67,13 @@ func TestConfigValidate_countModuleVar(t *testing.T) { } } +func TestConfigValidate_countNotInt(t *testing.T) { + c := testConfig(t, "validate-count-not-int") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_countResourceVar(t *testing.T) { c := testConfig(t, "validate-count-resource-var") if err := c.Validate(); err == nil { diff --git a/config/raw_config.go b/config/raw_config.go index f3b144800..e1c6c4108 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -79,24 +79,9 @@ func (r *RawConfig) Config() map[string]interface{} { // // If a variable key is missing, this will panic. func (r *RawConfig) Interpolate(vs map[string]string) error { - config, err := copystructure.Copy(r.Raw) - if err != nil { - return err - } - r.config = config.(map[string]interface{}) - - fn := func(i Interpolation) (string, error) { + return r.interpolate(func(i Interpolation) (string, error) { return i.Interpolate(vs) - } - - w := &interpolationWalker{F: fn, Replace: true} - err = reflectwalk.Walk(r.config, w) - if err != nil { - return err - } - - r.unknownKeys = w.unknownKeys - return nil + }) } func (r *RawConfig) init() error { @@ -126,6 +111,23 @@ func (r *RawConfig) init() error { return nil } +func (r *RawConfig) interpolate(fn interpolationWalkerFunc) error { + config, err := copystructure.Copy(r.Raw) + if err != nil { + return err + } + r.config = config.(map[string]interface{}) + + w := &interpolationWalker{F: fn, Replace: true} + err = reflectwalk.Walk(r.config, w) + if err != nil { + return err + } + + r.unknownKeys = w.unknownKeys + return nil +} + func (r *RawConfig) merge(r2 *RawConfig) *RawConfig { rawRaw, err := copystructure.Copy(r.Raw) if err != nil { diff --git a/config/test-fixtures/validate-count-int/main.tf b/config/test-fixtures/validate-count-int/main.tf new file mode 100644 index 000000000..7b27c2202 --- /dev/null +++ b/config/test-fixtures/validate-count-int/main.tf @@ -0,0 +1,5 @@ +variable "foo" {} + +resource "aws_instance" "web" { + count = "${var.foo}" +} diff --git a/config/test-fixtures/validate-count-not-int/main.tf b/config/test-fixtures/validate-count-not-int/main.tf new file mode 100644 index 000000000..af34d34c6 --- /dev/null +++ b/config/test-fixtures/validate-count-not-int/main.tf @@ -0,0 +1,5 @@ +variable "foo" {} + +resource "aws_instance" "web" { + count = "nope${var.foo}" +} From e4ba7373924237f3554578f9613ddd027160c0fa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 17:14:25 -0700 Subject: [PATCH 18/20] terraform: validate count is non-negative --- terraform/context.go | 17 +++++++++++++++++ terraform/context_test.go | 19 +++++++++++++++++++ .../validate-count-negative/main.tf | 3 +++ 3 files changed, 39 insertions(+) create mode 100644 terraform/test-fixtures/validate-count-negative/main.tf diff --git a/terraform/context.go b/terraform/context.go index a8894a73f..5dd2d2f05 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -991,6 +991,23 @@ func (c *walkContext) validateWalkFn() depgraph.WalkFunc { // If we're expanding, then expand the nodes, and then rewalk the graph if rn.ExpandMode > ResourceExpandNone { + // Interpolate the count and verify it is non-negative + rc := NewResourceConfig(rn.Config.RawCount) + rc.interpolate(c) + count, err := rn.Config.Count() + if err == nil { + if count < 0 { + err = fmt.Errorf( + "%s error: count must be positive", rn.Resource.Id) + } + } + if err != nil { + l.Lock() + defer l.Unlock() + meta.Errs = append(meta.Errs, err) + return nil + } + return c.genericWalkResource(rn, walkFn) } diff --git a/terraform/context_test.go b/terraform/context_test.go index b4ea97824..d44c499c2 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -107,6 +107,25 @@ func TestContextValidate_badVar(t *testing.T) { } } +func TestContextValidate_countNegative(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "validate-count-negative") + c := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + w, e := c.Validate() + if len(w) > 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) == 0 { + t.Fatalf("bad: %#v", e) + } +} + func TestContextValidate_moduleBadResource(t *testing.T) { m := testModule(t, "validate-module-bad-rc") p := testProvider("aws") diff --git a/terraform/test-fixtures/validate-count-negative/main.tf b/terraform/test-fixtures/validate-count-negative/main.tf new file mode 100644 index 000000000..d5bb04653 --- /dev/null +++ b/terraform/test-fixtures/validate-count-negative/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "test" { + count = "-5" +} From 221e40a3a9e190077ef4bc9262911b0fe0807743 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 17:18:40 -0700 Subject: [PATCH 19/20] terraform: test count == zero --- terraform/context.go | 6 ++++- terraform/context_test.go | 23 +++++++++++++++++++ terraform/terraform_test.go | 12 ++++++++++ .../test-fixtures/plan-count-zero/main.tf | 8 +++++++ 4 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 terraform/test-fixtures/plan-count-zero/main.tf diff --git a/terraform/context.go b/terraform/context.go index 5dd2d2f05..2f6ce15ee 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1559,7 +1559,6 @@ func (c *walkContext) computeResourceMultiVariable( // TODO: Not use only root module module := c.Context.state.RootModule() - // TODO: handle computed here count, err := cr.Count() if err != nil { return "", fmt.Errorf( @@ -1568,6 +1567,11 @@ func (c *walkContext) computeResourceMultiVariable( err) } + // If we have no count, return empty + if count == 0 { + return "", nil + } + var values []string for i := 0; i < count; i++ { id := fmt.Sprintf("%s.%d", v.ResourceId(), i) diff --git a/terraform/context_test.go b/terraform/context_test.go index d44c499c2..599acef97 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2490,6 +2490,29 @@ func TestContextPlan_countVar(t *testing.T) { } } +func TestContextPlan_countZero(t *testing.T) { + m := testModule(t, "plan-count-zero") + 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(testTerraformPlanCountZeroStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_countDecreaseToOne(t *testing.T) { m := testModule(t, "plan-count-dec") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 350a2354d..f0c1096ce 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -477,6 +477,18 @@ STATE: ` +const testTerraformPlanCountZeroStr = ` +DIFF: + +CREATE: aws_instance.bar + foo: "" => "" + type: "" => "aws_instance" + +STATE: + + +` + const testTerraformPlanCountVarStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-count-zero/main.tf b/terraform/test-fixtures/plan-count-zero/main.tf new file mode 100644 index 000000000..4845cbb0b --- /dev/null +++ b/terraform/test-fixtures/plan-count-zero/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + count = 0 + foo = "foo" +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.*.foo}" +} From 66c58788fe6031fb25a58adf718c05d54ce759be Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Oct 2014 17:24:22 -0700 Subject: [PATCH 20/20] terraform: test count = 1 variable access --- terraform/context.go | 13 +++++++---- terraform/context_test.go | 23 +++++++++++++++++++ terraform/terraform_test.go | 15 ++++++++++++ .../plan-count-one-index/main.tf | 8 +++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 terraform/test-fixtures/plan-count-one-index/main.tf diff --git a/terraform/context.go b/terraform/context.go index 2f6ce15ee..58cf0542b 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1498,10 +1498,15 @@ func (c *walkContext) computeResourceVariable( r, ok := module.Resources[id] if !ok { - return "", fmt.Errorf( - "Resource '%s' not found for variable '%s'", - id, - v.FullKey()) + if v.Multi && v.Index == 0 { + r, ok = module.Resources[v.ResourceId()] + } + if !ok { + return "", fmt.Errorf( + "Resource '%s' not found for variable '%s'", + id, + v.FullKey()) + } } if r.Primary == nil { diff --git a/terraform/context_test.go b/terraform/context_test.go index 599acef97..29c927838 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2513,6 +2513,29 @@ func TestContextPlan_countZero(t *testing.T) { } } +func TestContextPlan_countOneIndex(t *testing.T) { + m := testModule(t, "plan-count-one-index") + 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(testTerraformPlanCountOneIndexStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_countDecreaseToOne(t *testing.T) { m := testModule(t, "plan-count-dec") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index f0c1096ce..8041b139e 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -477,6 +477,21 @@ STATE: ` +const testTerraformPlanCountOneIndexStr = ` +DIFF: + +CREATE: aws_instance.bar + foo: "" => "foo" + type: "" => "aws_instance" +CREATE: aws_instance.foo + foo: "" => "foo" + type: "" => "aws_instance" + +STATE: + + +` + const testTerraformPlanCountZeroStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-count-one-index/main.tf b/terraform/test-fixtures/plan-count-one-index/main.tf new file mode 100644 index 000000000..58d4acf71 --- /dev/null +++ b/terraform/test-fixtures/plan-count-one-index/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + count = 1 + foo = "foo" +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.0.foo}" +}