Add remove_instance_on_destroy to per-instance config resources.

It's a bit counterintuitive that creating a per-instance config in an IGM spins up an instance but destroying it leaves the instance behind. Also fixed a bug related to the operation to detach the disk from the instance failing due to the instance having been deleted. Fixes GoogleCloudPlatform#9042 & #16621.
drcapulet · Dec 6, 2023 · 55f1a85 · 55f1a85
1 parent 1ea0673
commit 55f1a85
Show file tree

Hide file tree

Showing 10 changed files with 600 additions and 27 deletions.
diff --git a/mmv1/products/compute/PerInstanceConfig.yaml b/mmv1/products/compute/PerInstanceConfig.yaml
@@ -100,8 +100,19 @@ virtual_fields:
       - :REFRESH
       - :NONE
     default_value: :REPLACE
+  - !ruby/object:Api::Type::Boolean
+    name: 'remove_instance_on_destroy'
+    conflicts:
+      - remove_instance_state_on_destroy
+    description: |
+      When true, deleting this config will immediately remove the underlying instance.
+      When false, deleting this config will *not* remove the underlying instance but will
+      remove any specified state.
+    default_value: false
   - !ruby/object:Api::Type::Boolean
     name: 'remove_instance_state_on_destroy'
+    conflicts:
+      - remove_instance_on_destroy
     description: |
       When true, deleting this config will immediately remove any specified state from the underlying instance.
       When false, deleting this config will *not* immediately remove any state from the underlying instance.
@@ -110,7 +121,6 @@ virtual_fields:
 custom_code: !ruby/object:Provider::Terraform::CustomCode
   encoder: templates/terraform/encoders/compute_per_instance_config.go.erb
   update_encoder: templates/terraform/update_encoder/compute_per_instance_config.go.erb
-  pre_delete: templates/terraform/pre_delete/compute_per_instance_config.go.erb
   post_update: templates/terraform/post_update/compute_per_instance_config.go.erb
   custom_delete: templates/terraform/custom_delete/per_instance_config.go.erb
 parameters:

diff --git a/mmv1/products/compute/RegionPerInstanceConfig.yaml b/mmv1/products/compute/RegionPerInstanceConfig.yaml
@@ -101,8 +101,19 @@ virtual_fields:
       - :REFRESH
       - :NONE
     default_value: :REPLACE
+  - !ruby/object:Api::Type::Boolean
+    name: 'remove_instance_on_destroy'
+    conflicts:
+      - remove_instance_state_on_destroy
+    description: |
+      When true, deleting this config will immediately remove the underlying instance.
+      When false, deleting this config will *not* remove the underlying instance but will
+      remove any specified state.
+    default_value: false
   - !ruby/object:Api::Type::Boolean
     name: 'remove_instance_state_on_destroy'
+    conflicts:
+      - remove_instance_on_destroy
     description: |
       When true, deleting this config will immediately remove any specified state from the underlying instance.
       When false, deleting this config will *not* immediately remove any state from the underlying instance.
@@ -111,7 +122,6 @@ virtual_fields:
 custom_code: !ruby/object:Provider::Terraform::CustomCode
   encoder: templates/terraform/encoders/compute_per_instance_config.go.erb
   update_encoder: templates/terraform/update_encoder/compute_per_instance_config.go.erb
-  pre_delete: templates/terraform/pre_delete/compute_per_instance_config.go.erb
   post_update: templates/terraform/post_update/compute_region_per_instance_config.go.erb
   custom_delete: templates/terraform/custom_delete/region_per_instance_config.go.erb
 parameters:

diff --git a/mmv1/templates/terraform/custom_delete/per_instance_config.go.erb b/mmv1/templates/terraform/custom_delete/per_instance_config.go.erb
@@ -10,14 +10,31 @@
 	transport_tpg.MutexStore.Lock(lockName)
 	defer transport_tpg.MutexStore.Unlock(lockName)
 
-	url, err := tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{instance_group_manager}}/deletePerInstanceConfigs")
+	var url string
+	if d.Get("remove_instance_on_destroy").(bool) {
+		url, err = tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{instance_group_manager}}/deleteInstances")
+	} else {
+		url, err = tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{instance_group_manager}}/deletePerInstanceConfigs")
+	}
 	if err != nil {
 		return err
 	}
 
 	var obj map[string]interface{}
-	obj = map[string]interface{}{
-		"names": [1]string{d.Get("name").(string)},
+	if d.Get("remove_instance_on_destroy").(bool) {
+		// Instance name in deleteInstances request must include zone
+		instanceName, err := tpgresource.ReplaceVars(d, config, "zones/{{zone}}/instances/{{name}}")
+		if err != nil {
+			return err
+		}
+
+		obj = map[string]interface{}{
+			"instances": [1]string{instanceName},
+		}
+	} else {
+		obj = map[string]interface{}{
+			"names": [1]string{d.Get("name").(string)},
+		}
 	}
 	log.Printf("[DEBUG] Deleting PerInstanceConfig %q", d.Id())
 
@@ -42,8 +59,14 @@
 		return err
 	}
 
-	// Potentially delete the state managed by this config
-	if d.Get("remove_instance_state_on_destroy").(bool) {
+	if d.Get("remove_instance_on_destroy").(bool) {
+		err = transport_tpg.PollingWaitTime(resourceComputePerInstanceConfigInstancePollRead(d, meta, d.Get("name").(string)), PollCheckInstanceConfigInstanceDeleted, "Deleting PerInstanceConfig", d.Timeout(schema.TimeoutDelete), 1)
+		if err != nil {
+			return fmt.Errorf("Error waiting for instance delete on PerInstanceConfig %q: %s", d.Id(), err)
+		}
+	} else if d.Get("remove_instance_state_on_destroy").(bool) {
+		// Potentially delete the state managed by this config
+
 		// Instance name in applyUpdatesToInstances request must include zone
 		instanceName, err := tpgresource.ReplaceVars(d, config, "zones/{{zone}}/instances/{{name}}")
 		if err != nil {
@@ -85,7 +108,7 @@
 		err = transport_tpg.PollingWaitTime(resourceComputePerInstanceConfigPollRead(d, meta), PollCheckInstanceConfigDeleted, "Deleting PerInstanceConfig", d.Timeout(schema.TimeoutDelete), 1)
 		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)

diff --git a/mmv1/templates/terraform/custom_delete/region_per_instance_config.go.erb b/mmv1/templates/terraform/custom_delete/region_per_instance_config.go.erb
@@ -10,14 +10,31 @@
 	transport_tpg.MutexStore.Lock(lockName)
 	defer transport_tpg.MutexStore.Unlock(lockName)
 
-	url, err := tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/instanceGroupManagers/{{region_instance_group_manager}}/deletePerInstanceConfigs")
+	var url string
+	if d.Get("remove_instance_on_destroy").(bool) {
+		url, err = tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/instanceGroupManagers/{{region_instance_group_manager}}/deleteInstances")
+	} else {
+		url, err = tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/regions/{{region}}/instanceGroupManagers/{{region_instance_group_manager}}/deletePerInstanceConfigs")
+	}
 	if err != nil {
 		return err
 	}
 
 	var obj map[string]interface{}
-	obj = map[string]interface{}{
-		"names": [1]string{d.Get("name").(string)},
+	if d.Get("remove_instance_on_destroy").(bool) {
+		// Instance name in deleteInstances request must include zone
+		instanceName, err := findInstanceName(d, config)
+		if err != nil {
+			return err
+		}
+
+		obj = map[string]interface{}{
+			"instances": [1]string{instanceName},
+		}
+	} else {
+		obj = map[string]interface{}{
+			"names": [1]string{d.Get("name").(string)},
+		}
 	}
 	log.Printf("[DEBUG] Deleting RegionPerInstanceConfig %q", d.Id())
 
@@ -42,8 +59,13 @@
 		return err
 	}
 
-	// Potentially delete the state managed by this config
-	if d.Get("remove_instance_state_on_destroy").(bool) {
+	if d.Get("remove_instance_on_destroy").(bool) {
+		err = transport_tpg.PollingWaitTime(resourceComputeRegionPerInstanceConfigInstancePollRead(d, meta, d.Get("name").(string)), PollCheckInstanceConfigInstanceDeleted, "Deleting RegionPerInstanceConfig", d.Timeout(schema.TimeoutDelete), 1)
+		if err != nil {
+			return fmt.Errorf("Error waiting for instance delete on RegionPerInstanceConfig %q: %s", d.Id(), err)
+		}
+	} else if d.Get("remove_instance_state_on_destroy").(bool) {
+		// Potentially delete the state managed by this config
 		// Instance name in applyUpdatesToInstances request must include zone
 		instanceName, err := findInstanceName(d, config)
 		if err != nil {
@@ -86,7 +108,7 @@
 		err = transport_tpg.PollingWaitTime(resourceComputeRegionPerInstanceConfigPollRead(d, meta), PollCheckInstanceConfigDeleted, "Deleting RegionPerInstanceConfig", d.Timeout(schema.TimeoutDelete), 1)
 		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)

diff --git a/mmv1/templates/terraform/pre_delete/compute_per_instance_config.go.erb b/mmv1/templates/terraform/pre_delete/compute_per_instance_config.go.erb
diff --git a/mmv1/templates/terraform/pre_delete/detach_disk.erb b/mmv1/templates/terraform/pre_delete/detach_disk.erb
@@ -56,7 +56,8 @@ if v, ok := readRes["users"].([]interface{}); ok {
 		err = ComputeOperationWaitTime(config, op, call.project,
 			fmt.Sprintf("Detaching disk from %s/%s/%s", call.project, call.zone, call.instance), userAgent, d.Timeout(schema.TimeoutDelete))
 		if err != nil {
-			if opErr, ok := err.(ComputeOperationError); ok && len(opErr.Errors) == 1 && opErr.Errors[0].Code == "RESOURCE_NOT_FOUND" {
+			var opErr ComputeOperationError
+			if errors.As(err, &opErr) && len(opErr.Errors) == 1 && opErr.Errors[0].Code == "RESOURCE_NOT_FOUND" {
 				log.Printf("[WARN] instance %q was deleted while awaiting detach", call.instance)
 				continue
 			}