Skip to content

Commit

Permalink
Per instance config delete underlying instance (#3635) (#2187)
Browse files Browse the repository at this point in the history 
* Adding virtual field to control deletion of state after config is deleted

* Add support for deleting underlying instances for stateful MIG via virtual field. Poll for delete

* Update comment in test

* Remove token param if no token defined

* Add update tests

Signed-off-by: Modular Magician <[email protected]>
  • Loading branch information
@modular-magician @emilymye
modular-magician authored and emilymye committed Jun 17, 2020
1 parent 3170ee7 commit 6533eba
Show file tree
Hide file tree
Showing 8 changed files with 402 additions and 36 deletions.
6 changes: 6 additions & 0 deletions .changelog/3635.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:enhancement
compute: Added `remove_instance_state_on_destroy` to `google_compute_region_per_instance_config` to control deletion of underlying instance state. (beta only)
```
```release-note:enhancement
compute: Added `remove_instance_state_on_destroy` to `google_compute_per_instance_config` to control deletion of underlying instance state. (beta only)
```
53 changes: 50 additions & 3 deletions google-beta/resource_compute_per_instance_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ func resourceComputePerInstanceConfig() *schema.Resource {
Optional: true,
Default: "REPLACE",
},
"remove_instance_state_on_destroy": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
"project": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -163,7 +168,7 @@ func resourceComputePerInstanceConfigCreate(d *schema.ResourceData, meta interfa
return err
}

lockName, err := replaceVars(d, config, "instangeGroupManager/{{project}}/{{zone}}/{{instance_group_manager}}")
lockName, err := replaceVars(d, config, "instanceGroupManager/{{project}}/{{zone}}/{{instance_group_manager}}")
if err != nil {
return err
}
Expand Down Expand Up @@ -243,6 +248,9 @@ func resourceComputePerInstanceConfigRead(d *schema.ResourceData, meta interface
if _, ok := d.GetOk("most_disruptive_allowed_action"); !ok {
d.Set("most_disruptive_allowed_action", "REPLACE")
}
if _, ok := d.GetOk("remove_instance_state_on_destroy"); !ok {
d.Set("remove_instance_state_on_destroy", false)
}
if err := d.Set("project", project); err != nil {
return fmt.Errorf("Error reading PerInstanceConfig: %s", err)
}
Expand Down Expand Up @@ -284,7 +292,7 @@ func resourceComputePerInstanceConfigUpdate(d *schema.ResourceData, meta interfa
return err
}

lockName, err := replaceVars(d, config, "instangeGroupManager/{{project}}/{{zone}}/{{instance_group_manager}}")
lockName, err := replaceVars(d, config, "instanceGroupManager/{{project}}/{{zone}}/{{instance_group_manager}}")
if err != nil {
return err
}
Expand Down Expand Up @@ -362,7 +370,7 @@ func resourceComputePerInstanceConfigDelete(d *schema.ResourceData, meta interfa
return err
}

lockName, err := replaceVars(d, config, "instangeGroupManager/{{project}}/{{zone}}/{{instance_group_manager}}")
lockName, err := replaceVars(d, config, "instanceGroupManager/{{project}}/{{zone}}/{{instance_group_manager}}")
if err != nil {
return err
}
Expand Down Expand Up @@ -393,6 +401,44 @@ func resourceComputePerInstanceConfigDelete(d *schema.ResourceData, meta interfa
return err
}

// Potentially delete the state managed by this config
if d.Get("remove_instance_state_on_destroy").(bool) {
// Instance name in applyUpdatesToInstances request must include zone
instanceName, err := replaceVars(d, config, "zones/{{zone}}/instances/{{name}}")
if err != nil {
return err
}

obj = make(map[string]interface{})
obj["instances"] = []string{instanceName}

// The deletion must be applied to the instance after the PerInstanceConfig is deleted
url, err = replaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{instance_group_manager}}/applyUpdatesToInstances")
if err != nil {
return err
}

log.Printf("[DEBUG] Applying updates to PerInstanceConfig %q: %#v", d.Id(), obj)
res, err = sendRequestWithTimeout(config, "POST", project, url, obj, d.Timeout(schema.TimeoutUpdate))

if err != nil {
return fmt.Errorf("Error deleting PerInstanceConfig %q: %s", d.Id(), err)
}

err = computeOperationWaitTime(
config, res, project, "Applying update to PerInstanceConfig",
d.Timeout(schema.TimeoutUpdate))
if err != nil {
return fmt.Errorf("Error deleting PerInstanceConfig %q: %s", d.Id(), err)
}

// PerInstanceConfig goes into "DELETING" state while the instance is actually deleted
err = PollingWaitTime(resourceComputePerInstanceConfigPollRead(d, meta), PollCheckInstanceConfigDeleted, "Deleting PerInstanceConfig", d.Timeout(schema.TimeoutDelete))
if err != nil {
return fmt.Errorf("Error waiting for delete on PerInstanceConfig %q: %s", d.Id(), err)
}
}

log.Printf("[DEBUG] Finished deleting PerInstanceConfig %q: %#v", d.Id(), res)
return nil
}
Expand All @@ -418,6 +464,7 @@ func resourceComputePerInstanceConfigImport(d *schema.ResourceData, meta interfa
// Explicitly set virtual fields to default values on import
d.Set("minimal_action", "NONE")
d.Set("most_disruptive_allowed_action", "REPLACE")
d.Set("remove_instance_state_on_destroy", false)

return []*schema.ResourceData{d}, nil
}
Expand Down
89 changes: 75 additions & 14 deletions google-beta/resource_compute_per_instance_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
Config: testAccComputePerInstanceConfig_statefulBasic(context),
},
{
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
},
{
// Force-recreate old config
Expand All @@ -42,52 +43,109 @@ func TestAccComputePerInstanceConfig_statefulBasic(t *testing.T) {
),
},
{
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
},
{
// Add two new endpoints
Config: testAccComputePerInstanceConfig_statefulAdditional(context),
},
{
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
},
{
ResourceName: "google_compute_per_instance_config.with_disks",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"most_disruptive_allowed_action", "minimal_action"},
ImportStateVerifyIgnore: []string{"most_disruptive_allowed_action", "minimal_action", "remove_instance_state_on_destroy"},
},
{
ResourceName: "google_compute_per_instance_config.add2",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_compute_per_instance_config.add2",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
},
{
// delete all configs
Config: testAccComputePerInstanceConfig_igm(context),
Check: resource.ComposeTestCheckFunc(
// Config with remove_instance_state_on_destroy = false won't be destroyed (config4)
testAccCheckComputePerInstanceConfigDestroyed(t, igmId, context["config_name2"].(string)),
testAccCheckComputePerInstanceConfigDestroyed(t, igmId, context["config_name3"].(string)),
testAccCheckComputePerInstanceConfigDestroyed(t, igmId, context["config_name4"].(string)),
),
},
},
})
}

func TestAccComputePerInstanceConfig_update(t *testing.T) {
t.Parallel()

context := map[string]interface{}{
"random_suffix": randString(t, 10),
"config_name": fmt.Sprintf("instance-%s", randString(t, 10)),
}

vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
// Create one config
Config: testAccComputePerInstanceConfig_statefulBasic(context),
},
{
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
},
{
// Update an existing config
Config: testAccComputePerInstanceConfig_update(context),
},
{
ResourceName: "google_compute_per_instance_config.default",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_instance_state_on_destroy"},
},
},
})
}

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
preserved_state {
metadata = {
asdf = "asdf"
}
}
}
`, context) + testAccComputePerInstanceConfig_igm(context)
}

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
preserved_state {
metadata = {
asdf = "asdf"
update = "12345"
}
}
}
Expand All @@ -100,6 +158,7 @@ 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_name2}"
remove_instance_state_on_destroy = true
preserved_state {
metadata = {
asdf = "asdf"
Expand All @@ -115,6 +174,7 @@ 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_name2}"
remove_instance_state_on_destroy = true
preserved_state {
metadata = {
asdf = "asdf"
Expand All @@ -128,6 +188,7 @@ resource "google_compute_per_instance_config" "with_disks" {
name = "%{config_name3}"
most_disruptive_allowed_action = "REFRESH"
minimal_action = "REFRESH"
remove_instance_state_on_destroy = true
preserved_state {
metadata = {
meta = "123"
Expand Down
58 changes: 53 additions & 5 deletions google-beta/resource_compute_region_per_instance_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ func resourceComputeRegionPerInstanceConfig() *schema.Resource {
Optional: true,
Default: "REPLACE",
},
"remove_instance_state_on_destroy": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
"project": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -163,7 +168,7 @@ func resourceComputeRegionPerInstanceConfigCreate(d *schema.ResourceData, meta i
return err
}

lockName, err := replaceVars(d, config, "instangeGroupManager/{{project}}/{{region}}/{{region_instance_group_manager}}")
lockName, err := replaceVars(d, config, "instanceGroupManager/{{project}}/{{region}}/{{region_instance_group_manager}}")
if err != nil {
return err
}
Expand Down Expand Up @@ -243,6 +248,9 @@ func resourceComputeRegionPerInstanceConfigRead(d *schema.ResourceData, meta int
if _, ok := d.GetOk("most_disruptive_allowed_action"); !ok {
d.Set("most_disruptive_allowed_action", "REPLACE")
}
if _, ok := d.GetOk("remove_instance_state_on_destroy"); !ok {
d.Set("remove_instance_state_on_destroy", false)
}
if err := d.Set("project", project); err != nil {
return fmt.Errorf("Error reading RegionPerInstanceConfig: %s", err)
}
Expand Down Expand Up @@ -284,7 +292,7 @@ func resourceComputeRegionPerInstanceConfigUpdate(d *schema.ResourceData, meta i
return err
}

lockName, err := replaceVars(d, config, "instangeGroupManager/{{project}}/{{region}}/{{region_instance_group_manager}}")
lockName, err := replaceVars(d, config, "instanceGroupManager/{{project}}/{{region}}/{{region_instance_group_manager}}")
if err != nil {
return err
}
Expand All @@ -311,8 +319,8 @@ func resourceComputeRegionPerInstanceConfigUpdate(d *schema.ResourceData, meta i
return err
}

// Instance name in applyUpdatesToInstances request must include region
instanceName, err := replaceVars(d, config, "regoins/{{region}}/instances/{{name}}")
// Instance name in applyUpdatesToInstances request must include zone
instanceName, err := findInstanceName(d, config)
if err != nil {
return err
}
Expand Down Expand Up @@ -362,7 +370,7 @@ func resourceComputeRegionPerInstanceConfigDelete(d *schema.ResourceData, meta i
return err
}

lockName, err := replaceVars(d, config, "instangeGroupManager/{{project}}/{{region}}/{{region_instance_group_manager}}")
lockName, err := replaceVars(d, config, "instanceGroupManager/{{project}}/{{region}}/{{region_instance_group_manager}}")
if err != nil {
return err
}
Expand Down Expand Up @@ -393,6 +401,45 @@ func resourceComputeRegionPerInstanceConfigDelete(d *schema.ResourceData, meta i
return err
}

// Potentially delete the state managed by this config
if d.Get("remove_instance_state_on_destroy").(bool) {
// Instance name in applyUpdatesToInstances request must include zone
instanceName, err := findInstanceName(d, config)
if err != nil {
return err
}

obj = make(map[string]interface{})
obj["instances"] = []string{instanceName}

// Updates must be applied to the instance after deleting the PerInstanceConfig
url, err = replaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/instanceGroupManagers/{{region_instance_group_manager}}/applyUpdatesToInstances")
if err != nil {
return err
}

log.Printf("[DEBUG] Applying updates to PerInstanceConfig %q: %#v", d.Id(), obj)
res, err = sendRequestWithTimeout(config, "POST", project, url, obj, d.Timeout(schema.TimeoutUpdate))

if err != nil {
return fmt.Errorf("Error updating PerInstanceConfig %q: %s", d.Id(), err)
}

err = computeOperationWaitTime(
config, res, project, "Applying update to PerInstanceConfig",
d.Timeout(schema.TimeoutUpdate))

if err != nil {
return fmt.Errorf("Error deleting PerInstanceConfig %q: %s", d.Id(), err)
}

// RegionPerInstanceConfig goes into "DELETING" state while the instance is actually deleted
err = PollingWaitTime(resourceComputeRegionPerInstanceConfigPollRead(d, meta), PollCheckInstanceConfigDeleted, "Deleting RegionPerInstanceConfig", d.Timeout(schema.TimeoutDelete))
if err != nil {
return fmt.Errorf("Error waiting for delete on RegionPerInstanceConfig %q: %s", d.Id(), err)
}
}

log.Printf("[DEBUG] Finished deleting RegionPerInstanceConfig %q: %#v", d.Id(), res)
return nil
}
Expand All @@ -418,6 +465,7 @@ func resourceComputeRegionPerInstanceConfigImport(d *schema.ResourceData, meta i
// Explicitly set virtual fields to default values on import
d.Set("minimal_action", "NONE")
d.Set("most_disruptive_allowed_action", "REPLACE")
d.Set("remove_instance_state_on_destroy", false)

return []*schema.ResourceData{d}, nil
}
Expand Down
Loading

0 comments on commit 6533eba

Please sign in to comment.