Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't create separate provisioners for each module #22553

Merged
merged 2 commits into from
Aug 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions terraform/eval_context_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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()
}
Expand Down
4 changes: 2 additions & 2 deletions terraform/graph_builder_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,12 +478,11 @@ const testApplyGraphBuilderStr = `
meta.count-boundary (EachMode fixup)
module.child.test_object.other
test_object.other
module.child.provisioner.test
module.child.test_object.create
module.child.test_object.create (prepare state)
module.child.test_object.create (prepare state)
module.child.provisioner.test
provider.test
provisioner.test
module.child.test_object.other
module.child.test_object.create
module.child.test_object.other (prepare state)
Expand All @@ -493,6 +492,7 @@ provider.test
provider.test (close)
module.child.test_object.other
test_object.other
provisioner.test
provisioner.test (close)
module.child.test_object.create
root
Expand Down
17 changes: 0 additions & 17 deletions terraform/path.go

This file was deleted.

40 changes: 7 additions & 33 deletions terraform/transform_provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"fmt"
"log"

"github.com/hashicorp/terraform/addrs"

"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform/dag"
)
Expand Down Expand Up @@ -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]))
}
}
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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))
}
}

Expand Down Expand Up @@ -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
}
}

Expand Down
3 changes: 1 addition & 2 deletions terraform/transform_provisioner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ const testTransformMissingProvisionerModuleStr = `
aws_instance.foo
provisioner.shell
module.child.aws_instance.foo
module.child.provisioner.shell
module.child.provisioner.shell
provisioner.shell
provisioner.shell
`

Expand Down