core: avoid diff mismatch on NewRemoved fields during -/+
fixes #1508 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 node does its thing.) 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.
This commit is contained in:
parent
0bd7856942
commit
64f0897c82
|
@ -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) {
|
func TestAccComputeInstance_update(t *testing.T) {
|
||||||
var instance compute.Instance
|
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
|
// Update metadata, tags, and network_interface
|
||||||
const testAccComputeInstance_update = `
|
const testAccComputeInstance_update = `
|
||||||
resource "google_compute_instance" "foobar" {
|
resource "google_compute_instance" "foobar" {
|
||||||
|
|
|
@ -407,6 +407,12 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
|
||||||
|
|
||||||
_, ok := d2.Attributes[k]
|
_, ok := d2.Attributes[k]
|
||||||
if !ok {
|
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
|
// No exact match, but maybe this is a set containing computed
|
||||||
// values. So check if there is an approximate hash in the key
|
// values. So check if there is an approximate hash in the key
|
||||||
// and if so, try to match the key.
|
// and if so, try to match the key.
|
||||||
|
|
|
@ -519,12 +519,47 @@ func TestInstanceDiffSame(t *testing.T) {
|
||||||
true,
|
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 {
|
for i, tc := range cases {
|
||||||
same, reason := tc.One.Same(tc.Two)
|
same, reason := tc.One.Same(tc.Two)
|
||||||
if same != tc.Same {
|
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 {
|
if reason != tc.Reason {
|
||||||
t.Fatalf(
|
t.Fatalf(
|
||||||
|
|
Loading…
Reference in New Issue