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

-target destroys the wrong resources without cause #4515

Closed
justinclayton opened this issue Jan 5, 2016 · 5 comments · Fixed by #4574
Closed

-target destroys the wrong resources without cause #4515

justinclayton opened this issue Jan 5, 2016 · 5 comments · Fixed by #4574

Comments

@justinclayton
Copy link
Contributor

There seems to be a really scary pattern matching bug at play with the "resource address" method of referencing resources. Here's a simple repro:

resource "template_file" "foo" {
  count    = 13
  template = "lorem ipsum"
}

After a successful terraform apply, run the following and be afraid:

> terraform plan -target="template_file.foo[1]"
Refreshing Terraform state prior to plan...

template_file.foo.12: Refreshing state... (ID: 5e2bf57d3f40c4b6df69daf1936cb766f832374b4fc0259a7cbff06e2f70f269)
template_file.foo.10: Refreshing state... (ID: 5e2bf57d3f40c4b6df69daf1936cb766f832374b4fc0259a7cbff06e2f70f269)
template_file.foo.11: Refreshing state... (ID: 5e2bf57d3f40c4b6df69daf1936cb766f832374b4fc0259a7cbff06e2f70f269)
template_file.foo.1: Refreshing state... (ID: 5e2bf57d3f40c4b6df69daf1936cb766f832374b4fc0259a7cbff06e2f70f269)

<...>

- template_file.foo.10

- template_file.foo.11

- template_file.foo.12

Plan: 0 to add, 0 to change, 3 to destroy.

This is super-scary for two reasons:

  • State is refreshed for 4 resources instead of just 1
  • The 3 resources not targeted are suddenly scheduled for destruction!

We have reproduced this error across a couple of different providers, so it looks like a core bug for sure. In my reading of the docs, -target looks to be the only user of this "resource address" stuff. Everything else uses the TYPE.NAME.INDEX format everyone is used to. I'm not sure what the point of having two are, but that's a discussion for another time.

@justinclayton
Copy link
Contributor Author

A couple quick addendums:

  • The debug logging is very difficult for me to parse, but I am noticing that during one of the many evals performed on the graph, those collateral resources are getting marked with (orphan).
  • I have confirmed this bug has existed since -target was introduced in v0.4.0.

@phinze
Copy link
Contributor

phinze commented Jan 7, 2016

Thanks for the report, @justinclayton. Looking into it now.

In my reading of the docs, -target looks to be the only user of this "resource address" stuff. Everything else uses the TYPE.NAME.INDEX format everyone is used to. I'm not sure what the point of having two are, but that's a discussion for another time.

Before -target was implemented, there was no formalization in the code base of a way of addressing resources. So the idea was to start standardizing w/ target and apply that elsewhere.

@josephholsten
Copy link
Contributor

yep, we're seeing this as well. if I do terraform plan -target=module.non_existant it still wants to destroy everything it can.

@johnrengelman
Copy link
Contributor

Also just encountered this. My scenario is a bit different. We renamed some resources, so the normal plan would be to create the new ones and destroy the old ones. However, we need to do it in 2 phases, so my plan was to use -target to create the new resources and leave the old ones existing, and then destroy the old ones, but -target (even with plan) insists on deleting the resources that are no longer defined in the config.

phinze added a commit that referenced this issue 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.

Includes test coverage proving that it fixes #4515. Will do further
testing to determine whether this also fixes the various linked issues.
phinze added a commit that referenced this issue Jan 19, 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
phinze added a commit that referenced this issue Jan 19, 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
bigkraig pushed a commit to bigkraig/terraform that referenced this issue Mar 1, 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 hashicorp#4515
Fixes hashicorp#2538
Fixes hashicorp#4462
@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
Development

Successfully merging a pull request may close this issue.

5 participants