From 251790f05ab4e90524d6168a0de8899dfc1e013f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Jul 2014 16:58:31 -0700 Subject: [PATCH] terraform: add ID to diff implicitly --- .../aws/resource_aws_security_group.go | 4 +- terraform/context.go | 25 +++++++++- terraform/context_test.go | 35 ++++++++++++- terraform/diff.go | 14 ++++++ terraform/graph.go | 2 +- terraform/resource.go | 4 ++ terraform/terraform_test.go | 49 +++++++++++-------- terraform/test-fixtures/plan-empty/main.tf | 5 ++ 8 files changed, 112 insertions(+), 26 deletions(-) create mode 100644 terraform/test-fixtures/plan-empty/main.tf diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 9aadef1c4..a98644f08 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -4,9 +4,9 @@ import ( "fmt" "log" + "github.com/hashicorp/terraform/flatmap" "github.com/hashicorp/terraform/helper/diff" "github.com/hashicorp/terraform/terraform" - "github.com/hashicorp/terraform/flatmap" "github.com/mitchellh/goamz/ec2" ) @@ -132,7 +132,7 @@ func resource_aws_security_group_diff( "name": diff.AttrTypeCreate, "description": diff.AttrTypeCreate, "vpc_id": diff.AttrTypeUpdate, - "ingress": diff.AttrTypeUpdate, + "ingress": diff.AttrTypeUpdate, "egress": diff.AttrTypeUpdate, }, diff --git a/terraform/context.go b/terraform/context.go index 4820c6dc3..659772893 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -532,6 +532,26 @@ func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc { } } + if diff == nil { + diff = new(ResourceDiff) + } + + if diff.RequiresNew() && r.State.ID != "" { + // This will also require a destroy + diff.Destroy = true + } + + if diff.RequiresNew() || r.State.ID == "" { + // Add diff to compute new ID + diff.init() + diff.Attributes["id"] = &ResourceAttrDiff{ + Old: r.State.Attributes["id"], + NewComputed: true, + RequiresNew: true, + Type: DiffAttrOutput, + } + } + l.Lock() if !diff.Empty() { result.Diff.Resources[r.Id] = diff @@ -752,7 +772,10 @@ func (c *Context) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { }() // Call the callack - log.Printf("[INFO] Walking: %s", rn.Resource.Id) + log.Printf( + "[INFO] Walking: %s (Graph node: %s)", + rn.Resource.Id, + n.Name) if err := cb(rn.Resource); err != nil { log.Printf("[ERROR] Error walking '%s': %s", rn.Resource.Id, err) return err diff --git a/terraform/context_test.go b/terraform/context_test.go index ef059d9ab..06f91bc6b 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -180,8 +180,6 @@ func TestContextApply_Minimal(t *testing.T) { t.Fatalf("err: %s", err) } - ctx.variables = map[string]string{"value": "1"} - state, err := ctx.Apply() if err != nil { t.Fatalf("err: %s", err) @@ -242,7 +240,9 @@ func TestContextApply_cancel(t *testing.T) { // Start the Apply in a goroutine stateCh := make(chan *State) go func() { + println("START") state, err := ctx.Apply() + println("STOP") if err != nil { panic(err) } @@ -714,6 +714,29 @@ func TestContextPlan(t *testing.T) { } } +func TestContextPlan_minimal(t *testing.T) { + c := testConfig(t, "plan-empty") + 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) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanEmptyStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_nil(t *testing.T) { c := testConfig(t, "plan-nil") p := testProvider("aws") @@ -723,6 +746,14 @@ func TestContextPlan_nil(t *testing.T) { Providers: map[string]ResourceProviderFactory{ "aws": testProviderFuncFixed(p), }, + State: &State{ + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + ID: "bar", + Type: "aws_instance", + }, + }, + }, }) plan, err := ctx.Plan(nil) diff --git a/terraform/diff.go b/terraform/diff.go index d4f7545ae..6e7f6580d 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -115,6 +115,10 @@ func (d *Diff) String() string { keyLen := 0 keys := make([]string, 0, len(rdiff.Attributes)) for key, _ := range rdiff.Attributes { + if key == "id" { + continue + } + keys = append(keys, key) if len(key) > keyLen { keyLen = len(key) @@ -152,6 +156,8 @@ func (d *Diff) String() string { type ResourceDiff struct { Attributes map[string]*ResourceAttrDiff Destroy bool + + once sync.Once } // ResourceAttrDiff is the diff of a single attribute of a resource. @@ -177,6 +183,14 @@ const ( DiffAttrOutput ) +func (d *ResourceDiff) init() { + d.once.Do(func() { + if d.Attributes == nil { + d.Attributes = make(map[string]*ResourceAttrDiff) + } + }) +} + // Empty returns true if this diff encapsulates no changes. func (d *ResourceDiff) Empty() bool { if d == nil { diff --git a/terraform/graph.go b/terraform/graph.go index 4cdb5d23f..40c2a63e7 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -273,7 +273,7 @@ func graphAddDiff(g *depgraph.Graph, d *Diff) error { continue } - if rd.Destroy || rd.RequiresNew() { + if rd.Destroy { // If we're destroying, we create a new destroy node with // the proper dependencies. Perform a dirty copy operation. newNode := new(GraphNodeResource) diff --git a/terraform/resource.go b/terraform/resource.go index b89593507..609a21fe8 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -129,6 +129,10 @@ func (c *ResourceConfig) IsSet(k string) bool { } func (c *ResourceConfig) interpolate(ctx *Context) error { + if c == nil { + return nil + } + if ctx != nil { if err := ctx.computeVars(c.raw); err != nil { return err diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index caccef7b7..77a4ded34 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -110,10 +110,8 @@ aws_instance.foo: const testTerraformApplyMinimalStr = ` aws_instance.bar: ID = foo - type = aws_instance aws_instance.foo: ID = foo - type = aws_instance ` const testTerraformApplyDestroyStr = ` @@ -216,10 +214,10 @@ aws_instance.foo: const testTerraformPlanStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "2" type: "" => "aws_instance" -UPDATE: aws_instance.foo +CREATE: aws_instance.foo num: "" => "2" type: "" => "aws_instance" @@ -231,10 +229,10 @@ STATE: const testTerraformPlanComputedStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "" type: "" => "aws_instance" -UPDATE: aws_instance.foo +CREATE: aws_instance.foo foo: "" => "" num: "" => "2" type: "" => "aws_instance" @@ -247,10 +245,10 @@ STATE: const testTerraformPlanComputedIdStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "" type: "" => "aws_instance" -UPDATE: aws_instance.foo +CREATE: aws_instance.foo foo: "" => "" num: "" => "2" type: "" => "aws_instance" @@ -263,22 +261,22 @@ STATE: const testTerraformPlanCountStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "foo,foo,foo,foo,foo" type: "" => "aws_instance" -UPDATE: aws_instance.foo.0 +CREATE: aws_instance.foo.0 foo: "" => "foo" type: "" => "aws_instance" -UPDATE: aws_instance.foo.1 +CREATE: aws_instance.foo.1 foo: "" => "foo" type: "" => "aws_instance" -UPDATE: aws_instance.foo.2 +CREATE: aws_instance.foo.2 foo: "" => "foo" type: "" => "aws_instance" -UPDATE: aws_instance.foo.3 +CREATE: aws_instance.foo.3 foo: "" => "foo" type: "" => "aws_instance" -UPDATE: aws_instance.foo.4 +CREATE: aws_instance.foo.4 foo: "" => "foo" type: "" => "aws_instance" @@ -290,7 +288,7 @@ STATE: const testTerraformPlanCountDecreaseStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "bar" type: "" => "aws_instance" DESTROY: aws_instance.foo.1 @@ -311,13 +309,13 @@ aws_instance.foo.2: const testTerraformPlanCountIncreaseStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "bar" type: "" => "aws_instance" -UPDATE: aws_instance.foo.1 +CREATE: aws_instance.foo.1 foo: "" => "foo" type: "" => "aws_instance" -UPDATE: aws_instance.foo.2 +CREATE: aws_instance.foo.2 foo: "" => "foo" type: "" => "aws_instance" @@ -343,11 +341,22 @@ aws_instance.two: ID = baz ` +const testTerraformPlanEmptyStr = ` +DIFF: + +CREATE: aws_instance.bar +CREATE: aws_instance.foo + +STATE: + + +` + const testTerraformPlanOrphanStr = ` DIFF: DESTROY: aws_instance.baz -UPDATE: aws_instance.foo +CREATE: aws_instance.foo num: "" => "2" type: "" => "aws_instance" @@ -360,7 +369,7 @@ aws_instance.baz: const testTerraformPlanStateStr = ` DIFF: -UPDATE: aws_instance.bar +CREATE: aws_instance.bar foo: "" => "2" type: "" => "aws_instance" UPDATE: aws_instance.foo diff --git a/terraform/test-fixtures/plan-empty/main.tf b/terraform/test-fixtures/plan-empty/main.tf new file mode 100644 index 000000000..51e84d936 --- /dev/null +++ b/terraform/test-fixtures/plan-empty/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "foo" { +} + +resource "aws_instance" "bar" { +}