Skip to content

Commit

Permalink
Fix inconsistent locations in some resources (#4377) (#8224)
Browse files Browse the repository at this point in the history
* infer correct zone/region from provider

Co-authored-by: upodroid <[email protected]>

* add zone back in

Signed-off-by: Modular Magician <[email protected]>
  • Loading branch information
modular-magician authored Jan 15, 2021
1 parent 03d2ec2 commit d937ce4
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 46 deletions.
3 changes: 3 additions & 0 deletions .changelog/4377.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
compute: fixed an issue where `google_compute_region_per_instance_config`, `google_compute_per_instance_config`, `google_compute_region_instance_group_manager` resources were not inferring the region/zone from the provider.
```
14 changes: 7 additions & 7 deletions google/resource_compute_per_instance_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,6 @@ func resourceComputePerInstanceConfig() *schema.Resource {
ForceNew: true,
Description: `The name for this per-instance config and its corresponding instance.`,
},
"zone": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: compareSelfLinkOrResourceName,
Description: `Zone where the containing instance group manager is located`,
},
"preserved_state": {
Type: schema.TypeList,
Optional: true,
Expand All @@ -85,6 +78,13 @@ func resourceComputePerInstanceConfig() *schema.Resource {
},
},
},
"zone": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
DiffSuppressFunc: compareSelfLinkOrResourceName,
Description: `Zone where the containing instance group manager is located`,
},
"minimal_action": {
Type: schema.TypeString,
Optional: true,
Expand Down
17 changes: 7 additions & 10 deletions google/resource_compute_per_instance_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
"config_name4": fmt.Sprintf("instance-%s", randString(t, 10)),
}
igmId := fmt.Sprintf("projects/%s/zones/%s/instanceGroupManagers/%s",
getTestProjectFromEnv(), "us-central1-c", igmName)
getTestProjectFromEnv(), getTestZoneFromEnv(), igmName)

vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -38,7 +38,7 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "zone"},
},
{
// Force-recreate old config
Expand All @@ -51,7 +51,7 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "zone"},
},
{
// Add two new endpoints
Expand All @@ -61,7 +61,7 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "zone"},
},
{
ResourceName: "google_compute_per_instance_config.with_disks",
Expand All @@ -73,7 +73,7 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_per_instance_config.add2",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "zone"},
},
{
// delete all configs
Expand Down Expand Up @@ -109,7 +109,7 @@ func TestAccComputePerInstanceConfig_update(t *testing.T) {
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "zone"},
},
{
// Update an existing config
Expand All @@ -119,7 +119,7 @@ func TestAccComputePerInstanceConfig_update(t *testing.T) {
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "zone"},
},
},
})
Expand All @@ -128,7 +128,6 @@ func TestAccComputePerInstanceConfig_update(t *testing.T) {
func testAccComputePerInstanceConfig_statefulBasic(context map[string]interface{}) string {
return Nprintf(`
resource "google_compute_per_instance_config" "default" {
zone = google_compute_instance_group_manager.igm.zone
instance_group_manager = google_compute_instance_group_manager.igm.name
name = "%{config_name}"
remove_instance_state_on_destroy = true
Expand All @@ -144,7 +143,6 @@ resource "google_compute_per_instance_config" "default" {
func testAccComputePerInstanceConfig_update(context map[string]interface{}) string {
return Nprintf(`
resource "google_compute_per_instance_config" "default" {
zone = google_compute_instance_group_manager.igm.zone
instance_group_manager = google_compute_instance_group_manager.igm.name
name = "%{config_name}"
remove_instance_state_on_destroy = true
Expand Down Expand Up @@ -293,7 +291,6 @@ resource "google_compute_instance_group_manager" "igm" {
}
base_instance_name = "igm-no-tp"
zone = "us-central1-c"
}
`, context)
}
Expand Down
3 changes: 2 additions & 1 deletion google/resource_compute_region_instance_group_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ func resourceComputeRegionInstanceGroupManager() *schema.Resource {

"region": {
Type: schema.TypeString,
Required: true,
Computed: true,
Optional: true,
ForceNew: true,
Description: `The region where the managed instance group resides.`,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ resource "google_compute_region_instance_group_manager" "igm-basic" {
target_pools = [google_compute_target_pool.igm-basic.self_link]
base_instance_name = "igm-basic"
region = "us-central1"
target_size = 2
}
Expand Down
23 changes: 16 additions & 7 deletions google/resource_compute_region_per_instance_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,6 @@ func resourceComputeRegionPerInstanceConfig() *schema.Resource {
ForceNew: true,
Description: `The name for this per-instance config and its corresponding instance.`,
},
"region": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: compareSelfLinkOrResourceName,
Description: `Region where the containing instance group manager is located`,
},
"region_instance_group_manager": {
Type: schema.TypeString,
Required: true,
Expand Down Expand Up @@ -85,6 +78,14 @@ func resourceComputeRegionPerInstanceConfig() *schema.Resource {
},
},
},
"region": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
DiffSuppressFunc: compareSelfLinkOrResourceName,
Description: `Region where the containing instance group manager is located`,
},
"minimal_action": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -288,6 +289,14 @@ func resourceComputeRegionPerInstanceConfigRead(d *schema.ResourceData, meta int
return fmt.Errorf("Error reading RegionPerInstanceConfig: %s", err)
}

region, err := getRegion(d, config)
if err != nil {
return err
}
if err := d.Set("region", region); err != nil {
return fmt.Errorf("Error reading RegionPerInstanceConfig: %s", err)
}

if err := d.Set("name", flattenNestedComputeRegionPerInstanceConfigName(res["name"], d, config)); err != nil {
return fmt.Errorf("Error reading RegionPerInstanceConfig: %s", err)
}
Expand Down
17 changes: 7 additions & 10 deletions google/resource_compute_region_per_instance_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestAccComputeRegionPerInstanceConfig_statefulBasic(t *testing.T) {
"config_name4": fmt.Sprintf("instance-%s", randString(t, 10)),
}
rigmId := fmt.Sprintf("projects/%s/regions/%s/instanceGroupManagers/%s",
getTestProjectFromEnv(), "us-central1", rigmName)
getTestProjectFromEnv(), getTestRegionFromEnv(), rigmName)

vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -38,7 +38,7 @@ func TestAccComputeRegionPerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_region_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "region"},
},
{
// Force-recreate old config
Expand All @@ -51,7 +51,7 @@ func TestAccComputeRegionPerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_region_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "region"},
},
{
// Add two new endpoints
Expand All @@ -61,7 +61,7 @@ func TestAccComputeRegionPerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_region_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "region"},
},
{
ResourceName: "google_compute_region_per_instance_config.with_disks",
Expand All @@ -73,7 +73,7 @@ func TestAccComputeRegionPerInstanceConfig_statefulBasic(t *testing.T) {
ResourceName: "google_compute_region_per_instance_config.add2",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "region"},
},
{
// delete all configs
Expand Down Expand Up @@ -109,7 +109,7 @@ func TestAccComputeRegionPerInstanceConfig_update(t *testing.T) {
ResourceName: "google_compute_region_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "region"},
},
{
// Update an existing config
Expand All @@ -119,7 +119,7 @@ func TestAccComputeRegionPerInstanceConfig_update(t *testing.T) {
ResourceName: "google_compute_region_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy", "region"},
},
},
})
Expand All @@ -128,7 +128,6 @@ func TestAccComputeRegionPerInstanceConfig_update(t *testing.T) {
func testAccComputeRegionPerInstanceConfig_statefulBasic(context map[string]interface{}) string {
return Nprintf(`
resource "google_compute_region_per_instance_config" "default" {
region = google_compute_region_instance_group_manager.rigm.region
region_instance_group_manager = google_compute_region_instance_group_manager.rigm.name
name = "%{config_name}"
remove_instance_state_on_destroy = true
Expand All @@ -144,7 +143,6 @@ resource "google_compute_region_per_instance_config" "default" {
func testAccComputeRegionPerInstanceConfig_update(context map[string]interface{}) string {
return Nprintf(`
resource "google_compute_region_per_instance_config" "default" {
region = google_compute_region_instance_group_manager.rigm.region
region_instance_group_manager = google_compute_region_instance_group_manager.rigm.name
name = "%{config_name}"
remove_instance_state_on_destroy = true
Expand Down Expand Up @@ -293,7 +291,6 @@ resource "google_compute_region_instance_group_manager" "rigm" {
}
base_instance_name = "rigm-no-tp"
region = "us-central1"
update_policy {
instance_redistribution_type = "NONE"
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/compute_instance.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ The following arguments are supported:
* `name` - (Required) A unique name for the resource, required by GCE.
Changing this forces a new resource to be created.

* `zone` - (Required) The zone that the machine should be created in.
* `zone` - (Optional) The zone that the machine should be created in. If it is not provided, the provider zone is used.

* `network_interface` - (Required) Networks to attach to the instance. This can
be specified multiple times. Structure is documented below.
Expand Down
8 changes: 4 additions & 4 deletions website/docs/r/compute_per_instance_config.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ The following arguments are supported:
(Required)
The name for this per-instance config and its corresponding instance.

* `zone` -
(Required)
Zone where the containing instance group manager is located

* `instance_group_manager` -
(Required)
The instance group manager this instance config is part of.
Expand All @@ -132,6 +128,10 @@ The following arguments are supported:
The preserved state for this instance.
Structure is documented below.

* `zone` -
(Optional)
Zone where the containing instance group manager is located

* `project` - (Optional) The ID of the project in which the resource belongs.
If it is not provided, the provider project is used.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ The following arguments are supported:
[RFC1035](https://www.ietf.org/rfc/rfc1035.txt). Supported characters
include lowercase letters, numbers, and hyphens.

* `region` - (Required) The region where the managed instance group resides.
* `region` - (Optional) The region where the managed instance group resides. If not provided, the provider region is used.

- - -

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ The following arguments are supported:
(Required)
The name for this per-instance config and its corresponding instance.

* `region` -
(Required)
Region where the containing instance group manager is located

* `region_instance_group_manager` -
(Required)
The region instance group manager this instance config is part of.
Expand All @@ -133,6 +129,10 @@ The following arguments are supported:
The preserved state for this instance.
Structure is documented below.

* `region` -
(Optional)
Region where the containing instance group manager is located

* `project` - (Optional) The ID of the project in which the resource belongs.
If it is not provided, the provider project is used.

Expand Down

0 comments on commit d937ce4

Please sign in to comment.