-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feature gap: Implement targetSizePolicy.mode to InstanceGroupManager
#16615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -623,6 +623,24 @@ func ResourceComputeInstanceGroupManager() *schema.Resource { | |
| }, | ||
| }, | ||
| }, | ||
| {{ if ne $.TargetVersionName `ga` }} | ||
| "target_size_policy": { | ||
| Type: schema.TypeList, | ||
| Optional: true, | ||
| Description: `The target size policy for the instance group.`, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "mode": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| ForceNew: true, | ||
| Description: `The mode of the target size policy.`, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ValidateFunc: validation.StringInSlice([]string{"BULK", "INDIVIDUAL"}, false), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| {{- end }} | ||
| }, | ||
| UseJSONNumber: true, | ||
| } | ||
|
|
@@ -722,6 +740,7 @@ func resourceComputeInstanceGroupManagerCreate(d *schema.ResourceData, meta inte | |
| StatefulPolicy: expandStatefulPolicy(d), | ||
| ResourcePolicies: expandResourcePolicies(d.Get("resource_policies").([]interface{})), | ||
| {{- if ne $.TargetVersionName "ga" }} | ||
| TargetSizePolicy: expandTargetSizePolicy(d.Get("target_size_policy").([]interface{})), | ||
| Params: expandInstanceGroupManagerParams(d), | ||
| {{- end }} | ||
|
|
||
|
|
@@ -971,6 +990,11 @@ func resourceComputeInstanceGroupManagerRead(d *schema.ResourceData, meta interf | |
| if err = d.Set("resource_policies", flattenResourcePolicies(manager.ResourcePolicies)); err != nil { | ||
| return fmt.Errorf("Error setting resource_policies in state: %s", err.Error()) | ||
| } | ||
| {{ if ne $.TargetVersionName `ga` }} | ||
| if err = d.Set("target_size_policy", flattenTargetSizePolicy(manager.TargetSizePolicy)); err != nil { | ||
| return fmt.Errorf("Error setting target_size_policy in state: %s", err.Error()) | ||
| } | ||
| {{- end }} | ||
|
|
||
| // If unset in state set to default value | ||
| if d.Get("wait_for_instances_status").(string) == "" { | ||
|
|
@@ -1080,6 +1104,15 @@ func resourceComputeInstanceGroupManagerUpdate(d *schema.ResourceData, meta inte | |
| change = true | ||
| } | ||
|
|
||
| {{ if ne $.TargetVersionName `ga` }} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK this feature should go to GA. Please make sure whether it needs to go to Beta or straight to GA |
||
| // TODO: Target size policy is not updatable yet, but if it changes we need to update the manager with the new policy | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I may be wrong (I rarely work on TF) but shouldn't those field be set and let it fail on igm.Update? Now a caller will pass target_size_policy and it will be ignored on update. Not to mention that commented out code is not a good practice
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @askubis says that force new is the way to go in terraform. Please adjust code to that |
||
| // if d.HasChange("target_size_policy") { | ||
| // updatedManager.TargetSizePolicy = expandTargetSizePolicy(d.Get("target_size_policy").([]interface{})) | ||
| // updatedManager.ForceSendFields = append(updatedManager.ForceSendFields, "TargetSizePolicy") | ||
| // change = true | ||
| // } | ||
| {{- end }} | ||
|
|
||
| if change { | ||
| op, err := config.NewComputeClient(userAgent).InstanceGroupManagers.Patch(project, zone, d.Get("name").(string), updatedManager).Do() | ||
| if err != nil { | ||
|
|
@@ -1346,6 +1379,16 @@ func expandVersions(configured []interface{}) []*compute.InstanceGroupManagerVer | |
| return versions | ||
| } | ||
|
|
||
| func expandTargetSizePolicy(configured []interface{}) *compute.InstanceGroupManagerTargetSizePolicy { | ||
| if len(configured) == 0 || configured[0] == nil { | ||
| return nil | ||
| } | ||
| data := configured[0].(map[string]interface{}) | ||
| return &compute.InstanceGroupManagerTargetSizePolicy{ | ||
| Mode: data["mode"].(string), | ||
| } | ||
| } | ||
|
|
||
| func expandResourcePolicies(configured []interface{}) *compute.InstanceGroupManagerResourcePolicies { | ||
| resourcePolicies := &compute.InstanceGroupManagerResourcePolicies{} | ||
|
|
||
|
|
@@ -1734,6 +1777,17 @@ func flattenStatusAllInstancesConfig(allInstancesConfig *compute.InstanceGroupMa | |
| return results | ||
| } | ||
|
|
||
| func flattenTargetSizePolicy(targetSizePolicy *compute.InstanceGroupManagerTargetSizePolicy) []map[string]interface{} { | ||
| if targetSizePolicy == nil { | ||
| return nil | ||
| } | ||
| return []map[string]interface{}{ | ||
| { | ||
| "mode": targetSizePolicy.Mode, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func flattenResourcePolicies(resourcePolicies *compute.InstanceGroupManagerResourcePolicies) []map[string]interface{} { | ||
| results := []map[string]interface{}{} | ||
| if resourcePolicies != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2360,6 +2360,81 @@ resource "google_compute_instance_group_manager" "igm-workload-policy" { | |
|
|
||
|
|
||
| {{ if ne $.TargetVersionName `ga` -}} | ||
| func TestAccInstanceGroupManager_targetSizePolicy(t *testing.T) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it doable to write a test for failure case when target_size_policy is being updated? |
||
| t.Parallel() | ||
|
|
||
| templateName := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) | ||
| igmName := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) | ||
|
|
||
| acctest.VcrTest(t, resource.TestCase{ | ||
| PreCheck: func() { acctest.AccTestPreCheck(t) }, | ||
| ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), | ||
| CheckDestroy: testAccCheckInstanceGroupManagerDestroyProducer(t), | ||
| Steps: []resource.TestStep{ | ||
| { | ||
| Config: testAccInstanceGroupManager_targetSizePolicy(templateName, igmName), | ||
| Check: resource.ComposeTestCheckFunc( | ||
| resource.TestCheckResourceAttr("google_compute_instance_group_manager.igm-tsp", "target_size_policy.0.mode", "INDIVIDUAL"), | ||
| ), | ||
| }, | ||
| { | ||
| ResourceName: "google_compute_instance_group_manager.igm-tsp", | ||
| ImportState: true, | ||
| ImportStateVerify: true, | ||
| ImportStateVerifyIgnore: []string{"status"}, | ||
| }, | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| func testAccInstanceGroupManager_targetSizePolicy(template, igm string) string { | ||
| return fmt.Sprintf(` | ||
| data "google_compute_image" "my_image" { | ||
| family = "debian-11" | ||
| project = "debian-cloud" | ||
| } | ||
|
|
||
| resource "google_compute_instance_template" "igm-basic" { | ||
| name = "%s" | ||
| machine_type = "e2-medium" | ||
| can_ip_forward = false | ||
| tags = ["foo", "bar"] | ||
|
|
||
| disk { | ||
| source_image = data.google_compute_image.my_image.self_link | ||
| auto_delete = true | ||
| boot = true | ||
| } | ||
|
|
||
| network_interface { | ||
| network = "default" | ||
| } | ||
|
|
||
| service_account { | ||
| scopes = ["userinfo-email", "compute-ro", "storage-ro"] | ||
| } | ||
| } | ||
|
|
||
| resource "google_compute_instance_group_manager" "igm-tsp" { | ||
| description = "Terraform test instance group manager" | ||
| name = "%s" | ||
|
|
||
| version { | ||
| name = "primary" | ||
| instance_template = google_compute_instance_template.igm-basic.self_link | ||
| } | ||
|
|
||
| base_instance_name = "tf-test-igm-tsp" | ||
| zone = "us-central1-c" | ||
| target_size = 2 | ||
|
|
||
| target_size_policy { | ||
| mode = "INDIVIDUAL" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BULK would be more interesting IMO. INDIVIDUAL is the default value when target size policy is not set |
||
| } | ||
| } | ||
| `, template, igm) | ||
| } | ||
|
|
||
| func testAccInstanceGroupManager_resourceManagerTags(template_name, tag_name, igm_name, project_id string) string { | ||
| return fmt.Sprintf(` | ||
| data "google_compute_image" "my_image" { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -256,6 +256,8 @@ group. You can specify only one value. Structure is [documented below](#nested_a | |
|
|
||
| * `resource_policies` - (Optional) Resource policies for this managed instance group. Structure is [documented below](#nested_resource_policies). | ||
|
|
||
| * `target_size_policy` - (Optional [Beta](../guides/provider_versions.html.markdown)) The target size policy for the instance group. Structure is [documented below](#nested_target_size_policy). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I'd also prefer to have the official documentation of the fields |
||
|
|
||
| - - - | ||
|
|
||
| The `standby_policy` block supports: | ||
|
|
@@ -438,6 +440,10 @@ params{ | |
|
|
||
| * `workload_policy` - (Optional) The URL of the workload policy that is specified for this managed instance group. It can be a full or partial URL. | ||
|
|
||
| <a name="nested_target_size_policy"></a>The `target_size_policy` block supports: | ||
|
|
||
| * `mode` - (Required) The mode of the target size policy. Values: "BULK", "INDIVIDUAL". | ||
|
|
||
| ## Attributes Reference | ||
|
|
||
| In addition to the arguments listed above, the following computed attributes are | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the same documentation we have for the API field?
The policy that specifies how the MIG creates its VMs to achieve the target size.