diff --git a/rpc/mux_broker.go b/rpc/mux_broker.go index 639902a825..dbc98d1c20 100644 --- a/rpc/mux_broker.go +++ b/rpc/mux_broker.go @@ -3,9 +3,9 @@ package rpc import ( "encoding/binary" "fmt" + "math/rand" "net" "sync" - "sync/atomic" "time" "github.com/hashicorp/yamux" @@ -17,7 +17,6 @@ import ( // or accept a connection from, and the broker handles the details of // holding these channels open while they're being negotiated. type muxBroker struct { - nextId uint32 session *yamux.Session streams map[uint32]*muxBrokerPending @@ -95,9 +94,12 @@ func (m *muxBroker) Dial(id uint32) (net.Conn, error) { return stream, nil } -// NextId returns a unique ID to use next. +// NextId returns a unique ID to use next. There is no need for seeding the +// rand package as the returned ID's aren't stored or used anywhere outside +// the current runtime. So it's perfectly fine to get the same pseudo-random +// numbers each time terraform is running. func (m *muxBroker) NextId() uint32 { - return atomic.AddUint32(&m.nextId, 1) + return rand.Uint32() } // Run starts the brokering and should be executed in a goroutine, since it diff --git a/rpc/resource_provider.go b/rpc/resource_provider.go index e3fa4241cc..3fe6927de8 100644 --- a/rpc/resource_provider.go +++ b/rpc/resource_provider.go @@ -174,6 +174,10 @@ func (p *ResourceProvider) Resources() []terraform.ResourceType { return result } +func (p *ResourceProvider) Close() error { + return p.Client.Close() +} + // ResourceProviderServer is a net/rpc compatible structure for serving // a ResourceProvider. This should not be used directly. type ResourceProviderServer struct { diff --git a/rpc/resource_provider_test.go b/rpc/resource_provider_test.go index 91cf38bfe2..3efdbce25f 100644 --- a/rpc/resource_provider_test.go +++ b/rpc/resource_provider_test.go @@ -488,3 +488,31 @@ func TestResourceProvider_validateResource_warns(t *testing.T) { t.Fatalf("bad: %#v", w) } } + +func TestResourceProvider_close(t *testing.T) { + client, _ := testNewClientServer(t) + defer client.Close() + + provider, err := client.ResourceProvider() + if err != nil { + t.Fatalf("err: %s", err) + } + + var p interface{} + p = provider + pCloser, ok := p.(terraform.ResourceProviderCloser) + if !ok { + t.Fatal("should be a ResourceProviderCloser") + } + + if err := pCloser.Close(); err != nil { + t.Fatalf("failed to close provider: %s", err) + } + + // The connection should be closed now, so if we to make a + // new call we should get an error. + err = provider.Configure(&terraform.ResourceConfig{}) + if err == nil { + t.Fatal("should have error") + } +} diff --git a/rpc/resource_provisioner.go b/rpc/resource_provisioner.go index cf8c008120..715704d024 100644 --- a/rpc/resource_provisioner.go +++ b/rpc/resource_provisioner.go @@ -63,6 +63,10 @@ func (p *ResourceProvisioner) Apply( return err } +func (p *ResourceProvisioner) Close() error { + return p.Client.Close() +} + type ResourceProvisionerValidateArgs struct { Config *terraform.ResourceConfig } diff --git a/rpc/resource_provisioner_test.go b/rpc/resource_provisioner_test.go index 32b6c06f38..6fabdb6d41 100644 --- a/rpc/resource_provisioner_test.go +++ b/rpc/resource_provisioner_test.go @@ -132,3 +132,34 @@ func TestResourceProvisioner_validate_warns(t *testing.T) { t.Fatalf("bad: %#v", w) } } + +func TestResourceProvisioner_close(t *testing.T) { + client, _ := testNewClientServer(t) + defer client.Close() + + provisioner, err := client.ResourceProvisioner() + if err != nil { + t.Fatalf("err: %s", err) + } + + var p interface{} + p = provisioner + pCloser, ok := p.(terraform.ResourceProvisionerCloser) + if !ok { + t.Fatal("should be a ResourceProvisionerCloser") + } + + if err := pCloser.Close(); err != nil { + t.Fatalf("failed to close provisioner: %s", err) + } + + // The connection should be closed now, so if we to make a + // new call we should get an error. + o := &terraform.MockUIOutput{} + s := &terraform.InstanceState{} + c := &terraform.ResourceConfig{} + err = provisioner.Apply(o, s, c) + if err == nil { + t.Fatal("should have error") + } +} diff --git a/terraform/eval_context.go b/terraform/eval_context.go index 8a838afb71..f4427939ae 100644 --- a/terraform/eval_context.go +++ b/terraform/eval_context.go @@ -28,6 +28,9 @@ type EvalContext interface { // initialized) or returns nil if the provider isn't initialized. Provider(string) ResourceProvider + // CloseProvider closes provider connections that aren't needed anymore. + CloseProvider(string) error + // ConfigureProvider configures the provider with the given // configuration. This is a separate context call because this call // is used to store the provider configuration for inheritance lookups @@ -51,6 +54,10 @@ type EvalContext interface { // initialized) or returns nil if the provisioner isn't initialized. Provisioner(string) ResourceProvisioner + // CloseProvisioner closes provisioner connections that aren't needed + // anymore. + CloseProvisioner(string) error + // Interpolate takes the given raw configuration and completes // the interpolations, returning the processed ResourceConfig. // diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index efc7d04fe4..f7cdf27cc3 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -114,6 +114,28 @@ func (ctx *BuiltinEvalContext) Provider(n string) ResourceProvider { return ctx.ProviderCache[PathCacheKey(providerPath)] } +func (ctx *BuiltinEvalContext) CloseProvider(n string) error { + ctx.once.Do(ctx.init) + + ctx.ProviderLock.Lock() + defer ctx.ProviderLock.Unlock() + + providerPath := make([]string, len(ctx.Path())+1) + copy(providerPath, ctx.Path()) + providerPath[len(providerPath)-1] = n + + var provider interface{} + provider = ctx.ProviderCache[PathCacheKey(providerPath)] + if provider != nil { + if p, ok := provider.(ResourceProviderCloser); ok { + delete(ctx.ProviderCache, PathCacheKey(providerPath)) + return p.Close() + } + } + + return nil +} + func (ctx *BuiltinEvalContext) ConfigureProvider( n string, cfg *ResourceConfig) error { p := ctx.Provider(n) @@ -222,6 +244,28 @@ func (ctx *BuiltinEvalContext) Provisioner(n string) ResourceProvisioner { return ctx.ProvisionerCache[PathCacheKey(provPath)] } +func (ctx *BuiltinEvalContext) CloseProvisioner(n string) error { + ctx.once.Do(ctx.init) + + ctx.ProvisionerLock.Lock() + defer ctx.ProvisionerLock.Unlock() + + provPath := make([]string, len(ctx.Path())+1) + copy(provPath, ctx.Path()) + provPath[len(provPath)-1] = n + + var prov interface{} + prov = ctx.ProvisionerCache[PathCacheKey(provPath)] + if prov != nil { + if p, ok := prov.(ResourceProvisionerCloser); ok { + delete(ctx.ProvisionerCache, PathCacheKey(provPath)) + return p.Close() + } + } + + return nil +} + func (ctx *BuiltinEvalContext) Interpolate( cfg *config.RawConfig, r *Resource) (*ResourceConfig, error) { if cfg != nil { diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go index 2a5856829d..60d83c7240 100644 --- a/terraform/eval_context_mock.go +++ b/terraform/eval_context_mock.go @@ -25,6 +25,10 @@ type MockEvalContext struct { ProviderName string ProviderProvider ResourceProvider + CloseProviderCalled bool + CloseProviderName string + CloseProviderProvider ResourceProvider + ProviderInputCalled bool ProviderInputName string ProviderInputConfig map[string]interface{} @@ -55,6 +59,10 @@ type MockEvalContext struct { ProvisionerName string ProvisionerProvisioner ResourceProvisioner + CloseProvisionerCalled bool + CloseProvisionerName string + CloseProvisionerProvisioner ResourceProvisioner + InterpolateCalled bool InterpolateConfig *config.RawConfig InterpolateResource *Resource @@ -105,6 +113,12 @@ func (c *MockEvalContext) Provider(n string) ResourceProvider { return c.ProviderProvider } +func (c *MockEvalContext) CloseProvider(n string) error { + c.CloseProviderCalled = true + c.CloseProviderName = n + return nil +} + func (c *MockEvalContext) ConfigureProvider(n string, cfg *ResourceConfig) error { c.ConfigureProviderCalled = true c.ConfigureProviderName = n @@ -150,6 +164,12 @@ func (c *MockEvalContext) Provisioner(n string) ResourceProvisioner { return c.ProvisionerProvisioner } +func (c *MockEvalContext) CloseProvisioner(n string) error { + c.CloseProvisionerCalled = true + c.CloseProvisionerName = n + return nil +} + func (c *MockEvalContext) Interpolate( config *config.RawConfig, resource *Resource) (*ResourceConfig, error) { c.InterpolateCalled = true diff --git a/terraform/eval_provider.go b/terraform/eval_provider.go index e5205a556d..61efcc2352 100644 --- a/terraform/eval_provider.go +++ b/terraform/eval_provider.go @@ -71,6 +71,17 @@ func (n *EvalInitProvider) Eval(ctx EvalContext) (interface{}, error) { return ctx.InitProvider(n.Name) } +// EvalCloseProvider is an EvalNode implementation that closes provider +// connections that aren't needed anymore. +type EvalCloseProvider struct { + Name string +} + +func (n *EvalCloseProvider) Eval(ctx EvalContext) (interface{}, error) { + ctx.CloseProvider(n.Name) + return nil, nil +} + // EvalGetProvider is an EvalNode implementation that retrieves an already // initialized provider instance for the given name. type EvalGetProvider struct { diff --git a/terraform/eval_provider_test.go b/terraform/eval_provider_test.go index 5d50d746ba..5719b62c59 100644 --- a/terraform/eval_provider_test.go +++ b/terraform/eval_provider_test.go @@ -112,6 +112,22 @@ func TestEvalInitProvider(t *testing.T) { } } +func TestEvalCloseProvider(t *testing.T) { + n := &EvalCloseProvider{Name: "foo"} + provider := &MockResourceProvider{} + ctx := &MockEvalContext{CloseProviderProvider: provider} + if _, err := n.Eval(ctx); err != nil { + t.Fatalf("err: %s", err) + } + + if !ctx.CloseProviderCalled { + t.Fatal("should be called") + } + if ctx.CloseProviderName != "foo" { + t.Fatalf("bad: %#v", ctx.CloseProviderName) + } +} + func TestEvalGetProvider_impl(t *testing.T) { var _ EvalNode = new(EvalGetProvider) } diff --git a/terraform/eval_provisioner.go b/terraform/eval_provisioner.go index 362a26e845..89579c0557 100644 --- a/terraform/eval_provisioner.go +++ b/terraform/eval_provisioner.go @@ -15,6 +15,17 @@ func (n *EvalInitProvisioner) Eval(ctx EvalContext) (interface{}, error) { return ctx.InitProvisioner(n.Name) } +// EvalCloseProvisioner is an EvalNode implementation that closes provisioner +// connections that aren't needed anymore. +type EvalCloseProvisioner struct { + Name string +} + +func (n *EvalCloseProvisioner) Eval(ctx EvalContext) (interface{}, error) { + ctx.CloseProvisioner(n.Name) + return nil, nil +} + // EvalGetProvisioner is an EvalNode implementation that retrieves an already // initialized provisioner instance for the given name. type EvalGetProvisioner struct { diff --git a/terraform/eval_provisioner_test.go b/terraform/eval_provisioner_test.go index 1225c5abb7..390bc364d7 100644 --- a/terraform/eval_provisioner_test.go +++ b/terraform/eval_provisioner_test.go @@ -24,6 +24,22 @@ func TestEvalInitProvisioner(t *testing.T) { } } +func TestEvalCloseProvisioner(t *testing.T) { + n := &EvalCloseProvisioner{Name: "foo"} + provisioner := &MockResourceProvisioner{} + ctx := &MockEvalContext{CloseProvisionerProvisioner: provisioner} + if _, err := n.Eval(ctx); err != nil { + t.Fatalf("err: %s", err) + } + + if !ctx.CloseProvisionerCalled { + t.Fatal("should be called") + } + if ctx.CloseProvisionerName != "foo" { + t.Fatalf("bad: %#v", ctx.CloseProvisionerName) + } +} + func TestEvalGetProvisioner_impl(t *testing.T) { var _ EvalNode = new(EvalGetProvisioner) } diff --git a/terraform/evaltree_provider.go b/terraform/evaltree_provider.go index 59916d9b5e..6b26d43e95 100644 --- a/terraform/evaltree_provider.go +++ b/terraform/evaltree_provider.go @@ -72,3 +72,9 @@ func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { return &EvalSequence{Nodes: seq} } + +// CloseProviderEvalTree returns the evaluation tree for closing +// provider connections that aren't needed anymore. +func CloseProviderEvalTree(n string) EvalNode { + return &EvalCloseProvider{Name: n} +} diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index 28a76e5138..061edb599e 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -116,12 +116,14 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { // Provider-related transformations &MissingProviderTransformer{Providers: b.Providers}, &ProviderTransformer{}, + &CloseProviderTransformer{}, &PruneProviderTransformer{}, &DisableProviderTransformer{}, // Provisioner-related transformations &MissingProvisionerTransformer{Provisioners: b.Provisioners}, &ProvisionerTransformer{}, + &CloseProvisionerTransformer{}, &PruneProvisionerTransformer{}, // Run our vertex-level transforms diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index 1f7f250558..ee6a5e140c 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -196,6 +196,8 @@ aws_instance.db aws_instance.web aws_instance.db provider.aws +provider.aws (close) + aws_instance.web ` const testBuiltinGraphBuilderVerboseStr = ` @@ -213,8 +215,28 @@ aws_instance.web (destroy tainted) aws_instance.web (destroy) provider.aws provider.aws +provider.aws (close) + aws_instance.web ` +const testBuiltinGraphBuilderMultiLevelStr = ` +module.foo.module.bar.output.value + module.foo.module.bar.var.bar +module.foo.module.bar.plan-destroy +module.foo.module.bar.var.bar + module.foo.var.foo +module.foo.plan-destroy +module.foo.var.foo +root + module.foo.module.bar.output.value + module.foo.module.bar.plan-destroy + module.foo.plan-destroy +` + +/* +TODO: Commented out this const as it's likely this needs to +be updated when the TestBuiltinGraphBuilder_modules test is +enabled again. const testBuiltinGraphBuilderModuleStr = ` aws_instance.web aws_instance.web (destroy) @@ -231,17 +253,4 @@ module.consul (expanded) provider.aws provider.aws ` - -const testBuiltinGraphBuilderMultiLevelStr = ` -module.foo.module.bar.output.value - module.foo.module.bar.var.bar -module.foo.module.bar.plan-destroy -module.foo.module.bar.var.bar - module.foo.var.foo -module.foo.plan-destroy -module.foo.var.foo -root - module.foo.module.bar.output.value - module.foo.module.bar.plan-destroy - module.foo.plan-destroy -` +*/ diff --git a/terraform/resource_provider.go b/terraform/resource_provider.go index d8705232d0..ea23b031d3 100644 --- a/terraform/resource_provider.go +++ b/terraform/resource_provider.go @@ -71,6 +71,12 @@ type ResourceProvider interface { Refresh(*InstanceInfo, *InstanceState) (*InstanceState, error) } +// ResourceProviderCloser is an interface that providers that can close +// connections that aren't needed anymore must implement. +type ResourceProviderCloser interface { + Close() error +} + // ResourceType is a type of resource that a resource provider can manage. type ResourceType struct { Name string diff --git a/terraform/resource_provisioner.go b/terraform/resource_provisioner.go index acc80d054a..3327e30007 100644 --- a/terraform/resource_provisioner.go +++ b/terraform/resource_provisioner.go @@ -23,6 +23,12 @@ type ResourceProvisioner interface { Apply(UIOutput, *InstanceState, *ResourceConfig) error } +// ResourceProvisionerCloser is an interface that provisioners that can close +// connections that aren't needed anymore must implement. +type ResourceProvisionerCloser interface { + Close() error +} + // ResourceProvisionerFactory is a function type that creates a new instance // of a resource provisioner. type ResourceProvisionerFactory func() (ResourceProvisioner, error) diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 4863a4c987..3a017fee24 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -17,6 +17,13 @@ type GraphNodeProvider interface { ProviderConfig() *config.RawConfig } +// GraphNodeCloseProvider is an interface that nodes that can be a close +// provider must implement. The CloseProviderName returned is the name of +// the provider they satisfy. +type GraphNodeCloseProvider interface { + CloseProviderName() string +} + // GraphNodeProviderConsumer is an interface that nodes that require // a provider must implement. ProvidedBy must return the name of the provider // to use. @@ -98,6 +105,37 @@ func (t *ProviderTransformer) Transform(g *Graph) error { return err } +// CloseProviderTransformer is a GraphTransformer that adds nodes to the +// graph that will close open provider connections that aren't needed anymore. +// A provider connection is not needed anymore once all depended resources +// in the graph are evaluated. +type CloseProviderTransformer struct{} + +func (t *CloseProviderTransformer) Transform(g *Graph) error { + m := closeProviderVertexMap(g) + for _, v := range g.Vertices() { + if pv, ok := v.(GraphNodeProviderConsumer); ok { + for _, p := range pv.ProvidedBy() { + source := m[p] + + if source == nil { + // Create a new graphNodeCloseProvider and add it to the graph + source = &graphNodeCloseProvider{ProviderNameValue: p} + g.Add(source) + + // Make sure we also add the new graphNodeCloseProvider to the map + // so we don't create and add any duplicate graphNodeCloseProviders. + m[p] = source + } + + g.Connect(dag.BasicEdge(source, v)) + } + } + } + + return nil +} + // MissingProviderTransformer is a GraphTransformer that adds nodes // for missing providers into the graph. Specifically, it creates provider // configuration nodes for all the providers that we support. These are @@ -143,6 +181,28 @@ func (t *PruneProviderTransformer) Transform(g *Graph) error { return nil } +func providerVertexMap(g *Graph) map[string]dag.Vertex { + m := make(map[string]dag.Vertex) + for _, v := range g.Vertices() { + if pv, ok := v.(GraphNodeProvider); ok { + m[pv.ProviderName()] = v + } + } + + return m +} + +func closeProviderVertexMap(g *Graph) map[string]dag.Vertex { + m := make(map[string]dag.Vertex) + for _, v := range g.Vertices() { + if pv, ok := v.(GraphNodeCloseProvider); ok { + m[pv.CloseProviderName()] = v + } + } + + return m +} + type graphNodeDisabledProvider struct { GraphNodeProvider } @@ -258,6 +318,94 @@ func (n *graphNodeDisabledProviderFlat) DependentOn() []string { return result } +type graphNodeCloseProvider struct { + ProviderNameValue string +} + +func (n *graphNodeCloseProvider) Name() string { + return fmt.Sprintf("provider.%s (close)", n.ProviderNameValue) +} + +// GraphNodeEvalable impl. +func (n *graphNodeCloseProvider) EvalTree() EvalNode { + return CloseProviderEvalTree(n.ProviderNameValue) +} + +// GraphNodeDependable impl. +func (n *graphNodeCloseProvider) DependableName() []string { + return []string{n.Name()} +} + +func (n *graphNodeCloseProvider) CloseProviderName() string { + return n.ProviderNameValue +} + +// GraphNodeDotter impl. +func (n *graphNodeCloseProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { + return dot.NewNode(name, map[string]string{ + "label": n.Name(), + "shape": "diamond", + }) +} + +// GraphNodeDotterOrigin impl. +func (n *graphNodeCloseProvider) DotOrigin() bool { + return true +} + +// GraphNodeFlattenable impl. +func (n *graphNodeCloseProvider) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeCloseProviderFlat{ + graphNodeCloseProvider: n, + PathValue: p, + }, nil +} + +// Same as graphNodeCloseProvider, but for flattening +type graphNodeCloseProviderFlat struct { + *graphNodeCloseProvider + + PathValue []string +} + +func (n *graphNodeCloseProviderFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeCloseProvider.Name()) +} + +func (n *graphNodeCloseProviderFlat) Path() []string { + return n.PathValue +} + +func (n *graphNodeCloseProviderFlat) ProviderName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.graphNodeCloseProvider.CloseProviderName()) +} + +// GraphNodeDependable impl. +func (n *graphNodeCloseProviderFlat) DependableName() []string { + return []string{n.Name()} +} + +func (n *graphNodeCloseProviderFlat) DependentOn() []string { + var result []string + + // If we're in a module, then depend on our parent's provider + if len(n.PathValue) > 1 { + prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1]) + if prefix != "" { + prefix += "." + } + + result = append(result, fmt.Sprintf( + "%s%s", + prefix, n.graphNodeCloseProvider.Name())) + } + + return result +} + type graphNodeMissingProvider struct { ProviderNameValue string } @@ -305,17 +453,6 @@ func (n *graphNodeMissingProvider) Flatten(p []string) (dag.Vertex, error) { }, nil } -func providerVertexMap(g *Graph) map[string]dag.Vertex { - m := make(map[string]dag.Vertex) - for _, v := range g.Vertices() { - if pv, ok := v.(GraphNodeProvider); ok { - m[pv.ProviderName()] = v - } - } - - return m -} - // Same as graphNodeMissingProvider, but for flattening type graphNodeMissingProviderFlat struct { *graphNodeMissingProvider diff --git a/terraform/transform_provider_test.go b/terraform/transform_provider_test.go index fdf764c3f9..63ddfba0eb 100644 --- a/terraform/transform_provider_test.go +++ b/terraform/transform_provider_test.go @@ -30,6 +30,38 @@ func TestProviderTransformer(t *testing.T) { } } +func TestCloseProviderTransformer(t *testing.T) { + mod := testModule(t, "transform-provider-basic") + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + transform := &ProviderTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + transform := &CloseProviderTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformCloseProviderBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestMissingProviderTransformer(t *testing.T) { mod := testModule(t, "transform-provider-basic") @@ -41,9 +73,18 @@ func TestMissingProviderTransformer(t *testing.T) { } } - transform := &MissingProviderTransformer{Providers: []string{"foo"}} - if err := transform.Transform(&g); err != nil { - t.Fatalf("err: %s", err) + { + transform := &MissingProviderTransformer{Providers: []string{"foo"}} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + transform := &CloseProviderTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } } actual := strings.TrimSpace(g.String()) @@ -78,6 +119,13 @@ func TestPruneProviderTransformer(t *testing.T) { } } + { + transform := &CloseProviderTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + { transform := &PruneProviderTransformer{} if err := transform.Transform(&g); err != nil { @@ -117,6 +165,13 @@ func TestDisableProviderTransformer(t *testing.T) { } } + { + transform := &CloseProviderTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + { transform := &PruneProviderTransformer{} if err := transform.Transform(&g); err != nil { @@ -163,6 +218,13 @@ func TestDisableProviderTransformer_keep(t *testing.T) { } } + { + transform := &CloseProviderTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + { transform := &PruneProviderTransformer{} if err := transform.Transform(&g); err != nil { @@ -203,9 +265,19 @@ aws_instance.web provider.aws ` +const testTransformCloseProviderBasicStr = ` +aws_instance.web + provider.aws +provider.aws +provider.aws (close) + aws_instance.web +` + const testTransformMissingProviderBasicStr = ` aws_instance.web provider.aws +provider.aws (close) + aws_instance.web provider.foo ` @@ -213,12 +285,16 @@ const testTransformPruneProviderBasicStr = ` foo_instance.web provider.foo provider.foo +provider.foo (close) + foo_instance.web ` const testTransformDisableProviderBasicStr = ` module.child provider.aws (disabled) var.foo +provider.aws (close) + module.child provider.aws (disabled) var.foo ` @@ -230,5 +306,8 @@ module.child provider.aws var.foo provider.aws +provider.aws (close) + aws_instance.foo + module.child var.foo ` diff --git a/terraform/transform_provisioner.go b/terraform/transform_provisioner.go index bf661f88a4..a1107a1214 100644 --- a/terraform/transform_provisioner.go +++ b/terraform/transform_provisioner.go @@ -14,6 +14,13 @@ type GraphNodeProvisioner interface { ProvisionerName() string } +// GraphNodeCloseProvisioner is an interface that nodes that can be a close +// provisioner must implement. The CloseProvisionerName returned is the name +// of the provisioner they satisfy. +type GraphNodeCloseProvisioner interface { + CloseProvisionerName() string +} + // GraphNodeProvisionerConsumer is an interface that nodes that require // a provisioner must implement. ProvisionedBy must return the name of the // provisioner to use. @@ -49,6 +56,37 @@ func (t *ProvisionerTransformer) Transform(g *Graph) error { return err } +// CloseProvisionerTransformer is a GraphTransformer that adds nodes to the +// graph that will close open provisioner connections that aren't needed +// anymore. A provisioner connection is not needed anymore once all depended +// resources in the graph are evaluated. +type CloseProvisionerTransformer struct{} + +func (t *CloseProvisionerTransformer) Transform(g *Graph) error { + m := closeProvisionerVertexMap(g) + for _, v := range g.Vertices() { + if pv, ok := v.(GraphNodeProvisionerConsumer); ok { + for _, p := range pv.ProvisionedBy() { + source := m[p] + + if source == nil { + // Create a new graphNodeCloseProvisioner and add it to the graph + source = &graphNodeCloseProvisioner{ProvisionerNameValue: p} + g.Add(source) + + // Make sure we also add the new graphNodeCloseProvisioner to the map + // so we don't create and add any duplicate graphNodeCloseProvisioners. + m[p] = source + } + + g.Connect(dag.BasicEdge(source, v)) + } + } + } + + return nil +} + // MissingProvisionerTransformer is a GraphTransformer that adds nodes // for missing provisioners into the graph. Specifically, it creates provisioner // configuration nodes for all the provisioners that we support. These are @@ -94,6 +132,75 @@ func (t *PruneProvisionerTransformer) Transform(g *Graph) error { return nil } +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 { + m[pv.ProvisionerName()] = v + } + } + + return m +} + +func closeProvisionerVertexMap(g *Graph) map[string]dag.Vertex { + m := make(map[string]dag.Vertex) + for _, v := range g.Vertices() { + if pv, ok := v.(GraphNodeCloseProvisioner); ok { + m[pv.CloseProvisionerName()] = v + } + } + + return m +} + +type graphNodeCloseProvisioner struct { + ProvisionerNameValue string +} + +func (n *graphNodeCloseProvisioner) Name() string { + return fmt.Sprintf("provisioner.%s (close)", n.ProvisionerNameValue) +} + +// GraphNodeEvalable impl. +func (n *graphNodeCloseProvisioner) EvalTree() EvalNode { + return &EvalCloseProvisioner{Name: n.ProvisionerNameValue} +} + +func (n *graphNodeCloseProvisioner) CloseProvisionerName() string { + return n.ProvisionerNameValue +} + +// GraphNodeFlattenable impl. +func (n *graphNodeCloseProvisioner) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeCloseProvisionerFlat{ + graphNodeCloseProvisioner: n, + PathValue: p, + }, nil +} + +// Same as graphNodeCloseProvisioner, but for flattening +type graphNodeCloseProvisionerFlat struct { + *graphNodeCloseProvisioner + + PathValue []string +} + +func (n *graphNodeCloseProvisionerFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeCloseProvisioner.Name()) +} + +func (n *graphNodeCloseProvisionerFlat) Path() []string { + return n.PathValue +} + +func (n *graphNodeCloseProvisionerFlat) ProvisionerName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.graphNodeCloseProvisioner.CloseProvisionerName()) +} + type graphNodeMissingProvisioner struct { ProvisionerNameValue string } @@ -119,17 +226,6 @@ func (n *graphNodeMissingProvisioner) Flatten(p []string) (dag.Vertex, error) { }, nil } -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 { - m[pv.ProvisionerName()] = v - } - } - - return m -} - // Same as graphNodeMissingProvisioner, but for flattening type graphNodeMissingProvisionerFlat struct { *graphNodeMissingProvisioner diff --git a/terraform/transform_provisioner_test.go b/terraform/transform_provisioner_test.go index f38a0a0452..ea5faff770 100644 --- a/terraform/transform_provisioner_test.go +++ b/terraform/transform_provisioner_test.go @@ -18,9 +18,18 @@ func TestMissingProvisionerTransformer(t *testing.T) { } } - transform := &MissingProvisionerTransformer{Provisioners: []string{"foo"}} - if err := transform.Transform(&g); err != nil { - t.Fatalf("err: %s", err) + { + transform := &MissingProvisionerTransformer{Provisioners: []string{"foo"}} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + transform := &CloseProvisionerTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } } actual := strings.TrimSpace(g.String()) @@ -56,6 +65,13 @@ func TestPruneProvisionerTransformer(t *testing.T) { } } + { + transform := &CloseProvisionerTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + { transform := &PruneProvisionerTransformer{} if err := transform.Transform(&g); err != nil { @@ -86,10 +102,14 @@ func TestGraphNodeMissingProvisioner_ProvisionerName(t *testing.T) { const testTransformMissingProvisionerBasicStr = ` aws_instance.web provisioner.foo +provisioner.shell (close) + aws_instance.web ` const testTransformPruneProvisionerBasicStr = ` aws_instance.web provisioner.foo provisioner.foo +provisioner.foo (close) + aws_instance.web `