diff --git a/.changelog/8249.txt b/.changelog/8249.txt new file mode 100644 index 00000000000..d0212110e1a --- /dev/null +++ b/.changelog/8249.txt @@ -0,0 +1,3 @@ +```release-note:bug +compute: fixed logic when unsetting `google_compute_instance.min_cpu_platform` and switching to a `machine_type` that does not support `min_cpu_platform` at the same time +``` diff --git a/google/resource_compute_instance_test.go b/google/resource_compute_instance_test.go index b8a271314bf..e3fe277136d 100644 --- a/google/resource_compute_instance_test.go +++ b/google/resource_compute_instance_test.go @@ -22,6 +22,74 @@ import ( "google.golang.org/api/compute/v1" ) +func TestMinCpuPlatformDiffSuppress(t *testing.T) { + cases := map[string]struct { + Old, New string + ExpectDiffSuppress bool + }{ + "state: empty, conf: AUTOMATIC": { + Old: "", + New: "AUTOMATIC", + ExpectDiffSuppress: true, + }, + "state: empty, conf: automatic": { + Old: "", + New: "automatic", + ExpectDiffSuppress: true, + }, + "state: empty, conf: AuToMaTiC": { + Old: "", + New: "AuToMaTiC", + ExpectDiffSuppress: true, + }, + "state: empty, conf: Intel Haswell": { + Old: "", + New: "Intel Haswell", + ExpectDiffSuppress: false, + }, + // This case should never happen due to the field being + // Optional + Computed; however, including for completeness. + "state: Intel Haswell, conf: empty": { + Old: "Intel Haswell", + New: "", + ExpectDiffSuppress: false, + }, + // These cases should never happen given current API behavior; testing + // in case API behavior changes in the future. + "state: AUTOMATIC, conf: Intel Haswell": { + Old: "AUTOMATIC", + New: "Intel Haswell", + ExpectDiffSuppress: false, + }, + "state: Intel Haswell, conf: AUTOMATIC": { + Old: "Intel Haswell", + New: "AUTOMATIC", + ExpectDiffSuppress: false, + }, + "state: AUTOMATIC, conf: empty": { + Old: "AUTOMATIC", + New: "", + ExpectDiffSuppress: true, + }, + "state: automatic, conf: empty": { + Old: "automatic", + New: "", + ExpectDiffSuppress: true, + }, + "state: AuToMaTiC, conf: empty": { + Old: "AuToMaTiC", + New: "", + ExpectDiffSuppress: true, + }, + } + + for tn, tc := range cases { + if tpgcompute.ComputeInstanceMinCpuPlatformEmptyOrAutomaticDiffSuppress("min_cpu_platform", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { + t.Errorf("bad: %s, %q => %q expect DiffSuppress to return %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + } +} + func computeInstanceImportStep(zone, instanceName string, additionalImportIgnores []string) resource.TestStep { // metadata is only read into state if set in the config // importing doesn't know whether metadata.startup_script vs metadata_startup_script is set in the config, @@ -1430,7 +1498,15 @@ func TestAccComputeInstance_minCpuPlatform(t *testing.T) { testAccCheckComputeInstanceHasMinCpuPlatform(&instance, "Intel Haswell"), ), }, - computeInstanceImportStep("us-east1-d", instanceName, []string{}), + computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}), + { + Config: testAccComputeInstance_minCpuPlatform_remove(instanceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceHasMinCpuPlatform(&instance, ""), + ), + }, + computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}), }, }) } @@ -5212,6 +5288,35 @@ resource "google_compute_instance" "foobar" { } min_cpu_platform = "Intel Haswell" + allow_stopping_for_update = true +} +`, instance) +} + +func testAccComputeInstance_minCpuPlatform_remove(instance string) string { + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "debian-11" + project = "debian-cloud" +} + +resource "google_compute_instance" "foobar" { + name = "%s" + machine_type = "e2-micro" + zone = "us-east1-d" + + boot_disk { + initialize_params { + image = data.google_compute_image.my_image.self_link + } + } + + network_interface { + network = "default" + } + + min_cpu_platform = "AuToMaTiC" + allow_stopping_for_update = true } `, instance) } diff --git a/google/services/compute/resource_compute_instance.go b/google/services/compute/resource_compute_instance.go index 74ef64497a2..b3ffe4c5dba 100644 --- a/google/services/compute/resource_compute_instance.go +++ b/google/services/compute/resource_compute_instance.go @@ -94,6 +94,14 @@ func forceNewIfNetworkIPNotUpdatableFunc(d tpgresource.TerraformResourceDiff) er return nil } +// User may specify AUTOMATIC using any case; the API will accept it and return an empty string. +func ComputeInstanceMinCpuPlatformEmptyOrAutomaticDiffSuppress(k, old, new string, d *schema.ResourceData) bool { + old = strings.ToLower(old) + new = strings.ToLower(new) + defaultVal := "automatic" + return (old == "" && new == defaultVal) || (new == "" && old == defaultVal) +} + func ResourceComputeInstance() *schema.Resource { return &schema.Resource{ Create: resourceComputeInstanceCreate, @@ -588,10 +596,11 @@ func ResourceComputeInstance() *schema.Resource { }, "min_cpu_platform": { - Type: schema.TypeString, - Optional: true, - Computed: true, - Description: `The minimum CPU platform specified for the VM instance.`, + Type: schema.TypeString, + Optional: true, + Computed: true, + Description: `The minimum CPU platform specified for the VM instance.`, + DiffSuppressFunc: ComputeInstanceMinCpuPlatformEmptyOrAutomaticDiffSuppress, }, "project": { @@ -1965,40 +1974,34 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err } } - if d.HasChange("machine_type") { - mt, err := tpgresource.ParseMachineTypesFieldValue(d.Get("machine_type").(string), d, config) - if err != nil { - return err - } - req := &compute.InstancesSetMachineTypeRequest{ - MachineType: mt.RelativeLink(), + if d.HasChange("min_cpu_platform") { + minCpuPlatform := d.Get("min_cpu_platform") + req := &compute.InstancesSetMinCpuPlatformRequest{ + MinCpuPlatform: minCpuPlatform.(string), } - op, err := config.NewComputeClient(userAgent).Instances.SetMachineType(project, zone, instance.Name, req).Do() + op, err := config.NewComputeClient(userAgent).Instances.SetMinCpuPlatform(project, zone, instance.Name, req).Do() if err != nil { return err } - opErr := ComputeOperationWaitTime(config, op, project, "updating machinetype", userAgent, d.Timeout(schema.TimeoutUpdate)) + opErr := ComputeOperationWaitTime(config, op, project, "updating min cpu platform", userAgent, d.Timeout(schema.TimeoutUpdate)) if opErr != nil { return opErr } } - if d.HasChange("min_cpu_platform") { - minCpuPlatform, ok := d.GetOk("min_cpu_platform") - // Even though you don't have to set minCpuPlatform on create, you do have to set it to an - // actual value on update. "Automatic" is the default. This will be read back from the API as empty, - // so we don't need to worry about diffs. - if !ok { - minCpuPlatform = "Automatic" + if d.HasChange("machine_type") { + mt, err := tpgresource.ParseMachineTypesFieldValue(d.Get("machine_type").(string), d, config) + if err != nil { + return err } - req := &compute.InstancesSetMinCpuPlatformRequest{ - MinCpuPlatform: minCpuPlatform.(string), + req := &compute.InstancesSetMachineTypeRequest{ + MachineType: mt.RelativeLink(), } - op, err := config.NewComputeClient(userAgent).Instances.SetMinCpuPlatform(project, zone, instance.Name, req).Do() + op, err := config.NewComputeClient(userAgent).Instances.SetMachineType(project, zone, instance.Name, req).Do() if err != nil { return err } - opErr := ComputeOperationWaitTime(config, op, project, "updating min cpu platform", userAgent, d.Timeout(schema.TimeoutUpdate)) + opErr := ComputeOperationWaitTime(config, op, project, "updating machinetype", userAgent, d.Timeout(schema.TimeoutUpdate)) if opErr != nil { return opErr } diff --git a/website/docs/d/compute_instance.html.markdown b/website/docs/d/compute_instance.html.markdown index 2a3e1d03f40..d478a3eca5a 100644 --- a/website/docs/d/compute_instance.html.markdown +++ b/website/docs/d/compute_instance.html.markdown @@ -61,7 +61,7 @@ The following arguments are supported: * `metadata` - Metadata key/value pairs made available within the instance. -* `min_cpu_platform` - The minimum CPU platform specified for the VM instance. +* `min_cpu_platform` - The minimum CPU platform specified for the VM instance. Set to "AUTOMATIC" to remove a previously-set value. * `scheduling` - The scheduling strategy being used by the instance. Structure is [documented below](#nested_scheduling)