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

terraform: destroy resources in dependent providers first #10659

Merged
merged 2 commits into from
Dec 12, 2016

Conversation

mitchellh
Copy link
Contributor

@mitchellh mitchellh commented Dec 11, 2016

Fixes #4645

This is something that never worked (even in legacy graphs), but as we
push forward towards encouraging multi-provider usage especially with
things like the Vault data source, I want to make sure we have this
right for 0.8.

When you have a config like this:

resource "foo_type" "name" {}
provider "bar" { attr = "${foo_type.name.value}" }
resource "bar_type" "name" {}

Then the destruction ordering MUST be:

  1. bar_type
  2. foo_type

Since configuring the client for bar_type requires accessing data from
foo_type. Prior to this PR, these two would be done in parallel. This
properly pushes forward the dependency.

THIS PR:

  • ✅ Resources of dependent providers are destroyed first
  • ✅ Resources of dependent providers in child modules are destroyed first

There are likely more edge cases but I think I got the most common ones.

Fixes #4645

This is something that never worked (even in legacy graphs), but as we
push forward towards encouraging multi-provider usage especially with
things like the Vault data source, I want to make sure we have this
right for 0.8.

When you have a config like this:

```
resource "foo_type" "name" {}
provider "bar" { attr = "${foo_type.name.value}" }
resource "bar_type" "name" {}
```

Then the destruction ordering MUST be:

  1. `bar_type`
  2. `foo_type`

Since configuring the client for `bar_type` requires accessing data from
`foo_type`. Prior to this PR, these two would be done in parallel. This
properly pushes forward the dependency.

There are more cases I want to test but this is a basic case that is
fixed.
This extends the prior commit to also verify (and fix) that resources of
dependent providers are destroyed first even when they're within
modules.
Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

LGTM

@mitchellh mitchellh merged commit 80eab1d into master Dec 12, 2016
@mitchellh mitchellh deleted the b-multi-provider-dep branch December 12, 2016 19:49
@mtekel
Copy link

mtekel commented Dec 13, 2016

Looks like this could also fix #5340, but I'm not 100% sure...

@mitchellh
Copy link
Contributor Author

You're right that does look the same!

@ghost
Copy link

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

Destruction order for dependent providers is incorrect
4 participants