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

provider/google: instance template support for metadata_startup_script #8407

Closed
wants to merge 1 commit into from
Closed

provider/google: instance template support for metadata_startup_script #8407

wants to merge 1 commit into from

Conversation

cblecker
Copy link
Contributor

Fixes #7827.

This pull updates the google_compute_instance_template resource to use the same metadata arguments and validation functions as the google_compute_instance resource. That being said, while this directly addresses the issue, I'm not sure it's the most graceful way of doing it for a couple of reasons.

For users with existing configuration, this will force them to migrate their:

metadata {
    startup-script = "echo hi > /test.txt"
}

to:

metadata_startup_script = "echo hi > /test.txt"

This results in a plan that detects a change to the config and forces deletion and recreation of the template. If that template is in use by a google_compute_instance_group_manager resource, then the apply will fail as a template can't be deleted while in use. Then you'd need to taint the instance group which would delete and recreate all instances created by it.

Second, this change isn't strictly necessary. While it is good to have consistent configuration, functions, and validations where possible between the two resources, instance templates are more restrictive than instances. As an existing instance template can't be updated at all, any change of any metadata at all results in the above deletion and recreation scenario. The original reason as far as I can tell for splitting the main metadata apart from the startup script was that if the startup script changed, then the instance should be destroyed and recreated so that it can be re-run. This left the remaining metadata keys to be changed freely. As all metadata is treated the same here splitting the startup script out isn't really needed.

Perhaps a state migration would solve this? Or perhaps it's not worth the hassle and is simply a WONTFIX. Thoughts? :)

@cblecker cblecker changed the title GCE instance template support for metadata_startup_script provider/google: instance template support for metadata_startup_script Aug 23, 2016
@cblecker
Copy link
Contributor Author

cblecker commented Sep 8, 2016

@evandbrown @lwander -
If either of you Google ninjas have cycles, I'd love feedback on this direction 😀

@baskaran-md
Copy link

Any updates on this feature?

@stack72
Copy link
Contributor

stack72 commented Nov 1, 2016

ping @evandbrown :)

@evandbrown evandbrown self-assigned this Nov 1, 2016
@evandbrown
Copy link
Contributor

Hey @cblecker, sorry I missed this a few months ago! O_o

I'm actually not a fan of how google_compute_instance treats startup scripts as a special field (I'm guessing this was done to mimic how gcloud and the web UI level-up that feature).

I'd actually be in favor of reverting the google_compute_instance behavior with a migration, which shouldn't result in the need to taint or re-create resources.

How does that sound?

@cblecker
Copy link
Contributor Author

cblecker commented Nov 2, 2016

No worries, @evandbrown! I know you're a busy guy.

Yeah, I think the splitting out kind of over complicates things. I tried to dig and find where this was introduced, and it looks like all the way back in #2375.

I think either way is acceptable, but the big key for me is that there is consistent behaviour between the two resource types. It's extremely powerful to be able to take a working google_compute_instance resource, and then change it to a template by changing the resource type with little to no other changes required.

@cblecker
Copy link
Contributor Author

cblecker commented Dec 6, 2016

Closing in favour of #10537. Thanks @paddyforan!

@cblecker cblecker closed this Dec 6, 2016
@cblecker cblecker deleted the cblecker/gce-template-startupscript branch December 6, 2016 20:22
@ghost
Copy link

ghost commented Apr 19, 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 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.

@ghost ghost locked and limited conversation to collaborators Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GCE instance template support for metadata_startup_script
5 participants