provider/google: Support for unmanaged instance groups (google_compute_instance_group)

[ajcrowe]

I've added support for creating unmanaged Google Instance Groups. I've not been able to complete the suite of acceptance tests but I think they should work.

I've used a couple of functions found in google_compute_target_pool.go

• convertInstances() https://github.com/hashicorp/terraform/blob/master/builtin/providers/google/resource_compute_target_pool.go#L103
• convertStringArr() https://github.com/hashicorp/terraform/blob/master/builtin/providers/google/resource_compute_target_pool.go#L79
• calcAddRemove() https://github.com/hashicorp/terraform/blob/master/builtin/providers/google/resource_compute_target_pool.go#L167

Is having a dependency on these ok?

Let me know if anything needs changing more than happy to amend.

[sparkprime]

According to the API docs, this is not computed

[ajcrowe]

This is strange, the library docs list it as [Output Only] see https://godoc.org/google.golang.org/api/compute/v1#InstanceGroup

From my testing the adding of instances will populate the network into the state. or leave it blank should no instance be defined when it's created.

If you later add instance the network is populated correctly. However the network remains even after all instances are removed, this could be something to change?

[lwander]

@ajcrowe Provided tests all pass? LGTM, could you squash before I merge?

[ajcrowe]

I've squashed the commit and rebased to master. I've also removed the getNamedPorts function as this was pull into an update to the google_compute_instance_group_manager resource.

I've updated the docs to reflect the need to always use full self_link urls for instances and added a simple check for this. If you need any other changes let me know

[lwander]

Sweet! One last tiny nitpick - could you reformat your test configs to take a name parameter like here, and pass in a name with a random suffix like so? The motivation is explained in #4514. Your current chosen names shouldn't cause any conflicts, but we'd like to adopt this practice for all tests just in case.

[ajcrowe]

I've updated the acctests, I haven't been able to run them myself so please let me know if they are broken.

[lwander]

The tests are broken, 1, you're missing quotes around the instances resource you are referencing with ${}, and 2, those instances need to exist for the test to pass.

[ajcrowe]

This should be sorted now, sorry for the oversight!

[ajcrowe]

Thanks @lwander, sorry I missed this! I've pull your change in.

[lwander]

All set! sorry for the long wait.

[ajcrowe]

No worries Lars, pleased I could contribute to a great tool.

[ghost]

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.