From 883e4487a2c4f050363db5e0e3486462b15aab81 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 4 Sep 2020 14:03:45 -0400 Subject: [PATCH] terraform: add GraphNodeExecutable interface (#26132) This introduces a new GraphNode, GraphNodeExecutable, which will gradually replace GraphNodeEvalable as part of the overall removal of EvalTree()s. Terraform's Graph.walk function will now check if a node is GraphNodeExecutable and run walker.Execute instead of running through the EvalTree() and Eval(). For the time being, terraform will panic if a node implements both GraphNodeExecutable and GraphNodeEvalable. This will be removed when we've finished removing all GraphNodeEvalable implementations. The new GraphWalker function, Execute(), is meant to replace both EnterEvalTree and ExitEvalTree, and wraps the call to the GraphNodeExecutable's Execute function. --- terraform/execute.go | 9 +++++++++ terraform/graph.go | 18 ++++++++++++++--- terraform/graph_walk.go | 6 ++---- terraform/graph_walk_context.go | 35 +++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 terraform/execute.go diff --git a/terraform/execute.go b/terraform/execute.go new file mode 100644 index 0000000000..5bf06c4d04 --- /dev/null +++ b/terraform/execute.go @@ -0,0 +1,9 @@ +package terraform + +// GraphNodeExecutable is the interface that graph nodes must implement to +// enable execution. This is an alternative to GraphNodeEvalable, which is in +// the process of being removed. A given graph node should _not_ implement both +// GraphNodeExecutable and GraphNodeEvalable. +type GraphNodeExecutable interface { + Execute(EvalContext, walkOperation) error +} diff --git a/terraform/graph.go b/terraform/graph.go index 4c9f2f0c20..df1ecf1342 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -46,9 +46,6 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { log.Printf("[TRACE] vertex %q: visit complete", dag.VertexName(v)) }() - walker.EnterVertex(v) - defer walker.ExitVertex(v, diags) - // vertexCtx is the context that we use when evaluating. This // is normally the context of our graph but can be overridden // with a GraphNodeModuleInstance impl. @@ -58,6 +55,21 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { defer walker.ExitPath(pn.Path()) } + // If the node is exec-able, then execute it. + if ev, ok := v.(GraphNodeExecutable); ok { + // A node must not be both Evalable and Executable. This will be + // removed when GraphNodeEvalable is fully removed. + if _, ok := v.(GraphNodeEvalable); ok { + panic(fmt.Sprintf( + "%T implements both GraphNodeEvalable and GraphNodeExecutable", v, + )) + } + diags = diags.Append(walker.Execute(vertexCtx, ev)) + if diags.HasErrors() { + return + } + } + // If the node is eval-able, then evaluate it. if ev, ok := v.(GraphNodeEvalable); ok { tree := ev.EvalTree() diff --git a/terraform/graph_walk.go b/terraform/graph_walk.go index 706b7e0ab0..1ec099b3c5 100644 --- a/terraform/graph_walk.go +++ b/terraform/graph_walk.go @@ -12,10 +12,9 @@ type GraphWalker interface { EvalContext() EvalContext EnterPath(addrs.ModuleInstance) EvalContext ExitPath(addrs.ModuleInstance) - EnterVertex(dag.Vertex) - ExitVertex(dag.Vertex, tfdiags.Diagnostics) EnterEvalTree(dag.Vertex, EvalNode) EvalNode ExitEvalTree(dag.Vertex, interface{}, error) tfdiags.Diagnostics + Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics } // NullGraphWalker is a GraphWalker implementation that does nothing. @@ -26,9 +25,8 @@ type NullGraphWalker struct{} func (NullGraphWalker) EvalContext() EvalContext { return new(MockEvalContext) } func (NullGraphWalker) EnterPath(addrs.ModuleInstance) EvalContext { return new(MockEvalContext) } func (NullGraphWalker) ExitPath(addrs.ModuleInstance) {} -func (NullGraphWalker) EnterVertex(dag.Vertex) {} -func (NullGraphWalker) ExitVertex(dag.Vertex, tfdiags.Diagnostics) {} func (NullGraphWalker) EnterEvalTree(v dag.Vertex, n EvalNode) EvalNode { return n } func (NullGraphWalker) ExitEvalTree(dag.Vertex, interface{}, error) tfdiags.Diagnostics { return nil } +func (NullGraphWalker) Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics { return nil } diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 5025c98b66..4a2e26841a 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -162,3 +162,38 @@ func (w *ContextGraphWalker) init() { w.variableValues[""][k] = iv.Value } } + +func (w *ContextGraphWalker) Execute(ctx EvalContext, n GraphNodeExecutable) tfdiags.Diagnostics { + // Acquire a lock on the semaphore + w.Context.parallelSem.Acquire() + + err := n.Execute(ctx, w.Operation) + + // Release the semaphore + w.Context.parallelSem.Release() + + if err == nil { + return nil + } + + // Acquire the lock because anything is going to require a lock. + w.errorLock.Lock() + defer w.errorLock.Unlock() + + // If the error is non-fatal then we'll accumulate its diagnostics in our + // non-fatal list, rather than returning it directly, so that the graph + // walk can continue. + if nferr, ok := err.(tfdiags.NonFatalError); ok { + w.NonFatalDiagnostics = w.NonFatalDiagnostics.Append(nferr.Diagnostics) + return nil + } + + // Otherwise, we'll let our usual diagnostics machinery figure out how to + // unpack this as one or more diagnostic messages and return that. If we + // get down here then the returned diagnostics will contain at least one + // error, causing the graph walk to halt. + var diags tfdiags.Diagnostics + diags = diags.Append(err) + return diags + +}