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

core: fix resource targeting w/ provisioners #1544

Merged
merged 1 commit into from
Apr 30, 2015

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Apr 15, 2015

The TargetTransform was dropping provisioner nodes, which caused graph
validation to fail with messages about uninitialized provisioners.

closes #1541

@phinze
Copy link
Contributor Author

phinze commented Apr 15, 2015

Did a bit more investigation and confirmed this only applies in Destroy cases. This is because Destroy walks the other way to determine dependencies to keep. Other cases were already keeping provisioners because the targeted nodes depended on them.

Scoping down the commit message to indicate it's only in targeted destroy. 👍

@phinze phinze force-pushed the b-destroy-target-provisioner branch from 95aa294 to 66a5a39 Compare April 15, 2015 20:08
@mitchellh
Copy link
Contributor

Looks good, but I feel that we should inverse the logic of targeting. it seems brittle in the current approach of "if something, keep it." Instead I think it should be "if targetable and NOT what we're targeting, throw it away".

That way the graph contains everything except the non-targetable things. That would avoid this and any future issues of adding more graph nodes.

The `TargetTransform` was dropping provisioner nodes, which caused graph
validation to fail with messages about uninitialized provisioners when a
`terraform destroy` was attempted.

This was because `destroy` flops the dependency calculation to try and
address any nodes in the graph that "depend on" the target node. But we
still need to keep the provisioner node in the graph.

Here we switch the strategy for filtering nodes to only drop
addressable, non-targeted nodes. This should prevent us from having to
whitelist nodes to keep in the future.

closes #1541
@phinze phinze force-pushed the b-destroy-target-provisioner branch from 66a5a39 to 5e67657 Compare April 27, 2015 13:37
@phinze
Copy link
Contributor Author

phinze commented Apr 27, 2015

Instead I think it should be "if targetable and NOT what we're targeting, throw it away".

Good call - done! PTAL

@phinze
Copy link
Contributor Author

phinze commented Apr 30, 2015

@mitchellh bump for review of inverted strategy here

@mitchellh
Copy link
Contributor

LGTM

phinze added a commit that referenced this pull request Apr 30, 2015
core: fix resource targeting w/ provisioners
@phinze phinze merged commit 443c7e0 into master Apr 30, 2015
@phinze phinze deleted the b-destroy-target-provisioner branch April 30, 2015 21:03
@ghost
Copy link

ghost commented May 3, 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 May 3, 2020
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.

terraform destroy with -target depends on uninitialised provisioners
2 participants