Commit Graph

16 Commits

Author SHA1 Message Date
James Bardin c9e581e58a don't connect module closers to destroy nodes
One of the tenants of the graph transformations is that resource destroy
nodes can only be ordered relative to other resources, and can't be
referenced directly. This was broken by the module close node which
naively connected to all module nodes, creating cycles in some cases
when edges are reversed from CreateBeforeDestroy.
2020-09-09 12:23:23 -04:00
James Bardin 198c632e04 incorrect early return during module transformer
The recursive call should only return immediately on error.

The switch statement to find the current path should not use
ReferenceOutside, as we are getting the path for configuration, not for
references. This case would not have been taken currently, since all
GraphNodeReferenceOutside are also GraphNodeModulePath.
2020-06-06 21:45:05 -04:00
James Bardin d97a210ad3 don't connect destroyers to module expanders
Resource destroy nodes can only depend on other resources. Connecting
them to their module expander can introduce cycles when the module
expander depends on resources in the destroyer's subgraph.
2020-05-28 21:33:08 -04:00
James Bardin 082f91cd85 fix ModulePath for nodeExpandModule
ModulePath was incorrectly returning the parent module, because it did
not implement ReferenceOutside. With ReferenceOutside working correctly,
we can have ModulePath return the real path and remove the special case
for this during pruning.
2020-05-28 21:30:44 -04:00
James Bardin c638252210 pruneUnusedNodesTransformer
Create a single transformer to remove all unused nodes from the apply
graph. This is similar to the combination of the resource pruning done
in the destroy edge transformer, and the unused values transformer. In
addition to resources, variables, locals, and outputs, we now need to
remove unused module expansion nodes as well. Since these can all be
interdependent, we need to process them as whole in a single
transformation.
2020-05-28 21:30:42 -04:00
James Bardin 7122b271f9 restore the ordering of nested modules
In order for depends_on to work, modules need to implicitly depend on
their child modules. This will have little effect on terraform's
concurrency, as configuration trees are always much wider than they are
deep.
2020-05-28 21:30:12 -04:00
James Bardin 009a136fa2 requiresInstanceExpansion and instanceExpander
create interfaces that nodes can implement to declare whether they
expand into instances of some sort, using the instances.Expander, and/or
whether use the instances.Expander to find instances.

included is a rough transformer implementation to remove these nodes
from the apply graph.
2020-05-28 21:30:12 -04:00
James Bardin 700e20de5d connect references to the module closer
NodeModuleRemoved is redundant now with the concept of
nodeCloseModule, so we can replace it within the graph. This does mean
that nodeCloseModule needs to know if it's evaluating an orphaned module
that can't be expanded, but the overhead to checking this isn't too
bad.

Now that nodeModuleClose is referenceable, and we can ensure it's always
in the graph at the correct time, we can eliminate the need to connect
each resource to every single node within a module it references, and
instead connect only to the nodeModuleClose, which acts as the module
root. Since module expansion can cause exponential growth in the number
of edges in graphs, this will help with performance problems when
transforming and reducing these graphs by eliminating the bulk of
redundant edges. This will also help with general debugging, making the
graphs easier to read.
2020-04-08 12:30:35 -04:00
James Bardin e23aa02560 modules expansion validate test 2020-04-06 09:13:43 -04:00
James Bardin 73492fd2d5 add module expansion to validation
We cannot evaluate expansion during validation, since the values may not
be known at that time.

Inject a nodeValidateModule, using the "Concrete" pattern used for other
node types during graph building. This node will always evaluate to a
single module instance, so that we have a valid context within which to
evaluate all sub resources.
2020-04-05 12:13:48 -04:00
James Bardin 0dcca1bc37 make the root node a nodeCloseModule for root
Replace the graphNodeRoot for the main graph with a nodeCloseModule for
the root module. USe a new transformer as well, so as to not change any
behavior of DynamicExpand graphs.

Closing out the root module like we do with sub modules means we no
longer need the OrphanResourceTransformer, or the NodeDestroyResource.
The old resource destroy logic has mostly moved into the instance nodes,
and the remaining resource node was just for cleanup, which need to be
done again by the module since there isn't always a NodeDestroyResource
to be evaluated.

The more-correct state caused a few tests to fail, which need to be
cleaned up to match the state without empty resource husks.
2020-04-02 16:00:36 -04:00
James Bardin 2df7127943 add nodeCloseModule
During plan, anything dependent on a module can connect to the module
expansion node, because all instance nodes are created during
DynamicExpand. During apply the instance nodes are created from the
diff, so we need a root module to terminate the logical module subgraph.

Besides providing an anchor for the completion of a module, the
nodeCloseModule can also be used to cleanup the orphan resource and
module placeholders in the state.
2020-04-02 16:00:36 -04:00
James Bardin 215f60d5cf remove module shims from module expansion nodes 2020-03-10 17:25:11 -04:00
James Bardin bd9cfca794 rename GraphNodeSubPath -> GraphNodeModuleInstance 2020-03-10 17:25:11 -04:00
Pam Selle c249943360
Module Expansion: Part 2 (#24154)
* WIP: dynamic expand

* WIP: add variable and local support

* WIP: outputs

* WIP: Add referencer

* String representation, fixing tests it impacts

* Fixes TestContext2Apply_outputOrphanModule

* Fix TestContext2Apply_plannedDestroyInterpolatedCount

* Update DestroyOutputTransformer and associated types to reflect PlannableOutputs

* Remove comment about locals

* Remove module count enablement

* Removes allowing count for modules, and reverts the test,
while adding a Skip()'d test that works when you re-enable
the config

* update TargetDownstream signature to match master

* remove unnecessary method

Co-authored-by: James Bardin <j.bardin@gmail.com>
2020-02-24 17:42:32 -05:00
Martin Atkins 68b900928d core: Use instances.Expander to handle resource count and for_each
This is a minimal integration of instances.Expander used just for resource
count and for_each, for now just forcing modules to always be singletons
because the rest of Terraform Core isn't ready to deal with expanding
module calls yet.

This doesn't integrate super cleanly yet because we still have some
cleanup work to do in the design of the plan walk, to make it explicit
that the nodes in the plan graph represent static configuration objects
rather than expanded instances, including for modules. To make this work
in the meantime, there is some shimming between addrs.Module and
addrs.ModuleInstance to correct for the discontinuities that result from
the fact that Terraform currently assumes that modules are always
singletons.
2020-02-14 15:20:07 -08:00