From 2b4695eecb1c0e28c41d950b669d9de3aba2880a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 21 Aug 2019 19:06:25 -0400 Subject: [PATCH] only create one provisioner instance per type There's no reason to start individual provisioners per module path, as they are not configured per module (or independently at all for that matter). --- terraform/eval_context_builtin.go | 13 +++------- terraform/path.go | 17 ------------- terraform/transform_provisioner.go | 40 ++++++------------------------ 3 files changed, 11 insertions(+), 59 deletions(-) delete mode 100644 terraform/path.go diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 20b37933d..3ca02ad8b 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -225,14 +225,12 @@ func (ctx *BuiltinEvalContext) InitProvisioner(n string) (provisioners.Interface ctx.ProvisionerLock.Lock() defer ctx.ProvisionerLock.Unlock() - key := PathObjectCacheKey(ctx.Path(), n) - - p, err := ctx.Components.ResourceProvisioner(n, key) + p, err := ctx.Components.ResourceProvisioner(n, "") if err != nil { return nil, err } - ctx.ProvisionerCache[key] = p + ctx.ProvisionerCache[n] = p return p, nil } @@ -243,8 +241,7 @@ func (ctx *BuiltinEvalContext) Provisioner(n string) provisioners.Interface { ctx.ProvisionerLock.Lock() defer ctx.ProvisionerLock.Unlock() - key := PathObjectCacheKey(ctx.Path(), n) - return ctx.ProvisionerCache[key] + return ctx.ProvisionerCache[n] } func (ctx *BuiltinEvalContext) ProvisionerSchema(n string) *configschema.Block { @@ -259,9 +256,7 @@ func (ctx *BuiltinEvalContext) CloseProvisioner(n string) error { ctx.ProvisionerLock.Lock() defer ctx.ProvisionerLock.Unlock() - key := PathObjectCacheKey(ctx.Path(), n) - - prov := ctx.ProvisionerCache[key] + prov := ctx.ProvisionerCache[n] if prov != nil { return prov.Close() } diff --git a/terraform/path.go b/terraform/path.go deleted file mode 100644 index 9757446bb..000000000 --- a/terraform/path.go +++ /dev/null @@ -1,17 +0,0 @@ -package terraform - -import ( - "fmt" - - "github.com/hashicorp/terraform/addrs" -) - -// PathObjectCacheKey is like PathCacheKey but includes an additional name -// to be included in the key, for module-namespaced objects. -// -// The result of this function is guaranteed unique for any distinct pair -// of path and name, but is not guaranteed to be in any particular format -// and in particular should never be shown to end-users. -func PathObjectCacheKey(path addrs.ModuleInstance, objectName string) string { - return fmt.Sprintf("%s|%s", path.String(), objectName) -} diff --git a/terraform/transform_provisioner.go b/terraform/transform_provisioner.go index fe4cf0e9e..b31026655 100644 --- a/terraform/transform_provisioner.go +++ b/terraform/transform_provisioner.go @@ -4,8 +4,6 @@ import ( "fmt" "log" - "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/dag" ) @@ -43,16 +41,15 @@ func (t *ProvisionerTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { if pv, ok := v.(GraphNodeProvisionerConsumer); ok { for _, p := range pv.ProvisionedBy() { - key := provisionerMapKey(p, pv) - if m[key] == nil { + if m[p] == nil { err = multierror.Append(err, fmt.Errorf( "%s: provisioner %s couldn't be found", dag.VertexName(v), p)) continue } - log.Printf("[TRACE] ProvisionerTransformer: %s is provisioned by %s (%q)", dag.VertexName(v), key, dag.VertexName(m[key])) - g.Connect(dag.BasicEdge(v, m[key])) + log.Printf("[TRACE] ProvisionerTransformer: %s is provisioned by %s (%q)", dag.VertexName(v), p, dag.VertexName(m[p])) + g.Connect(dag.BasicEdge(v, m[p])) } } } @@ -85,18 +82,8 @@ func (t *MissingProvisionerTransformer) Transform(g *Graph) error { continue } - // If this node has a subpath, then we use that as a prefix - // into our map to check for an existing provider. - path := addrs.RootModuleInstance - if sp, ok := pv.(GraphNodeSubPath); ok { - path = sp.Path() - } - for _, p := range pv.ProvisionedBy() { - // Build the key for storing in the map - key := provisionerMapKey(p, pv) - - if _, ok := m[key]; ok { + if _, ok := m[p]; ok { // This provisioner already exists as a configure node continue } @@ -110,12 +97,11 @@ func (t *MissingProvisionerTransformer) Transform(g *Graph) error { // Build the vertex var newV dag.Vertex = &NodeProvisioner{ NameValue: p, - PathValue: path, } // Add the missing provisioner node to the graph - m[key] = g.Add(newV) - log.Printf("[TRACE] MissingProviderTransformer: added implicit provisioner %s, first implied by %s", key, dag.VertexName(v)) + m[p] = g.Add(newV) + log.Printf("[TRACE] MissingProviderTransformer: added implicit provisioner %s, first implied by %s", p, dag.VertexName(v)) } } @@ -153,23 +139,11 @@ func (t *CloseProvisionerTransformer) Transform(g *Graph) error { return nil } -// provisionerMapKey is a helper that gives us the key to use for the -// maps returned by things such as provisionerVertexMap. -func provisionerMapKey(k string, v dag.Vertex) string { - pathPrefix := "" - if sp, ok := v.(GraphNodeSubPath); ok { - pathPrefix = sp.Path().String() + "." - } - - return pathPrefix + k -} - func provisionerVertexMap(g *Graph) map[string]dag.Vertex { m := make(map[string]dag.Vertex) for _, v := range g.Vertices() { if pv, ok := v.(GraphNodeProvisioner); ok { - key := provisionerMapKey(pv.ProvisionerName(), v) - m[key] = v + m[pv.ProvisionerName()] = v } }