From 3337a625afb571350ede79f839333273a5369d35 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 20:11:58 -0700 Subject: [PATCH 1/7] config: support count meta-parameter --- config/config.go | 1 + config/loader_libucl.go | 18 ++++++++++++++++++ config/loader_test.go | 13 +++++++------ config/test-fixtures/basic.tf | 1 + 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/config/config.go b/config/config.go index 8c8b80fa5dcb..63c5651a5967 100644 --- a/config/config.go +++ b/config/config.go @@ -31,6 +31,7 @@ type ProviderConfig struct { type Resource struct { Name string Type string + Count int RawConfig *RawConfig } diff --git a/config/loader_libucl.go b/config/loader_libucl.go index 0f80da50ae5e..4d5644a26870 100644 --- a/config/loader_libucl.go +++ b/config/loader_libucl.go @@ -253,6 +253,9 @@ func loadResourcesLibucl(o *libucl.Object) ([]*Resource, error) { err) } + // Remove the "count" from the config, since we treat that special + delete(config, "count") + rawConfig, err := NewRawConfig(config) if err != nil { return nil, fmt.Errorf( @@ -262,9 +265,24 @@ func loadResourcesLibucl(o *libucl.Object) ([]*Resource, error) { err) } + // If we have a count, then figure it out + var count int = 1 + if o := r.Get("count"); o != nil { + err = o.Decode(&count) + o.Close() + if err != nil { + return nil, fmt.Errorf( + "Error parsing count for %s[%s]: %s", + t.Key(), + r.Key(), + err) + } + } + result = append(result, &Resource{ Name: r.Key(), Type: t.Key(), + Count: count, RawConfig: rawConfig, }) } diff --git a/config/loader_test.go b/config/loader_test.go index 6742d44c2598..4a9a6f939379 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -145,9 +145,10 @@ func resourcesStr(rs []*Resource) string { result := "" for _, r := range rs { result += fmt.Sprintf( - "%s[%s]\n", + "%s[%s] (x%d)\n", r.Type, - r.Name) + r.Name, + r.Count) ks := make([]string, 0, len(r.RawConfig.Raw)) for k, _ := range r.RawConfig.Raw { @@ -229,8 +230,8 @@ do ` const basicResourcesStr = ` -aws_security_group[firewall] -aws_instance[web] +aws_security_group[firewall] (x5) +aws_instance[web] (x1) ami network_interface security_groups @@ -251,8 +252,8 @@ aws ` const importResourcesStr = ` -aws_security_group[db] -aws_security_group[web] +aws_security_group[db] (x1) +aws_security_group[web] (x1) ` const importVariablesStr = ` diff --git a/config/test-fixtures/basic.tf b/config/test-fixtures/basic.tf index 04d71bbbf41e..f389b1bf058e 100644 --- a/config/test-fixtures/basic.tf +++ b/config/test-fixtures/basic.tf @@ -13,6 +13,7 @@ provider "do" { } resource "aws_security_group" "firewall" { + count = 5 } resource aws_instance "web" { From 5e79ddf7c662b4bb8083dfce6af8a80539b0da6a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 20:41:26 -0700 Subject: [PATCH 2/7] config: detect variables in form of resource.name.*.blah --- config/loader_test.go | 4 ++++ config/test-fixtures/basic.tf | 4 ++++ config/variable.go | 2 +- config/variable_test.go | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/config/loader_test.go b/config/loader_test.go index 4a9a6f939379..b9340d3a0801 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -238,6 +238,10 @@ aws_instance[web] (x1) vars resource: aws_security_group.firewall.foo user: var.foo +aws_instance[db] (x1) + security_groups + vars + resource: aws_security_group.firewall.*.id ` const basicVariablesStr = ` diff --git a/config/test-fixtures/basic.tf b/config/test-fixtures/basic.tf index f389b1bf058e..07aa5ff955ed 100644 --- a/config/test-fixtures/basic.tf +++ b/config/test-fixtures/basic.tf @@ -28,3 +28,7 @@ resource aws_instance "web" { description = "Main network interface" } } + +resource "aws_instance" "db" { + security_groups = "${aws_security_group.firewall.*.id}" +} diff --git a/config/variable.go b/config/variable.go index 841981a39bec..40a3b28df537 100644 --- a/config/variable.go +++ b/config/variable.go @@ -13,7 +13,7 @@ import ( var varRegexp *regexp.Regexp func init() { - varRegexp = regexp.MustCompile(`(?i)(\$+)\{([-.a-z0-9_]+)\}`) + varRegexp = regexp.MustCompile(`(?i)(\$+)\{([*-.a-z0-9_]+)\}`) } // ReplaceVariables takes a configuration and a mapping of variables diff --git a/config/variable_test.go b/config/variable_test.go index 88fdb23e1085..7e3b5fc11074 100644 --- a/config/variable_test.go +++ b/config/variable_test.go @@ -87,6 +87,22 @@ func TestVariableDetectWalker_resource(t *testing.T) { } } +func TestVariableDetectWalker_resourceMulti(t *testing.T) { + w := new(variableDetectWalker) + + str := `foo ${ec2.foo.*.bar}` + if err := w.Primitive(reflect.ValueOf(str)); err != nil { + t.Fatalf("err: %s", err) + } + + if len(w.Variables) != 1 { + t.Fatalf("bad: %#v", w.Variables) + } + if w.Variables["ec2.foo.*.bar"].(*ResourceVariable).FullKey() != "ec2.foo.*.bar" { + t.Fatalf("bad: %#v", w.Variables) + } +} + func TestVariableDetectWalker_bad(t *testing.T) { w := new(variableDetectWalker) From e7b7644cbfb8ea3c657f720d1c98170cd7706d38 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 20:42:29 -0700 Subject: [PATCH 3/7] terraform: turn multi-counts into multiple nodes --- depgraph/graph.go | 30 +++++++- terraform/graph.go | 84 ++++++++++++++++----- terraform/graph_test.go | 34 +++++++++ terraform/test-fixtures/graph-count/main.tf | 7 ++ 4 files changed, 137 insertions(+), 18 deletions(-) create mode 100644 terraform/test-fixtures/graph-count/main.tf diff --git a/depgraph/graph.go b/depgraph/graph.go index 4e34e361fefc..acca4ce6c9fe 100644 --- a/depgraph/graph.go +++ b/depgraph/graph.go @@ -9,6 +9,7 @@ import ( "bytes" "fmt" "sort" + "strings" "sync" "github.com/hashicorp/terraform/digraph" @@ -42,7 +43,34 @@ type ValidateError struct { } func (v *ValidateError) Error() string { - return "The depedency graph is not valid" + var msgs []string + + if v.MissingRoot { + msgs = append(msgs, "The graph has no single root") + } + + for _, n := range v.Unreachable { + msgs = append(msgs, fmt.Sprintf( + "Unreachable node: %s", n.Name)) + } + + for _, c := range v.Cycles { + cycleNodes := make([]string, len(c)) + for i, n := range c { + cycleNodes[i] = n.Name + } + + msgs = append(msgs, fmt.Sprintf( + "Cycle: %s", strings.Join(cycleNodes, " -> "))) + } + + for i, m := range msgs { + msgs[i] = fmt.Sprintf("* %s", m) + } + + return fmt.Sprintf( + "The dependency graph is not valid:\n\n%s", + strings.Join(msgs, "\n")) } // ConstraintError is used to return detailed violation diff --git a/terraform/graph.go b/terraform/graph.go index 486996cc4ef4..01ef19cfcb71 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -45,8 +45,12 @@ type GraphOpts struct { // graph. This node is just a placemarker and has no associated functionality. const GraphRootNode = "root" -// GraphNodeResource is a node type in the graph that represents a resource. +// GraphNodeResource is a node type in the graph that represents a resource +// that will be created or managed. Unlike the GraphNodeResourceMeta node, +// this represents a _single_, _resource_ to be managed, not a set of resources +// or a component of a resource. type GraphNodeResource struct { + Index int Type string Config *config.Resource Orphan bool @@ -54,6 +58,15 @@ type GraphNodeResource struct { ResourceProviderID string } +// 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 { + 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 { @@ -152,18 +165,57 @@ func graphAddConfigResources( } } - noun := &depgraph.Noun{ - Name: r.Id(), - Meta: &GraphNodeResource{ - Type: r.Type, - Config: r, - Resource: &Resource{ - Id: r.Id(), - State: state, + resourceNouns := make([]*depgraph.Noun, r.Count) + for i := 0; i < r.Count; i++ { + name := r.Id() + + // 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) + } + + resourceNouns[i] = &depgraph.Noun{ + Name: name, + Meta: &GraphNodeResource{ + Type: r.Type, + Config: r, + Resource: &Resource{ + Id: name, + State: state, + }, }, - }, + } + } + + // 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{ + Name: r.Id(), + 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 + } + + for _, n := range resourceNouns { + nouns[n.Name] = n } - nouns[noun.Name] = noun } // Build the list of nouns that we iterate over @@ -357,7 +409,10 @@ func graphAddProviderConfigs(g *depgraph.Graph, c *config.Config) { nounsList := make([]*depgraph.Noun, 0, 2) pcNouns := make(map[string]*depgraph.Noun) for _, noun := range g.Nouns { - resourceNode := noun.Meta.(*GraphNodeResource) + resourceNode, ok := noun.Meta.(*GraphNodeResource) + if !ok { + continue + } // Look up the provider config for this resource pcName := config.ProviderConfigName(resourceNode.Type, c.ProviderConfigs) @@ -401,11 +456,6 @@ func graphAddProviderConfigs(g *depgraph.Graph, c *config.Config) { func graphAddRoot(g *depgraph.Graph) { root := &depgraph.Noun{Name: GraphRootNode} for _, n := range g.Nouns { - // The root only needs to depend on all the resources - if _, ok := n.Meta.(*GraphNodeResource); !ok { - continue - } - root.Deps = append(root.Deps, &depgraph.Dependency{ Name: n.Name, Source: root, diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 4e2b89e2e895..d395f0b35b34 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -27,6 +27,21 @@ func TestGraph_configRequired(t *testing.T) { } } +func TestGraph_count(t *testing.T) { + config := testConfig(t, "graph-count") + + g, err := Graph(&GraphOpts{Config: config}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphCountStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestGraph_cycle(t *testing.T) { config := testConfig(t, "graph-cycle") @@ -226,6 +241,25 @@ root root -> openstack_floating_ip.random ` +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 + root -> aws_instance.web + root -> aws_instance.web.0 + root -> aws_instance.web.1 + root -> aws_instance.web.2 + root -> aws_load_balancer.weblb +` + const testTerraformGraphDiffStr = ` root: root aws_instance.foo diff --git a/terraform/test-fixtures/graph-count/main.tf b/terraform/test-fixtures/graph-count/main.tf new file mode 100644 index 000000000000..b35995faacbf --- /dev/null +++ b/terraform/test-fixtures/graph-count/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "web" { + count = 3 +} + +resource "aws_load_balancer" "weblb" { + members = "${aws_instance.web.*.id}" +} From ba144ef93305a5e80a2b1f46d20e484adf29e519 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 20:51:31 -0700 Subject: [PATCH 4/7] terraform: clean up root deps on the graph --- terraform/graph.go | 18 ++++++++++++++++++ terraform/graph_test.go | 3 --- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/terraform/graph.go b/terraform/graph.go index 01ef19cfcb71..8c7846a769f7 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -168,16 +168,19 @@ func graphAddConfigResources( 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 } resourceNouns[i] = &depgraph.Noun{ Name: name, Meta: &GraphNodeResource{ + Index: index, Type: r.Type, Config: r, Resource: &Resource{ @@ -391,6 +394,7 @@ func graphAddOrphans(g *depgraph.Graph, c *config.Config, s *State) { noun := &depgraph.Noun{ Name: k, Meta: &GraphNodeResource{ + Index: -1, Type: rs.Type, Orphan: true, Resource: &Resource{ @@ -456,6 +460,20 @@ func graphAddProviderConfigs(g *depgraph.Graph, c *config.Config) { func graphAddRoot(g *depgraph.Graph) { root := &depgraph.Noun{Name: GraphRootNode} for _, n := range g.Nouns { + switch m := n.Meta.(type) { + case *GraphNodeResource: + // If the resource is part of a group, we don't need to make a dep + if m.Index != -1 { + continue + } + 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. + continue + } + root.Deps = append(root.Deps, &depgraph.Dependency{ Name: n.Name, Source: root, diff --git a/terraform/graph_test.go b/terraform/graph_test.go index d395f0b35b34..f8d460e1c849 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -254,9 +254,6 @@ aws_load_balancer.weblb aws_load_balancer.weblb -> aws_instance.web root root -> aws_instance.web - root -> aws_instance.web.0 - root -> aws_instance.web.1 - root -> aws_instance.web.2 root -> aws_load_balancer.weblb ` From a616218d13eeeb65cc88bc68815a0ed3529cec80 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 21:24:17 -0700 Subject: [PATCH 5/7] terraform: planning and applying multi-count resources tests --- terraform/context.go | 81 ++++++++++++++++++++++ terraform/context_test.go | 27 ++++++++ terraform/graph.go | 4 +- terraform/terraform_test.go | 27 ++++++++ terraform/test-fixtures/plan-count/main.tf | 8 +++ 5 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 terraform/test-fixtures/plan-count/main.tf diff --git a/terraform/context.go b/terraform/context.go index ebf4d7bd3813..3f68c245aa96 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "log" + "strings" "sync" "sync/atomic" @@ -509,6 +510,9 @@ func (c *Context) genericWalkFn( vars[fmt.Sprintf("var.%s", k)] = v } + // This will keep track of the counts of multi-count resources + counts := make(map[string]int) + // This will keep track of whether we're stopped or not var stop uint32 = 0 @@ -523,8 +527,20 @@ func (c *Context) genericWalkFn( return nil } + // Calculate any aggregate interpolated variables if we have to. + // Aggregate variables (such as "test_instance.foo.*.id") are not + // pre-computed since the fanout would be expensive. We calculate + // them on-demand here. + computeAggregateVars(&l, n, counts, vars) + switch m := n.Meta.(type) { case *GraphNodeResource: + case *GraphNodeResourceMeta: + // Record the count and then just ignore + l.Lock() + counts[m.ID] = m.Count + l.Unlock() + return nil case *GraphNodeResourceProvider: var rc *ResourceConfig if m.Config != nil { @@ -543,6 +559,8 @@ func (c *Context) genericWalkFn( } return nil + default: + panic(fmt.Sprintf("unknown graph node: %#v", n.Meta)) } rn := n.Meta.(*GraphNodeResource) @@ -603,3 +621,66 @@ func (c *Context) genericWalkFn( return nil } } + +func computeAggregateVars( + l *sync.RWMutex, + n *depgraph.Noun, + cs map[string]int, + vs map[string]string) { + var ivars map[string]config.InterpolatedVariable + switch m := n.Meta.(type) { + case *GraphNodeResource: + ivars = m.Config.RawConfig.Variables + case *GraphNodeResourceProvider: + if m.Config != nil { + ivars = m.Config.RawConfig.Variables + } + } + if len(ivars) == 0 { + return + } + + for _, v := range ivars { + rv, ok := v.(*config.ResourceVariable) + if !ok { + continue + } + + idx := strings.Index(rv.Field, ".") + if idx == -1 { + // It isn't an aggregated var + continue + } + if rv.Field[:idx] != "*" { + // It isn't an aggregated var + continue + } + field := rv.Field[idx+1:] + + // Get the meta node so that we can determine the count + key := fmt.Sprintf("%s.%s", rv.Type, rv.Name) + l.RLock() + count, ok := cs[key] + l.RUnlock() + if !ok { + // This should never happen due to semantic checks + panic(fmt.Sprintf( + "non-existent resource variable access: %s\n\n%#v", key, rv)) + } + + var values []string + for i := 0; i < count; i++ { + key := fmt.Sprintf( + "%s.%s.%d.%s", + rv.Type, + rv.Name, + i, + field) + if v, ok := vs[key]; ok { + values = append(values, v) + } + } + + vs[rv.FullKey()] = strings.Join(values, ",") + } +} diff --git a/terraform/context_test.go b/terraform/context_test.go index 3f51483de34b..e2eb5826a0b4 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -641,6 +641,33 @@ func TestContextPlan_computed(t *testing.T) { } } +func TestContextPlan_count(t *testing.T) { + c := testConfig(t, "plan-count") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + plan, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + if len(plan.Diff.Resources) < 6 { + t.Fatalf("bad: %#v", plan.Diff.Resources) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanCountStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_destroy(t *testing.T) { c := testConfig(t, "plan-destroy") p := testProvider("aws") diff --git a/terraform/graph.go b/terraform/graph.go index 8c7846a769f7..2dd3a06168cb 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -62,6 +62,7 @@ type GraphNodeResource struct { // 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 @@ -197,7 +198,8 @@ func graphAddConfigResources( metaNoun := &depgraph.Noun{ Name: r.Id(), Meta: &GraphNodeResourceMeta{ - Name: r.Id(), + ID: r.Id(), + Name: r.Name, Type: r.Type, Count: r.Count, }, diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 04d79de8dce5..7183b20c7b14 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -155,6 +155,33 @@ STATE: ` +const testTerraformPlanCountStr = ` +DIFF: + +UPDATE: aws_instance.bar + foo: "" => "foo,foo,foo,foo,foo" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.0 + foo: "" => "foo" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.1 + foo: "" => "foo" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.2 + foo: "" => "foo" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.3 + foo: "" => "foo" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.4 + foo: "" => "foo" + type: "" => "aws_instance" + +STATE: + + +` + const testTerraformPlanDestroyStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-count/main.tf b/terraform/test-fixtures/plan-count/main.tf new file mode 100644 index 000000000000..32e61dc287c3 --- /dev/null +++ b/terraform/test-fixtures/plan-count/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + count = 5 + foo = "foo" +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.*.foo}" +} From c5a0b9cb408e93ff8c6445ca8419aa512ce5e13e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 21:42:18 -0700 Subject: [PATCH 6/7] terraform: test case for decreasing the count --- terraform/context.go | 4 +- terraform/context_test.go | 69 +++++++++++++++++-- terraform/graph.go | 26 ++++--- terraform/state.go | 6 ++ terraform/terraform_test.go | 21 ++++++ .../test-fixtures/plan-count-dec/main.tf | 7 ++ 6 files changed, 118 insertions(+), 15 deletions(-) create mode 100644 terraform/test-fixtures/plan-count-dec/main.tf diff --git a/terraform/context.go b/terraform/context.go index 3f68c245aa96..66e553738f7b 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -630,7 +630,9 @@ func computeAggregateVars( var ivars map[string]config.InterpolatedVariable switch m := n.Meta.(type) { case *GraphNodeResource: - ivars = m.Config.RawConfig.Variables + if m.Config != nil { + ivars = m.Config.RawConfig.Variables + } case *GraphNodeResourceProvider: if m.Config != nil { ivars = m.Config.RawConfig.Variables diff --git a/terraform/context_test.go b/terraform/context_test.go index e2eb5826a0b4..25bb48aafc84 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -668,6 +668,50 @@ func TestContextPlan_count(t *testing.T) { } } +func TestContextPlan_countDecrease(t *testing.T) { + c := testConfig(t, "plan-count-dec") + p := testProvider("aws") + p.DiffFn = testDiffFn + s := &State{ + Resources: map[string]*ResourceState{ + "aws_instance.foo.0": &ResourceState{ + ID: "bar", + Type: "aws_instance", + Attributes: map[string]string{ + "foo": "foo", + "type": "aws_instance", + }, + }, + "aws_instance.foo.1": &ResourceState{ + ID: "bar", + Type: "aws_instance", + }, + "aws_instance.foo.2": &ResourceState{ + ID: "bar", + Type: "aws_instance", + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: s, + }) + + plan, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanCountDecreaseStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_destroy(t *testing.T) { c := testConfig(t, "plan-destroy") p := testProvider("aws") @@ -944,10 +988,6 @@ func testDiffFn( c *ResourceConfig) (*ResourceDiff, error) { var diff ResourceDiff diff.Attributes = make(map[string]*ResourceAttrDiff) - diff.Attributes["type"] = &ResourceAttrDiff{ - Old: "", - New: s.Type, - } for k, v := range c.Raw { if _, ok := v.(string); !ok { @@ -1009,6 +1049,27 @@ func testDiffFn( } } + for k, v := range diff.Attributes { + if v.NewComputed { + continue + } + + old, ok := s.Attributes[k] + if !ok { + continue + } + if old == v.New { + delete(diff.Attributes, k) + } + } + + if !diff.Empty() { + diff.Attributes["type"] = &ResourceAttrDiff{ + Old: "", + New: s.Type, + } + } + return &diff, nil } diff --git a/terraform/graph.go b/terraform/graph.go index 2dd3a06168cb..fd7a40cff363 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -156,16 +156,6 @@ func graphAddConfigResources( // This tracks all the resource nouns nouns := make(map[string]*depgraph.Noun) for _, r := range c.Resources { - var state *ResourceState - if s != nil { - state = s.Resources[r.Id()] - } - if state == nil { - state = &ResourceState{ - Type: r.Type, - } - } - resourceNouns := make([]*depgraph.Noun, r.Count) for i := 0; i < r.Count; i++ { name := r.Id() @@ -178,6 +168,22 @@ func graphAddConfigResources( index = i } + var state *ResourceState + if s != nil { + state = s.Resources[name] + + // If the count is one, check the state for ".0" appended, which + // might exist if we go from count > 1 to count == 1. + if state == nil && r.Count == 1 { + state = s.Resources[r.Id()+".0"] + } + } + if state == nil { + state = &ResourceState{ + Type: r.Type, + } + } + resourceNouns[i] = &depgraph.Noun{ Name: name, Meta: &GraphNodeResource{ diff --git a/terraform/state.go b/terraform/state.go index 171870d0b990..104459155b2a 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -38,6 +38,12 @@ func (s *State) Orphans(c *config.Config) []string { for _, r := range c.Resources { delete(keys, r.Id()) + + // If there is only one of this instance, then we alias that + // to the ".0" version as well so that it can count + if r.Count == 1 { + delete(keys, r.Id()+".0") + } } result := make([]string, 0, len(keys)) diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 7183b20c7b14..84c360e16dd2 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -182,6 +182,27 @@ STATE: ` +const testTerraformPlanCountDecreaseStr = ` +DIFF: + +UPDATE: aws_instance.bar + foo: "" => "bar" + type: "" => "aws_instance" +DESTROY: aws_instance.foo.1 +DESTROY: aws_instance.foo.2 + +STATE: + +aws_instance.foo.0: + ID = bar + foo = foo + type = aws_instance +aws_instance.foo.1: + ID = bar +aws_instance.foo.2: + ID = bar +` + const testTerraformPlanDestroyStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-count-dec/main.tf b/terraform/test-fixtures/plan-count-dec/main.tf new file mode 100644 index 000000000000..e4cba316cf45 --- /dev/null +++ b/terraform/test-fixtures/plan-count-dec/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "foo" { + foo = "foo" +} + +resource "aws_instance" "bar" { + foo = "bar" +} From 3b3c9e140ae59775091b3fb89b10d997fa94f068 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2014 21:47:07 -0700 Subject: [PATCH 7/7] terraform: tests for increasing count from 1 to > 1 --- terraform/context_test.go | 38 ++++++++++++++++++- terraform/graph.go | 16 ++++++-- terraform/terraform_test.go | 21 ++++++++++ .../test-fixtures/plan-count-inc/main.tf | 8 ++++ 4 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 terraform/test-fixtures/plan-count-inc/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 25bb48aafc84..c67faec6cb25 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -668,7 +668,7 @@ func TestContextPlan_count(t *testing.T) { } } -func TestContextPlan_countDecrease(t *testing.T) { +func TestContextPlan_countDecreaseToOne(t *testing.T) { c := testConfig(t, "plan-count-dec") p := testProvider("aws") p.DiffFn = testDiffFn @@ -712,6 +712,42 @@ func TestContextPlan_countDecrease(t *testing.T) { } } +func TestContextPlan_countIncreaseFromOne(t *testing.T) { + c := testConfig(t, "plan-count-inc") + p := testProvider("aws") + p.DiffFn = testDiffFn + s := &State{ + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + ID: "bar", + Type: "aws_instance", + Attributes: map[string]string{ + "foo": "foo", + "type": "aws_instance", + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: s, + }) + + plan, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanCountIncreaseStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_destroy(t *testing.T) { c := testConfig(t, "plan-destroy") p := testProvider("aws") diff --git a/terraform/graph.go b/terraform/graph.go index fd7a40cff363..ff97e5fbf38a 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -172,10 +172,18 @@ func graphAddConfigResources( if s != nil { state = s.Resources[name] - // If the count is one, check the state for ".0" appended, which - // might exist if we go from count > 1 to count == 1. - if state == nil && r.Count == 1 { - state = s.Resources[r.Id()+".0"] + 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 = s.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 = s.Resources[r.Id()] + } } } if state == nil { diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 84c360e16dd2..dfb0f0e37c02 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -203,6 +203,27 @@ aws_instance.foo.2: ID = bar ` +const testTerraformPlanCountIncreaseStr = ` +DIFF: + +UPDATE: aws_instance.bar + foo: "" => "bar" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.1 + foo: "" => "foo" + type: "" => "aws_instance" +UPDATE: aws_instance.foo.2 + foo: "" => "foo" + type: "" => "aws_instance" + +STATE: + +aws_instance.foo: + ID = bar + foo = foo + type = aws_instance +` + const testTerraformPlanDestroyStr = ` DIFF: diff --git a/terraform/test-fixtures/plan-count-inc/main.tf b/terraform/test-fixtures/plan-count-inc/main.tf new file mode 100644 index 000000000000..d5a3d843478c --- /dev/null +++ b/terraform/test-fixtures/plan-count-inc/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + foo = "foo" + count = 3 +} + +resource "aws_instance" "bar" { + foo = "bar" +}