Merge pull request hashicorp#27549 from hashicorp/jbardin/provisioner…

…-lifecycle

Provisioner lifecycle

builtin/provisioners/file/resource_provisioner.go

@@ -181,6 +181,5 @@ func (p *provisioner) Stop() error {
 }
 
 func (p *provisioner) Close() error {
-	p.cancel()
 	return nil
 }

builtin/provisioners/local-exec/resource_provisioner.go

@@ -181,7 +181,6 @@ func (p *provisioner) Stop() error {
 }
 
 func (p *provisioner) Close() error {
-	p.cancel()
 	return nil
 }
 

builtin/provisioners/remote-exec/resource_provisioner.go

@@ -116,7 +116,6 @@ func (p *provisioner) Stop() error {
 }
 
 func (p *provisioner) Close() error {
-	p.cancel()
 	return nil
 }
 

command/e2etest/provisioner_test.go

@@ -0,0 +1,44 @@
+package e2etest
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/e2e"
+)
+
+// TestProviderDevOverrides is a test that terraform can execute a 3rd party
+// provisioner plugin.
+func TestProvisioner(t *testing.T) {
+	t.Parallel()
+
+	// This test reaches out to releases.hashicorp.com to download the
+	// template and null providers, so it can only run if network access is
+	// allowed.
+	skipIfCannotAccessNetwork(t)
+
+	tf := e2e.NewBinary(terraformBin, "testdata/provisioner")
+	defer tf.Close()
+
+	//// INIT
+	_, stderr, err := tf.Run("init")
+	if err != nil {
+		t.Fatalf("unexpected init error: %s\nstderr:\n%s", err, stderr)
+	}
+
+	//// PLAN
+	_, stderr, err = tf.Run("plan", "-out=tfplan")
+	if err != nil {
+		t.Fatalf("unexpected plan error: %s\nstderr:\n%s", err, stderr)
+	}
+
+	//// APPLY
+	stdout, stderr, err := tf.Run("apply", "tfplan")
+	if err != nil {
+		t.Fatalf("unexpected apply error: %s\nstderr:\n%s", err, stderr)
+	}
+
+	if !strings.Contains(stdout, "HelloProvisioner") {
+		t.Fatalf("missing provisioner output:\n%s", stdout)
+	}
+}

command/e2etest/testdata/provisioner/main.tf

@@ -0,0 +1,5 @@
+resource "null_resource" "a" {
+  provisioner "local-exec" {
+    command = "echo HelloProvisioner"
+  }
+}

terraform/context_apply_test.go

@@ -9801,22 +9801,25 @@ func TestContext2Apply_plannedConnectionRefs(t *testing.T) {
 		return resp
 	}
 
-	pr := testProvisioner()
-	pr.ProvisionResourceFn = func(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
-		host := req.Connection.GetAttr("host")
-		if host.IsNull() || !host.IsKnown() {
-			resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("invalid host value: %#v", host))
-		}
+	provisionerFactory := func() (provisioners.Interface, error) {
+		pr := testProvisioner()
+		pr.ProvisionResourceFn = func(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
+			host := req.Connection.GetAttr("host")
+			if host.IsNull() || !host.IsKnown() {
+				resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("invalid host value: %#v", host))
+			}
 
-		return resp
+			return resp
+		}
+		return pr, nil
 	}
 
 	Providers := map[addrs.Provider]providers.Factory{
 		addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
 	}
 
 	provisioners := map[string]provisioners.Factory{
-		"shell": testProvisionerFuncFixed(pr),
+		"shell": provisionerFactory,
 	}
 
 	hook := &testHook{}
@@ -12163,6 +12166,7 @@ output "out" {
 func TestContext2Apply_provisionerSensitive(t *testing.T) {
 	m := testModule(t, "apply-provisioner-sensitive")
 	p := testProvider("aws")
+
 	pr := testProvisioner()
 	pr.ProvisionResourceFn = func(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
 		if req.Config.ContainsMarked() {
@@ -12201,6 +12205,9 @@ func TestContext2Apply_provisionerSensitive(t *testing.T) {
 		t.Fatal("plan failed")
 	}
 
+	// "restart" provisioner
+	pr.CloseCalled = false
+
 	state, diags := ctx.Apply()
 	if diags.HasErrors() {
 		logDiagnostics(t, diags)

terraform/eval_context.go

@@ -77,22 +77,16 @@ type EvalContext interface {
 	ProviderInput(addrs.AbsProviderConfig) map[string]cty.Value
 	SetProviderInput(addrs.AbsProviderConfig, map[string]cty.Value)
 
-	// InitProvisioner initializes the provisioner with the given name.
-	// It is an error to initialize the same provisioner more than once.
-	InitProvisioner(string) error
-
-	// Provisioner gets the provisioner instance with the given name (already
-	// initialized) or returns nil if the provisioner isn't initialized.
-	Provisioner(string) provisioners.Interface
+	// Provisioner gets the provisioner instance with the given name.
+	Provisioner(string) (provisioners.Interface, error)
 
 	// ProvisionerSchema retrieves the main configuration schema for a
 	// particular provisioner, which must have already been initialized with
 	// InitProvisioner.
 	ProvisionerSchema(string) *configschema.Block
 
-	// CloseProvisioner closes provisioner connections that aren't needed
-	// anymore.
-	CloseProvisioner(string) error
+	// CloseProvisioner closes all provisioner plugins.
+	CloseProvisioners() error
 
 	// EvaluateBlock takes the given raw configuration block and associated
 	// schema and evaluates it to produce a value of an object type that

terraform/eval_context_builtin.go

@@ -228,48 +228,41 @@ func (ctx *BuiltinEvalContext) SetProviderInput(pc addrs.AbsProviderConfig, c ma
 	ctx.ProviderLock.Unlock()
 }
 
-func (ctx *BuiltinEvalContext) InitProvisioner(n string) error {
-	// If we already initialized, it is an error
-	if p := ctx.Provisioner(n); p != nil {
-		return fmt.Errorf("Provisioner '%s' already initialized", n)
-	}
-
-	// Warning: make sure to acquire these locks AFTER the call to Provisioner
-	// above, since it also acquires locks.
+func (ctx *BuiltinEvalContext) Provisioner(n string) (provisioners.Interface, error) {
 	ctx.ProvisionerLock.Lock()
 	defer ctx.ProvisionerLock.Unlock()
 
-	p, err := ctx.Components.ResourceProvisioner(n)
-	if err != nil {
-		return err
-	}
-
-	ctx.ProvisionerCache[n] = p
-
-	return nil
-}
+	p, ok := ctx.ProvisionerCache[n]
+	if !ok {
+		var err error
+		p, err = ctx.Components.ResourceProvisioner(n)
+		if err != nil {
+			return nil, err
+		}
 
-func (ctx *BuiltinEvalContext) Provisioner(n string) provisioners.Interface {
-	ctx.ProvisionerLock.Lock()
-	defer ctx.ProvisionerLock.Unlock()
+		ctx.ProvisionerCache[n] = p
+	}
 
-	return ctx.ProvisionerCache[n]
+	return p, nil
 }
 
 func (ctx *BuiltinEvalContext) ProvisionerSchema(n string) *configschema.Block {
 	return ctx.Schemas.ProvisionerConfig(n)
 }
 
-func (ctx *BuiltinEvalContext) CloseProvisioner(n string) error {
+func (ctx *BuiltinEvalContext) CloseProvisioners() error {
+	var diags tfdiags.Diagnostics
 	ctx.ProvisionerLock.Lock()
 	defer ctx.ProvisionerLock.Unlock()
 
-	prov := ctx.ProvisionerCache[n]
-	if prov != nil {
-		return prov.Close()
+	for name, prov := range ctx.ProvisionerCache {
+		err := prov.Close()
+		if err != nil {
+			diags = diags.Append(fmt.Errorf("provisioner.Close %s: %s", name, err))
+		}
 	}
 
-	return nil
+	return diags.Err()
 }
 
 func (ctx *BuiltinEvalContext) EvaluateBlock(body hcl.Body, schema *configschema.Block, self addrs.Referenceable, keyData InstanceKeyEvalData) (cty.Value, hcl.Body, tfdiags.Diagnostics) {

terraform/eval_context_mock.go

@@ -63,11 +63,6 @@ type MockEvalContext struct {
 	ConfigureProviderConfig cty.Value
 	ConfigureProviderDiags  tfdiags.Diagnostics
 
-	InitProvisionerCalled      bool
-	InitProvisionerName        string
-	InitProvisionerProvisioner provisioners.Interface
-	InitProvisionerError       error
-
 	ProvisionerCalled      bool
 	ProvisionerName        string
 	ProvisionerProvisioner provisioners.Interface
@@ -76,9 +71,7 @@ type MockEvalContext struct {
 	ProvisionerSchemaName   string
 	ProvisionerSchemaSchema *configschema.Block
 
-	CloseProvisionerCalled      bool
-	CloseProvisionerName        string
-	CloseProvisionerProvisioner provisioners.Interface
+	CloseProvisionersCalled bool
 
 	EvaluateBlockCalled     bool
 	EvaluateBlockBody       hcl.Body
@@ -208,16 +201,10 @@ func (c *MockEvalContext) SetProviderInput(addr addrs.AbsProviderConfig, vals ma
 	c.SetProviderInputValues = vals
 }
 
-func (c *MockEvalContext) InitProvisioner(n string) error {
-	c.InitProvisionerCalled = true
-	c.InitProvisionerName = n
-	return c.InitProvisionerError
-}
-
-func (c *MockEvalContext) Provisioner(n string) provisioners.Interface {
+func (c *MockEvalContext) Provisioner(n string) (provisioners.Interface, error) {
 	c.ProvisionerCalled = true
 	c.ProvisionerName = n
-	return c.ProvisionerProvisioner
+	return c.ProvisionerProvisioner, nil
 }
 
 func (c *MockEvalContext) ProvisionerSchema(n string) *configschema.Block {
@@ -226,9 +213,8 @@ func (c *MockEvalContext) ProvisionerSchema(n string) *configschema.Block {
 	return c.ProvisionerSchemaSchema
 }
 
-func (c *MockEvalContext) CloseProvisioner(n string) error {
-	c.CloseProvisionerCalled = true
-	c.CloseProvisionerName = n
+func (c *MockEvalContext) CloseProvisioners() error {
+	c.CloseProvisionersCalled = true
 	return nil
 }
 

terraform/graph_builder_apply.go

@@ -108,10 +108,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Provisioner-related transformations
-		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
-		&ProvisionerTransformer{},
-
 		// add providers
 		TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config),
 
@@ -162,7 +158,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 
 		// Close opened plugin connections
 		&CloseProviderTransformer{},
-		&CloseProvisionerTransformer{},
 
 		// close the root module
 		&CloseRootModuleTransformer{},

terraform/graph_builder_apply_test.go

@@ -423,70 +423,6 @@ func TestApplyGraphBuilder_moduleDestroy(t *testing.T) {
 	)
 }
 
-func TestApplyGraphBuilder_provisioner(t *testing.T) {
-	changes := &plans.Changes{
-		Resources: []*plans.ResourceInstanceChangeSrc{
-			{
-				Addr: mustResourceInstanceAddr("test_object.foo"),
-				ChangeSrc: plans.ChangeSrc{
-					Action: plans.Create,
-				},
-			},
-		},
-	}
-
-	b := &ApplyGraphBuilder{
-		Config:     testModule(t, "graph-builder-apply-provisioner"),
-		Changes:    changes,
-		Components: simpleMockComponentFactory(),
-		Schemas:    simpleTestSchemas(),
-	}
-
-	g, err := b.Build(addrs.RootModuleInstance)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	testGraphContains(t, g, "provisioner.test")
-	testGraphHappensBefore(
-		t, g,
-		"provisioner.test",
-		"test_object.foo",
-	)
-}
-
-func TestApplyGraphBuilder_provisionerDestroy(t *testing.T) {
-	changes := &plans.Changes{
-		Resources: []*plans.ResourceInstanceChangeSrc{
-			{
-				Addr: mustResourceInstanceAddr("test_object.foo"),
-				ChangeSrc: plans.ChangeSrc{
-					Action: plans.Delete,
-				},
-			},
-		},
-	}
-
-	b := &ApplyGraphBuilder{
-		Config:     testModule(t, "graph-builder-apply-provisioner"),
-		Changes:    changes,
-		Components: simpleMockComponentFactory(),
-		Schemas:    simpleTestSchemas(),
-	}
-
-	g, err := b.Build(addrs.RootModuleInstance)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	testGraphContains(t, g, "provisioner.test")
-	testGraphHappensBefore(
-		t, g,
-		"provisioner.test",
-		"test_object.foo (destroy)",
-	)
-}
-
 func TestApplyGraphBuilder_targetModule(t *testing.T) {
 	changes := &plans.Changes{
 		Resources: []*plans.ResourceInstanceChangeSrc{
@@ -784,7 +720,6 @@ module.child.test_object.create
 module.child.test_object.create (expand)
   module.child (expand)
   provider["registry.terraform.io/hashicorp/test"]
-  provisioner.test
 module.child.test_object.other
   module.child.test_object.create
   module.child.test_object.other (expand)
@@ -795,13 +730,9 @@ provider["registry.terraform.io/hashicorp/test"]
 provider["registry.terraform.io/hashicorp/test"] (close)
   module.child.test_object.other
   test_object.other
-provisioner.test
-provisioner.test (close)
-  module.child.test_object.create
 root
   meta.count-boundary (EachMode fixup)
   provider["registry.terraform.io/hashicorp/test"] (close)
-  provisioner.test (close)
 test_object.create
   test_object.create (expand)
 test_object.create (expand)

terraform/graph_builder_plan.go

@@ -115,10 +115,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Provisioner-related transformations
-		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
-		&ProvisionerTransformer{},
-
 		// add providers
 		TransformProviders(b.Components.ResourceProviders(), b.ConcreteProvider, b.Config),