diff --git a/builtin/providers/google/resource_compute_instance_test.go b/builtin/providers/google/resource_compute_instance_test.go index 9c53fae04..612282b16 100644 --- a/builtin/providers/google/resource_compute_instance_test.go +++ b/builtin/providers/google/resource_compute_instance_test.go @@ -168,6 +168,34 @@ func TestAccComputeInstance_update_deprecated_network(t *testing.T) { }) } +func TestAccComputeInstance_forceNewAndChangeMetadata(t *testing.T) { + var instance compute.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeInstance_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists( + "google_compute_instance.foobar", &instance), + ), + }, + resource.TestStep{ + Config: testAccComputeInstance_forceNewAndChangeMetadata, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists( + "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceMetadata( + &instance, "qux", "true"), + ), + }, + }, + }) +} + func TestAccComputeInstance_update(t *testing.T) { var instance compute.Instance @@ -432,6 +460,30 @@ resource "google_compute_instance" "foobar" { } }` +// Update zone to ForceNew, and change metadata k/v entirely +// Generates diff mismatch +const testAccComputeInstance_forceNewAndChangeMetadata = ` +resource "google_compute_instance" "foobar" { + name = "terraform-test" + machine_type = "n1-standard-1" + zone = "us-central1-a" + zone = "us-central1-b" + tags = ["baz"] + + disk { + image = "debian-7-wheezy-v20140814" + } + + network_interface { + network = "default" + access_config { } + } + + metadata { + qux = "true" + } +}` + // Update metadata, tags, and network_interface const testAccComputeInstance_update = ` resource "google_compute_instance" "foobar" { diff --git a/terraform/diff.go b/terraform/diff.go index 97fb6cc47..96b8c654a 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -407,6 +407,12 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) { _, ok := d2.Attributes[k] if !ok { + // If there's no new attribute, and the old diff expected the attribute + // to be removed, that's just fine. + if diffOld.NewRemoved { + continue + } + // No exact match, but maybe this is a set containing computed // values. So check if there is an approximate hash in the key // and if so, try to match the key. diff --git a/terraform/diff_test.go b/terraform/diff_test.go index d65ff455d..4eeb8d387 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -519,12 +519,47 @@ func TestInstanceDiffSame(t *testing.T) { true, "", }, + + // In a DESTROY/CREATE scenario, the plan diff will be run against the + // state of the old instance, while the apply diff will be run against an + // empty state (because the state is cleared when the destroy runs.) + // For complex attributes, this can result in keys that seem to disappear + // between the two diffs, when in reality everything is working just fine. + // + // Same() needs to take into account this scenario by analyzing NewRemoved + // and treating as "Same" a diff that does indeed have that key removed. + { + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "somemap.oldkey": &ResourceAttrDiff{ + Old: "long ago", + New: "", + NewRemoved: true, + }, + "somemap.newkey": &ResourceAttrDiff{ + Old: "", + New: "brave new world", + }, + }, + }, + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "somemap.newkey": &ResourceAttrDiff{ + Old: "", + New: "brave new world", + }, + }, + }, + true, + "", + }, } for i, tc := range cases { same, reason := tc.One.Same(tc.Two) if same != tc.Same { - t.Fatalf("%d:\n\n%#v\n\n%#v", i, tc.One, tc.Two) + t.Fatalf("%d: expected same: %t, got %t (%s)\n\n one: %#v\n\ntwo: %#v", + i, tc.Same, same, reason, tc.One, tc.Two) } if reason != tc.Reason { t.Fatalf(