In the case of highly-connected graphs, the TransitiveReduction process
was far too computationally intensive. Since no operations are applied
to the nodes, and the walk order is not even user visible, we don't need
to sort them n^2 times.
The removeEdge test could fail intermittently with the wrong order.
The precondition of a 1->2->3 order wasn't met, because there was no
edge from 1->3, so 3->1->2 was also a valid ordering.
The other failure was a bookkeeping error, were the recorded order may
not match the visited order. What happened in this case was the gateCh
was closed by V2, allowing V3 to run which could beat V2 to recording
its visit. Now the visit is recorded as part of the vertex walk, and the
gate is released as the final operation.
The order is deterministic now, so remove the brute-force test loop.
Because the vertex visit was record after the Update call, Updated
vertices may have been visited before the visit was recorded, causing
occasional test failures.
The order is now deterministic, and we can remove the brute-force loop.
Fixes#11052
It appears that historically nodes did not expect DotOpts to ever be
nil. To avoid nil panics in general I'm in agreement with this behavior
so this modifies dag to always pass in a non-nil DotOpts. Tests
included.
Add `terraform debug json2dot` to convert debug log graphs to dot
format. This is not meant to be in place of more advanced debug
visualization, but may continue to be a useful way to work with the
debug output.
This encodes vertex debug information into the graph log when a vertex
is visited during a walk operation. These can ordered to show how the
Graph was walked.
Add a mutex to the encoder so it can be used during a parallel walk.
Moved string literal constants used for marshaling to pre-defined constants.
Did some renaming to make the marshal* structures more consistent.
The method marks the start of a set of operations on the Graph, with
extra information optionally provided in the second paramter. This
returns a function with a single End method to mark the end of the set
in the logs.
Refactor the existing graph Begin/End Operation calls to use this single
method. Remove the *string types in the marshal structs, these are
strictly informational and don't need to differentiate empty vs unset
strings.
Add calls to DebugOperation for each step while building the graph.
dag.Graph is used as a value, but contains a sync data structure. This
prevents copying the value, and is needed if one wants to upgrade a Graph
to an AcyclicGraph.
The sync.Once only guards the init() method, which can be guarded
internally with nil checks on the fields. The init method could be
removed entirely with a proper constructor later on of we so choose.
The AnnotateVertex and AnnotateEdge Graph methods will allow terraform
to insert arbitray information into the encoded graph stream for later
processing.
The external api provided here is simply
dag.Graph.SetDebugWriter(io.Writer). When a writer is provided to a
Graph, it will immediately encode itself to the stream, and subsequently
encode any additional transformations to the graph. This will allow
easier logging of graph transformations without writing complete graphs
to the logs at every step. Since the marshalGraph can also be dot
encoded, this will allow translation from the JSON logs to dot graphs.
To maintain the same output, the Graph.Dot implementation needs to be
aware of GraphNodeDotter. Copy the interface into the dag package, and
make the Dot marshaler aware of which nodes implemented the interface.
This way we can remove most of the remaining dot code from terraform.
The dot format generation was done with a mix of code from the terraform
package and the dot package. Unify the dot generation code, and it into
the dag package.
Use an intermediate structure to allow a dag.Graph to marshal itself
directly. This structure will be ablt to marshal directly to JSON, or be
translated to dot format. This was we can record more information about
the graph in the debug logs, and provide a way to translate those logged
structures to dot, which is convenient for viewing the graphs.
Set the default log package output to iotuil.Discard during tests if the
`-v` flag isn't set. If we are verbose, then apply the filter according
to the TF_LOG env variable.
The report in #7378 led us into a deep rabbit hole that turned out to
expose a bug in the graph walk implementation being used by the
`NoopTransformer`. The problem ended up being when two nodes in a single
dependency chain both reported `Noop() -> true` and needed to be
removed. This was breaking the walk and preventing the second node from
ever being visited.
Fixes#7378
We weren't marking skipped nodes as failing, so any
grandchild-and-deeper dependencies would still evaluate.
For example:
A -> B -> C -> D
If B failed, C would be skipped, but D would still be evaluated.
This fixes the behavior so C, D, and any further descendents will all be
skipped when B fails.
Addresses crashing aspect of #2955 and likely a lot of other confusing
failure modes.
When you specify `-verbose` you'll get the whole graph of operations,
which gives a better idea of the operations terraform performs and in
what order.
The DOT graph is now generated with a small internal library instead of
simple string building. This allows us to ensure the graph generation is
as consistent as possible, among other benefits.
We set `newrank = true` in the graph, which I've found does just as good
a job organizing things visually as manually attempting to rank the nodes
based on depth.
This also fixes `-module-depth`, which was broken post-AST refector.
Modules are now expanded into subgraphs with labels and borders. We
have yet to regain the plan graphing functionality, so I removed that
from the docs for now.
Finally, if `-draw-cycles` is added, extra colored edges will be drawn
to indicate the path of any cycles detected in the graph.
A notable implementation change included here is that
{Reverse,}DepthFirstWalk has been made deterministic. (Before it was
dependent on `map` ordering.) This turned out to be unnecessary to gain
determinism in the final DOT-level implementation, but it seemed
a desirable enough of a property that I left it in.
Add `-target=resource` flag to core operations, allowing users to
target specific resources in their infrastructure. When `-target` is
used, the operation will only apply to that resource and its
dependencies.
The calculated dependencies are different depending on whether we're
running a normal operation or a `terraform destroy`.
Generally, "dependencies" refers to ancestors: resources falling
_before_ the target in the graph, because their changes are required to
accurately act on the target.
For destroys, "dependencies" are descendents: those resources which fall
_after_ the target. These resources depend on our target, which is going
to be destroyed, so they should also be destroyed.
James Bardin