google_compute_instance ForceNew argument create_timeout added without state migration

[apparentlymart]

In 49c7d27 a new argument create_timeout was added that has a default value and sets ForceNew true. This forces a replace diff for anyone upgrading from an older Terraform version where this attribute wasn't present.

This commit seems to have shown up for the first time in 0.8, so this can be an upgrade blocker for people coming from 0.7.

The general solution to this sort of problem is to provide a state migration function so that a default can be inserted into the state. In this case however it seems that this attribute is used only on create anyway, so perhaps a simpler fix is to just drop the ForceNew altogether... it feels weird to assume that if someone changes the creation timeout that they necessarily want to recreate the instance in order to experience the new timeout. 😀

---

For anyone that hits this bug and wants a workaround to make progress:

Manually editing the state is probably the least-effort workaround here, though care must always be taken when editing the state since the state is Terraform's "memory" and careless editing can cause Terraform to misbehave.

To adjust this, locate the relevant google_compute_instance resource in the state file and add the new attribute with its default value of 4:

    "create_timeout": "4"

Be sure to preserve correct JSON syntax by adding a comma to the end to delimit from a following attribute value. The value must be quoted because Terraform treats all state attribute values as strings.

If you have remote state enabled, the file to edit is .terraform/terraform.tfstate and you must also increment the serial value near the top of the file so Terraform can understand that this was an intentional change rather than state corruption.

If you do not have remote state enabled, the file to edit is terraform.tfstate.

[cblecker]

Completely agree with the dropping of ForceNew. If resource creation fails because of timeout, the resource would be marked as tainted already, wouldn't it?

[paddycarver]

I'm on board with both dropping the ForceNew and adding a migration to set a default. Definitely don't want to block people on updating or have un-labeled backwards incompatibilities. Thanks for bringing it up. :) Just want to mention @aditya87 (the original author) and @evandbrown (reviewer) here to make sure we're not missing nuance or context, but I'll go ahead and make a PR for this today.

[cblecker]

If we drop ForceNew, would we end up needing a state migration? The next apply should set the default, wouldn't it?

[paddycarver]

My thoughts on that (correct me if I'm wrong!) are that if we do the state migration, people don't get a dirty diff that isn't going to change their infrastructure at all.

For example, without the migration, my understanding is we'll have a 0.7.x state, upgrade to 0.8, plan, and it will show us a diff, when the API call isn't really going to do anything to our infrastructure.

With the migration, my understanding is we'll have a 0.7.x state, upgrade to 0.8, plan, and there will be no diff.

It's possible I'm misunderstanding, and could probably work up a sample of each of those if there's contention about what we think would happen.

[evandbrown]

Removing ForceNew seems legit. @paddyforan I believe you're correct that a state migration to set a default value would be necessary to prevent the diff (even though it's a noop).

[paddycarver]

This has been merged and will be part of 0.8.3. :) Thanks for reporting it!

[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.