This is a "should never happen" case, but we have reports of it actually
happening. In order to try to collect a bit more data about what's going
on here, we're changing what was previously a hard panic into a normal
error message that can include the address of the instance we were working
on and the action we were trying to do to it at the time.
The hope is to narrow down what situations can trigger this in order to
find a reliable reproduction case in order to debug further. This also
means that for those who _do_ encounter this problem in the meantime
Terraform will have a chance to shut down cleanly and therefore be more
likely to be able to recover on a subsequent plan/apply cycle.
Further investigation of this will follow once we see a report or two of
this updated error message.
Since a planned destroy can no longer indicate it is a full destroy,
unused values were being left in the apply graph for evaluation. If
these values contains interpolations that can fail, (for example, a
zipmap with mismatched list sizes), it will cause the apply to abort.
The PrunUnusedValuesTransformer was only previously run during destroy,
more out of conservatism than for any other particular reason. Adapt it
to always remove unused values from the graph, with the exception being
the root module outputs, which must be retained when we don't have a
clear indication that a full destroy is being executed.
The resource cleanup node does not need a provider. We can't directly
remove the ProvidedBy method, but this node only needs to be eval-able
so we can remove all the NodeAbstractResource methods at once.
References from a resource-level connection blocks were not returned
from NodeAbstractResource.References, causing the provisioner connection
attributes to sometimes be evaluated too early.
* huge change to weave new addrs.Provider into addrs.ProviderConfig
* terraform: do not include an empty string in the returned Providers /
Provisioners
- Fixed a minor bug where results included an extra empty string
In order to make this work reasonably we can't avoid using some funny
heuristics, which are somewhat reasonable to apply within the context of
Terraform itself but would not be good to add to the general "logutils".
Specifically, this is adding the additional heuristic that lines starting
with spaces are continuation lines and so should inherit the log level
of the most recent non-continuation line.
* terraform/context: use new addrs.Provider as map key in provider factories
* added NewLegacyProviderType and LegacyString funcs to make it explicit that these are temporary placeholders
This PR introduces a new concept, provider fully-qualified name (FQN), encapsulated by the `addrs.Provider` struct.
NoEach and Each list both have this check, but it was missing in
EachMap. Refactor the EachList check to remove a level of indentation,
and make the check consistently near the start of the block.
The CreateBeforeDestroy transformer correctly handles the edge referred
to in the comment, and going forward it will probably be easier to use
the knowledge of this edge for CBD anyway.
A number of tests had no, or incomplete state for the transformations
they wanted to test. Add states state with the correct dependencies for
these tests.
An earlier change to eliminate the large amount of duplicate edges being
added by the original CreateBeforeDestroy dependency mapper mistakingly
prevented adding edges when there are multiple CBD dependencies.
This updates the algorithm to use a map to collect all possible edges
and de-deplucating them before processing.
EvalRefreshDependencies is used to update resource dependencies when
they don't exist, allow broken or old states to be updated. While
appending any newly found dependencies is tempting to have the largest
set available, changes to the config could conflict with the prior
dependencies causing cycles.
Since a create node cannot both depend on its destroy node AND be
CreateBeforeDestroy, the same goes for its dependencies. While we do
connect resources with dependency destroy nodes so that updates are
ordered correctly, this ordering does not make sense in the
CreateBeforeDestroy case.
If resource node is CreateBeforeDestroy, we need to remove any direct
dependencies from it to destroy nodes to prevent cycles. Since we don't
know for certain if a crate node is going to be CreateBeforeDestroy at
the time the edge is added in the graph, we add it unconditionally and
prune it out later on. The pruning happens during the CBD transformer
when the CBD destroy node reverses it's own destroy edge. The reason
this works for detecting the original edge, is that dependencies of CBD
resources are forced to be CBD themselves. This does have a false
positive where the case of the original node is NOT CBD, but this can be
taken care of later when we gather enough information in the graph to
prevent the connection in the first place.
The DestroyEdgeTransformer cannot determine ordering from the graph when
the destroyers are from orphaned resources, because there are no
references to resolve. The new stored Dependencies provides what we need
to connect the instances in this case.
We also add the StateDependencies method directly in the
GraphNodeResourceInstance interface, since all instances already
implement this, and we don't need another optional interface to check.
The old code in DestroyEdgeTransformer may no longer be needed in the
long run, but that can be determined separately, since too many of the
tests start with an incomplete state and rely on the Dependencies being
determined from the configuration alone.
Refresh should load any new dependencies found because of configuration
or state changes, but retain any dependencies already in the state.
Orphaned resources would not be in config, but we do not want to lose
the destroy ordering for the later apply.
Make use of the new Dependencies field in the instance state.
The inter-instance dependencies will be determined from the complete
reference graph, so that absolute addresses can be stored, rather than
just references within a module. The Dependencies are added to the node
in the same manner as state, i.e. via an "attacher" interface and
transformer. This is because dependencies are calculated from the graph
itself, and not from the config.
We need to be able to reference all possible dependencies for ordering
when the configuration is no longer present, which means that absolute
addresses must be used. Since this is only to recreate the proper
ordering for instance destruction, only resources addresses need to be
listed rather than individual instance addresses.
If a resource is only destroying instances, there is no reason to
prepare the state and we can remove the Resource (prepare state) nodes.
They normally have pose no issue, but if the instances are being
destroyed along with their dependencies, the resource node may fail to
evaluate due to the missing dependencies (since destroy happens in the
reverse order).
These failures were previously blocked by there being a cycle when the
destroy nodes were directly attached to the resource nodes.
Destroy nodes do not need to be connected to the resource (prepare
state) node when adding them to the graph. Destroy nodes already have a
complete state in the graph (which is being destroyed), any references
will be added in the ReferenceTransformer, and the proper
connection to the create node will be added in the
DestroyEdgeTransformer.
Under normal circumstances this makes no difference, as create and
destroy nodes always have an dependency, so having the prepare state
handled before both only linearizes the operation slightly in the
normal destroy-then-create scenario.
However if there is a dependency on a resource being replaced in another
module, there will be a dependency between the destroy nodes in each
module (to complete the destroy ordering), while the resource node will
depend on the variable->output->resource chain. If both the destroy and
create nodes depend on the resource node, there will be a cycle.
The CBDEdgeTransformer tests worked on fake data structures, with a
synthetic graph, and configs that didn't match. Update them to generate
a more complete graph, with real node implementations, from real
configs.
The output graph is filtered down to instances, and the results still
functionally match the original expected test results, with some minor
additions due to using the real implementation.
When looking for dependencies to fix when handling
create_before_destroy, we need to look past more than one edge, as
dependencies may appear transitively through outputs and variables. Use
Descendants rather than UpEdges.
We have the full graph to use for the CBD transformation, so there's no
longer any need to create a temporary graph, which may differ from the
original.
James Bardin