After a lot of fun debugging with @mitchellh we finally have a diagnosis
for #1056.
I'm going to attempt to reproduce the diagnosis in prose to test out my
understanding.
----
The `DestroyTransformer` runs twice:
* `DestroyPrimary` mode creates destroy nodes for normal resources
* `DestroyTainted` mode creates destroy nodes for tainted resources
These destroy nodes are specializations of `GraphConfigNode`, which has
a `DynamicExpand` step.
In `DynamicExpand` we have a race condition between the normal destroy
node and the tainted destroy node for a given resource when
`CreateBeforeDestroy` is off.
The `DestroyTainted` `GraphConfigNode` must run the `TaintedTransform`
to draw out tainted nodes, since it is reponsible for destroying them
for replacement.
The `DestroyPrimary` `GraphConfigNode` _also_ runs `TaintedTransform` -
this is to catch `Deposed` nodes from CBD, which are piggy backing on
the end of the `Tainted` list.
Here's the bug: when CBD is off and you start an apply with a tainted
resource in your state, both `DestroyPrimary` and `DestroyTainted` catch
it and create their own `graphNodeTaintedResource` in their respective
subgraphs.
If parallelism is disabled, this doesn't happen because the
`DestroyPrimary` subgraph resolves completely before the
`DestroyTainted` node does its `DynamicExpand`, so the `Tainted` list
has been cleared by the time `DestroyTainted` is expanding. With
parallelism, each of these two subgraphs plays out in its own goroutine
simultaneously, and each picks up the tainted resource(s) that the apply
starts with. So you get two `graphNodeTaintedResource` nodes, and two
destroys.
This band-aid fixes that by skipping the TaintedTransform alltogether in
the `DestroyPrimary` node if CBD is off.
A better fix will follow, which involves reworking the `Deposed` concept
so it no longer piggybacks on `Tainted`.
fixes#1056
