google_compute_instance shouldn't favor metadata_startup_script over startup-script key in metadata

[JDiPierro]

Fixes #9999

[paddycarver]

See also #8407 :)

[cblecker]

@JDiPierro --
What happens if you use both? I'm assuming one overwrites the other which wouldn't be good.

[paddycarver]

Oh, also, sounds like this breaks some stuff: #3507.

Sorry, I'm working right now to try to find a good answer to this problem that can get us on a forward path again.

[cblecker]

@paddyforan --
I think @evandbrown's suggestion here is probably the best way forward. Remove the metadata startup script, and do a state migration to put that key in one place. Just have instance metadata be in the metadata block, and work the same if you're using an instance or instance_group.

[paddycarver]

Hey @cblecker,

In investigating this issue, I actually looked into that option--looking back at the original pull request, it looks like the metadata_startup_script attribute was included in #2375, because there's no way to make metadata.startup-script force instance recreation, which is the AWS behaviour. So I'm hesitant to roll back to that point without some discussion of pros and cons. There are a few ways forward that I can see, I'm just seeking some wisdom from some more experienced folks on the Terraform team before I commit to any of them. Apologies for the lengthy process, but I promise I'm investigating and trying to get this moving again. We have (from what I can tell) about 3 open PRs and an open issue that are all centered around this, so it has drawn my attention. :)

[cblecker]

Totally understand the need for discussion and review! Just wanted to bring Evan's piece into it. I'd also note that forcing the instance to recreate when metadata is updated isn't always preferable in the GCP world (where as it might be in AWS). You can update metadata manually on an instance in Google and it doesn't force instance recreation.

Look forward to hearing what the result of those internal talks is :)

[paddycarver]

Appreciate the direction. And the note; I totally see the value in updating the startup script without recreating things. I know it certainly makes sharing configuration code between instances and instance templates trickier, as forcing recreation of an instance template can have some pretty wide consequences. It's a balancing act at this point of trying to manage complexity while supporting as many use cases as possible, and prioritising those use cases.

I'm trying not to keep this as an internal discussion, but I did want to get feedback from other Terraform team members before I put my foot in my mouth, given how complicated this trade-off is.

[JDiPierro]

re: What happens if you use both?
The provider yells at you that only one can be specified: https://github.com/hashicorp/terraform/blob/master/builtin/providers/google/resource_compute_instance.go#L793

Glad to see this is being worked on, hope to have a clean way to manage this in a future version :) Feel free to close this PR in favor of a future one that addresses the issue.

[paddycarver]

This should be covered by #10537. :) Thanks for the PR!

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