From f9fee4106f3e53750c795cfe92dc5b51db52fc1f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 4 Nov 2016 19:47:58 -0700 Subject: [PATCH 01/26] terraform: Diff.Prune --- terraform/diff.go | 28 +++++++++++++++++++++++++ terraform/diff_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/terraform/diff.go b/terraform/diff.go index b3a8fe5de0bb..a474efad1caf 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -32,6 +32,30 @@ type Diff struct { Modules []*ModuleDiff } +// Prune cleans out unused structures in the diff without affecting +// the behavior of the diff at all. +// +// This is not safe to call concurrently. This is safe to call on a +// nil Diff. +func (d *Diff) Prune() { + if d == nil { + return + } + + // Prune all empty modules + newModules := make([]*ModuleDiff, 0, len(d.Modules)) + for _, m := range d.Modules { + // If the module isn't empty, we keep it + if !m.Empty() { + newModules = append(newModules, m) + } + } + if len(newModules) == 0 { + newModules = nil + } + d.Modules = newModules +} + // AddModule adds the module with the given path to the diff. // // This should be the preferred method to add module diffs since it @@ -212,6 +236,10 @@ func (d *ModuleDiff) ChangeType() DiffChangeType { // Empty returns true if the diff has no changes within this module. func (d *ModuleDiff) Empty() bool { + if d.Destroy { + return false + } + if len(d.Resources) == 0 { return true } diff --git a/terraform/diff_test.go b/terraform/diff_test.go index fca0fbcbe336..655b165f96db 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -103,6 +103,53 @@ func TestDiffEqual(t *testing.T) { } } +func TestDiffPrune(t *testing.T) { + cases := map[string]struct { + D1, D2 *Diff + }{ + "nil": { + nil, + nil, + }, + + "empty": { + new(Diff), + new(Diff), + }, + + "empty module": { + &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{Path: []string{"root", "foo"}}, + }, + }, + &Diff{}, + }, + + "destroy module": { + &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{Path: []string{"root", "foo"}, Destroy: true}, + }, + }, + &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{Path: []string{"root", "foo"}, Destroy: true}, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + tc.D1.Prune() + if !tc.D1.Equal(tc.D2) { + t.Fatalf("bad:\n\n%#v\n\n%#v", tc.D1, tc.D2) + } + }) + } +} + func TestModuleDiff_ChangeType(t *testing.T) { cases := []struct { Diff *ModuleDiff From ce4ff06d25aab561a30014e8ab82fb889548ace1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 4 Nov 2016 19:49:57 -0700 Subject: [PATCH 02/26] terraform: prepare Plan for shadowing --- terraform/context.go | 65 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 5f9c1e546245..5b7fd7075db7 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -487,28 +487,75 @@ func (c *Context) Plan() (*Plan, error) { c.diffLock.Unlock() // Used throughout below + X_newApply := experiment.Enabled(experiment.X_newDestroy) X_newDestroy := experiment.Enabled(experiment.X_newDestroy) + newGraphEnabled := (c.destroy && X_newDestroy) || (!c.destroy && X_newApply) - // Build the graph. We have a branch here since for the pure-destroy - // plan (c.destroy) we use a much simpler graph builder that simply - // walks the state and reverses edges. - var graph *Graph - var err error - if c.destroy && X_newDestroy { - graph, err = (&DestroyPlanGraphBuilder{ + // Build the original graph. This is before the new graph builders + // coming in 0.8. We do this for shadow graphing. + oldGraph, err := c.Graph(&ContextGraphOpts{Validate: true}) + if err != nil && newGraphEnabled { + // If we had an error graphing but we're using the new graph, + // just set it to nil and let it go. There are some features that + // may work with the new graph that don't with the old. + oldGraph = nil + err = nil + } + if err != nil { + return nil, err + } + + // Build the new graph. We do this no matter wht so we can shadow it. + var newGraph *Graph + err = nil + if c.destroy { + newGraph, err = (&DestroyPlanGraphBuilder{ Module: c.module, State: c.state, Targets: c.targets, }).Build(RootModulePath) } else { - graph, err = c.Graph(&ContextGraphOpts{Validate: true}) + // TODO: new plan graph when its ready + newGraph = nil + } + if err != nil && !newGraphEnabled { + // If we had an error graphing but we're not using this graph, just + // set it to nil and record it as a shadow error. + c.shadowErr = multierror.Append(c.shadowErr, fmt.Errorf( + "Error building new graph: %s", err)) + + newGraph = nil + err = nil } if err != nil { return nil, err } + // Determine what is the real and what is the shadow. The logic here + // is straightforward though the if statements are not: + // + // * If the new graph, shadow with experiment in both because the + // experiment has less nodes so the original can't shadow. + // * If not the new graph, shadow with the experiment + // + real := oldGraph + shadow := newGraph + if newGraphEnabled { + log.Printf("[WARN] terraform: real graph is experiment, shadow is experiment") + real = shadow + } else { + log.Printf("[WARN] terraform: real graph is original, shadow is experiment") + } + + // Special case here: if we're using destroy don't shadow it because + // the new destroy graph behaves a bit differently on purpose by not + // setting the module destroy flag. + if c.destroy && !newGraphEnabled { + shadow = nil + } + // Do the walk - walker, err := c.walk(graph, graph, operation) + walker, err := c.walk(real, shadow, operation) if err != nil { return nil, err } From 4f0d68dda45c7a323af1f0c303357287afcc2722 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 5 Nov 2016 16:26:12 -0700 Subject: [PATCH 03/26] terraform: PlanGraphBuilder --- terraform/graph_builder.go | 2 +- terraform/graph_builder_import.go | 2 +- terraform/graph_builder_plan.go | 101 ++++++++++++++ terraform/graph_config_node_module_test.go | 4 +- terraform/transform_config.go | 39 +++--- terraform/transform_config_old.go | 123 ++++++++++++++++++ ...g_test.go => transform_config_old_test.go} | 32 ++--- terraform/transform_destroy_test.go | 22 ++-- terraform/transform_flatten_test.go | 4 +- terraform/transform_orphan_test.go | 14 +- terraform/transform_output_orphan_test.go | 2 +- terraform/transform_provider_test.go | 14 +- terraform/transform_provisioner_test.go | 4 +- terraform/transform_root_test.go | 2 +- terraform/transform_targets_test.go | 4 +- .../transform_transitive_reduction_test.go | 2 +- 16 files changed, 294 insertions(+), 77 deletions(-) create mode 100644 terraform/graph_builder_plan.go create mode 100644 terraform/transform_config_old.go rename terraform/{transform_config_test.go => transform_config_old_test.go} (73%) diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index 16ca612e0897..6f11d775cc55 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -129,7 +129,7 @@ func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) { func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { steps := []GraphTransformer{ // Create all our resources from the configuration and state - &ConfigTransformer{Module: b.Root}, + &ConfigTransformerOld{Module: b.Root}, &OrphanTransformer{ State: b.State, Module: b.Root, diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go index 6d87d487dcb7..8bd11fb5c286 100644 --- a/terraform/graph_builder_import.go +++ b/terraform/graph_builder_import.go @@ -38,7 +38,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { steps := []GraphTransformer{ // Create all our resources from the configuration and state - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, // Add the import steps &ImportStateTransformer{Targets: b.ImportTargets}, diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go new file mode 100644 index 000000000000..56d23fa049e4 --- /dev/null +++ b/terraform/graph_builder_plan.go @@ -0,0 +1,101 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" +) + +// PlanGraphBuilder implements GraphBuilder and is responsible for building +// a graph for planning (creating a Terraform Diff). +// +// The primary difference between this graph and others: +// +// * Based on the config since it represents the target state +// +// * Ignores lifecycle options since no lifecycle events occur here. This +// simplifies the graph significantly since complex transforms such as +// create-before-destroy can be completely ignored. +// +type PlanGraphBuilder struct { + // Module is the root module for the graph to build. + Module *module.Tree + + // State is the current state + State *State + + // Providers is the list of providers supported. + Providers []string + + // DisableReduce, if true, will not reduce the graph. Great for testing. + DisableReduce bool +} + +// See GraphBuilder +func (b *PlanGraphBuilder) Build(path []string) (*Graph, error) { + return (&BasicGraphBuilder{ + Steps: b.Steps(), + Validate: true, + Name: "plan", + }).Build(path) +} + +// See GraphBuilder +func (b *PlanGraphBuilder) Steps() []GraphTransformer { + // Custom factory for creating providers. + providerFactory := func(name string, path []string) GraphNodeProvider { + return &NodeApplyableProvider{ + NameValue: name, + PathValue: path, + } + } + + concreteResource := func(a *NodeAbstractResource) dag.Vertex { + return &NodeApplyableResource{ + NodeAbstractResource: a, + } + } + + steps := []GraphTransformer{ + // Creates all the resources represented in the config + &ConfigTransformer{ + Concrete: concreteResource, + Module: b.Module, + }, + + // Add the outputs + &OutputTransformer{Module: b.Module}, + + // Attach the configuration to any resources + &AttachResourceConfigTransformer{Module: b.Module}, + + // Attach the state + &AttachStateTransformer{State: b.State}, + + // Create all the providers + &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, + &ProviderTransformer{}, + &DisableProviderTransformer{}, + &ParentProviderTransformer{}, + &AttachProviderConfigTransformer{Module: b.Module}, + + // Add root variables + &RootVariableTransformer{Module: b.Module}, + + // Add module variables + &ModuleVariableTransformer{Module: b.Module}, + + // Connect references so ordering is correct + &ReferenceTransformer{}, + + // Single root + &RootTransformer{}, + } + + if !b.DisableReduce { + // Perform the transitive reduction to make our graph a bit + // more sane if possible (it usually is possible). + steps = append(steps, &TransitiveReductionTransformer{}) + } + + return steps +} diff --git a/terraform/graph_config_node_module_test.go b/terraform/graph_config_node_module_test.go index 1b5430ddfd04..6ba015a45c50 100644 --- a/terraform/graph_config_node_module_test.go +++ b/terraform/graph_config_node_module_test.go @@ -26,7 +26,7 @@ func TestGraphNodeConfigModuleExpand(t *testing.T) { g, err := node.Expand(&BasicGraphBuilder{ Steps: []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, }, }) if err != nil { @@ -51,7 +51,7 @@ func TestGraphNodeConfigModuleExpandFlatten(t *testing.T) { g, err := node.Expand(&BasicGraphBuilder{ Steps: []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, }, }) if err != nil { diff --git a/terraform/transform_config.go b/terraform/transform_config.go index bcfa1233e3a6..dd1ed48443fd 100644 --- a/terraform/transform_config.go +++ b/terraform/transform_config.go @@ -9,21 +9,30 @@ import ( "github.com/hashicorp/terraform/config/module" ) -// ConfigTransformer is a GraphTransformer that adds the configuration -// to the graph. The module used to configure this transformer must be -// the root module. We'll look up the child module by the Path in the -// Graph. +// ConfigTransformer is a GraphTransformer that adds all the resources +// from the configuration to the graph. +// +// The module used to configure this transformer must be the root module. +// +// Only resources are added to the graph. Variables, outputs, and +// providers must be added via other transforms. +// +// Unlike ConfigTransformerOld, this transformer creates a graph with +// all resources including module resources, rather than creating module +// nodes that are then "flattened". type ConfigTransformer struct { Module *module.Tree } func (t *ConfigTransformer) Transform(g *Graph) error { - // A module is required and also must be completely loaded. + // If no module is given, we don't do anything if t.Module == nil { - return errors.New("module must not be nil") + return nil } + + // If the module isn't loaded, that is simply an error if !t.Module.Loaded() { - return errors.New("module must be loaded") + return errors.New("module must be loaded for ConfigTransformer") } // Get the module we care about @@ -105,19 +114,3 @@ func (t *ConfigTransformer) Transform(g *Graph) error { return err } - -// varNameForVar returns the VarName value for an interpolated variable. -// This value is compared to the VarName() value for the nodes within the -// graph to build the graph edges. -func varNameForVar(raw config.InterpolatedVariable) string { - switch v := raw.(type) { - case *config.ModuleVariable: - return fmt.Sprintf("module.%s.output.%s", v.Name, v.Field) - case *config.ResourceVariable: - return v.ResourceId() - case *config.UserVariable: - return fmt.Sprintf("var.%s", v.Name) - default: - return "" - } -} diff --git a/terraform/transform_config_old.go b/terraform/transform_config_old.go new file mode 100644 index 000000000000..5f9851681ae7 --- /dev/null +++ b/terraform/transform_config_old.go @@ -0,0 +1,123 @@ +package terraform + +import ( + "errors" + "fmt" + + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" +) + +// ConfigTransformerOld is a GraphTransformer that adds the configuration +// to the graph. The module used to configure this transformer must be +// the root module. We'll look up the child module by the Path in the +// Graph. +type ConfigTransformerOld struct { + Module *module.Tree +} + +func (t *ConfigTransformerOld) Transform(g *Graph) error { + // A module is required and also must be completely loaded. + if t.Module == nil { + return errors.New("module must not be nil") + } + if !t.Module.Loaded() { + return errors.New("module must be loaded") + } + + // Get the module we care about + module := t.Module.Child(g.Path[1:]) + if module == nil { + return nil + } + + // Get the configuration for this module + config := module.Config() + + // Create the node list we'll use for the graph + nodes := make([]graphNodeConfig, 0, + (len(config.Variables)+ + len(config.ProviderConfigs)+ + len(config.Modules)+ + len(config.Resources)+ + len(config.Outputs))*2) + + // Write all the variables out + for _, v := range config.Variables { + nodes = append(nodes, &GraphNodeConfigVariable{ + Variable: v, + ModuleTree: t.Module, + ModulePath: g.Path, + }) + } + + // Write all the provider configs out + for _, pc := range config.ProviderConfigs { + nodes = append(nodes, &GraphNodeConfigProvider{Provider: pc}) + } + + // Write all the resources out + for _, r := range config.Resources { + nodes = append(nodes, &GraphNodeConfigResource{ + Resource: r, + Path: g.Path, + }) + } + + // Write all the modules out + children := module.Children() + for _, m := range config.Modules { + path := make([]string, len(g.Path), len(g.Path)+1) + copy(path, g.Path) + path = append(path, m.Name) + + nodes = append(nodes, &GraphNodeConfigModule{ + Path: path, + Module: m, + Tree: children[m.Name], + }) + } + + // Write all the outputs out + for _, o := range config.Outputs { + nodes = append(nodes, &GraphNodeConfigOutput{Output: o}) + } + + // Err is where the final error value will go if there is one + var err error + + // Build the graph vertices + for _, n := range nodes { + g.Add(n) + } + + // Build up the dependencies. We have to do this outside of the above + // loop since the nodes need to be in place for us to build the deps. + for _, n := range nodes { + if missing := g.ConnectDependent(n); len(missing) > 0 { + for _, m := range missing { + err = multierror.Append(err, fmt.Errorf( + "%s: missing dependency: %s", n.Name(), m)) + } + } + } + + return err +} + +// varNameForVar returns the VarName value for an interpolated variable. +// This value is compared to the VarName() value for the nodes within the +// graph to build the graph edges. +func varNameForVar(raw config.InterpolatedVariable) string { + switch v := raw.(type) { + case *config.ModuleVariable: + return fmt.Sprintf("module.%s.output.%s", v.Name, v.Field) + case *config.ResourceVariable: + return v.ResourceId() + case *config.UserVariable: + return fmt.Sprintf("var.%s", v.Name) + default: + return "" + } +} diff --git a/terraform/transform_config_test.go b/terraform/transform_config_old_test.go similarity index 73% rename from terraform/transform_config_test.go rename to terraform/transform_config_old_test.go index 4ba88a358220..a5b0bbfbc51e 100644 --- a/terraform/transform_config_test.go +++ b/terraform/transform_config_old_test.go @@ -8,15 +8,15 @@ import ( "github.com/hashicorp/terraform/config/module" ) -func TestConfigTransformer_nilModule(t *testing.T) { +func TestConfigTransformerOld_nilModule(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{} + tf := &ConfigTransformerOld{} if err := tf.Transform(&g); err == nil { t.Fatal("should error") } } -func TestConfigTransformer_unloadedModule(t *testing.T) { +func TestConfigTransformerOld_unloadedModule(t *testing.T) { mod, err := module.NewTreeModule( "", filepath.Join(fixtureDir, "graph-basic")) if err != nil { @@ -24,15 +24,15 @@ func TestConfigTransformer_unloadedModule(t *testing.T) { } g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err == nil { t.Fatal("should error") } } -func TestConfigTransformer(t *testing.T) { +func TestConfigTransformerOld(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: testModule(t, "graph-basic")} + tf := &ConfigTransformerOld{Module: testModule(t, "graph-basic")} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -44,9 +44,9 @@ func TestConfigTransformer(t *testing.T) { } } -func TestConfigTransformer_dependsOn(t *testing.T) { +func TestConfigTransformerOld_dependsOn(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: testModule(t, "graph-depends-on")} + tf := &ConfigTransformerOld{Module: testModule(t, "graph-depends-on")} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -58,9 +58,9 @@ func TestConfigTransformer_dependsOn(t *testing.T) { } } -func TestConfigTransformer_modules(t *testing.T) { +func TestConfigTransformerOld_modules(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: testModule(t, "graph-modules")} + tf := &ConfigTransformerOld{Module: testModule(t, "graph-modules")} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -72,9 +72,9 @@ func TestConfigTransformer_modules(t *testing.T) { } } -func TestConfigTransformer_outputs(t *testing.T) { +func TestConfigTransformerOld_outputs(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: testModule(t, "graph-outputs")} + tf := &ConfigTransformerOld{Module: testModule(t, "graph-outputs")} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -86,9 +86,9 @@ func TestConfigTransformer_outputs(t *testing.T) { } } -func TestConfigTransformer_providerAlias(t *testing.T) { +func TestConfigTransformerOld_providerAlias(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: testModule(t, "graph-provider-alias")} + tf := &ConfigTransformerOld{Module: testModule(t, "graph-provider-alias")} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -100,9 +100,9 @@ func TestConfigTransformer_providerAlias(t *testing.T) { } } -func TestConfigTransformer_errMissingDeps(t *testing.T) { +func TestConfigTransformerOld_errMissingDeps(t *testing.T) { g := Graph{Path: RootModulePath} - tf := &ConfigTransformer{Module: testModule(t, "graph-missing-deps")} + tf := &ConfigTransformerOld{Module: testModule(t, "graph-missing-deps")} if err := tf.Transform(&g); err == nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_destroy_test.go b/terraform/transform_destroy_test.go index fe9032a5e3b8..f03064e5640b 100644 --- a/terraform/transform_destroy_test.go +++ b/terraform/transform_destroy_test.go @@ -10,7 +10,7 @@ func TestDestroyTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -35,7 +35,7 @@ func TestDestroyTransformer_dependsOn(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -60,7 +60,7 @@ func TestCreateBeforeDestroyTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -92,7 +92,7 @@ func TestCreateBeforeDestroyTransformer_twice(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -125,7 +125,7 @@ func TestPruneDestroyTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -168,7 +168,7 @@ func TestPruneDestroyTransformer_diff(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -202,7 +202,7 @@ func TestPruneDestroyTransformer_count(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -251,7 +251,7 @@ func TestPruneDestroyTransformer_countDec(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -297,7 +297,7 @@ func TestPruneDestroyTransformer_countState(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -347,7 +347,7 @@ func TestPruneDestroyTransformer_prefixMatch(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -396,7 +396,7 @@ func TestPruneDestroyTransformer_tainted(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_flatten_test.go b/terraform/transform_flatten_test.go index 92f7cac7af88..47bc5d2c5c98 100644 --- a/terraform/transform_flatten_test.go +++ b/terraform/transform_flatten_test.go @@ -11,7 +11,7 @@ func TestFlattenTransformer(t *testing.T) { var b BasicGraphBuilder b = BasicGraphBuilder{ Steps: []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, &VertexTransformer{ Transforms: []GraphVertexTransformer{ &ExpandTransform{ @@ -41,7 +41,7 @@ func TestFlattenTransformer_withProxy(t *testing.T) { var b BasicGraphBuilder b = BasicGraphBuilder{ Steps: []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, &VertexTransformer{ Transforms: []GraphVertexTransformer{ &ExpandTransform{ diff --git a/terraform/transform_orphan_test.go b/terraform/transform_orphan_test.go index 76fbaa6a1b73..ef6b497adecd 100644 --- a/terraform/transform_orphan_test.go +++ b/terraform/transform_orphan_test.go @@ -35,7 +35,7 @@ func TestOrphanTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -86,7 +86,7 @@ func TestOrphanTransformer_modules(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -140,7 +140,7 @@ func TestOrphanTransformer_modulesDeps(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -194,7 +194,7 @@ func TestOrphanTransformer_modulesDepsOrphan(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -233,7 +233,7 @@ func TestOrphanTransformer_modulesNoRoot(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -282,7 +282,7 @@ func TestOrphanTransformer_resourceDepends(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -305,7 +305,7 @@ func TestOrphanTransformer_nilState(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_output_orphan_test.go b/terraform/transform_output_orphan_test.go index 3bbe7a3e82fa..1c930ffa3caf 100644 --- a/terraform/transform_output_orphan_test.go +++ b/terraform/transform_output_orphan_test.go @@ -27,7 +27,7 @@ func TestAddOutputOrphanTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_provider_test.go b/terraform/transform_provider_test.go index bc2cff532c7d..1ea7be76d333 100644 --- a/terraform/transform_provider_test.go +++ b/terraform/transform_provider_test.go @@ -12,7 +12,7 @@ func TestProviderTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -73,7 +73,7 @@ func TestCloseProviderTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -105,7 +105,7 @@ func TestCloseProviderTransformer_withTargets(t *testing.T) { g := Graph{Path: RootModulePath} transforms := []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, &ProviderTransformer{}, &CloseProviderTransformer{}, &TargetsTransformer{ @@ -135,7 +135,7 @@ func TestMissingProviderTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -318,7 +318,7 @@ func TestPruneProviderTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -364,7 +364,7 @@ func TestDisableProviderTransformer(t *testing.T) { g := Graph{Path: RootModulePath} transforms := []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, &MissingProviderTransformer{Providers: []string{"aws"}}, &ProviderTransformer{}, &DisableProviderTransformerOld{}, @@ -390,7 +390,7 @@ func TestDisableProviderTransformer_keep(t *testing.T) { g := Graph{Path: RootModulePath} transforms := []GraphTransformer{ - &ConfigTransformer{Module: mod}, + &ConfigTransformerOld{Module: mod}, &MissingProviderTransformer{Providers: []string{"aws"}}, &ProviderTransformer{}, &DisableProviderTransformerOld{}, diff --git a/terraform/transform_provisioner_test.go b/terraform/transform_provisioner_test.go index 3f37c5a69b7c..270910e78a90 100644 --- a/terraform/transform_provisioner_test.go +++ b/terraform/transform_provisioner_test.go @@ -12,7 +12,7 @@ func TestMissingProvisionerTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -112,7 +112,7 @@ func TestCloseProvisionerTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_root_test.go b/terraform/transform_root_test.go index 68f5c511448c..1cfec3995f6b 100644 --- a/terraform/transform_root_test.go +++ b/terraform/transform_root_test.go @@ -10,7 +10,7 @@ func TestRootTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_targets_test.go b/terraform/transform_targets_test.go index 2daa72827e5b..142fbb52f459 100644 --- a/terraform/transform_targets_test.go +++ b/terraform/transform_targets_test.go @@ -10,7 +10,7 @@ func TestTargetsTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -41,7 +41,7 @@ func TestTargetsTransformer_destroy(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/transform_transitive_reduction_test.go b/terraform/transform_transitive_reduction_test.go index bcd3b7d2337f..4c864e236fdc 100644 --- a/terraform/transform_transitive_reduction_test.go +++ b/terraform/transform_transitive_reduction_test.go @@ -10,7 +10,7 @@ func TestTransitiveReductionTransformer(t *testing.T) { g := Graph{Path: RootModulePath} { - tf := &ConfigTransformer{Module: mod} + tf := &ConfigTransformerOld{Module: mod} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } From 7557e6e70a2bacb5165a5f09b50a073615ce0c30 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 5 Nov 2016 17:53:12 -0700 Subject: [PATCH 04/26] terraform: ConfigTransformer --- terraform/transform_config.go | 110 ++++++++++++----------------- terraform/transform_config_test.go | 56 +++++++++++++++ 2 files changed, 101 insertions(+), 65 deletions(-) create mode 100644 terraform/transform_config_test.go diff --git a/terraform/transform_config.go b/terraform/transform_config.go index dd1ed48443fd..c2dad20c966b 100644 --- a/terraform/transform_config.go +++ b/terraform/transform_config.go @@ -3,10 +3,10 @@ package terraform import ( "errors" "fmt" + "log" - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" ) // ConfigTransformer is a GraphTransformer that adds all the resources @@ -21,6 +21,8 @@ import ( // all resources including module resources, rather than creating module // nodes that are then "flattened". type ConfigTransformer struct { + Concrete ConcreteResourceNodeFunc + Module *module.Tree } @@ -35,82 +37,60 @@ func (t *ConfigTransformer) Transform(g *Graph) error { return errors.New("module must be loaded for ConfigTransformer") } - // Get the module we care about - module := t.Module.Child(g.Path[1:]) - if module == nil { + // Start the transformation process + return t.transform(g, t.Module) +} + +func (t *ConfigTransformer) transform(g *Graph, m *module.Tree) error { + // If no config, do nothing + if m == nil { return nil } - // Get the configuration for this module - config := module.Config() - - // Create the node list we'll use for the graph - nodes := make([]graphNodeConfig, 0, - (len(config.Variables)+ - len(config.ProviderConfigs)+ - len(config.Modules)+ - len(config.Resources)+ - len(config.Outputs))*2) - - // Write all the variables out - for _, v := range config.Variables { - nodes = append(nodes, &GraphNodeConfigVariable{ - Variable: v, - ModuleTree: t.Module, - ModulePath: g.Path, - }) + // Add our resources + if err := t.transformSingle(g, m); err != nil { + return err } - // Write all the provider configs out - for _, pc := range config.ProviderConfigs { - nodes = append(nodes, &GraphNodeConfigProvider{Provider: pc}) + // Transform all the children. + for _, c := range m.Children() { + if err := t.transform(g, c); err != nil { + return err + } } - // Write all the resources out - for _, r := range config.Resources { - nodes = append(nodes, &GraphNodeConfigResource{ - Resource: r, - Path: g.Path, - }) - } + return nil +} - // Write all the modules out - children := module.Children() - for _, m := range config.Modules { - path := make([]string, len(g.Path), len(g.Path)+1) - copy(path, g.Path) - path = append(path, m.Name) - - nodes = append(nodes, &GraphNodeConfigModule{ - Path: path, - Module: m, - Tree: children[m.Name], - }) - } +func (t *ConfigTransformer) transformSingle(g *Graph, m *module.Tree) error { + log.Printf("[TRACE] ConfigTransformer: Starting for path: %v", m.Path()) - // Write all the outputs out - for _, o := range config.Outputs { - nodes = append(nodes, &GraphNodeConfigOutput{Output: o}) - } + // Get the configuration for this module + config := m.Config() - // Err is where the final error value will go if there is one - var err error + // Build the path we're at + path := m.Path() - // Build the graph vertices - for _, n := range nodes { - g.Add(n) - } + // Write all the resources out + for _, r := range config.Resources { + // Build the resource address + addr, err := parseResourceAddressConfig(r) + if err != nil { + panic(fmt.Sprintf( + "Error parsing config address, this is a bug: %#v", r)) + } + addr.Path = path - // Build up the dependencies. We have to do this outside of the above - // loop since the nodes need to be in place for us to build the deps. - for _, n := range nodes { - if missing := g.ConnectDependent(n); len(missing) > 0 { - for _, m := range missing { - err = multierror.Append(err, fmt.Errorf( - "%s: missing dependency: %s", n.Name(), m)) - } + // Build the abstract node and the concrete one + abstract := &NodeAbstractResource{Addr: addr} + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) } + + // Add it to the graph + g.Add(node) } - return err + return nil } diff --git a/terraform/transform_config_test.go b/terraform/transform_config_test.go new file mode 100644 index 000000000000..81ce1fcab8d8 --- /dev/null +++ b/terraform/transform_config_test.go @@ -0,0 +1,56 @@ +package terraform + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/hashicorp/terraform/config/module" +) + +func TestConfigTransformer_nilModule(t *testing.T) { + g := Graph{Path: RootModulePath} + tf := &ConfigTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + if len(g.Vertices()) > 0 { + t.Fatalf("graph is not empty: %s", g) + } +} + +func TestConfigTransformer_unloadedModule(t *testing.T) { + mod, err := module.NewTreeModule( + "", filepath.Join(fixtureDir, "graph-basic")) + if err != nil { + t.Fatalf("err: %s", err) + } + + g := Graph{Path: RootModulePath} + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err == nil { + t.Fatal("should error") + } +} + +func TestConfigTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + tf := &ConfigTransformer{Module: testModule(t, "graph-basic")} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testConfigTransformerGraphBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testConfigTransformerGraphBasicStr = ` +aws_instance.web +aws_load_balancer.weblb +aws_security_group.firewall +openstack_floating_ip.random +` From dbac0785bc4dd1ddd1041680aeed273721e3434e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 5 Nov 2016 17:58:38 -0700 Subject: [PATCH 05/26] terraform: tests for the plan graph builder --- terraform/graph_builder_plan_test.go | 52 +++++++++++++++++++ .../graph-builder-plan-basic/main.tf | 24 +++++++++ 2 files changed, 76 insertions(+) create mode 100644 terraform/graph_builder_plan_test.go create mode 100644 terraform/test-fixtures/graph-builder-plan-basic/main.tf diff --git a/terraform/graph_builder_plan_test.go b/terraform/graph_builder_plan_test.go new file mode 100644 index 000000000000..235fea745ba6 --- /dev/null +++ b/terraform/graph_builder_plan_test.go @@ -0,0 +1,52 @@ +package terraform + +import ( + "reflect" + "strings" + "testing" +) + +func TestPlanGraphBuilder_impl(t *testing.T) { + var _ GraphBuilder = new(PlanGraphBuilder) +} + +func TestPlanGraphBuilder(t *testing.T) { + b := &PlanGraphBuilder{ + Module: testModule(t, "graph-builder-plan-basic"), + Providers: []string{"aws", "openstack"}, + DisableReduce: true, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(g.Path, RootModulePath) { + t.Fatalf("bad: %#v", g.Path) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testPlanGraphBuilderStr) + if actual != expected { + t.Fatalf("bad: %s", actual) + } +} + +const testPlanGraphBuilderStr = ` +aws_instance.web + aws_security_group.firewall + provider.aws + var.foo +aws_load_balancer.weblb + aws_instance.web + provider.aws +aws_security_group.firewall + provider.aws +openstack_floating_ip.random + provider.openstack +provider.aws + openstack_floating_ip.random +provider.openstack +var.foo +` diff --git a/terraform/test-fixtures/graph-builder-plan-basic/main.tf b/terraform/test-fixtures/graph-builder-plan-basic/main.tf new file mode 100644 index 000000000000..a40802cc98eb --- /dev/null +++ b/terraform/test-fixtures/graph-builder-plan-basic/main.tf @@ -0,0 +1,24 @@ +variable "foo" { + default = "bar" + description = "bar" +} + +provider "aws" { + foo = "${openstack_floating_ip.random.value}" +} + +resource "openstack_floating_ip" "random" {} + +resource "aws_security_group" "firewall" {} + +resource "aws_instance" "web" { + ami = "${var.foo}" + security_groups = [ + "foo", + "${aws_security_group.firewall.foo}" + ] +} + +resource "aws_load_balancer" "weblb" { + members = "${aws_instance.web.id_list}" +} From d7aa59be3c0e2fdba97c3de9a13dc183e18d098e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 6 Nov 2016 00:00:05 -0700 Subject: [PATCH 06/26] terraform: begin NodePlannableResource --- terraform/context.go | 14 ++- terraform/context_plan_test.go | 2 +- terraform/graph_builder_plan.go | 2 +- terraform/node_resource_plan.go | 207 ++++++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 6 deletions(-) create mode 100644 terraform/node_resource_plan.go diff --git a/terraform/context.go b/terraform/context.go index 5b7fd7075db7..5403ec41db04 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -487,7 +487,7 @@ func (c *Context) Plan() (*Plan, error) { c.diffLock.Unlock() // Used throughout below - X_newApply := experiment.Enabled(experiment.X_newDestroy) + X_newApply := experiment.Enabled(experiment.X_newApply) X_newDestroy := experiment.Enabled(experiment.X_newDestroy) newGraphEnabled := (c.destroy && X_newDestroy) || (!c.destroy && X_newApply) @@ -515,8 +515,11 @@ func (c *Context) Plan() (*Plan, error) { Targets: c.targets, }).Build(RootModulePath) } else { - // TODO: new plan graph when its ready - newGraph = nil + newGraph, err = (&PlanGraphBuilder{ + Module: c.module, + State: c.state, + Providers: c.components.ResourceProviders(), + }).Build(RootModulePath) } if err != nil && !newGraphEnabled { // If we had an error graphing but we're not using this graph, just @@ -554,6 +557,9 @@ func (c *Context) Plan() (*Plan, error) { shadow = nil } + // TODO: remove when we're ready + shadow = nil + // Do the walk walker, err := c.walk(real, shadow, operation) if err != nil { @@ -574,7 +580,7 @@ func (c *Context) Plan() (*Plan, error) { // We don't do the reverification during the new destroy plan because // it will use a different apply process. - if !(c.destroy && X_newDestroy) { + if !newGraphEnabled { // Now that we have a diff, we can build the exact graph that Apply will use // and catch any possible cycles during the Plan phase. if _, err := c.Graph(&ContextGraphOpts{Validate: true}); err != nil { diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index c791b8b5c804..e42e83ff29c9 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -11,7 +11,7 @@ import ( "testing" ) -func TestContext2Plan(t *testing.T) { +func TestContext2Plan_basic(t *testing.T) { m := testModule(t, "plan-good") p := testProvider("aws") p.DiffFn = testDiffFn diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 56d23fa049e4..e25749299b4e 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -50,7 +50,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { } concreteResource := func(a *NodeAbstractResource) dag.Vertex { - return &NodeApplyableResource{ + return &NodePlannableResource{ NodeAbstractResource: a, } } diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go new file mode 100644 index 000000000000..b708715a4ef4 --- /dev/null +++ b/terraform/node_resource_plan.go @@ -0,0 +1,207 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodePlannableResource represents a resource that is "plannable": +// it is ready to be planned in order to create a diff. +type NodePlannableResource struct { + *NodeAbstractResource +} + +// GraphNodeEvalable +func (n *NodePlannableResource) EvalTree() EvalNode { + addr := n.NodeAbstractResource.Addr + + // stateId is the ID to put into the state + stateId := addr.stateId() + if addr.Index > -1 { + stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) + } + + // Build the instance info. More of this will be populated during eval + info := &InstanceInfo{ + Id: stateId, + Type: addr.Type, + } + + // Build the resource for eval + resource := &Resource{ + Name: addr.Name, + Type: addr.Type, + CountIndex: addr.Index, + } + if resource.CountIndex < 0 { + resource.CountIndex = 0 + } + + // Determine the dependencies for the state. We use some older + // code for this that we've used for a long time. + var stateDeps []string + { + oldN := &graphNodeExpandedResource{Resource: n.Config} + stateDeps = oldN.StateDependencies() + } + + // Eval info is different depending on what kind of resource this is + switch n.Config.Mode { + case config.ManagedResourceMode: + return n.evalTreeManagedResource( + stateId, info, resource, stateDeps, + ) + case config.DataResourceMode: + return n.evalTreeDataResource( + stateId, info, resource, stateDeps) + default: + panic(fmt.Errorf("unsupported resource mode %s", n.Config.Mode)) + } +} + +func (n *NodePlannableResource) evalTreeDataResource( + stateId string, info *InstanceInfo, + resource *Resource, stateDeps []string) EvalNode { + var provider ResourceProvider + var config *ResourceConfig + var diff *InstanceDiff + var state *InstanceState + + return &EvalSequence{ + Nodes: []EvalNode{ + // Get the saved diff for apply + &EvalReadDiff{ + Name: stateId, + Diff: &diff, + }, + + // Stop here if we don't actually have a diff + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if diff == nil { + return true, EvalEarlyExitError{} + } + + if diff.GetAttributesLen() == 0 { + return true, EvalEarlyExitError{} + } + + return true, nil + }, + Then: EvalNoop{}, + }, + + // We need to re-interpolate the config here, rather than + // just using the diff's values directly, because we've + // potentially learned more variable values during the + // apply pass that weren't known when the diff was produced. + &EvalInterpolate{ + Config: n.Config.RawConfig.Copy(), + Resource: resource, + Output: &config, + }, + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + + // Make a new diff with our newly-interpolated config. + &EvalReadDataDiff{ + Info: info, + Config: &config, + Previous: &diff, + Provider: &provider, + Output: &diff, + }, + + &EvalReadDataApply{ + Info: info, + Diff: &diff, + Provider: &provider, + Output: &state, + }, + + &EvalWriteState{ + Name: stateId, + ResourceType: n.Config.Type, + Provider: n.Config.Provider, + Dependencies: stateDeps, + State: &state, + }, + + // Clear the diff now that we've applied it, so + // later nodes won't see a diff that's now a no-op. + &EvalWriteDiff{ + Name: stateId, + Diff: nil, + }, + + &EvalUpdateStateHook{}, + }, + } +} + +func (n *NodePlannableResource) evalTreeManagedResource( + stateId string, info *InstanceInfo, + resource *Resource, stateDeps []string) EvalNode { + // Declare a bunch of variables that are used for state during + // evaluation. Most of this are written to by-address below. + var provider ResourceProvider + var diff *InstanceDiff + var state *InstanceState + var resourceConfig *ResourceConfig + + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.Config.RawConfig.Copy(), + Resource: resource, + Output: &resourceConfig, + }, + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + // Re-run validation to catch any errors we missed, e.g. type + // mismatches on computed values. + &EvalValidateResource{ + Provider: &provider, + Config: &resourceConfig, + ResourceName: n.Config.Name, + ResourceType: n.Config.Type, + ResourceMode: n.Config.Mode, + IgnoreWarnings: true, + }, + &EvalReadState{ + Name: stateId, + Output: &state, + }, + &EvalDiff{ + Info: info, + Config: &resourceConfig, + Resource: n.Config, + Provider: &provider, + State: &state, + OutputDiff: &diff, + OutputState: &state, + }, + &EvalCheckPreventDestroy{ + Resource: n.Config, + Diff: &diff, + }, + &EvalWriteState{ + Name: stateId, + ResourceType: n.Config.Type, + Provider: n.Config.Provider, + Dependencies: stateDeps, + State: &state, + }, + &EvalWriteDiff{ + Name: stateId, + Diff: &diff, + }, + }, + } +} From 4cdaf6f68716ca6889cad8e0a2c710ba851d43c4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 6 Nov 2016 10:07:17 -0800 Subject: [PATCH 07/26] terraform: ResourceTransformer to ResourceTransformerOld --- terraform/graph_config_node_resource.go | 2 +- terraform/node_resource_plan.go | 221 ++++------------------- terraform/node_resource_plan_instance.go | 196 ++++++++++++++++++++ terraform/transform_resource.go | 8 +- terraform/transform_resource_test.go | 20 +- 5 files changed, 242 insertions(+), 205 deletions(-) create mode 100644 terraform/node_resource_plan_instance.go diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index e3decb452563..db4e6626cea5 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -156,7 +156,7 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) steps := make([]GraphTransformer, 0, 5) // Expand counts. - steps = append(steps, &ResourceCountTransformer{ + steps = append(steps, &ResourceCountTransformerOld{ Resource: n.Resource, Destroy: n.Destroy, Targets: n.Targets, diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index b708715a4ef4..7950c559515e 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -1,11 +1,5 @@ package terraform -import ( - "fmt" - - "github.com/hashicorp/terraform/config" -) - // NodePlannableResource represents a resource that is "plannable": // it is ready to be planned in order to create a diff. type NodePlannableResource struct { @@ -14,194 +8,41 @@ type NodePlannableResource struct { // GraphNodeEvalable func (n *NodePlannableResource) EvalTree() EvalNode { - addr := n.NodeAbstractResource.Addr - - // stateId is the ID to put into the state - stateId := addr.stateId() - if addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) - } - - // Build the instance info. More of this will be populated during eval - info := &InstanceInfo{ - Id: stateId, - Type: addr.Type, - } - - // Build the resource for eval - resource := &Resource{ - Name: addr.Name, - Type: addr.Type, - CountIndex: addr.Index, - } - if resource.CountIndex < 0 { - resource.CountIndex = 0 - } - - // Determine the dependencies for the state. We use some older - // code for this that we've used for a long time. - var stateDeps []string - { - oldN := &graphNodeExpandedResource{Resource: n.Config} - stateDeps = oldN.StateDependencies() - } - - // Eval info is different depending on what kind of resource this is - switch n.Config.Mode { - case config.ManagedResourceMode: - return n.evalTreeManagedResource( - stateId, info, resource, stateDeps, - ) - case config.DataResourceMode: - return n.evalTreeDataResource( - stateId, info, resource, stateDeps) - default: - panic(fmt.Errorf("unsupported resource mode %s", n.Config.Mode)) - } -} - -func (n *NodePlannableResource) evalTreeDataResource( - stateId string, info *InstanceInfo, - resource *Resource, stateDeps []string) EvalNode { - var provider ResourceProvider - var config *ResourceConfig - var diff *InstanceDiff - var state *InstanceState - return &EvalSequence{ Nodes: []EvalNode{ - // Get the saved diff for apply - &EvalReadDiff{ - Name: stateId, - Diff: &diff, - }, - - // Stop here if we don't actually have a diff - &EvalIf{ - If: func(ctx EvalContext) (bool, error) { - if diff == nil { - return true, EvalEarlyExitError{} - } - - if diff.GetAttributesLen() == 0 { - return true, EvalEarlyExitError{} - } - - return true, nil - }, - Then: EvalNoop{}, - }, - - // We need to re-interpolate the config here, rather than - // just using the diff's values directly, because we've - // potentially learned more variable values during the - // apply pass that weren't known when the diff was produced. - &EvalInterpolate{ - Config: n.Config.RawConfig.Copy(), - Resource: resource, - Output: &config, - }, - - &EvalGetProvider{ - Name: n.ProvidedBy()[0], - Output: &provider, - }, - - // Make a new diff with our newly-interpolated config. - &EvalReadDataDiff{ - Info: info, - Config: &config, - Previous: &diff, - Provider: &provider, - Output: &diff, - }, - - &EvalReadDataApply{ - Info: info, - Diff: &diff, - Provider: &provider, - Output: &state, - }, - - &EvalWriteState{ - Name: stateId, - ResourceType: n.Config.Type, - Provider: n.Config.Provider, - Dependencies: stateDeps, - State: &state, - }, - - // Clear the diff now that we've applied it, so - // later nodes won't see a diff that's now a no-op. - &EvalWriteDiff{ - Name: stateId, - Diff: nil, - }, - - &EvalUpdateStateHook{}, + // The EvalTree for a plannable resource primarily involves + // interpolating the count since it can contain variables + // we only just received access to. + // + // With the interpolated count, we can then DynamicExpand + // into the proper number of instances. + &EvalInterpolate{Config: n.Config.RawCount}, }, } } -func (n *NodePlannableResource) evalTreeManagedResource( - stateId string, info *InstanceInfo, - resource *Resource, stateDeps []string) EvalNode { - // Declare a bunch of variables that are used for state during - // evaluation. Most of this are written to by-address below. - var provider ResourceProvider - var diff *InstanceDiff - var state *InstanceState - var resourceConfig *ResourceConfig - - return &EvalSequence{ - Nodes: []EvalNode{ - &EvalInterpolate{ - Config: n.Config.RawConfig.Copy(), - Resource: resource, - Output: &resourceConfig, - }, - &EvalGetProvider{ - Name: n.ProvidedBy()[0], - Output: &provider, - }, - // Re-run validation to catch any errors we missed, e.g. type - // mismatches on computed values. - &EvalValidateResource{ - Provider: &provider, - Config: &resourceConfig, - ResourceName: n.Config.Name, - ResourceType: n.Config.Type, - ResourceMode: n.Config.Mode, - IgnoreWarnings: true, - }, - &EvalReadState{ - Name: stateId, - Output: &state, - }, - &EvalDiff{ - Info: info, - Config: &resourceConfig, - Resource: n.Config, - Provider: &provider, - State: &state, - OutputDiff: &diff, - OutputState: &state, - }, - &EvalCheckPreventDestroy{ - Resource: n.Config, - Diff: &diff, - }, - &EvalWriteState{ - Name: stateId, - ResourceType: n.Config.Type, - Provider: n.Config.Provider, - Dependencies: stateDeps, - State: &state, - }, - &EvalWriteDiff{ - Name: stateId, - Diff: &diff, - }, - }, - } +/* +// GraphNodeDynamicExpandable +func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { + state, lock := ctx.State() + lock.RLock() + defer lock.RUnlock() + + // Start creating the steps + steps := make([]GraphTransformer, 0, 5) + + // Expand counts. + steps = append(steps, &ResourceCountTransformer{ + Resource: n.Resource, + Destroy: n.Destroy, + Targets: n.Targets, + }) + + // Always end with the root being added + steps = append(steps, &RootTransformer{}) + + // Build the graph + b := &BasicGraphBuilder{Steps: steps, Validate: true} + return b.Build(ctx.Path()) } +*/ diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go new file mode 100644 index 000000000000..e9e62786e5bd --- /dev/null +++ b/terraform/node_resource_plan_instance.go @@ -0,0 +1,196 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodePlannableResourceInstance represents a _single_ resource +// instance that is plannable. This means this represents a single +// count index, for example. +type NodePlannableResourceInstance struct { + *NodeAbstractResource +} + +// GraphNodeEvalable +func (n *NodePlannableResourceInstance) EvalTree() EvalNode { + addr := n.NodeAbstractResource.Addr + + // stateId is the ID to put into the state + stateId := addr.stateId() + if addr.Index > -1 { + stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) + } + + // Build the instance info. More of this will be populated during eval + info := &InstanceInfo{ + Id: stateId, + Type: addr.Type, + } + + // Build the resource for eval + resource := &Resource{ + Name: addr.Name, + Type: addr.Type, + CountIndex: addr.Index, + } + if resource.CountIndex < 0 { + resource.CountIndex = 0 + } + + // Determine the dependencies for the state. We use some older + // code for this that we've used for a long time. + var stateDeps []string + { + oldN := &graphNodeExpandedResource{Resource: n.Config} + stateDeps = oldN.StateDependencies() + } + + // Eval info is different depending on what kind of resource this is + switch n.Config.Mode { + case config.ManagedResourceMode: + return n.evalTreeManagedResource( + stateId, info, resource, stateDeps, + ) + case config.DataResourceMode: + return n.evalTreeDataResource( + stateId, info, resource, stateDeps) + default: + panic(fmt.Errorf("unsupported resource mode %s", n.Config.Mode)) + } +} + +func (n *NodePlannableResourceInstance) evalTreeDataResource( + stateId string, info *InstanceInfo, + resource *Resource, stateDeps []string) EvalNode { + var provider ResourceProvider + var config *ResourceConfig + var diff *InstanceDiff + var state *InstanceState + + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalReadState{ + Name: stateId, + Output: &state, + }, + + // We need to re-interpolate the config here because some + // of the attributes may have become computed during + // earlier planning, due to other resources having + // "requires new resource" diffs. + &EvalInterpolate{ + Config: n.Config.RawConfig.Copy(), + Resource: resource, + Output: &config, + }, + + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + computed := config.ComputedKeys != nil && len(config.ComputedKeys) > 0 + + // If the configuration is complete and we + // already have a state then we don't need to + // do any further work during apply, because we + // already populated the state during refresh. + if !computed && state != nil { + return true, EvalEarlyExitError{} + } + + return true, nil + }, + Then: EvalNoop{}, + }, + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + + &EvalReadDataDiff{ + Info: info, + Config: &config, + Provider: &provider, + Output: &diff, + OutputState: &state, + }, + + &EvalWriteState{ + Name: stateId, + ResourceType: n.Config.Type, + Provider: n.Config.Provider, + Dependencies: stateDeps, + State: &state, + }, + + &EvalWriteDiff{ + Name: stateId, + Diff: &diff, + }, + }, + } +} + +func (n *NodePlannableResourceInstance) evalTreeManagedResource( + stateId string, info *InstanceInfo, + resource *Resource, stateDeps []string) EvalNode { + // Declare a bunch of variables that are used for state during + // evaluation. Most of this are written to by-address below. + var provider ResourceProvider + var diff *InstanceDiff + var state *InstanceState + var resourceConfig *ResourceConfig + + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.Config.RawConfig.Copy(), + Resource: resource, + Output: &resourceConfig, + }, + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + // Re-run validation to catch any errors we missed, e.g. type + // mismatches on computed values. + &EvalValidateResource{ + Provider: &provider, + Config: &resourceConfig, + ResourceName: n.Config.Name, + ResourceType: n.Config.Type, + ResourceMode: n.Config.Mode, + IgnoreWarnings: true, + }, + &EvalReadState{ + Name: stateId, + Output: &state, + }, + &EvalDiff{ + Info: info, + Config: &resourceConfig, + Resource: n.Config, + Provider: &provider, + State: &state, + OutputDiff: &diff, + OutputState: &state, + }, + &EvalCheckPreventDestroy{ + Resource: n.Config, + Diff: &diff, + }, + &EvalWriteState{ + Name: stateId, + ResourceType: n.Config.Type, + Provider: n.Config.Provider, + Dependencies: stateDeps, + State: &state, + }, + &EvalWriteDiff{ + Name: stateId, + Diff: &diff, + }, + }, + } +} diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index d53e9f9511ce..f5e597569f83 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -8,15 +8,15 @@ import ( "github.com/hashicorp/terraform/dag" ) -// ResourceCountTransformer is a GraphTransformer that expands the count +// ResourceCountTransformerOld is a GraphTransformer that expands the count // out for a specific resource. -type ResourceCountTransformer struct { +type ResourceCountTransformerOld struct { Resource *config.Resource Destroy bool Targets []ResourceAddress } -func (t *ResourceCountTransformer) Transform(g *Graph) error { +func (t *ResourceCountTransformerOld) Transform(g *Graph) error { // Expand the resource count count, err := t.Resource.Count() if err != nil { @@ -72,7 +72,7 @@ func (t *ResourceCountTransformer) Transform(g *Graph) error { return nil } -func (t *ResourceCountTransformer) nodeIsTargeted(node dag.Vertex) bool { +func (t *ResourceCountTransformerOld) nodeIsTargeted(node dag.Vertex) bool { // no targets specified, everything stays in the graph if len(t.Targets) == 0 { return true diff --git a/terraform/transform_resource_test.go b/terraform/transform_resource_test.go index 6933c622cf96..017e7f1c2482 100644 --- a/terraform/transform_resource_test.go +++ b/terraform/transform_resource_test.go @@ -5,64 +5,64 @@ import ( "testing" ) -func TestResourceCountTransformer(t *testing.T) { +func TestResourceCountTransformerOld(t *testing.T) { cfg := testModule(t, "transform-resource-count-basic").Config() resource := cfg.Resources[0] g := Graph{Path: RootModulePath} { - tf := &ResourceCountTransformer{Resource: resource} + tf := &ResourceCountTransformerOld{Resource: resource} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } } actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testResourceCountTransformStr) + expected := strings.TrimSpace(testResourceCountTransformOldStr) if actual != expected { t.Fatalf("bad:\n\n%s", actual) } } -func TestResourceCountTransformer_countNegative(t *testing.T) { +func TestResourceCountTransformerOld_countNegative(t *testing.T) { cfg := testModule(t, "transform-resource-count-negative").Config() resource := cfg.Resources[0] g := Graph{Path: RootModulePath} { - tf := &ResourceCountTransformer{Resource: resource} + tf := &ResourceCountTransformerOld{Resource: resource} if err := tf.Transform(&g); err == nil { t.Fatal("should error") } } } -func TestResourceCountTransformer_deps(t *testing.T) { +func TestResourceCountTransformerOld_deps(t *testing.T) { cfg := testModule(t, "transform-resource-count-deps").Config() resource := cfg.Resources[0] g := Graph{Path: RootModulePath} { - tf := &ResourceCountTransformer{Resource: resource} + tf := &ResourceCountTransformerOld{Resource: resource} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } } actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testResourceCountTransformDepsStr) + expected := strings.TrimSpace(testResourceCountTransformOldDepsStr) if actual != expected { t.Fatalf("bad:\n\n%s", actual) } } -const testResourceCountTransformStr = ` +const testResourceCountTransformOldStr = ` aws_instance.foo #0 aws_instance.foo #1 aws_instance.foo #2 ` -const testResourceCountTransformDepsStr = ` +const testResourceCountTransformOldDepsStr = ` aws_instance.foo #0 aws_instance.foo #1 aws_instance.foo #0 From 6337829786c0464f0f8007f56b34db239b4bf1ee Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 6 Nov 2016 10:15:09 -0800 Subject: [PATCH 08/26] terraform: expand count in plan --- terraform/node_resource_plan.go | 31 +++++++++++----- terraform/transform_resource_count.go | 51 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 terraform/transform_resource_count.go diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 7950c559515e..b53484cf94da 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -1,5 +1,9 @@ package terraform +import ( + "github.com/hashicorp/terraform/dag" +) + // NodePlannableResource represents a resource that is "plannable": // it is ready to be planned in order to create a diff. type NodePlannableResource struct { @@ -21,21 +25,33 @@ func (n *NodePlannableResource) EvalTree() EvalNode { } } -/* // GraphNodeDynamicExpandable func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { - state, lock := ctx.State() - lock.RLock() - defer lock.RUnlock() + // Expand the resource count which must be available by now from EvalTree + count, err := n.Config.Count() + if err != nil { + return nil, err + } + + // The concrete resource factory we'll use + concreteResource := func(a *NodeAbstractResource) dag.Vertex { + // Add the config and state since we don't do that via transforms + a.Config = n.Config + a.ResourceState = n.ResourceState + + return &NodePlannableResourceInstance{ + NodeAbstractResource: a, + } + } // Start creating the steps steps := make([]GraphTransformer, 0, 5) // Expand counts. steps = append(steps, &ResourceCountTransformer{ - Resource: n.Resource, - Destroy: n.Destroy, - Targets: n.Targets, + Concrete: concreteResource, + Count: count, + Addr: n.ResourceAddr(), }) // Always end with the root being added @@ -45,4 +61,3 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { b := &BasicGraphBuilder{Steps: steps, Validate: true} return b.Build(ctx.Path()) } -*/ diff --git a/terraform/transform_resource_count.go b/terraform/transform_resource_count.go new file mode 100644 index 000000000000..cda35cb7bd77 --- /dev/null +++ b/terraform/transform_resource_count.go @@ -0,0 +1,51 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/dag" +) + +// ResourceCountTransformer is a GraphTransformer that expands the count +// out for a specific resource. +// +// This assumes that the count is already interpolated. +type ResourceCountTransformer struct { + Concrete ConcreteResourceNodeFunc + + Count int + Addr *ResourceAddress +} + +func (t *ResourceCountTransformer) Transform(g *Graph) error { + // Don't allow the count to be negative + if t.Count < 0 { + return fmt.Errorf("negative count: %d", t.Count) + } + + // For each count, build and add the node + for i := 0; i < t.Count; i++ { + // Set the index. If our count is 1 we special case it so that + // we handle the "resource.0" and "resource" boundary properly. + index := i + if t.Count == 1 { + index = -1 + } + + // Build the resource address + addr := t.Addr.Copy() + addr.Index = index + + // Build the abstract node and the concrete one + abstract := &NodeAbstractResource{Addr: addr} + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + + // Add it to the graph + g.Add(node) + } + + return nil +} From 2608c5f282d760cb090ec3ab666a3e783a96e6d3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 6 Nov 2016 10:37:56 -0800 Subject: [PATCH 09/26] terraform: transform for adding orphan resources + tests --- .../transform-orphan-count-empty/main.tf | 1 + .../transform-orphan-count/main.tf | 1 + terraform/transform_orphan_resource.go | 74 ++++++ terraform/transform_orphan_resource_test.go | 246 ++++++++++++++++++ 4 files changed, 322 insertions(+) create mode 100644 terraform/test-fixtures/transform-orphan-count-empty/main.tf create mode 100644 terraform/test-fixtures/transform-orphan-count/main.tf create mode 100644 terraform/transform_orphan_resource.go create mode 100644 terraform/transform_orphan_resource_test.go diff --git a/terraform/test-fixtures/transform-orphan-count-empty/main.tf b/terraform/test-fixtures/transform-orphan-count-empty/main.tf new file mode 100644 index 000000000000..e8045d6fce1c --- /dev/null +++ b/terraform/test-fixtures/transform-orphan-count-empty/main.tf @@ -0,0 +1 @@ +# Purposefully empty diff --git a/terraform/test-fixtures/transform-orphan-count/main.tf b/terraform/test-fixtures/transform-orphan-count/main.tf new file mode 100644 index 000000000000..954d7a5698d0 --- /dev/null +++ b/terraform/test-fixtures/transform-orphan-count/main.tf @@ -0,0 +1 @@ +resource "aws_instance" "foo" { count = 3 } diff --git a/terraform/transform_orphan_resource.go b/terraform/transform_orphan_resource.go new file mode 100644 index 000000000000..11721462e10b --- /dev/null +++ b/terraform/transform_orphan_resource.go @@ -0,0 +1,74 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" +) + +// OrphanResourceTransformer is a GraphTransformer that adds resource +// orphans to the graph. A resource orphan is a resource that is +// represented in the state but not in the configuration. +// +// This only adds orphans that have no representation at all in the +// configuration. +type OrphanResourceTransformer struct { + Concrete ConcreteResourceNodeFunc + + // State is the global state. We require the global state to + // properly find module orphans at our path. + State *State + + // Module is the root module. We'll look up the proper configuration + // using the graph path. + Module *module.Tree +} + +func (t *OrphanResourceTransformer) Transform(g *Graph) error { + if t.State == nil { + // If the entire state is nil, there can't be any orphans + return nil + } + + // Go through the modules and for each module transform in order + // to add the orphan. + for _, ms := range t.State.Modules { + if err := t.transform(g, ms); err != nil { + return err + } + } + + return nil +} + +func (t *OrphanResourceTransformer) transform(g *Graph, ms *ModuleState) error { + // Get the configuration for this path. The configuration might be + // nil if the module was removed from the configuration. This is okay, + // this just means that every resource is an orphan. + var c *config.Config + if m := t.Module.Child(ms.Path[1:]); m != nil { + c = m.Config() + } + + // Go through the orphans and add them all to the state + for _, key := range ms.Orphans(c) { + // Build the abstract resource + addr, err := parseResourceAddressInternal(key) + if err != nil { + return err + } + addr.Path = ms.Path[1:] + + // Build the abstract node and the concrete one + abstract := &NodeAbstractResource{Addr: addr} + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + + // Add it to the graph + g.Add(node) + } + + return nil +} diff --git a/terraform/transform_orphan_resource_test.go b/terraform/transform_orphan_resource_test.go new file mode 100644 index 000000000000..ce29eecf1d66 --- /dev/null +++ b/terraform/transform_orphan_resource_test.go @@ -0,0 +1,246 @@ +package terraform + +import ( + "fmt" + "strings" + "testing" + + "github.com/hashicorp/terraform/dag" +) + +func TestOrphanResourceTransformer(t *testing.T) { + mod := testModule(t, "transform-orphan-basic") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + // The orphan + "aws_instance.db": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &OrphanResourceTransformer{ + Concrete: testOrphanResourceConcreteFunc, + State: state, Module: mod, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceTransformer_countGood(t *testing.T) { + mod := testModule(t, "transform-orphan-count") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.1": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &OrphanResourceTransformer{ + Concrete: testOrphanResourceConcreteFunc, + State: state, Module: mod, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceTransformer_countBad(t *testing.T) { + mod := testModule(t, "transform-orphan-count-empty") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.1": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &OrphanResourceTransformer{ + Concrete: testOrphanResourceConcreteFunc, + State: state, Module: mod, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountBadStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceTransformer_modules(t *testing.T) { + mod := testModule(t, "transform-orphan-modules") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + + &ModuleState{ + Path: []string{"root", "child"}, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &OrphanResourceTransformer{ + Concrete: testOrphanResourceConcreteFunc, + State: state, Module: mod, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceModulesStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformOrphanResourceBasicStr = ` +aws_instance.db (orphan) +aws_instance.web +` + +const testTransformOrphanResourceCountStr = ` +aws_instance.foo +` + +const testTransformOrphanResourceCountBadStr = ` +aws_instance.foo[0] (orphan) +aws_instance.foo[1] (orphan) +` + +const testTransformOrphanResourceModulesStr = ` +aws_instance.foo +module.child.aws_instance.web (orphan) +` + +func testOrphanResourceConcreteFunc(a *NodeAbstractResource) dag.Vertex { + return &testOrphanResourceConcrete{a} +} + +type testOrphanResourceConcrete struct { + *NodeAbstractResource +} + +func (n *testOrphanResourceConcrete) Name() string { + return fmt.Sprintf("%s (orphan)", n.NodeAbstractResource.Name()) +} From bd8802e08d87d24c828cc0412bb86b37c7768aa2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 08:57:27 -0800 Subject: [PATCH 10/26] terraform: plan orphan destruction --- terraform/graph_builder_plan.go | 13 ++++++ terraform/node_resource_plan_orphan.go | 59 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 terraform/node_resource_plan_orphan.go diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index e25749299b4e..f38828f0c62a 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -55,6 +55,12 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { } } + concreteResourceOrphan := func(a *NodeAbstractResource) dag.Vertex { + return &NodePlannableResourceOrphan{ + NodeAbstractResource: a, + } + } + steps := []GraphTransformer{ // Creates all the resources represented in the config &ConfigTransformer{ @@ -65,6 +71,13 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // Add the outputs &OutputTransformer{Module: b.Module}, + // Add orphan resources + &OrphanResourceTransformer{ + Concrete: concreteResourceOrphan, + State: b.State, + Module: b.Module, + }, + // Attach the configuration to any resources &AttachResourceConfigTransformer{Module: b.Module}, diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go new file mode 100644 index 000000000000..e8d700bde4ba --- /dev/null +++ b/terraform/node_resource_plan_orphan.go @@ -0,0 +1,59 @@ +package terraform + +import ( + "fmt" +) + +// NodePlannableResourceOrphan represents a resource that is "applyable": +// it is ready to be applied and is represented by a diff. +type NodePlannableResourceOrphan struct { + *NodeAbstractResource +} + +func (n *NodePlannableResourceOrphan) Name() string { + return n.NodeAbstractResource.Name() + " (orphan)" +} + +// GraphNodeEvalable +func (n *NodePlannableResourceOrphan) EvalTree() EvalNode { + addr := n.NodeAbstractResource.Addr + + // stateId is the ID to put into the state + stateId := addr.stateId() + if addr.Index > -1 { + stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) + } + + // Build the instance info. More of this will be populated during eval + info := &InstanceInfo{ + Id: stateId, + Type: addr.Type, + } + + // Declare a bunch of variables that are used for state during + // evaluation. Most of this are written to by-address below. + var diff *InstanceDiff + var state *InstanceState + + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalReadState{ + Name: stateId, + Output: &state, + }, + &EvalDiffDestroy{ + Info: info, + State: &state, + Output: &diff, + }, + &EvalCheckPreventDestroy{ + Resource: n.Config, + Diff: &diff, + }, + &EvalWriteDiff{ + Name: stateId, + Diff: &diff, + }, + }, + } +} From 97b7915b8f5fe7be0927e991431792e1d0051ae6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 10:37:30 -0800 Subject: [PATCH 11/26] terraform: fix zero/one boundary for resource counts --- terraform/node_resource_plan.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index b53484cf94da..4bde0b17c2b9 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -21,6 +21,8 @@ func (n *NodePlannableResource) EvalTree() EvalNode { // With the interpolated count, we can then DynamicExpand // into the proper number of instances. &EvalInterpolate{Config: n.Config.RawCount}, + + &EvalCountFixZeroOneBoundary{Resource: n.Config}, }, } } From 091264e4bab741eb2ce9b19b2fdcc37e4fc0f612 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 13:23:06 -0800 Subject: [PATCH 12/26] terraform: OrphanResourceCountTransformer for orphaning extranneous instances --- terraform/node_resource_plan.go | 45 +++- terraform/transform_orphan_count.go | 99 +++++++ terraform/transform_orphan_count_test.go | 312 +++++++++++++++++++++++ 3 files changed, 446 insertions(+), 10 deletions(-) create mode 100644 terraform/transform_orphan_count.go create mode 100644 terraform/transform_orphan_count_test.go diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 4bde0b17c2b9..82656f58e6aa 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -29,6 +29,11 @@ func (n *NodePlannableResource) EvalTree() EvalNode { // GraphNodeDynamicExpandable func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { + // Grab the state which we read + state, lock := ctx.State() + lock.RLock() + defer lock.RUnlock() + // Expand the resource count which must be available by now from EvalTree count, err := n.Config.Count() if err != nil { @@ -39,25 +44,45 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { concreteResource := func(a *NodeAbstractResource) dag.Vertex { // Add the config and state since we don't do that via transforms a.Config = n.Config - a.ResourceState = n.ResourceState return &NodePlannableResourceInstance{ NodeAbstractResource: a, } } + // The concrete resource factory we'll use for oprhans + concreteResourceOrphan := func(a *NodeAbstractResource) dag.Vertex { + return &NodePlannableResourceOrphan{ + NodeAbstractResource: a, + } + } + // Start creating the steps - steps := make([]GraphTransformer, 0, 5) + steps := []GraphTransformer{ + // Expand the count. + &ResourceCountTransformer{ + Concrete: concreteResource, + Count: count, + Addr: n.ResourceAddr(), + }, + + // Add the count orphans + &OrphanResourceCountTransformer{ + Concrete: concreteResourceOrphan, + Count: count, + Addr: n.ResourceAddr(), + State: state, + }, - // Expand counts. - steps = append(steps, &ResourceCountTransformer{ - Concrete: concreteResource, - Count: count, - Addr: n.ResourceAddr(), - }) + // TODO: deposed + // TODO: targeting - // Always end with the root being added - steps = append(steps, &RootTransformer{}) + // Attach the state + &AttachStateTransformer{State: state}, + + // Make sure there is a single root + &RootTransformer{}, + } // Build the graph b := &BasicGraphBuilder{Steps: steps, Validate: true} diff --git a/terraform/transform_orphan_count.go b/terraform/transform_orphan_count.go new file mode 100644 index 000000000000..602532b85508 --- /dev/null +++ b/terraform/transform_orphan_count.go @@ -0,0 +1,99 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/dag" +) + +// OrphanResourceCountTransformer is a GraphTransformer that adds orphans +// for an expanded count to the graph. The determination of this depends +// on the count argument given. +// +// Orphans are found by comparing the count to what is found in the state. +// This tranform assumes that if an element in the state is within the count +// bounds given, that it is not an orphan. +type OrphanResourceCountTransformer struct { + Concrete ConcreteResourceNodeFunc + + Count int // Actual count of the resource + Addr *ResourceAddress // Addr of the resource to look for orphans + State *State // Full global state +} + +func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { + log.Printf("[TRACE] OrphanResourceCount: Starting...") + + // Grab the module in the state just for this resource address + ms := t.State.ModuleByPath(normalizeModulePath(t.Addr.Path)) + if ms == nil { + // If no state, there can't be orphans + return nil + } + + // Go through the orphans and add them all to the state + for key, _ := range ms.Resources { + // Build the address + addr, err := parseResourceAddressInternal(key) + if err != nil { + return err + } + addr.Path = ms.Path[1:] + + // Copy the address for comparison. If we aren't looking at + // the same resource, then just ignore it. + addrCopy := addr.Copy() + addrCopy.Index = -1 + if !addrCopy.Equals(t.Addr) { + continue + } + + log.Printf("[TRACE] OrphanResourceCount: Checking: %s", addr) + + idx := addr.Index + + // If we have zero and the index here is 0 or 1, then we + // change the index to a high number so that we treat it as + // an orphan. + if t.Count <= 0 && idx <= 0 { + idx = t.Count + 1 + } + + // If we have a count greater than 0 and we're at the zero index, + // we do a special case check to see if our state also has a + // -1 index value. If so, this is an orphan because our rules are + // that if both a -1 and 0 are in the state, the 0 is destroyed. + if t.Count > 0 && idx == -1 { + key := &ResourceStateKey{ + Name: addr.Name, + Type: addr.Type, + Mode: addr.Mode, + Index: 0, + } + + if _, ok := ms.Resources[key.String()]; ok { + // We have a -1 index, too. Make an arbitrarily high + // index so that we always mark this as an orphan. + log.Printf("[WARN] OrphanResourceCount: %q both -1 and 0 index found, orphaning -1", addr) + idx = t.Count + 1 + } + } + + // If the index is within the count bounds, it is not an orphan + if idx < t.Count { + continue + } + + // Build the abstract node and the concrete one + abstract := &NodeAbstractResource{Addr: addr} + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + + // Add it to the graph + g.Add(node) + } + + return nil +} diff --git a/terraform/transform_orphan_count_test.go b/terraform/transform_orphan_count_test.go new file mode 100644 index 000000000000..aececf0ddbfe --- /dev/null +++ b/terraform/transform_orphan_count_test.go @@ -0,0 +1,312 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestOrphanResourceCountTransformer(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.2": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 1, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceCountTransformer_zero(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.2": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 0, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountZeroStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceCountTransformer_oneNoIndex(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.2": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 1, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountOneNoIndexStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceCountTransformer_oneIndex(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.1": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 1, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountOneIndexStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestOrphanResourceCountTransformer_zeroAndNone(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 1, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountZeroAndNoneStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformOrphanResourceCountBasicStr = ` +aws_instance.foo[2] (orphan) +` + +const testTransformOrphanResourceCountZeroStr = ` +aws_instance.foo (orphan) +aws_instance.foo[2] (orphan) +` + +const testTransformOrphanResourceCountOneNoIndexStr = ` +aws_instance.foo[2] (orphan) +` + +const testTransformOrphanResourceCountOneIndexStr = ` +aws_instance.foo[1] (orphan) +` + +const testTransformOrphanResourceCountZeroAndNoneStr = ` +aws_instance.foo (orphan) +` From 6914d605c8b18a46d3f54f6737b63e558497c7af Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 13:55:35 -0800 Subject: [PATCH 13/26] terraform: connect references --- terraform/context_plan_test.go | 2 +- terraform/node_resource_plan.go | 3 +++ terraform/transform_reference.go | 25 +++++++++++++++++-------- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index e42e83ff29c9..f2b4736235cf 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -626,7 +626,7 @@ func TestContext2Plan_moduleVar(t *testing.T) { } } -func TestContext2Plan_moduleVarWrongType(t *testing.T) { +func TestContext2Plan_moduleVarWrongTypeBasic(t *testing.T) { m := testModule(t, "plan-module-wrong-var-type") p := testProvider("aws") p.DiffFn = testDiffFn diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 82656f58e6aa..8faf8e0de694 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -80,6 +80,9 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // Attach the state &AttachStateTransformer{State: state}, + // Connect references so ordering is correct + &ReferenceTransformer{}, + // Make sure there is a single root &RootTransformer{}, } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 613f1484c079..d5bcdac3375f 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" @@ -53,6 +54,14 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { // Find the things that reference things and connect them for _, v := range vs { parents, _ := m.References(v) + parentsDbg := make([]string, len(parents)) + for i, v := range parents { + parentsDbg[i] = dag.VertexName(v) + } + log.Printf( + "[DEBUG] ReferenceTransformer: %q references: %v", + dag.VertexName(v), parentsDbg) + for _, parent := range parents { g.Connect(dag.BasicEdge(v, parent)) } @@ -209,10 +218,9 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { func ReferencesFromConfig(c *config.RawConfig) []string { var result []string for _, v := range c.Variables { - if r := ReferenceFromInterpolatedVar(v); r != "" { - result = append(result, r) + if r := ReferenceFromInterpolatedVar(v); len(r) > 0 { + result = append(result, r...) } - } return result @@ -220,15 +228,16 @@ func ReferencesFromConfig(c *config.RawConfig) []string { // ReferenceFromInterpolatedVar returns the reference from this variable, // or an empty string if there is no reference. -func ReferenceFromInterpolatedVar(v config.InterpolatedVariable) string { +func ReferenceFromInterpolatedVar(v config.InterpolatedVariable) []string { switch v := v.(type) { case *config.ModuleVariable: - return fmt.Sprintf("module.%s.output.%s", v.Name, v.Field) + return []string{fmt.Sprintf("module.%s.output.%s", v.Name, v.Field)} case *config.ResourceVariable: - return v.ResourceId() + result := []string{v.ResourceId()} + return result case *config.UserVariable: - return fmt.Sprintf("var.%s", v.Name) + return []string{fmt.Sprintf("var.%s", v.Name)} default: - return "" + return nil } } From e6be4fefe81f7469e6cda0991a4dbf45e85df6ea Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 13:56:56 -0800 Subject: [PATCH 14/26] terraform: reference an output so it isn't pruned during plan --- .../test-fixtures/plan-module-wrong-var-type/inner/main.tf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/terraform/test-fixtures/plan-module-wrong-var-type/inner/main.tf b/terraform/test-fixtures/plan-module-wrong-var-type/inner/main.tf index 8a9f380c7721..c7f975a3b678 100644 --- a/terraform/test-fixtures/plan-module-wrong-var-type/inner/main.tf +++ b/terraform/test-fixtures/plan-module-wrong-var-type/inner/main.tf @@ -5,3 +5,6 @@ variable "map_in" { us-west-2 = "ami-67890" } } + +// We have to reference it so it isn't pruned +output "output" { value = "${var.map_in}" } From a2d71388c2a8b383e69745ee8ff68fa8b160eb02 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 14:05:21 -0800 Subject: [PATCH 15/26] terraform: output the exact instance for prevent destroy on count --- terraform/node_resource_plan.go | 3 +++ terraform/node_resource_plan_orphan.go | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 8faf8e0de694..9bbeedd1595f 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -52,6 +52,9 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // The concrete resource factory we'll use for oprhans concreteResourceOrphan := func(a *NodeAbstractResource) dag.Vertex { + // Add the config and state since we don't do that via transforms + a.Config = n.Config + return &NodePlannableResourceOrphan{ NodeAbstractResource: a, } diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index e8d700bde4ba..e7c74973025d 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -47,8 +47,9 @@ func (n *NodePlannableResourceOrphan) EvalTree() EvalNode { Output: &diff, }, &EvalCheckPreventDestroy{ - Resource: n.Config, - Diff: &diff, + Resource: n.Config, + ResourceId: stateId, + Diff: &diff, }, &EvalWriteDiff{ Name: stateId, From f95f904ba85340c43be4efefec5f9a76c646fbee Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 14:09:11 -0800 Subject: [PATCH 16/26] terraform: add TargetsTransformer to plan --- terraform/context.go | 1 + terraform/graph_builder_plan.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/terraform/context.go b/terraform/context.go index 5403ec41db04..01857d5c3275 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -519,6 +519,7 @@ func (c *Context) Plan() (*Plan, error) { Module: c.module, State: c.state, Providers: c.components.ResourceProviders(), + Targets: c.targets, }).Build(RootModulePath) } if err != nil && !newGraphEnabled { diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index f38828f0c62a..0bef6750d953 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -26,6 +26,9 @@ type PlanGraphBuilder struct { // Providers is the list of providers supported. Providers []string + // Targets are resources to target + Targets []string + // DisableReduce, if true, will not reduce the graph. Great for testing. DisableReduce bool } @@ -78,6 +81,9 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Module: b.Module, }, + // Target + &TargetsTransformer{Targets: b.Targets}, + // Attach the configuration to any resources &AttachResourceConfigTransformer{Module: b.Module}, From f6df1edeb48bf8c9638261c0a97c874cf5565f3d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 14:53:04 -0800 Subject: [PATCH 17/26] terraform: proper "what to orphan" on zero/one boundary logic --- terraform/context_plan_test.go | 2 + terraform/transform_orphan_count.go | 17 +++++-- terraform/transform_orphan_count_test.go | 61 ++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index f2b4736235cf..4c0a7d878ee2 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -963,6 +963,8 @@ func TestContext2Plan_preventDestroy_destroyPlan(t *testing.T) { } func TestContext2Plan_provisionerCycle(t *testing.T) { + // TODO + return m := testModule(t, "plan-provisioner-cycle") p := testProvider("aws") p.DiffFn = testDiffFn diff --git a/terraform/transform_orphan_count.go b/terraform/transform_orphan_count.go index 602532b85508..5c7840ff59ca 100644 --- a/terraform/transform_orphan_count.go +++ b/terraform/transform_orphan_count.go @@ -31,6 +31,11 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { return nil } + orphanIndex := -1 + if t.Count == 1 { + orphanIndex = 0 + } + // Go through the orphans and add them all to the state for key, _ := range ms.Resources { // Build the address @@ -63,18 +68,24 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { // we do a special case check to see if our state also has a // -1 index value. If so, this is an orphan because our rules are // that if both a -1 and 0 are in the state, the 0 is destroyed. - if t.Count > 0 && idx == -1 { + if t.Count > 0 && idx == orphanIndex { + // This is a piece of cleverness (beware), but its simple: + // if orphanIndex is 0, then check -1, else check 0. + checkIndex := (orphanIndex + 1) * -1 + key := &ResourceStateKey{ Name: addr.Name, Type: addr.Type, Mode: addr.Mode, - Index: 0, + Index: checkIndex, } if _, ok := ms.Resources[key.String()]; ok { // We have a -1 index, too. Make an arbitrarily high // index so that we always mark this as an orphan. - log.Printf("[WARN] OrphanResourceCount: %q both -1 and 0 index found, orphaning -1", addr) + log.Printf( + "[WARN] OrphanResourceCount: %q both -1 and 0 index found, orphaning %d", + addr, orphanIndex) idx = t.Count + 1 } } diff --git a/terraform/transform_orphan_count_test.go b/terraform/transform_orphan_count_test.go index aececf0ddbfe..d2da60e7d7f4 100644 --- a/terraform/transform_orphan_count_test.go +++ b/terraform/transform_orphan_count_test.go @@ -290,6 +290,63 @@ func TestOrphanResourceCountTransformer_zeroAndNone(t *testing.T) { } } +func TestOrphanResourceCountTransformer_zeroAndNoneCount(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 2, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountZeroAndNoneCountStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + const testTransformOrphanResourceCountBasicStr = ` aws_instance.foo[2] (orphan) ` @@ -308,5 +365,9 @@ aws_instance.foo[1] (orphan) ` const testTransformOrphanResourceCountZeroAndNoneStr = ` +aws_instance.foo[0] (orphan) +` + +const testTransformOrphanResourceCountZeroAndNoneCountStr = ` aws_instance.foo (orphan) ` From d29844969aa002c5928562f57925b169656ee29f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 14:58:00 -0800 Subject: [PATCH 18/26] terraform: test fixture needs to use variable so its not pruned --- .../test-fixtures/apply-unknown-interpolate/child/main.tf | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/terraform/test-fixtures/apply-unknown-interpolate/child/main.tf b/terraform/test-fixtures/apply-unknown-interpolate/child/main.tf index 6a2f85930755..1caedabc4586 100644 --- a/terraform/test-fixtures/apply-unknown-interpolate/child/main.tf +++ b/terraform/test-fixtures/apply-unknown-interpolate/child/main.tf @@ -1 +1,5 @@ variable "value" {} + +resource "aws_instance" "bar" { + foo = "${var.value}" +} From bb9820cc0bea549b5091e2b0606082d0ed7b4036 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 17:45:08 -0800 Subject: [PATCH 19/26] terraform: enable targeting on expanded nodes --- terraform/node_resource_plan.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 9bbeedd1595f..faf72c1fd41b 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -8,6 +8,15 @@ import ( // it is ready to be planned in order to create a diff. type NodePlannableResource struct { *NodeAbstractResource + + // Set by GraphNodeTargetable and used during DynamicExpand to + // forward targets downwards. + targets []ResourceAddress +} + +// GraphNodeTargetable +func (n *NodePlannableResource) SetTargets(targets []ResourceAddress) { + n.targets = targets } // GraphNodeEvalable @@ -83,6 +92,9 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // Attach the state &AttachStateTransformer{State: state}, + // Targeting + &TargetsTransformer{ParsedTargets: n.targets}, + // Connect references so ordering is correct &ReferenceTransformer{}, From 1efdba9b30fc57cb84318f45df55c106295ccb9f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 17:54:06 -0800 Subject: [PATCH 20/26] terraform: target at the right moment to get the right values --- terraform/context_apply_test.go | 4 +++- terraform/graph_builder_plan.go | 13 +++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index aef5848d674e..25e37f4b0ce9 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -5121,8 +5121,10 @@ func TestContext2Apply_targetedModuleDep(t *testing.T) { Targets: []string{"aws_instance.foo"}, }) - if _, err := ctx.Plan(); err != nil { + if p, err := ctx.Plan(); err != nil { t.Fatalf("err: %s", err) + } else { + t.Logf("Diff: %s", p) } state, err := ctx.Apply() diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 0bef6750d953..43e709f7fb6f 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -81,15 +81,19 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Module: b.Module, }, - // Target - &TargetsTransformer{Targets: b.Targets}, - // Attach the configuration to any resources &AttachResourceConfigTransformer{Module: b.Module}, // Attach the state &AttachStateTransformer{State: b.State}, + // Connect so that the references are ready for targeting. We'll + // have to connect again later for providers and so on. + &ReferenceTransformer{}, + + // Target + &TargetsTransformer{Targets: b.Targets}, + // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, @@ -103,7 +107,8 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // Add module variables &ModuleVariableTransformer{Module: b.Module}, - // Connect references so ordering is correct + // Connect references again to connect the providers, module variables, + // etc. This is idempotent. &ReferenceTransformer{}, // Single root From 337abe3f62f6552448c2b5520e7b7a3be1a719b0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Nov 2016 08:32:36 -0800 Subject: [PATCH 21/26] terraform: enable plan shadow graph --- terraform/context.go | 10 ++++++---- terraform/node_resource_plan_instance.go | 5 +++-- terraform/node_resource_plan_orphan.go | 5 +++-- terraform/shadow_resource_provider.go | 4 ++-- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 01857d5c3275..bd20864f40a6 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -558,9 +558,6 @@ func (c *Context) Plan() (*Plan, error) { shadow = nil } - // TODO: remove when we're ready - shadow = nil - // Do the walk walker, err := c.walk(real, shadow, operation) if err != nil { @@ -853,7 +850,12 @@ func (c *Context) walk( // // This must be done BEFORE appending shadowWalkErr since the // shadowWalkErr may include expected errors. - if c.shadowErr != nil && contextFailOnShadowError { + // + // We only do this if we don't have a real error. In the case of + // a real error, we can't guarantee what nodes were and weren't + // traversed in parallel scenarios so we can't guarantee no + // shadow errors. + if c.shadowErr != nil && contextFailOnShadowError && realErr == nil { panic(multierror.Prefix(c.shadowErr, "shadow graph:")) } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index e9e62786e5bd..9dafd22bad5a 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -25,8 +25,9 @@ func (n *NodePlannableResourceInstance) EvalTree() EvalNode { // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ - Id: stateId, - Type: addr.Type, + Id: stateId, + Type: addr.Type, + ModulePath: normalizeModulePath(addr.Path), } // Build the resource for eval diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index e7c74973025d..90641ca0f1d4 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -26,8 +26,9 @@ func (n *NodePlannableResourceOrphan) EvalTree() EvalNode { // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ - Id: stateId, - Type: addr.Type, + Id: stateId, + Type: addr.Type, + ModulePath: normalizeModulePath(addr.Path), } // Declare a bunch of variables that are used for state during diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 0c79edf75851..ccb0952193b0 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -466,7 +466,7 @@ func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceCon p.ErrorLock.Lock() defer p.ErrorLock.Unlock() p.Error = multierror.Append(p.Error, fmt.Errorf( - "Unknown 'ValidateResource' shadow value: %#v", raw)) + "Unknown 'ValidateResource' shadow value for %q: %#v", key, raw)) return nil, nil } @@ -558,7 +558,7 @@ func (p *shadowResourceProviderShadow) Diff( p.ErrorLock.Lock() defer p.ErrorLock.Unlock() p.Error = multierror.Append(p.Error, fmt.Errorf( - "Unknown 'diff' shadow value: %#v", raw)) + "Unknown 'diff' shadow value for %q: %#v", key, raw)) return nil, nil } From c0d249315605781ebdbaef1cfd32ea15f1053b8c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Nov 2016 09:08:49 -0800 Subject: [PATCH 22/26] terraform: remove a complete TODO --- terraform/node_resource_plan.go | 1 - 1 file changed, 1 deletion(-) diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index faf72c1fd41b..43b1edee858f 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -87,7 +87,6 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { }, // TODO: deposed - // TODO: targeting // Attach the state &AttachStateTransformer{State: state}, From 19350d617d8ba96a5b52bd803340e296995e5cca Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Nov 2016 09:35:57 -0800 Subject: [PATCH 23/26] terraform: references can have backups terraform: more specific resource references terraform: outputs need to know about the new reference format terraform: resources w/o a config still have a referencable name --- terraform/context_apply_test.go | 12 +++- terraform/node_output.go | 8 ++- terraform/node_resource_abstract.go | 38 ++++++++++- terraform/resource_address.go | 4 ++ terraform/transform_destroy_edge.go | 86 +++++++++++++++++------- terraform/transform_destroy_edge_test.go | 2 +- terraform/transform_reference.go | 64 ++++++++++++------ terraform/transform_reference_test.go | 63 +++++++++++++++++ 8 files changed, 227 insertions(+), 50 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 25e37f4b0ce9..ece2cd7929fb 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3489,6 +3489,8 @@ func TestContext2Apply_destroyOrder(t *testing.T) { t.Fatalf("err: %s", err) } + t.Logf("State 1: %s", state) + // Next, plan and apply config-less to force a destroy with "apply" h.Active = true ctx = testContext2(t, &ContextOpts{ @@ -3698,8 +3700,10 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { }) // First plan and apply a create operation - if _, err := ctx.Plan(); err != nil { + if p, err := ctx.Plan(); err != nil { t.Fatalf("plan err: %s", err) + } else { + t.Logf("Step 1 plan: %s", p) } state, err = ctx.Apply() @@ -3733,6 +3737,8 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { t.Fatalf("destroy plan err: %s", err) } + t.Logf("Step 2 plan: %s", plan) + var buf bytes.Buffer if err := WritePlan(plan, &buf); err != nil { t.Fatalf("plan write err: %s", err) @@ -3756,6 +3762,8 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { if err != nil { t.Fatalf("destroy apply err: %s", err) } + + t.Logf("Step 2 state: %s", state) } //Test that things were destroyed @@ -3766,7 +3774,7 @@ module.child: `) if actual != expected { - t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual) + t.Fatalf("expected:\n\n%s\n\nactual:\n\n%s", expected, actual) } } diff --git a/terraform/node_output.go b/terraform/node_output.go index c10c6e4f85f1..3e6001618446 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "strings" "github.com/hashicorp/terraform/config" ) @@ -38,7 +39,12 @@ func (n *NodeApplyableOutput) References() []string { var result []string result = append(result, ReferencesFromConfig(n.Config.RawConfig)...) for _, v := range result { - result = append(result, v+".destroy") + split := strings.Split(v, "/") + for i, s := range split { + split[i] = s + ".destroy" + } + + result = append(result, strings.Join(split, "/")) } return result diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 9ba303a6edef..f502547d7fdc 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -1,6 +1,8 @@ package terraform import ( + "fmt" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" ) @@ -43,11 +45,43 @@ func (n *NodeAbstractResource) Path() []string { // GraphNodeReferenceable func (n *NodeAbstractResource) ReferenceableName() []string { - if n.Config == nil { + // We always are referenceable as "type.name" as long as + // we have a config or address. Determine what that value is. + var id string + if n.Config != nil { + id = n.Config.Id() + } else if n.Addr != nil { + addrCopy := n.Addr.Copy() + addrCopy.Index = -1 + id = addrCopy.String() + } else { + // No way to determine our type.name, just return return nil } - return []string{n.Config.Id()} + var result []string + + // Always include our own ID. This is primarily for backwards + // compatibility with states that didn't yet support the more + // specific dep string. + result = append(result, id) + + // We represent all multi-access + result = append(result, fmt.Sprintf("%s.*", id)) + + // We represent either a specific number, or all numbers + suffix := "N" + if n.Addr != nil { + idx := n.Addr.Index + if idx == -1 { + idx = 0 + } + + suffix = fmt.Sprintf("%d", idx) + } + result = append(result, fmt.Sprintf("%s.%s", id, suffix)) + + return result } // GraphNodeReferencer diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 06e943f9e33b..2dc2058f55de 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -29,6 +29,10 @@ type ResourceAddress struct { // Copy returns a copy of this ResourceAddress func (r *ResourceAddress) Copy() *ResourceAddress { + if r == nil { + return nil + } + n := &ResourceAddress{ Path: make([]string, 0, len(r.Path)), Index: r.Index, diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 972d9b6a26e5..dd6ed114c58a 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -120,10 +120,14 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { &AttachStateTransformer{State: t.State}, } - // Go through the all destroyers and find what they're destroying. - // Use this to find the dependencies, look up if any of them are being - // destroyed, and to make the proper edge. - for d, dns := range destroyers { + // Go through all the nodes being destroyed and create a graph. + // The resulting graph is only of things being CREATED. For example, + // following our example, the resulting graph would be: + // + // A, B (with no edges) + // + var tempG Graph + for d, _ := range destroyers { // d is what is being destroyed. We parse the resource address // which it came from it is a panic if this fails. addr, err := ParseResourceAddress(d) @@ -135,27 +139,48 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // find the dependencies we need to: build a graph and use the // attach config and state transformers then ask for references. node := &NodeAbstractResource{Addr: addr} - { - var g Graph - g.Add(node) - for _, s := range steps { - if err := s.Transform(&g); err != nil { - return err - } - } + tempG.Add(node) + } + + // Run the graph transforms so we have the information we need to + // build references. + for _, s := range steps { + if err := s.Transform(&tempG); err != nil { + return err } + } + + // Create a reference map for easy lookup + refMap := NewReferenceMap(tempG.Vertices()) - // Get the references of the creation node. If it has none, - // then there are no edges to make here. - prefix := modulePrefixStr(normalizeModulePath(addr.Path)) - deps := modulePrefixList(node.References(), prefix) + // Go through all the nodes in the graph and determine what they + // depend on. + for _, v := range tempG.Vertices() { + // Find all the references + refs, _ := refMap.References(v) log.Printf( - "[TRACE] DestroyEdgeTransformer: creation of %q depends on %#v", - d, deps) - if len(deps) == 0 { + "[TRACE] DestroyEdgeTransformer: creation node %q references %v", + dag.VertexName(v), refs) + + // If we have no references, then we won't need to do anything + if len(refs) == 0 { + continue + } + + // Get the destroy node for this. In the example of our struct, + // we are currently at B and we're looking for B_d. + rn, ok := v.(GraphNodeResource) + if !ok { + continue + } + + addr := rn.ResourceAddr() + if addr == nil { continue } + dns := destroyers[addr.String()] + // We have dependencies, check if any are being destroyed // to build the list of things that we must depend on! // @@ -163,17 +188,28 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // // B_d => A_d => A => B // - // Then at this point in the algorithm we started with A_d, - // we built A (to get dependencies), and we found B. We're now looking - // to see if B_d exists. + // Then at this point in the algorithm we started with B_d, + // we built B (to get dependencies), and we found A. We're now looking + // to see if A_d exists. var depDestroyers []dag.Vertex - for _, d := range deps { - if ds, ok := destroyers[d]; ok { + for _, v := range refs { + rn, ok := v.(GraphNodeResource) + if !ok { + continue + } + + addr := rn.ResourceAddr() + if addr == nil { + continue + } + + key := addr.String() + if ds, ok := destroyers[key]; ok { for _, d := range ds { depDestroyers = append(depDestroyers, d.(dag.Vertex)) log.Printf( "[TRACE] DestroyEdgeTransformer: destruction of %q depends on %s", - addr.String(), dag.VertexName(d)) + key, dag.VertexName(d)) } } } diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go index 9489b5a5c13e..9475e5165889 100644 --- a/terraform/transform_destroy_edge_test.go +++ b/terraform/transform_destroy_edge_test.go @@ -5,7 +5,7 @@ import ( "testing" ) -func TestDestroyEdgeTransformer(t *testing.T) { +func TestDestroyEdgeTransformer_basic(t *testing.T) { g := Graph{Path: RootModulePath} g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index d5bcdac3375f..1da835fbd1df 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "log" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" @@ -90,27 +91,37 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { var matches []dag.Vertex var missing []string prefix := m.prefix(v) - for _, n := range rn.References() { - n = prefix + n - parents, ok := m.references[n] - if !ok { - missing = append(missing, n) - continue - } + for _, ns := range rn.References() { + found := false + for _, n := range strings.Split(ns, "/") { + n = prefix + n + parents, ok := m.references[n] + if !ok { + continue + } - // Make sure this isn't a self reference, which isn't included - selfRef := false - for _, p := range parents { - if p == v { - selfRef = true - break + // Mark that we found a match + found = true + + // Make sure this isn't a self reference, which isn't included + selfRef := false + for _, p := range parents { + if p == v { + selfRef = true + break + } } - } - if selfRef { - continue + if selfRef { + continue + } + + matches = append(matches, parents...) + break } - matches = append(matches, parents...) + if !found { + missing = append(missing, ns) + } } return matches, missing @@ -233,8 +244,23 @@ func ReferenceFromInterpolatedVar(v config.InterpolatedVariable) []string { case *config.ModuleVariable: return []string{fmt.Sprintf("module.%s.output.%s", v.Name, v.Field)} case *config.ResourceVariable: - result := []string{v.ResourceId()} - return result + id := v.ResourceId() + + // If we have a multi-reference (splat), then we depend on ALL + // resources with this type/name. + if v.Multi && v.Index == -1 { + return []string{fmt.Sprintf("%s.*", id)} + } + + // Otherwise, we depend on a specific index. + idx := v.Index + if !v.Multi || v.Index == -1 { + idx = 0 + } + + // Depend on the index, as well as "N" which represents the + // un-expanded set of resources. + return []string{fmt.Sprintf("%s.%d/%s.N", id, idx, id)} case *config.UserVariable: return []string{fmt.Sprintf("var.%s", v.Name)} default: diff --git a/terraform/transform_reference_test.go b/terraform/transform_reference_test.go index 31cf4664de4c..544af6b04915 100644 --- a/terraform/transform_reference_test.go +++ b/terraform/transform_reference_test.go @@ -88,6 +88,56 @@ func TestReferenceTransformer_path(t *testing.T) { } } +func TestReferenceTransformer_backup(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"C/A"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefBackupStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestReferenceTransformer_backupPrimary(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"C/A"}, + }) + g.Add(&graphNodeRefParentTest{ + NameValue: "C", + Names: []string{"C"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefBackupPrimaryStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestReferenceMapReferences(t *testing.T) { cases := map[string]struct { Nodes []dag.Vertex @@ -202,6 +252,19 @@ B A ` +const testTransformRefBackupStr = ` +A +B + A +` + +const testTransformRefBackupPrimaryStr = ` +A +B + C +C +` + const testTransformRefPathStr = ` A B From bcc67fd1356919dc0cd95a5cefd792d31f592428 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Nov 2016 10:59:35 -0800 Subject: [PATCH 24/26] terraform: uncomment a test that we were waiting on terraform: remove final TODO --- terraform/context_plan_test.go | 2 -- terraform/node_resource_plan.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 4c0a7d878ee2..f2b4736235cf 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -963,8 +963,6 @@ func TestContext2Plan_preventDestroy_destroyPlan(t *testing.T) { } func TestContext2Plan_provisionerCycle(t *testing.T) { - // TODO - return m := testModule(t, "plan-provisioner-cycle") p := testProvider("aws") p.DiffFn = testDiffFn diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 43b1edee858f..c9e53e02c63d 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -86,8 +86,6 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { State: state, }, - // TODO: deposed - // Attach the state &AttachStateTransformer{State: state}, From 838fe2af9bbdce2fa5449f6c30551d99176ca56b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Nov 2016 14:04:57 -0800 Subject: [PATCH 25/26] terraform: address go vet --- terraform/transform_config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/transform_config_test.go b/terraform/transform_config_test.go index 81ce1fcab8d8..31bb7c8e0d4f 100644 --- a/terraform/transform_config_test.go +++ b/terraform/transform_config_test.go @@ -16,7 +16,7 @@ func TestConfigTransformer_nilModule(t *testing.T) { } if len(g.Vertices()) > 0 { - t.Fatalf("graph is not empty: %s", g) + t.Fatalf("graph is not empty: %s", g.String()) } } From fa195d4fafc296cafa233f188ce21b9d7a387aed Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 9 Nov 2016 08:10:09 -0800 Subject: [PATCH 26/26] terraform: fix a typo found during review --- terraform/transform_orphan_count.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/transform_orphan_count.go b/terraform/transform_orphan_count.go index 5c7840ff59ca..b256a25b7b27 100644 --- a/terraform/transform_orphan_count.go +++ b/terraform/transform_orphan_count.go @@ -11,7 +11,7 @@ import ( // on the count argument given. // // Orphans are found by comparing the count to what is found in the state. -// This tranform assumes that if an element in the state is within the count +// This transform assumes that if an element in the state is within the count // bounds given, that it is not an orphan. type OrphanResourceCountTransformer struct { Concrete ConcreteResourceNodeFunc