allow updating additional_zones, turn it into a set #152
Conversation
google/resource_container_cluster.go
Outdated
| if waitErr != nil { | ||
| return waitErr | ||
| if d.HasChange("additional_zones") { | ||
| azs := convertStringArr(d.Get("additional_zones").(*schema.Set).List()) |
There was a problem hiding this comment.
convertStringArr is defined in resource_compute_target_pool.go - is there a better place to put it?
There was a problem hiding this comment.
I put it in provider.go, I think that's the best option we have righ tnow.
google/resource_container_cluster.go
Outdated
| return waitErr | ||
| if d.HasChange("additional_zones") { | ||
| azs := convertStringArr(d.Get("additional_zones").(*schema.Set).List()) | ||
| locations := append(azs, zoneName) |
There was a problem hiding this comment.
I'm guessing there's no problem if a location is repeated multiple times (i.e. the API doesn't throw an error)? Otherwise it might make sense to do this call before converting it to a list
There was a problem hiding this comment.
Good catch! The API does throw an error. I just added a check like we have in Create
| switch v { | ||
| case 0: | ||
| log.Println("[INFO] Found Container Cluster State v0; migrating to v1") | ||
| is, err := migrateClusterStateV0toV1(is) |
There was a problem hiding this comment.
I think this can be shorted to just
return migrateCLusterStateV0toV1(is)
| } | ||
| } | ||
|
|
||
| func migrateClusterStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { |
There was a problem hiding this comment.
nit: capital 't' in the to part of migrateClusterStateV0toV1?
There was a problem hiding this comment.
Leaving it lowercase to match the other files
| @@ -219,6 +231,10 @@ func testAccCheckContainerClusterDestroy(s *terraform.State) error { | |||
| return nil | |||
| } | |||
|
|
|||
| var setFields map[string]struct{} = map[string]struct{}{ | |||
There was a problem hiding this comment.
Could this just be a list/set of strings rather than having an unused struct{} value?
There was a problem hiding this comment.
This is basically how you do a set in go, since there's no native set type (there's the terraform-specific one but I don't want to rely on terraform implementation in tests), and lists don't have a contains function so you'd have to scan the whole thing.
| "change additional_zones from list to set": { | ||
| StateVersion: 0, | ||
| Attributes: map[string]string{ | ||
| "additional_zones.#": "1", |
<!-- This change is generated by MagicModules. --> /cc @danawillow
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #104.
additional_zoneshad to be turned into a set because GCP returns them in alphabetical order regardless of the order they're set in the request.