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

Deprecate name_prefix #1035

Merged
merged 2 commits into from
Apr 2, 2018
Merged

Deprecate name_prefix #1035

merged 2 commits into from
Apr 2, 2018

Conversation

danawillow
Copy link
Contributor

Since you can generate prefixes with the random provider, there's no need for name_prefix anywhere anymore and it only makes things more complicated.

Related: #780.

@rosbo
Copy link
Contributor

rosbo commented Feb 1, 2018

What's the migration story for our users?

If they change their config to start using the random provider, it will create a diff on the name... For existing state/config, they would need to update the state so that the random provider value matches the one generated by the name prefix field. Is there an easier way to deal with this?

@danawillow
Copy link
Contributor Author

Quick thoughts before I head out- name is Computed for all of these resources. If we make name_prefix computed as well, then a user who's already using name_prefix can safely remove it from their config. Then they don't have to start actually using the random provider until they wish to recreate the resource.

@sl1pm4t
Copy link
Contributor

sl1pm4t commented Feb 6, 2018

Hi @danawillow + @rosbo.
There is one situation where having a name_prefix attribute directly on the resource is very useful.
With google_container_node_pool resource, I rely on the name_prefix + create_before_destroy flag to ensure we can replace a node_pool without an outage. I've done this to update the machine_type or auth scopes of a node pool for example.
If the name_prefix attribute was removed, I would have to manually taint the random_X resource to get the same behaviour, which will be confusing for our users.

I know there has been a lot of focus on the google_container_cluster and google_container_node_pool resources lately, which I have not stayed up to date with, so maybe this kind of updates of a node_pool is baked into the resources now?!

Thanks!

@danawillow
Copy link
Contributor Author

@sl1pm4t I have an answer for you! I'm going to put it on #1054 to try to have a centralized place for discussion (no fault on your part, I created the issue after I had already done the PR)

@danawillow
Copy link
Contributor Author

@paddycarver I'm thinking these changes should probably get in before 2.0.0 so we can remove the feature entirely in 2.0.0, right?

@danawillow danawillow merged commit 7596164 into hashicorp:master Apr 2, 2018
@danawillow danawillow deleted the nameprefix branch April 2, 2018 17:29
@danawillow danawillow mentioned this pull request Apr 2, 2018
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
* deprecate name_prefix

* make name_prefix computed and add migration instructions
@ghost
Copy link

ghost commented Nov 19, 2018

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 unassigned rosbo Nov 19, 2018
@ghost ghost locked and limited conversation to collaborators Nov 19, 2018
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.

4 participants