From c9a541d95ba01ad7765aa4252cc4599437e47873 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 27 Aug 2014 15:26:15 -0700 Subject: [PATCH] helper/schema: generate a full diff in destroy/create cycle --- .../providers/aws/resource_aws_instance.go | 1 + helper/schema/resource.go | 9 +- helper/schema/schema.go | 58 +++++++++++ helper/schema/schema_test.go | 98 +++++++++++++++++++ 4 files changed, 160 insertions(+), 6 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 1b15a4052..bd4d43add 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -91,6 +91,7 @@ func resourceAwsInstance() *schema.Resource { "security_groups": &schema.Schema{ Type: schema.TypeSet, Optional: true, + Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: func(v interface{}) int { return hashcode.String(v.(string)) diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 8cef2c05f..480b1e547 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -82,11 +82,8 @@ func (r *Resource) Apply( return data.State(), err } - // Reset the data to be empty - data, err = schemaMap(r.Schema).Data(nil, d) - if err != nil { - return nil, err - } + // Make sure the ID is gone. + data.SetId("") } // If we're only destroying, and not creating, then return @@ -97,7 +94,7 @@ func (r *Resource) Apply( } err = nil - if s.ID == "" { + if data.Id() == "" { // We're creating, it is a new resource. err = r.Create(data, meta) } else { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 6de1407d3..60848b969 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -177,6 +177,64 @@ func (m schemaMap) Diff( } } + // If the diff requires a new resource, then we recompute the diff + // so we have the complete new resource diff, and preserve the + // RequiresNew fields where necessary so the user knows exactly what + // caused that. + if result.RequiresNew() { + // Create the new diff + result2 := new(terraform.ResourceDiff) + result2.Attributes = make(map[string]*terraform.ResourceAttrDiff) + + // Reset the data to not contain state + d.state = nil + + // Perform the diff again + for k, schema := range m { + err := m.diff(k, schema, result2, d) + if err != nil { + return nil, err + } + } + + // Force all the fields to not force a new since we know what we + // want to force new. + for k, attr := range result2.Attributes { + if attr == nil { + continue + } + + if attr.RequiresNew { + attr.RequiresNew = false + } + + if s != nil { + attr.Old = s.Attributes[k] + } + } + + // Now copy in all the requires new diffs... + for k, attr := range result.Attributes { + if attr == nil { + continue + } + + newAttr, ok := result2.Attributes[k] + if !ok { + newAttr = attr + } + + if attr.RequiresNew { + newAttr.RequiresNew = true + } + + result2.Attributes[k] = newAttr + } + + // And set the diff! + result = result2 + } + // Remove any nil diffs just to keep things clean for k, v := range result.Attributes { if v == nil { diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 5b32c985a..cbf995704 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -764,6 +764,104 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + + /* + * ForceNews + */ + + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + ForceNew: true, + }, + + "address": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + + State: &terraform.ResourceState{ + Attributes: map[string]string{ + "availability_zone": "bar", + "address": "foo", + }, + }, + + Config: map[string]interface{}{ + "availability_zone": "foo", + }, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "bar", + New: "foo", + RequiresNew: true, + }, + + "address": &terraform.ResourceAttrDiff{ + Old: "foo", + New: "", + NewComputed: true, + }, + }, + }, + + Err: false, + }, + + // Set + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + ForceNew: true, + }, + + "ports": &Schema{ + Type: TypeSet, + Optional: true, + Computed: true, + Elem: &Schema{Type: TypeInt}, + Set: func(v interface{}) int { return v.(int) }, + }, + }, + + State: &terraform.ResourceState{ + Attributes: map[string]string{ + "availability_zone": "bar", + "ports.#": "1", + "ports.0": "80", + }, + }, + + Config: map[string]interface{}{ + "availability_zone": "foo", + }, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "bar", + New: "foo", + RequiresNew: true, + }, + + "ports.#": &terraform.ResourceAttrDiff{ + Old: "1", + New: "", + NewComputed: true, + }, + }, + }, + + Err: false, + }, } for i, tc := range cases {