terraform: new plan graph

terraform/context.go

@@ -487,28 +487,79 @@ func (c *Context) Plan() (*Plan, error) {
 	c.diffLock.Unlock()
 
 	// Used throughout below
+	X_newApply := experiment.Enabled(experiment.X_newApply)
 	X_newDestroy := experiment.Enabled(experiment.X_newDestroy)
+	newGraphEnabled := (c.destroy && X_newDestroy) || (!c.destroy && X_newApply)
+
+	// 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 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 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})
+		newGraph, err = (&PlanGraphBuilder{
+			Module:    c.module,
+			State:     c.state,
+			Providers: c.components.ResourceProviders(),
+			Targets:   c.targets,
+		}).Build(RootModulePath)
+	}
+	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
 	}
@@ -527,7 +578,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 {
@@ -799,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:"))
 		}
 

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:
   <no state>
 		`)
 	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)
 	}
 }
 
@@ -5121,8 +5129,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()

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
@@ -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

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
 	}

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

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,

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},