Skip to content
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

Fix BackendService group hash when instance groups use beta features #522

Merged
merged 2 commits into from
Oct 4, 2017

Conversation

danawillow
Copy link
Contributor

Fixes #462.

The issue in #462 was that backend.group takes a self link of an instance group. Instance Group Managers have a beta field, auto_healing_policies. If that is set, then its self link will be the beta one since we have to read from the beta API in order to read that field. In that case, the example config ended up such that state had the v1 self link (because it reads from the v1 BackendServices API) but plan wanted to change it to a beta self link (because it reads from the beta InstanceGroupManagers API).

The link of the instance group is one of the factors that makes up the hash for the backend. In order to make sure changes in api version don't impact the backend hash, I've changed it to use the relative path instead of the full self link. Since this changes the value of the hash, I added a state migration.

While I was in the neighborhood, I also changed group to Required since it'll fail server-side if it's not set (and our documentation actually says it's Required) and added a self link DiffSuppress for it.

$ make testacc TEST=./google TESTARGS='-run=TestComputeBackendServiceMigrateState'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestComputeBackendServiceMigrateState -timeout 120m
=== RUN   TestComputeBackendServiceMigrateState
2017/10/03 16:46:42 [INFO] Found Compute Backend Service State v0; migrating to v1
2017/10/03 16:46:42 [DEBUG] Attributes before migration: map[string]string{"backend.242332812.balancing_mode":"UTILIZATION", "backend.242332812.max_utilization":"0.8", "backend.#":"1", "backend.242332812.group":"https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instanceGroups/igName"}
2017/10/03 16:46:42 [DEBUG] Attributes after migration: map[string]string{"backend.2573491210.balancing_mode":"UTILIZATION", "backend.2573491210.max_utilization":"0.8", "backend.2573491210.group":"https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instanceGroups/igName", "backend.#":"1"}
--- PASS: TestComputeBackendServiceMigrateState (0.00s)
=== RUN   TestComputeBackendServiceMigrateState_empty
2017/10/03 16:46:42 [DEBUG] Empty InstanceState; nothing to migrate.
2017/10/03 16:46:42 [DEBUG] Empty InstanceState; nothing to migrate.
--- PASS: TestComputeBackendServiceMigrateState_empty (0.00s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	0.159s
$ make testacc TEST=./google TESTARGS='-run=TestAccComputeBackendService_withBackend'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccComputeBackendService_withBackend -timeout 120m
=== RUN   TestAccComputeBackendService_withBackend
--- PASS: TestAccComputeBackendService_withBackend (93.22s)
=== RUN   TestAccComputeBackendService_withBackendAndUpdate
--- PASS: TestAccComputeBackendService_withBackendAndUpdate (105.21s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	198.636s

continue
}

if k == "backend.#" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could move it in the if above with a ||.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

StateVersion: 0,
Attributes: map[string]string{
"backend.#": "1",
"backend.242332812.group": "https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instanceGroups/igName",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add also a test with a beta group. Even though it wouldn't add much value I guess...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, not going to bother on this one since this test doesn't make any API calls or really look at the version

@danawillow danawillow merged commit 754e6da into hashicorp:master Oct 4, 2017
@danawillow danawillow deleted the is-462 branch October 4, 2017 23:49
negz pushed a commit to negz/terraform-provider-google that referenced this pull request Oct 17, 2017
negz pushed a commit to negz/terraform-provider-google that referenced this pull request Oct 17, 2017
luis-silva pushed a commit to luis-silva/terraform-provider-google that referenced this pull request May 21, 2019
<!-- This change is generated by MagicModules. -->
/cc @rileykarson
@ghost
Copy link

ghost commented Mar 30, 2020

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 ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backend Service cannot reconsolidate state
2 participants