Commit Graph

1606 Commits

Author SHA1 Message Date
Mitchell Hashimoto
14d079f914
terraform: destroy resources in dependent providers first
Fixes #4645

This is something that never worked (even in legacy graphs), but as we
push forward towards encouraging multi-provider usage especially with
things like the Vault data source, I want to make sure we have this
right for 0.8.

When you have a config like this:

```
resource "foo_type" "name" {}
provider "bar" { attr = "${foo_type.name.value}" }
resource "bar_type" "name" {}
```

Then the destruction ordering MUST be:

  1. `bar_type`
  2. `foo_type`

Since configuring the client for `bar_type` requires accessing data from
`foo_type`. Prior to this PR, these two would be done in parallel. This
properly pushes forward the dependency.

There are more cases I want to test but this is a basic case that is
fixed.
2016-12-10 20:11:24 -05:00
Mitchell Hashimoto
92faace610 release: clean up after v0.8.0-rc3 2016-12-09 05:07:23 +00:00
Mitchell Hashimoto
fce2be1773
v0.8.0-rc3 2016-12-09 05:01:06 +00:00
Mitchell Hashimoto
d5135c0f61 Merge pull request #10522 from hashicorp/b-destroy-cbd
terraform: apply resource must depend on destroy deps
2016-12-05 21:37:09 -08:00
Mitchell Hashimoto
a4ceb4a772 Merge pull request #10518 from hashicorp/b-graph
command/graph: work with new graphs
2016-12-05 21:36:33 -08:00
Martin Atkins
e772b45970 "external" data source, for integrating with external programs (#8768)
* "external" provider for gluing in external logic

This provider will become a bit of glue to help people interface external
programs with Terraform without writing a full Terraform provider.

It will be nowhere near as capable as a first-class provider, but is
intended as a light-touch way to integrate some pre-existing or custom
system into Terraform.

* Unit test for the "resourceProvider" utility function

This small function determines the dependable name of a provider for
a given resource name and optional provider alias. It's simple but it's
a key part of how resource nodes get connected to provider nodes so
worth specifying the intended behavior in the form of a test.

* Allow a provider to export a resource with the provider's name

If a provider only implements one resource of each type (managed vs. data)
then it can be reasonable for the resource names to exactly match the
provider name, if the provider name is descriptive enough for the
purpose of the each resource to be obvious.

* provider/external: data source

A data source that executes a child process, expecting it to support a
particular gateway protocol, and exports its result. This can be used as
a straightforward way to retrieve data from sources that Terraform
doesn't natively support..

* website: documentation for the "external" provider
2016-12-05 17:24:57 +00:00
Mitchell Hashimoto
0e4a6e3e89
terraform: apply resource must depend on destroy deps
Fixes #10440

This updates the behavior of "apply" resources to depend on the
destroy versions of their dependencies.

We make an exception to this behavior when the "apply" resource is CBD.
This is odd and not 100% correct, but it mimics the behavior of the
legacy graphs and avoids us having to do major core work to support the
100% correct solution.

I'll explain this in examples...

Given the following configuration:

    resource "null_resource" "a" {
       count = "${var.count}"
    }

    resource "null_resource" "b" {
      triggers { key = "${join(",", null_resource.a.*.id)}" }
    }

Assume we've successfully created this configuration with count = 2.
When going from count = 2 to count = 1, `null_resource.b` should wait
for `null_resource.a.1` to destroy.

If it doesn't, then it is a race: depending when we interpolate the
`triggers.key` attribute of `null_resource.b`, we may get 1 value or 2.
If `null_resource.a.1` is destroyed, we'll get 1. Otherwise, we'll get
2. This was the root cause of #10440

In the legacy graphs, `null_resource.b` would depend on the destruction
of any `null_resource.a` (orphans, tainted, anything!). This would
ensure proper ordering. We mimic that behavior here.

The difference is CBD. If `null_resource.b` has CBD enabled, then the
ordering **in the legacy graph** becomes:

  1. null_resource.b (create)
  2. null_resource.b (destroy)
  3. null_resource.a (destroy)

In this case, the update would always have 2 values for `triggers.key`,
even though we were destroying a resource later! This scenario required
two `terraform apply` operations.

This is what the CBD check is for in this PR. We do this to mimic the
behavior of the legacy graph.

The correct solution to do one day is to allow splat references
(`null_resource.a.*.id`) to happen in parallel and only read up to to
the `count` amount in the state. This requires some fairly significant
work close to the 0.8 release date, so we can defer this to later and
adopt the 0.7.x behavior for now.
2016-12-03 23:54:29 -08:00
Mitchell Hashimoto
26ac58bc97
terraform: refactor NodeApplyableProvider to use NodeAbstractProvider
This is important so that the graph looks correct.
2016-12-03 15:27:38 -08:00
Mitchell Hashimoto
fb8f2e2753
terraform: new Graph API that can return the graph for each op 2016-12-02 22:56:22 -05:00
Mitchell Hashimoto
9197422881
terraform: new graph nodes implement Dotter 2016-12-02 22:26:40 -05:00
Clint
7a6aa1292f fix typo
fix typo
2016-12-02 16:23:07 -06:00
James Bardin
f833958505 Merge pull request #10502 from hashicorp/jbardin/validate-crash
Make sure that a Context.diff is never nil
2016-12-02 15:15:49 -05:00
Mitchell Hashimoto
8ffe25ef9b release: clean up after v0.8.0-rc2 2016-12-02 20:09:28 +00:00
Mitchell Hashimoto
a6ac5bed69
v0.8.0-rc2 2016-12-02 20:05:22 +00:00
James Bardin
6a8df0cbe2 Make sure that a Context.diff is never nil
The context and diff passed along during a walk, and the diff is assumed
to be valid.
2016-12-02 11:52:18 -05:00
Mitchell Hashimoto
cfb440ea60
terraform: don't prune state on init()
Init should only _add_ values, not remove them.

During graph execution, there are steps that expect that a state isn't
being actively pruned out from under it. Namely: writing deposed states.

Writing deposed states has no way to handle if a state changes
underneath it because the only way to uniquely identify a deposed state
is its index in the deposed array. When destroying deposed resources, we
set the value to `<nil>`. If the array is pruned before the next deposed
destroy, then the indexes have changed, and this can cause a crash.

This PR does the following (with more details below):

  * `init()` no longer prunes.

  * `ReadState()` always prunes before returning. I can't think of a
    scenario where this is unsafe since generally we can always START
    from a pruned state, its just causing problems to prune
    mid-execution.

  * Exported State APIs updated to be robust against nil ModuleStates.

Instead, I think we should adopt the following semantics for init/prune
in our structures that support it (Diff, for example). By having
consistent semantics around these functions, we can avoid this in the
future and have set expectations working with them.

  * `init()` (in anything) will only ever be additive, and won't change
    ordering or existing values. It won't remove values.

  * `prune()` is destructive, expectedly.

  * Functions on a structure must not assume a pruned structure 100% of
    the time. They must be robust to handle nils. This is especially
    important because in many cases values such as `Modules` in state
    are exported so end users can simply modify them outside of the
    exported APIs.

This PR may expose us to unknown crashes but I've tried to cover our
cases in exposed APIs by checking for nil.
2016-12-02 11:48:34 -05:00
Mitchell Hashimoto
08a56304bb Merge pull request #10455 from hashicorp/b-non-cbd-promote
terraform: when promoting non-CBD to CBD, mark the config as such
2016-12-02 09:51:27 -05:00
Mitchell Hashimoto
95239a7fe6
terraform: when promoting non-CBD to CBD, mark the config as such
This brings the change for the  new graph. See #10455
2016-12-02 09:46:42 -05:00
Mitchell Hashimoto
f3a62c694d
terraform: when promoting non-CBD to CBD, mark the config as such
Fixes #10439

When a CBD resource depends on a non-CBD resource, the non-CBD resource
is auto-promoted to CBD. This was done in
cf3a259. This PR makes it so that we
also set the config CBD to true. This causes the proper runtime
execution behavior to occur where we depose state and so on.

So in addition to simple graph edge tricks we also treat the non-CBD
resources as CBD resources.
2016-12-02 09:46:04 -05:00
Mitchell Hashimoto
a2f3259e51 Merge pull request #10404 from hashicorp/b-plan-deposed
terraform: diff shows pure deposed destroy
2016-11-30 16:57:44 -08:00
Mitchell Hashimoto
2f8bf5b7ec
terraform: add variables to Interpolator value
Fixes #10412

The context wasn't properly adding variable values to the Interpolator
instance which made it so that the `console` command couldn't access
variables set via tfvars and the CLI.

This also adds better test coverage in command itself for this.
2016-11-30 11:56:31 -08:00
Mitchell Hashimoto
bd1ef07a3c Merge pull request #10416 from hashicorp/b-deposed-order
terraform: Destroy node should only include deposed for specific index
2016-11-29 09:40:44 -08:00
James Bardin
7677bd94ed Merge pull request #10325 from hashicorp/jbardin/GH-10187
Fix some cases for nested maps and lists
2016-11-29 12:24:53 -05:00
Mitchell Hashimoto
3b2282ca57
terraform: Destroy node should only include deposed for specific index
Fixes #10338

The destruction step for a resource was included the deposed resources
for _all_ resources with that name (ignoring the "index"). For example:
`aws_instance.foo.0` was including destroying deposed for
`aws_instance.foo.1`.

This changes the config to the deposed transformer to properly include
that index.

This change includes a larger change of changing `stateId` to include
the index. This affected more parts but was ultimately the issue in
question.
2016-11-29 09:16:18 -08:00
Mitchell Hashimoto
80457b689c
terraform: do the deposed check within EvalDiff
There is never any reason to separate the two.
2016-11-28 14:34:24 -08:00
Mitchell Hashimoto
41c56ad002
terraform: new apply graph understands destroying deposed only 2016-11-28 14:34:24 -08:00
Mitchell Hashimoto
6ee3a08799
terraform: deposed shows up in plan, tests 2016-11-28 14:32:42 -08:00
Mitchell Hashimoto
3bf93501a1
terraform: test applying tainted + deposed (passes)
This is added just trying to reproduce a crash I saw. It passes so
adding it to the master tests.
2016-11-28 14:29:38 -08:00
James Bardin
16597d4a55 Nested lists and maps fail in GetRaw
When referencing a list of maps variable from within a resource, only
the first list element is included the plan. This is because GetRaw
can't access the interpolated values. Add some tests to document this
behavior for both Get and GetRaw.
2016-11-28 09:04:12 -05:00
Mitchell Hashimoto
1ddc242271 release: clean up after v0.8.0-rc1 2016-11-23 18:42:12 +00:00
Mitchell Hashimoto
aac47ecf84
v0.8.0-rc1 2016-11-23 18:35:39 +00:00
Mitchell Hashimoto
28d933f6dd
nitpicks for #10310 2016-11-23 09:40:11 -08:00
Mitchell Hashimoto
c15754c365 Merge pull request #10310 from spangenberg/custom-import-provider
Implements import with specified provider
2016-11-23 09:37:04 -08:00
Mitchell Hashimoto
f50c7acf95
terraform: record dependency to self (other index)
Fixes #10313

The new graph wasn't properly recording resource dependencies to a
specific index of itself. For example: `foo.bar.2` depending on
`foo.bar.0` wasn't shown in the state when it should've been.

This adds a test to verify this and fixes it.
2016-11-23 09:25:20 -08:00
Daniel Spangenberg
804a5bd3c5 Implements import with specified provider
This allows the import with a specified provider.
So far it was not possible to get resources imported from a different provider
than the default.
2016-11-23 11:58:58 +01:00
James Bardin
47b66a28b9 Merge pull request #10279 from hashicorp/jbardin/GH-9996
Set proper Mode when moving a data source in state
2016-11-22 08:49:25 -05:00
Mitchell Hashimoto
f35d02cbee
terraform: test for alias inheritance [GH-4789] 2016-11-21 21:51:07 -08:00
Mitchell Hashimoto
71a1e215d9
terraform: add more Same test cases to cover #9171 2016-11-21 17:31:47 -08:00
James Bardin
705527e6fe Set proper Mode when moving a data source in state
ResourceAddr.Mode wasn't properly set when moving a module, so data
sources would lose the "data." prefix when their module was moved within
the State.
2016-11-21 18:26:29 -05:00
Mitchell Hashimoto
b6687aa287
terraform: write lock on post state updates
Found race here: https://travis-ci.org/hashicorp/terraform/builds/177814212

Since WriteState calls `prune` and `init`, we're actually modifying the
state structure so we need a full write lock to perform this operation.
2016-11-21 15:21:35 -08:00
James Bardin
c908f3ceae convert TestStateAdd to subtests 2016-11-21 18:13:15 -05:00
James Bardin
91378d0499 Merge pull request #10233 from hashicorp/jbardin/GH-10229
An empty module in state can panic
2016-11-21 17:35:33 -05:00
James Bardin
e3ef8e6f8a Merge pull request #10228 from hashicorp/jbardin/map-crash
ResourceConfig.get should never return (nil, true)
2016-11-21 12:01:30 -05:00
James Bardin
8f6583c264 Add test for ResourceConfig.Get
ResourceConfig.Get could previously return (nil, true) when looking up
an interpolated map in a list because of the indexing ambiguity. Make
sure we test that a non-existent value always returns false.
2016-11-21 10:09:21 -05:00
James Bardin
e045a9cc79 Merge pull request #10133 from hashicorp/jbardin/debug
DebugVisitInfo
2016-11-21 09:13:51 -05:00
James Bardin
5108182690 Merge pull request #10199 from hashicorp/jbardin/GH-10155
Catch map type errors on variable assignment
2016-11-21 09:13:16 -05:00
Mitchell Hashimoto
e7d59ab245
terraform: test for interpolation escapes 2016-11-20 21:14:16 -08:00
James Bardin
a5cb530571 Move the state module cleanup from init to prune
It makes for sense for this to happen in State.prune(). Also move a
redundant pruning from ResourceState.init, and make sure
ResourceState.prune is called from the parent's prune method.
2016-11-20 11:33:41 -05:00
James Bardin
def55e52ca An empty module in state can panic
An empty module, or a module with an empty path can panic during graph
traversal. Make sure we clear these out when reading and writing the
state.
2016-11-18 17:59:07 -05:00
James Bardin
68ba2d6ff0 ResourceConfig.get should never return (nil, true)
Fixes a case where ResourceConfig.get inadvertently returns a nil value.

Add an integration test where assigning a map to a list via
interpolation would panic.
2016-11-18 16:24:40 -05:00