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

Orphan addressing / targeting #4574

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Orphan addressing / targeting #4574

merged 1 commit into from
Jan 21, 2016

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Jan 8, 2016

Instead of trying to skip non-targeted orphans as they are added to
the graph in OrphanTransformer, remove knowledge of targeting from
OrphanTransformer and instead make the orphan resource nodes properly
addressable.

That allows us to use existing logic in TargetTransformer to filter out
the nodes appropriately. This does require adding TargetTransformer to the
list of transforms that run during DynamicExpand so that targeting can
be applied to nodes with expanded counts.

Fixes #4515
Fixes #2538
Fixes #4462

@StefanSmith
Copy link

I'm finding that with in releases since about 0.6.6, when I "apply -target=...", orphaned resources are destroyed. Does 4574 change the behaviour such that resources in the state file which are no longer defined in the *.tf files will be ignored rather than destroyed on "apply"?

Instead of trying to skip non-targeted orphans as they are added to
the graph in OrphanTransformer, remove knowledge of targeting from
OrphanTransformer and instead make the orphan resource nodes properly
addressable.

That allows us to use existing logic in TargetTransformer to filter out
the nodes appropriately. This does require adding TargetTransformer to the
list of transforms that run during DynamicExpand so that targeting can
be applied to nodes with expanded counts.

Fixes #4515
Fixes #2538
Fixes #4462
@phinze phinze removed the wip label Jan 19, 2016
@phinze phinze changed the title [WIP] Orphan addressing / targeting Orphan addressing / targeting Jan 19, 2016
@phinze
Copy link
Contributor Author

phinze commented Jan 19, 2016

Alright @mitchellh / @jen20 this should be ready for review. Confirmed it fixes several open bugs and included a covering test for each one.

@mitchellh
Copy link
Contributor

LGTM! I still think at some point we should just measure twice and cut once the targeting syntax to make sure we can get it fully right and test the full supported syntax. But this doesn't appear to introduce any except make the old one work properly so LGTM.

@phinze
Copy link
Contributor Author

phinze commented Jan 21, 2016

Thanks for the review! Merging. 🚢

I still think at some point we should just measure twice and cut once the targeting syntax to make sure we can get it fully right and test the full supported syntax.

What's there in ResourceAddress has been specced out, docced, and tested pretty extensively. We need to get the same syntax ported over to terraform taint and we need to come up with a story for exclude/inverse targeting (#2253), but other than that I wasn't aware of anything missing. Can you clarify a bit more?

phinze added a commit that referenced this pull request Jan 21, 2016
@phinze phinze merged commit 8d8b287 into master Jan 21, 2016
@phinze phinze deleted the phinze/orphan-addressing branch January 21, 2016 21:07
@josephholsten
Copy link
Contributor

So excited to see this merged. Will be verifying this fixes our issues today.

@jjshoe
Copy link

jjshoe commented Jan 25, 2016

Can confirm this works as of e865c34

@ghost
Copy link

ghost commented Apr 28, 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 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants