terraform: EvalNode removal, continued

This commit continues the overall EvalNode removal project.

Something to note: the NodeRefreshableDataResourceInstance's Execute()
function is intentionally refactored in the bare minimum,
hardly-a-refactor style, because we have another ongoing project which
aims to remove NodeRefreshable*s. It is not worth the effort at this
time. We may revisit this decision in the future.

terraform/eval_provider.go

@@ -46,6 +46,22 @@ func buildProviderConfig(ctx EvalContext, addr addrs.AbsProviderConfig, config *
 	}
 }
 
+// GetProvider returns the providers interface and schema for a given provider.
+func GetProvider(ctx EvalContext, addr addrs.AbsProviderConfig) (providers.Interface, *ProviderSchema, error) {
+	if addr.Provider.Type == "" {
+		// Should never happen
+		panic("EvalGetProvider used with uninitialized provider configuration address")
+	}
+	provider := ctx.Provider(addr)
+	if provider == nil {
+		return nil, &ProviderSchema{}, fmt.Errorf("provider %s not initialized", addr)
+	}
+	// Not all callers require a schema, so we will leave checking for a nil
+	// schema to the callers.
+	schema := ctx.ProviderSchema(addr)
+	return provider, schema, nil
+}
+
 // EvalConfigProvider is an EvalNode implementation that configures
 // a provider that is already initialized and retrieved.
 type EvalConfigProvider struct {

terraform/node_data_destroy.go

@@ -1,9 +1,6 @@
 package terraform
 
-import (
+import "log"
-	"github.com/hashicorp/terraform/providers"
-	"github.com/hashicorp/terraform/states"
-)
 
 // NodeDestroyableDataResourceInstance represents a resource that is "destroyable":
 // it is ready to be destroyed.
@@ -11,30 +8,13 @@ type NodeDestroyableDataResourceInstance struct {
 	*NodeAbstractResourceInstance
 }
 
-// GraphNodeEvalable
+var (
-func (n *NodeDestroyableDataResourceInstance) EvalTree() EvalNode {
+	_ GraphNodeExecutable = (*NodeDestroyableDataResourceInstance)(nil)
-	addr := n.ResourceInstanceAddr()
+)
 
-	var providerSchema *ProviderSchema
+// GraphNodeExecutable
-	// We don't need the provider, but we're calling EvalGetProvider to load the
+func (n *NodeDestroyableDataResourceInstance) Execute(ctx EvalContext, op *walkOperation) error {
-	// schema.
+	log.Printf("[TRACE] NodeDestroyableDataResourceInstance: removing state object for %s", n.Addr)
-	var provider providers.Interface
+	ctx.State().SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider)
-
+	return nil
-	// Just destroy it.
-	var state *states.ResourceInstanceObject
-	return &EvalSequence{
-		Nodes: []EvalNode{
-			&EvalGetProvider{
-				Addr:   n.ResolvedProvider,
-				Output: &provider,
-				Schema: &providerSchema,
-			},
-			&EvalWriteState{
-				Addr:           addr.Resource,
-				State:          &state,
-				ProviderAddr:   n.ResolvedProvider,
-				ProviderSchema: &providerSchema,
-			},
-		},
-	}
 }

terraform/node_data_destroy_test.go

@@ -0,0 +1,48 @@
+package terraform
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/states"
+)
+
+func TestNodeDataDestroyExecute(t *testing.T) {
+	state := states.NewState()
+	state.Module(addrs.RootModuleInstance).SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.DataResourceMode,
+			Type: "test_instance",
+			Name: "foo",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"dynamic":{"type":"string","value":"hello"}}`),
+		},
+		addrs.AbsProviderConfig{
+			Provider: addrs.NewDefaultProvider("test"),
+			Module:   addrs.RootModule,
+		},
+	)
+	ctx := &MockEvalContext{
+		StateState: state.SyncWrapper(),
+	}
+
+	node := NodeDestroyableDataResourceInstance{&NodeAbstractResourceInstance{
+		Addr: addrs.Resource{
+			Mode: addrs.DataResourceMode,
+			Type: "test_instance",
+			Name: "foo",
+		}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+	}}
+
+	err := node.Execute(ctx, nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err.Error())
+	}
+
+	// verify resource removed from state
+	if state.HasResources() {
+		t.Fatal("resources still in state after NodeDataDestroy.Execute")
+	}
+}

terraform/node_data_refresh.go

@@ -4,7 +4,6 @@ import (
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/dag"
 	"github.com/hashicorp/terraform/plans"
-	"github.com/hashicorp/terraform/providers"
 	"github.com/hashicorp/terraform/states"
 	"github.com/hashicorp/terraform/tfdiags"
 )
@@ -191,91 +190,97 @@ type NodeRefreshableDataResourceInstance struct {
 	*NodeAbstractResourceInstance
 }
 
-// GraphNodeEvalable
+// GraphNodeExecutable
-func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode {
+func (n *NodeRefreshableDataResourceInstance) Execute(ctx EvalContext, op *walkOperation) error {
 	addr := n.ResourceInstanceAddr()
 
 	// These variables are the state for the eval sequence below, and are
 	// updated through pointers.
-	var provider providers.Interface
-	var providerSchema *ProviderSchema
 	var change *plans.ResourceInstanceChange
 	var state *states.ResourceInstanceObject
 
-	return &EvalSequence{
+	provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider)
-		Nodes: []EvalNode{
+	if err != nil {
-			&EvalGetProvider{
+		return err
-				Addr:   n.ResolvedProvider,
+	}
-				Output: &provider,
-				Schema: &providerSchema,
-			},
 
-			&EvalReadState{
+	// EvalReadState
-				Addr:           addr.Resource,
+	readStateReq := &EvalReadState{
-				Provider:       &provider,
+		Addr:           addr.Resource,
-				ProviderSchema: &providerSchema,
+		Provider:       &provider,
-				Output:         &state,
+		ProviderSchema: &providerSchema,
-			},
+		Output:         &state,
+	}
+	_, err = readStateReq.Eval(ctx)
+	if err != nil {
+		return err
+	}
 
-			// EvalReadDataRefresh will _attempt_ to read the data source, but
+	// EvalReadDataRefresh will _attempt_ to read the data source, but
-			// may generate an incomplete planned object if the configuration
+	// may generate an incomplete planned object if the configuration
-			// includes values that won't be known until apply.
+	// includes values that won't be known until apply.
-			&evalReadDataRefresh{
+	readDataRefreshReq := &evalReadDataRefresh{
-				evalReadData{
+		evalReadData{
-					Addr:           addr.Resource,
+			Addr:           addr.Resource,
-					Config:         n.Config,
+			Config:         n.Config,
-					Provider:       &provider,
+			Provider:       &provider,
-					ProviderAddr:   n.ResolvedProvider,
+			ProviderAddr:   n.ResolvedProvider,
-					ProviderMetas:  n.ProviderMetas,
+			ProviderMetas:  n.ProviderMetas,
-					ProviderSchema: &providerSchema,
+			ProviderSchema: &providerSchema,
-					OutputChange:   &change,
+			OutputChange:   &change,
-					State:          &state,
+			State:          &state,
-					dependsOn:      n.dependsOn,
+			dependsOn:      n.dependsOn,
-					forceDependsOn: n.forceDependsOn,
+			forceDependsOn: n.forceDependsOn,
-				},
-			},
-
-			&EvalIf{
-				If: func(ctx EvalContext) (bool, error) {
-					return change == nil, nil
-
-				},
-				Then: &EvalSequence{
-					Nodes: []EvalNode{
-						&EvalWriteState{
-							Addr:           addr.Resource,
-							ProviderAddr:   n.ResolvedProvider,
-							State:          &state,
-							ProviderSchema: &providerSchema,
-						},
-						&EvalUpdateStateHook{},
-					},
-				},
-				Else: &EvalSequence{
-					// We can't deal with this yet, so we'll repeat this step
-					// during the plan walk to produce a planned change to read
-					// this during the apply walk. However, we do still need to
-					// save the generated change and partial state so that
-					// results from it can be included in other data resources
-					// or provider configurations during the refresh walk.
-					// (The planned object we save in the state here will be
-					// pruned out at the end of the refresh walk, returning
-					// it back to being unset again for subsequent walks.)
-					Nodes: []EvalNode{
-						&EvalWriteDiff{
-							Addr:           addr.Resource,
-							Change:         &change,
-							ProviderSchema: &providerSchema,
-						},
-						&EvalWriteState{
-							Addr:           addr.Resource,
-							ProviderAddr:   n.ResolvedProvider,
-							State:          &state,
-							ProviderSchema: &providerSchema,
-						},
-					},
-				},
-			},
 		},
 	}
+	_, err = readDataRefreshReq.Eval(ctx)
+	if err != nil {
+		return err
+	}
+
+	if change == nil {
+		// EvalWriteState
+		writeStateRequest := EvalWriteState{
+			Addr:           addr.Resource,
+			ProviderAddr:   n.ResolvedProvider,
+			State:          &state,
+			ProviderSchema: &providerSchema,
+		}
+		_, err := writeStateRequest.Eval(ctx)
+		if err != nil {
+			return err
+		}
+
+		// EvalUpdateStateHook
+		updateStateHookReq := EvalUpdateStateHook{}
+		_, err = updateStateHookReq.Eval(ctx)
+		if err != nil {
+			return err
+		}
+	} else {
+		// EvalWriteDiff
+		writeDiffReq := &EvalWriteDiff{
+			Addr:           addr.Resource,
+			Change:         &change,
+			ProviderSchema: &providerSchema,
+		}
+		_, err = writeDiffReq.Eval(ctx)
+		if err != nil {
+			return err
+		}
+		// EvalWriteState
+		writeStateRequest := EvalWriteState{
+			Addr:           addr.Resource,
+			ProviderAddr:   n.ResolvedProvider,
+			State:          &state,
+			ProviderSchema: &providerSchema,
+		}
+		_, err := writeStateRequest.Eval(ctx)
+		if err != nil {
+			return err
+		}
+	}
+
+	return err
+
 }

terraform/node_resource_refresh.go

@@ -199,7 +199,7 @@ var (
 	_ GraphNodeResourceInstance     = (*NodeRefreshableManagedResourceInstance)(nil)
 	_ GraphNodeAttachResourceConfig = (*NodeRefreshableManagedResourceInstance)(nil)
 	_ GraphNodeAttachResourceState  = (*NodeRefreshableManagedResourceInstance)(nil)
-	_ GraphNodeEvalable             = (*NodeRefreshableManagedResourceInstance)(nil)
+	_ GraphNodeExecutable           = (*NodeRefreshableManagedResourceInstance)(nil)
 )
 
 // GraphNodeDestroyer
@@ -209,7 +209,7 @@ func (n *NodeRefreshableManagedResourceInstance) DestroyAddr() *addrs.AbsResourc
 }
 
 // GraphNodeEvalable
-func (n *NodeRefreshableManagedResourceInstance) EvalTree() EvalNode {
+func (n *NodeRefreshableManagedResourceInstance) Execute(ctx EvalContext, op *walkOperation) error {
 	addr := n.ResourceInstanceAddr()
 
 	// Eval info is different depending on what kind of resource this is
@@ -217,15 +217,17 @@ func (n *NodeRefreshableManagedResourceInstance) EvalTree() EvalNode {
 	case addrs.ManagedResourceMode:
 		if n.instanceState == nil {
 			log.Printf("[TRACE] NodeRefreshableManagedResourceInstance: %s has no existing state to refresh", addr)
-			return n.evalTreeManagedResourceNoState()
+			_, err := n.evalTreeManagedResourceNoState().Eval(ctx)
+			return err
 		}
 		log.Printf("[TRACE] NodeRefreshableManagedResourceInstance: %s will be refreshed", addr)
-		return n.evalTreeManagedResource()
+		_, err := n.evalTreeManagedResource().Eval(ctx)
+		return err
 
 	case addrs.DataResourceMode:
 		// Get the data source node. If we don't have a configuration
 		// then it is an orphan so we destroy it (remove it from the state).
-		var dn GraphNodeEvalable
+		var dn GraphNodeExecutable
 		if n.Config != nil {
 			dn = &NodeRefreshableDataResourceInstance{
 				NodeAbstractResourceInstance: n.NodeAbstractResourceInstance,
@@ -236,7 +238,7 @@ func (n *NodeRefreshableManagedResourceInstance) EvalTree() EvalNode {
 			}
 		}
 
-		return dn.EvalTree()
+		return dn.Execute(ctx, nil)
 	default:
 		panic(fmt.Errorf("unsupported resource mode %s", addr.Resource.Resource.Mode))
 	}

terraform/node_resource_refresh_test.go

@@ -1,10 +1,8 @@
 package terraform
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/addrs"
@@ -159,20 +157,3 @@ root - terraform.graphNodeRoot
 		t.Fatalf("Expected:\n%s\nGot:\n%s", expected, actual)
 	}
 }
-
-func TestNodeRefreshableManagedResourceEvalTree_scaleOut(t *testing.T) {
-	m := testModule(t, "refresh-resource-scale-inout")
-
-	n := &NodeRefreshableManagedResourceInstance{
-		NodeAbstractResourceInstance: NewNodeAbstractResourceInstance(addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "aws_instance", "foo").Instance(addrs.IntKey(2))),
-	}
-
-	n.AttachResourceConfig(m.Module.ManagedResources["aws_instance.foo"])
-
-	actual := n.EvalTree()
-	expected := n.evalTreeManagedResourceNoState()
-
-	if !reflect.DeepEqual(expected, actual) {
-		t.Fatalf("Expected:\n\n%s\nGot:\n\n%s\n", spew.Sdump(expected), spew.Sdump(actual))
-	}
-}