provider/google: Fixed instance group manager to allow 0 instance #13492
Conversation
| @@ -144,6 +144,7 @@ func resourceComputeInstanceGroupManagerCreate(d *schema.ResourceData, meta inte | |||
| BaseInstanceName: d.Get("base_instance_name").(string), | |||
| InstanceTemplate: d.Get("instance_template").(string), | |||
| TargetSize: target_size, | |||
| ForceSendFields: []string{"Name", "BaseInstanceName", "InstanceTemplate", "TargetSize"}, | |||
There was a problem hiding this comment.
This is required, otherwise TargetSize: 0 will be dropped when request is made to the Google API.
See googleapis/google-api-go-client#197 for the detail.
| @@ -256,7 +257,6 @@ func resourceComputeInstanceGroupManagerRead(d *schema.ResourceData, meta interf | |||
| d.Set("named_port", flattenNamedPorts(manager.NamedPorts)) | |||
| d.Set("fingerprint", manager.Fingerprint) | |||
| d.Set("instance_group", manager.InstanceGroup) | |||
| d.Set("target_size", manager.TargetSize) | |||
There was a problem hiding this comment.
This was the duplicated line; target_size is set on the 5 lines above.
|
|
||
| Key: "from_port", | ||
| Value: 0, | ||
| Ok: true, |
|
|
||
| Key: "ports", | ||
| Value: []interface{}{}, | ||
| Ok: true, |
|
|
||
| Key: "availability_zone", | ||
| Value: "", | ||
| Ok: true, |
|
Hello, can someone review this & #13493 pulls? |
|
Hey @tmshn! Thanks so much for the PR and your interest in contributing to Terraform, it's really appreciated. This is actually a long-running, complicated, and out of my depth issue. In particular, there have historically been challenges with representing unset values in diffs that make the behavior of There's a lot going on this PR, and it spans two different areas of expertise (the GCP provider and the core Terraform code). Because of that, it's unlikely to get a review in its current form, just because no one person on the team has the context and domain knowledge in both those areas of the codebase to be able to give it the attention and thought it deserves. I think the best way forward for this is:
Hopefully that sounds OK. I definitely don't want to discourage you from contributing or from trying to tackle this problem, but it's going to be hard to tackle both the provider-level changes and the schema-level changes in a single PR. |
|
@paddycarver I got it, thanks for the advice! I'll open the new and smaller-scoped pull requests. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Problem solved
As pointed in #5690, even if you configure the
google_compute_instance_group_managerwithtarget_size = 0, that is ignored on the resulting resource (both on first creation or update).This PR will fix this.
Discussion point
To fix this, I added
GetOkAllowZeromethod to theResourceData, which is wanted in #5694 (note: #993 is also relevant PR).But actually, I don't understand whole code of the Terraform, so I don't know if this is the good way of achieving it.
Also, naming of the method can be questioned.