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: Ensure EvalReadDataApply is called on expanded destroy nodes #6922

Merged

Conversation

vancluever
Copy link
Contributor

@vancluever vancluever commented May 29, 2016

Ref: #6913

As per the discussion with @apparentlymart, I did a bit more digging in the code and the debug logs, and found the issue is actually with the ResourceCountTransformer. Log snip below:

2016/05/28 21:06:42 [TRACE] [walkDestroy] Exiting eval tree: provider.aws
2016/05/28 21:06:42 [DEBUG] vertex data.aws_ami_description.nat_ami (destroy), got dep: provider.aws
2016/05/28 21:06:42 [DEBUG] vertex root.data.aws_ami_description.nat_ami (destroy): walking
2016/05/28 21:06:42 [DEBUG] vertex root.data.aws_ami_description.nat_ami (destroy): evaluating
2016/05/28 21:06:42 [TRACE] [walkDestroy] Entering eval tree: data.aws_ami_description.nat_ami (destroy)
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalSequence
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalInterpolate
2016/05/28 21:06:42 [DEBUG] root: eval: terraform.EvalNoop
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalCountFixZeroOneBoundary
2016/05/28 21:06:42 [TRACE] [walkDestroy] Exiting eval tree: data.aws_ami_description.nat_ami (destroy)
2016/05/28 21:06:42 [DEBUG] vertex root.data.aws_ami_description.nat_ami (destroy): expanding/walking dynamic subgraph
2016/05/28 21:06:42 [TRACE] Graph after step *terraform.ResourceCountTransformer:

data.aws_ami_description.nat_ami (destroy) - *terraform.graphNodeExpandedResourceDestroy
2016/05/28 21:06:42 [TRACE] Graph after step *terraform.OrphanTransformer:

data.aws_ami_description.nat_ami (destroy) - *terraform.graphNodeExpandedResourceDestroy
2016/05/28 21:06:42 [TRACE] Graph after step *terraform.DeposedTransformer:

data.aws_ami_description.nat_ami (destroy) - *terraform.graphNodeExpandedResourceDestroy
2016/05/28 21:06:42 [TRACE] Graph after step *terraform.TargetsTransformer:

data.aws_ami_description.nat_ami (destroy) - *terraform.graphNodeExpandedResourceDestroy
2016/05/28 21:06:42 [TRACE] Graph after step *terraform.RootTransformer:

data.aws_ami_description.nat_ami (destroy) - *terraform.graphNodeExpandedResourceDestroy
2016/05/28 21:06:42 [DEBUG] vertex root.data.aws_ami_description.nat_ami (destroy): walking
2016/05/28 21:06:42 [DEBUG] vertex root.data.aws_ami_description.nat_ami (destroy): evaluating
2016/05/28 21:06:42 [TRACE] [walkDestroy] Entering eval tree: data.aws_ami_description.nat_ami (destroy)
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalOpFilter
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalSequence
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalReadDiff
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalFilterDiff
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalIf
2016/05/28 21:06:42 [DEBUG] root: eval: terraform.EvalNoop
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalGetProvider
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalReadState
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalRequireState
2016/05/28 21:06:42 [DEBUG] root: eval: *terraform.EvalApply
2016/05/28 21:06:42 [DEBUG] apply: data.aws_ami_description.nat_ami: executing Apply
panic: bad, not supposed to be here!

The node gets transformed into a graphNodeExpandedResourceDestroy which was not handling for data sources, putting all nodes through EvalApply regardless of what they were.

The fix was simple - handle the apply case properly with an EvalIf. Not too sure if an entirely new node type is necessary for this.

During accpeptance tests of some of the first data sources (see
hashicorp#6881 and hashicorp#6911),
"unknown resource type" errors have been coming up. Traced it down to
the ResourceCountTransformer, which transforms destroy nodes to a
graphNodeExpandedResourceDestroy node. This node's EvalTree() was still
indiscriminately using EvalApply for all resource types, versus
EvalReadDataApply. This accounts for both cases via EvalIf.
@jen20
Copy link
Contributor

jen20 commented May 29, 2016

👍 This is a better fix than preventing fall through as I was trying to do. Thanks @vancluever!

@jen20 jen20 merged commit 4ca379f into hashicorp:master May 29, 2016
@vancluever
Copy link
Contributor Author

No problem and thank you @jen20!

@vancluever vancluever deleted the paybyphone_expanded_datasource_destroyfix branch May 30, 2016 01:47
@ghost
Copy link

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

3 participants