diff --git a/command/graph.go b/command/graph.go index aa5f14d1a..45299787a 100644 --- a/command/graph.go +++ b/command/graph.go @@ -158,7 +158,8 @@ Options: -no-color If specified, output won't contain any color. -type=plan Type of graph to output. Can be: plan, plan-destroy, apply, - legacy. + validate, input, refresh. + ` return strings.TrimSpace(helpText) diff --git a/helper/experiment/experiment.go b/helper/experiment/experiment.go index 991011018..d3094291a 100644 --- a/helper/experiment/experiment.go +++ b/helper/experiment/experiment.go @@ -50,9 +50,6 @@ import ( // of definition and use. This allows the compiler to enforce references // so it becomes easy to remove the features. var ( - // Reuse the old graphs from TF 0.7.x. These will be removed at some point. - X_legacyGraph = newBasicID("legacy-graph", "LEGACY_GRAPH", false) - // Shadow graph. This is already on by default. Disabling it will be // allowed for awhile in order for it to not block operations. X_shadow = newBasicID("shadow", "SHADOW", true) @@ -75,7 +72,6 @@ var ( func init() { // The list of all experiments, update this when an experiment is added. All = []ID{ - X_legacyGraph, X_shadow, x_force, } diff --git a/terraform/context.go b/terraform/context.go index 066ffa379..616d4766f 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -262,30 +262,11 @@ func (c *Context) Graph(typ GraphType, opts *ContextGraphOpts) (*Graph, error) { Targets: c.targets, Validate: opts.Validate, }).Build(RootModulePath) - - case GraphTypeLegacy: - return c.graphBuilder(opts).Build(RootModulePath) } return nil, fmt.Errorf("unknown graph type: %s", typ) } -// GraphBuilder returns the GraphBuilder that will be used to create -// the graphs for this context. -func (c *Context) graphBuilder(g *ContextGraphOpts) GraphBuilder { - return &BuiltinGraphBuilder{ - Root: c.module, - Diff: c.diff, - Providers: c.components.ResourceProviders(), - Provisioners: c.components.ResourceProvisioners(), - State: c.state, - Targets: c.targets, - Destroy: c.destroy, - Validate: g.Validate, - Verbose: g.Verbose, - } -} - // ShadowError returns any errors caught during a shadow operation. // // A shadow operation is an operation run in parallel to a real operation @@ -465,15 +446,8 @@ func (c *Context) Apply() (*State, error) { // Copy our own state c.state = c.state.DeepCopy() - // Enable the new graph by default - X_legacyGraph := experiment.Enabled(experiment.X_legacyGraph) - // Build the graph. - graphType := GraphTypeLegacy - if !X_legacyGraph { - graphType = GraphTypeApply - } - graph, err := c.Graph(graphType, nil) + graph, err := c.Graph(GraphTypeApply, nil) if err != nil { return nil, err } @@ -541,17 +515,10 @@ func (c *Context) Plan() (*Plan, error) { c.diff.init() c.diffLock.Unlock() - // Used throughout below - X_legacyGraph := experiment.Enabled(experiment.X_legacyGraph) - // Build the graph. - graphType := GraphTypeLegacy - if !X_legacyGraph { - if c.destroy { - graphType = GraphTypePlanDestroy - } else { - graphType = GraphTypePlan - } + graphType := GraphTypePlan + if c.destroy { + graphType = GraphTypePlanDestroy } graph, err := c.Graph(graphType, nil) if err != nil { @@ -576,15 +543,17 @@ func (c *Context) Plan() (*Plan, error) { p.Diff.DeepCopy() } - // We don't do the reverification during the new destroy plan because - // it will use a different apply process. - if X_legacyGraph { - // 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(GraphTypeLegacy, nil); err != nil { - return nil, err + /* + // We don't do the reverification during the new destroy plan because + // it will use a different apply process. + if X_legacyGraph { + // 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(GraphTypeLegacy, nil); err != nil { + return nil, err + } } - } + */ var errs error if len(walker.ValidationErrors) > 0 { @@ -606,15 +575,8 @@ func (c *Context) Refresh() (*State, error) { // Copy our own state c.state = c.state.DeepCopy() - // Used throughout below - X_legacyGraph := experiment.Enabled(experiment.X_legacyGraph) - // Build the graph. - graphType := GraphTypeLegacy - if !X_legacyGraph { - graphType = GraphTypeRefresh - } - graph, err := c.Graph(graphType, nil) + graph, err := c.Graph(GraphTypeRefresh, nil) if err != nil { return nil, err } diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index fb850df18..6374bb904 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -4,8 +4,6 @@ import ( "fmt" "log" "strings" - - "github.com/hashicorp/terraform/config/module" ) // GraphBuilder is an interface that can be implemented and used with @@ -77,160 +75,3 @@ func (b *BasicGraphBuilder) Build(path []string) (*Graph, error) { return g, nil } - -// BuiltinGraphBuilder is responsible for building the complete graph that -// Terraform uses for execution. It is an opinionated builder that defines -// the step order required to build a complete graph as is used and expected -// by Terraform. -// -// If you require a custom graph, you'll have to build it up manually -// on your own by building a new GraphBuilder implementation. -type BuiltinGraphBuilder struct { - // Root is the root module of the graph to build. - Root *module.Tree - - // Diff is the diff. The proper module diffs will be looked up. - Diff *Diff - - // State is the global state. The proper module states will be looked - // up by graph path. - State *State - - // Providers is the list of providers supported. - Providers []string - - // Provisioners is the list of provisioners supported. - Provisioners []string - - // Targets is the user-specified list of resources to target. - Targets []string - - // Destroy is set to true when we're in a `terraform destroy` or a - // `terraform plan -destroy` - Destroy bool - - // Determines whether the GraphBuilder should perform graph validation before - // returning the Graph. Generally you want this to be done, except when you'd - // like to inspect a problematic graph. - Validate bool - - // Verbose is set to true when the graph should be built "worst case", - // skipping any prune steps. This is used for early cycle detection during - // Validate and for manual inspection via `terraform graph -verbose`. - Verbose bool -} - -// Build builds the graph according to the steps returned by Steps. -func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) { - basic := &BasicGraphBuilder{ - Steps: b.Steps(path), - Validate: b.Validate, - Name: "BuiltinGraphBuilder", - } - - return basic.Build(path) -} - -// Steps returns the ordered list of GraphTransformers that must be executed -// to build a complete graph. -func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { - steps := []GraphTransformer{ - // Create all our resources from the configuration and state - &ConfigTransformerOld{Module: b.Root}, - &OrphanTransformer{ - State: b.State, - Module: b.Root, - }, - - // Output-related transformations - &AddOutputOrphanTransformer{State: b.State}, - - // Provider-related transformations - &MissingProviderTransformer{Providers: b.Providers}, - &ProviderTransformer{}, - &DisableProviderTransformerOld{}, - - // Provisioner-related transformations - &MissingProvisionerTransformer{Provisioners: b.Provisioners}, - &ProvisionerTransformer{}, - - // Run our vertex-level transforms - &VertexTransformer{ - Transforms: []GraphVertexTransformer{ - // Expand any statically expanded nodes, such as module graphs - &ExpandTransform{ - Builder: b, - }, - }, - }, - - // Flatten stuff - &FlattenTransformer{}, - - // Make sure all the connections that are proxies are connected through - &ProxyTransformer{}, - } - - // If we're on the root path, then we do a bunch of other stuff. - // We don't do the following for modules. - if len(path) <= 1 { - steps = append(steps, - // Optionally reduces the graph to a user-specified list of targets and - // their dependencies. - &TargetsTransformer{Targets: b.Targets, Destroy: b.Destroy}, - - // Create orphan output nodes - &OrphanOutputTransformer{Module: b.Root, State: b.State}, - - // Prune the providers. This must happen only once because flattened - // modules might depend on empty providers. - &PruneProviderTransformer{}, - - // Create the destruction nodes - &DestroyTransformer{FullDestroy: b.Destroy}, - b.conditional(&conditionalOpts{ - If: func() bool { return !b.Destroy }, - Then: &CreateBeforeDestroyTransformer{}, - }), - b.conditional(&conditionalOpts{ - If: func() bool { return !b.Verbose }, - Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State}, - }), - - // Remove the noop nodes - &PruneNoopTransformer{Diff: b.Diff, State: b.State}, - - // Insert nodes to close opened plugin connections - &CloseProviderTransformer{}, - &CloseProvisionerTransformer{}, - - // Perform the transitive reduction to make our graph a bit - // more sane if possible (it usually is possible). - &TransitiveReductionTransformer{}, - ) - } - - // Make sure we have a single root - steps = append(steps, &RootTransformer{}) - - // Remove nils - for i, s := range steps { - if s == nil { - steps = append(steps[:i], steps[i+1:]...) - } - } - - return steps -} - -type conditionalOpts struct { - If func() bool - Then GraphTransformer -} - -func (b *BuiltinGraphBuilder) conditional(o *conditionalOpts) GraphTransformer { - if o.If != nil && o.Then != nil && o.If() { - return o.Then - } - return nil -} diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index f73308b2c..6d7f4b61b 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -65,212 +65,6 @@ func TestBasicGraphBuilder_validateOff(t *testing.T) { } } -func TestBuiltinGraphBuilder_impl(t *testing.T) { - var _ GraphBuilder = new(BuiltinGraphBuilder) -} - -// This test is not meant to test all the transforms but rather just -// to verify we get some basic sane graph out. Special tests to ensure -// specific ordering of steps should be added in other tests. -func TestBuiltinGraphBuilder(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-basic"), - Validate: true, - } - - g, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testBuiltinGraphBuilderBasicStr) - if actual != expected { - t.Fatalf("bad: %s", actual) - } -} - -func TestBuiltinGraphBuilder_Verbose(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-basic"), - Validate: true, - Verbose: true, - } - - g, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testBuiltinGraphBuilderVerboseStr) - if actual != expected { - t.Fatalf("bad: %s", actual) - } -} - -// This tests that the CreateBeforeDestoryTransformer is not present when -// we perform a "terraform destroy" operation. We don't actually do anything -// else. -func TestBuiltinGraphBuilder_CreateBeforeDestroy_Destroy_Bypass(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-basic"), - Validate: true, - Destroy: true, - } - - steps := b.Steps([]string{}) - - actual := false - expected := false - for _, v := range steps { - switch v.(type) { - case *CreateBeforeDestroyTransformer: - actual = true - } - } - - if actual != expected { - t.Fatalf("bad: CreateBeforeDestroyTransformer still in root path") - } -} - -// This tests that the CreateBeforeDestoryTransformer *is* present -// during a non-destroy operation (ie: Destroy not set). -func TestBuiltinGraphBuilder_CreateBeforeDestroy_NonDestroy_Present(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-basic"), - Validate: true, - } - - steps := b.Steps([]string{}) - - actual := false - expected := true - for _, v := range steps { - switch v.(type) { - case *CreateBeforeDestroyTransformer: - actual = true - } - } - - if actual != expected { - t.Fatalf("bad: CreateBeforeDestroyTransformer not in root path") - } -} - -// This tests a cycle we got when a CBD resource depends on a non-CBD -// resource. This cycle shouldn't happen in the general case anymore. -func TestBuiltinGraphBuilder_cbdDepNonCbd(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-cbd-non-cbd"), - Validate: true, - } - - _, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } -} - -// This now returns no errors due to a general fix while building the graph -func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-cbd-non-cbd"), - Validate: true, - Verbose: true, - } - - _, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } -} - -func TestBuiltinGraphBuilder_multiLevelModule(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-multi-level-module"), - Validate: true, - } - - g, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testBuiltinGraphBuilderMultiLevelStr) - if actual != expected { - t.Fatalf("bad: %s", actual) - } -} - -func TestBuiltinGraphBuilder_orphanDeps(t *testing.T) { - state := &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: rootModulePath, - Resources: map[string]*ResourceState{ - "aws_instance.foo": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "foo", - }, - }, - - "aws_instance.bar": &ResourceState{ - Type: "aws_instance", - Dependencies: []string{"aws_instance.foo"}, - Primary: &InstanceState{ - ID: "bar", - }, - }, - }, - }, - }, - } - - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-orphan-deps"), - State: state, - Validate: true, - } - - g, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testBuiltinGraphBuilderOrphanDepsStr) - if actual != expected { - t.Fatalf("bad: %s", actual) - } -} - -/* -TODO: This exposes a really bad bug we need to fix after we merge -the f-ast-branch. This bug still exists in master. - -// This test tests that the graph builder properly expands modules. -func TestBuiltinGraphBuilder_modules(t *testing.T) { - b := &BuiltinGraphBuilder{ - Root: testModule(t, "graph-builder-modules"), - } - - g, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testBuiltinGraphBuilderModuleStr) - if actual != expected { - t.Fatalf("bad: %s", actual) - } -} -*/ - type testBasicGraphBuilderTransform struct { V dag.Vertex } @@ -283,76 +77,3 @@ func (t *testBasicGraphBuilderTransform) Transform(g *Graph) error { const testBasicGraphBuilderStr = ` 1 ` - -const testBuiltinGraphBuilderBasicStr = ` -aws_instance.db - provider.aws -aws_instance.web - aws_instance.db -provider.aws -provider.aws (close) - aws_instance.web -` - -const testBuiltinGraphBuilderVerboseStr = ` -aws_instance.db - aws_instance.db (destroy) -aws_instance.db (destroy) - aws_instance.web (destroy) -aws_instance.web - aws_instance.db -aws_instance.web (destroy) - provider.aws -provider.aws -provider.aws (close) - aws_instance.web -` - -const testBuiltinGraphBuilderMultiLevelStr = ` -module.foo.module.bar.output.value - module.foo.module.bar.var.bar - module.foo.var.foo -module.foo.module.bar.plan-destroy -module.foo.module.bar.var.bar - module.foo.var.foo -module.foo.plan-destroy -module.foo.var.foo -root - module.foo.module.bar.output.value - module.foo.module.bar.plan-destroy - module.foo.module.bar.var.bar - module.foo.plan-destroy - module.foo.var.foo -` - -const testBuiltinGraphBuilderOrphanDepsStr = ` -aws_instance.bar (orphan) - provider.aws -aws_instance.foo (orphan) - aws_instance.bar (orphan) -provider.aws -provider.aws (close) - aws_instance.foo (orphan) -` - -/* -TODO: Commented out this const as it's likely this needs to -be updated when the TestBuiltinGraphBuilder_modules test is -enabled again. -const testBuiltinGraphBuilderModuleStr = ` -aws_instance.web - aws_instance.web (destroy) -aws_instance.web (destroy) - aws_security_group.firewall - module.consul (expanded) - provider.aws -aws_security_group.firewall - aws_security_group.firewall (destroy) -aws_security_group.firewall (destroy) - provider.aws -module.consul (expanded) - aws_security_group.firewall - provider.aws -provider.aws -` -*/