Merge pull request #1515 from hashicorp/b-diff-mismatch-on-instance-replacement

core: avoid diff mismatch on NewRemoved fields during -/+

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" {

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.

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(