From f8c7b639c99590ac4be62871a03317c34a703101 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 11:33:26 -0800
Subject: [PATCH 01/23] terraform: switch to Context for stop, Stoppable
 provisioners

This switches to the Go "context" package for cancellation and threads
the context through all the way to evaluation to allow behavior based on
stopping deep within graph execution.

This also adds the Stop API to provisioners so they can quickly exit
when stop is called.
---
 terraform/context.go                          | 156 +++++++++++-------
 terraform/context_apply_test.go               |  63 +++++++
 terraform/context_import.go                   |   3 +-
 terraform/eval_context.go                     |   4 +
 terraform/eval_context_builtin.go             |  13 ++
 terraform/eval_context_mock.go                |   8 +
 terraform/graph_walk_context.go               |   7 +-
 terraform/resource_provisioner.go             |  20 +++
 terraform/resource_provisioner_mock.go        |  23 ++-
 terraform/shadow_context.go                   |   3 +-
 terraform/shadow_resource_provisioner.go      |  11 ++
 .../apply-cancel-provisioner/main.tf          |   7 +
 12 files changed, 252 insertions(+), 66 deletions(-)
 create mode 100644 terraform/test-fixtures/apply-cancel-provisioner/main.tf

diff --git a/terraform/context.go b/terraform/context.go
index 066ffa379..52b95f058 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"sort"
@@ -91,8 +92,9 @@ type Context struct {
 	l                   sync.Mutex // Lock acquired during any task
 	parallelSem         Semaphore
 	providerInputConfig map[string]map[string]interface{}
-	runCh               <-chan struct{}
-	stopCh              chan struct{}
+	runLock             sync.Mutex
+	runContext          context.Context
+	runContextCancel    context.CancelFunc
 	shadowErr           error
 }
 
@@ -339,8 +341,7 @@ func (c *Context) Interpolater() *Interpolater {
 // This modifies the configuration in-place, so asking for Input twice
 // may result in different UI output showing different current values.
 func (c *Context) Input(mode InputMode) error {
-	v := c.acquireRun("input")
-	defer c.releaseRun(v)
+	defer c.acquireRun("input")()
 
 	if mode&InputModeVar != 0 {
 		// Walk the variables first for the root module. We walk them in
@@ -459,8 +460,7 @@ func (c *Context) Input(mode InputMode) error {
 // In addition to returning the resulting state, this context is updated
 // with the latest state.
 func (c *Context) Apply() (*State, error) {
-	v := c.acquireRun("apply")
-	defer c.releaseRun(v)
+	defer c.acquireRun("apply")()
 
 	// Copy our own state
 	c.state = c.state.DeepCopy()
@@ -504,8 +504,7 @@ func (c *Context) Apply() (*State, error) {
 // Plan also updates the diff of this context to be the diff generated
 // by the plan, so Apply can be called after.
 func (c *Context) Plan() (*Plan, error) {
-	v := c.acquireRun("plan")
-	defer c.releaseRun(v)
+	defer c.acquireRun("plan")()
 
 	p := &Plan{
 		Module:  c.module,
@@ -600,8 +599,7 @@ func (c *Context) Plan() (*Plan, error) {
 // Even in the case an error is returned, the state will be returned and
 // will potentially be partially updated.
 func (c *Context) Refresh() (*State, error) {
-	v := c.acquireRun("refresh")
-	defer c.releaseRun(v)
+	defer c.acquireRun("refresh")()
 
 	// Copy our own state
 	c.state = c.state.DeepCopy()
@@ -635,29 +633,32 @@ func (c *Context) Refresh() (*State, error) {
 // Stop will block until the task completes.
 func (c *Context) Stop() {
 	c.l.Lock()
-	ch := c.runCh
 
-	// If we aren't running, then just return
-	if ch == nil {
-		c.l.Unlock()
-		return
+	// If we're running, then stop
+	if c.runContextCancel != nil {
+		// Tell the hook we want to stop
+		c.sh.Stop()
+
+		// Stop the context
+		c.runContextCancel()
+		c.runContextCancel = nil
 	}
 
-	// Tell the hook we want to stop
-	c.sh.Stop()
+	// Grab the context before we unlock
+	ctx := c.runContext
 
-	// Close the stop channel
-	close(c.stopCh)
-
-	// Wait for us to stop
+	// Unlock
 	c.l.Unlock()
-	<-ch
+
+	// Wait if we have a context
+	if ctx != nil {
+		<-ctx.Done()
+	}
 }
 
 // Validate validates the configuration and returns any warnings or errors.
 func (c *Context) Validate() ([]string, []error) {
-	v := c.acquireRun("validate")
-	defer c.releaseRun(v)
+	defer c.acquireRun("validate")()
 
 	var errs error
 
@@ -718,26 +719,26 @@ func (c *Context) SetVariable(k string, v interface{}) {
 	c.variables[k] = v
 }
 
-func (c *Context) acquireRun(phase string) chan<- struct{} {
+func (c *Context) acquireRun(phase string) func() {
+	// Acquire the runlock first. This is the lock that is held for
+	// the duration of a run to prevent multiple runs.
+	c.runLock.Lock()
+
+	// With the run lock held, grab the context lock to make changes
+	// to the run context.
 	c.l.Lock()
 	defer c.l.Unlock()
 
+	// Setup debugging
 	dbug.SetPhase(phase)
 
-	// Wait for no channel to exist
-	for c.runCh != nil {
-		c.l.Unlock()
-		ch := c.runCh
-		<-ch
-		c.l.Lock()
+	// runContext should never be non-nil, check that here
+	if c.runContext != nil {
+		panic("acquireRun called with runContext != nil")
 	}
 
-	// Create the new channel
-	ch := make(chan struct{})
-	c.runCh = ch
-
-	// Reset the stop channel so we can watch that
-	c.stopCh = make(chan struct{})
+	// Create a new run context
+	c.runContext, c.runContextCancel = context.WithCancel(context.Background())
 
 	// Reset the stop hook so we're not stopped
 	c.sh.Reset()
@@ -745,10 +746,11 @@ func (c *Context) acquireRun(phase string) chan<- struct{} {
 	// Reset the shadow errors
 	c.shadowErr = nil
 
-	return ch
+	return c.releaseRun
 }
 
-func (c *Context) releaseRun(ch chan<- struct{}) {
+func (c *Context) releaseRun() {
+	// Grab the context lock so that we can make modifications to fields
 	c.l.Lock()
 	defer c.l.Unlock()
 
@@ -757,9 +759,17 @@ func (c *Context) releaseRun(ch chan<- struct{}) {
 	// phase
 	dbug.SetPhase("INVALID")
 
-	close(ch)
-	c.runCh = nil
-	c.stopCh = nil
+	// End our run. We check if runContext is non-nil because it can be
+	// set to nil if it was cancelled via Stop()
+	if c.runContextCancel != nil {
+		c.runContextCancel()
+	}
+
+	// Unset the context
+	c.runContext = nil
+
+	// Unlock the run lock
+	c.runLock.Unlock()
 }
 
 func (c *Context) walk(
@@ -791,13 +801,14 @@ func (c *Context) walk(
 	log.Printf("[DEBUG] Starting graph walk: %s", operation.String())
 
 	walker := &ContextGraphWalker{
-		Context:   realCtx,
-		Operation: operation,
+		Context:     realCtx,
+		Operation:   operation,
+		StopContext: c.runContext,
 	}
 
 	// Watch for a stop so we can call the provider Stop() API.
 	doneCh := make(chan struct{})
-	go c.watchStop(walker, c.stopCh, doneCh)
+	go c.watchStop(walker, doneCh)
 
 	// Walk the real graph, this will block until it completes
 	realErr := graph.Walk(walker)
@@ -892,7 +903,15 @@ func (c *Context) walk(
 	return walker, realErr
 }
 
-func (c *Context) watchStop(walker *ContextGraphWalker, stopCh, doneCh <-chan struct{}) {
+func (c *Context) watchStop(walker *ContextGraphWalker, doneCh <-chan struct{}) {
+	// Get the stop channel. runContext might be nil only during tests.
+	// If this is called during a proper run operation, this will never
+	// be nil.
+	var stopCh <-chan struct{}
+	if ctx := c.runContext; ctx != nil {
+		stopCh = ctx.Done()
+	}
+
 	// Wait for a stop or completion
 	select {
 	case <-stopCh:
@@ -904,20 +923,39 @@ func (c *Context) watchStop(walker *ContextGraphWalker, stopCh, doneCh <-chan st
 
 	// If we're here, we're stopped, trigger the call.
 
-	// Copy the providers so that a misbehaved blocking Stop doesn't
-	// completely hang Terraform.
-	walker.providerLock.Lock()
-	ps := make([]ResourceProvider, 0, len(walker.providerCache))
-	for _, p := range walker.providerCache {
-		ps = append(ps, p)
-	}
-	defer walker.providerLock.Unlock()
+	{
+		// Copy the providers so that a misbehaved blocking Stop doesn't
+		// completely hang Terraform.
+		walker.providerLock.Lock()
+		ps := make([]ResourceProvider, 0, len(walker.providerCache))
+		for _, p := range walker.providerCache {
+			ps = append(ps, p)
+		}
+		defer walker.providerLock.Unlock()
 
-	for _, p := range ps {
-		// We ignore the error for now since there isn't any reasonable
-		// action to take if there is an error here, since the stop is still
-		// advisory: Terraform will exit once the graph node completes.
-		p.Stop()
+		for _, p := range ps {
+			// We ignore the error for now since there isn't any reasonable
+			// action to take if there is an error here, since the stop is still
+			// advisory: Terraform will exit once the graph node completes.
+			p.Stop()
+		}
+	}
+
+	{
+		// Call stop on all the provisioners
+		walker.provisionerLock.Lock()
+		ps := make([]ResourceProvisioner, 0, len(walker.provisionerCache))
+		for _, p := range walker.provisionerCache {
+			ps = append(ps, p)
+		}
+		defer walker.provisionerLock.Unlock()
+
+		for _, p := range ps {
+			// We ignore the error for now since there isn't any reasonable
+			// action to take if there is an error here, since the stop is still
+			// advisory: Terraform will exit once the graph node completes.
+			p.Stop()
+		}
 	}
 }
 
diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index a96ac1a98..20cfe7cd7 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -1720,6 +1720,69 @@ func TestContext2Apply_cancel(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_cancelProvisioner(t *testing.T) {
+	m := testModule(t, "apply-cancel-provisioner")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	pr := testProvisioner()
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+	})
+
+	prStopped := make(chan struct{})
+	pr.ApplyFn = func(rs *InstanceState, c *ResourceConfig) error {
+		// Start the stop process
+		go ctx.Stop()
+
+		<-prStopped
+		return nil
+	}
+	pr.StopFn = func() error {
+		close(prStopped)
+		return nil
+	}
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Start the Apply in a goroutine
+	var applyErr error
+	stateCh := make(chan *State)
+	go func() {
+		state, err := ctx.Apply()
+		if err != nil {
+			applyErr = err
+		}
+
+		stateCh <- state
+	}()
+
+	// Wait for completion
+	state := <-stateCh
+	if applyErr != nil {
+		t.Fatalf("err: %s", applyErr)
+	}
+
+	checkStateString(t, state, `
+aws_instance.foo: (tainted)
+  ID = foo
+  num = 2
+  type = aws_instance
+	`)
+
+	if !pr.StopCalled {
+		t.Fatal("stop should be called")
+	}
+}
+
 func TestContext2Apply_compute(t *testing.T) {
 	m := testModule(t, "apply-compute")
 	p := testProvider("aws")
diff --git a/terraform/context_import.go b/terraform/context_import.go
index afc9a43c0..f1d57760d 100644
--- a/terraform/context_import.go
+++ b/terraform/context_import.go
@@ -40,8 +40,7 @@ type ImportTarget struct {
 // imported.
 func (c *Context) Import(opts *ImportOpts) (*State, error) {
 	// Hold a lock since we can modify our own state here
-	v := c.acquireRun("import")
-	defer c.releaseRun(v)
+	defer c.acquireRun("import")()
 
 	// Copy our own state
 	c.state = c.state.DeepCopy()
diff --git a/terraform/eval_context.go b/terraform/eval_context.go
index f2867511d..a1f815b7d 100644
--- a/terraform/eval_context.go
+++ b/terraform/eval_context.go
@@ -8,6 +8,10 @@ import (
 
 // EvalContext is the interface that is given to eval nodes to execute.
 type EvalContext interface {
+	// Stopped returns a channel that is closed when evaluation is stopped
+	// via Terraform.Context.Stop()
+	Stopped() <-chan struct{}
+
 	// Path is the current module path.
 	Path() []string
 
diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go
index 032f79f9d..3dcfb2275 100644
--- a/terraform/eval_context_builtin.go
+++ b/terraform/eval_context_builtin.go
@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"strings"
@@ -12,6 +13,9 @@ import (
 // BuiltinEvalContext is an EvalContext implementation that is used by
 // Terraform by default.
 type BuiltinEvalContext struct {
+	// StopContext is the context used to track whether we're complete
+	StopContext context.Context
+
 	// PathValue is the Path that this context is operating within.
 	PathValue []string
 
@@ -43,6 +47,15 @@ type BuiltinEvalContext struct {
 	once sync.Once
 }
 
+func (ctx *BuiltinEvalContext) Stopped() <-chan struct{} {
+	// This can happen during tests. During tests, we just block forever.
+	if ctx.StopContext == nil {
+		return nil
+	}
+
+	return ctx.StopContext.Done()
+}
+
 func (ctx *BuiltinEvalContext) Hook(fn func(Hook) (HookAction, error)) error {
 	for _, h := range ctx.Hooks {
 		action, err := fn(h)
diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go
index 4f5c23bc4..4f90d5b12 100644
--- a/terraform/eval_context_mock.go
+++ b/terraform/eval_context_mock.go
@@ -9,6 +9,9 @@ import (
 // MockEvalContext is a mock version of EvalContext that can be used
 // for tests.
 type MockEvalContext struct {
+	StoppedCalled bool
+	StoppedValue  <-chan struct{}
+
 	HookCalled bool
 	HookHook   Hook
 	HookError  error
@@ -85,6 +88,11 @@ type MockEvalContext struct {
 	StateLock   *sync.RWMutex
 }
 
+func (c *MockEvalContext) Stopped() <-chan struct{} {
+	c.StoppedCalled = true
+	return c.StoppedValue
+}
+
 func (c *MockEvalContext) Hook(fn func(Hook) (HookAction, error)) error {
 	c.HookCalled = true
 	if c.HookHook != nil {
diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go
index 459fcdec9..19fd47ceb 100644
--- a/terraform/graph_walk_context.go
+++ b/terraform/graph_walk_context.go
@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"sync"
@@ -15,8 +16,9 @@ type ContextGraphWalker struct {
 	NullGraphWalker
 
 	// Configurable values
-	Context   *Context
-	Operation walkOperation
+	Context     *Context
+	Operation   walkOperation
+	StopContext context.Context
 
 	// Outputs, do not set these. Do not read these while the graph
 	// is being walked.
@@ -65,6 +67,7 @@ func (w *ContextGraphWalker) EnterPath(path []string) EvalContext {
 	w.interpolaterVarLock.Unlock()
 
 	ctx := &BuiltinEvalContext{
+		StopContext:         w.StopContext,
 		PathValue:           path,
 		Hooks:               w.Context.hooks,
 		InputValue:          w.Context.uiInput,
diff --git a/terraform/resource_provisioner.go b/terraform/resource_provisioner.go
index 3327e3000..361ec1ec0 100644
--- a/terraform/resource_provisioner.go
+++ b/terraform/resource_provisioner.go
@@ -21,6 +21,26 @@ type ResourceProvisioner interface {
 	// is provided since provisioners only run after a resource has been
 	// newly created.
 	Apply(UIOutput, *InstanceState, *ResourceConfig) error
+
+	// Stop is called when the provisioner should halt any in-flight actions.
+	//
+	// This can be used to make a nicer Ctrl-C experience for Terraform.
+	// Even if this isn't implemented to do anything (just returns nil),
+	// Terraform will still cleanly stop after the currently executing
+	// graph node is complete. However, this API can be used to make more
+	// efficient halts.
+	//
+	// Stop doesn't have to and shouldn't block waiting for in-flight actions
+	// to complete. It should take any action it wants and return immediately
+	// acknowledging it has received the stop request. Terraform core will
+	// automatically not make any further API calls to the provider soon
+	// after Stop is called (technically exactly once the currently executing
+	// graph nodes are complete).
+	//
+	// The error returned, if non-nil, is assumed to mean that signaling the
+	// stop somehow failed and that the user should expect potentially waiting
+	// a longer period of time.
+	Stop() error
 }
 
 // ResourceProvisionerCloser is an interface that provisioners that can close
diff --git a/terraform/resource_provisioner_mock.go b/terraform/resource_provisioner_mock.go
index be04e9814..f471a5182 100644
--- a/terraform/resource_provisioner_mock.go
+++ b/terraform/resource_provisioner_mock.go
@@ -21,6 +21,10 @@ type MockResourceProvisioner struct {
 	ValidateFn           func(c *ResourceConfig) ([]string, []error)
 	ValidateReturnWarns  []string
 	ValidateReturnErrors []error
+
+	StopCalled      bool
+	StopFn          func() error
+	StopReturnError error
 }
 
 func (p *MockResourceProvisioner) Validate(c *ResourceConfig) ([]string, []error) {
@@ -40,14 +44,29 @@ func (p *MockResourceProvisioner) Apply(
 	state *InstanceState,
 	c *ResourceConfig) error {
 	p.Lock()
-	defer p.Unlock()
 
 	p.ApplyCalled = true
 	p.ApplyOutput = output
 	p.ApplyState = state
 	p.ApplyConfig = c
 	if p.ApplyFn != nil {
-		return p.ApplyFn(state, c)
+		fn := p.ApplyFn
+		p.Unlock()
+		return fn(state, c)
 	}
+
+	defer p.Unlock()
 	return p.ApplyReturnError
 }
+
+func (p *MockResourceProvisioner) Stop() error {
+	p.Lock()
+	defer p.Unlock()
+
+	p.StopCalled = true
+	if p.StopFn != nil {
+		return p.StopFn()
+	}
+
+	return p.StopReturnError
+}
diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go
index 5e0e31609..5f7914328 100644
--- a/terraform/shadow_context.go
+++ b/terraform/shadow_context.go
@@ -88,7 +88,8 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) {
 		// l - no copy
 		parallelSem:         c.parallelSem,
 		providerInputConfig: c.providerInputConfig,
-		runCh:               c.runCh,
+		runContext:          c.runContext,
+		runContextCancel:    c.runContextCancel,
 		shadowErr:           c.shadowErr,
 	}
 
diff --git a/terraform/shadow_resource_provisioner.go b/terraform/shadow_resource_provisioner.go
index 6e405c09d..60a490889 100644
--- a/terraform/shadow_resource_provisioner.go
+++ b/terraform/shadow_resource_provisioner.go
@@ -112,6 +112,10 @@ func (p *shadowResourceProvisionerReal) Apply(
 	return err
 }
 
+func (p *shadowResourceProvisionerReal) Stop() error {
+	return p.ResourceProvisioner.Stop()
+}
+
 // shadowResourceProvisionerShadow is the shadow resource provisioner. Function
 // calls never affect real resources. This is paired with the "real" side
 // which must be called properly to enable recording.
@@ -228,6 +232,13 @@ func (p *shadowResourceProvisionerShadow) Apply(
 	return result.ResultErr
 }
 
+func (p *shadowResourceProvisionerShadow) Stop() error {
+	// For the shadow, we always just return nil since a Stop indicates
+	// that we were interrupted and shadows are disabled during interrupts
+	// anyways.
+	return nil
+}
+
 // The structs for the various function calls are put below. These structs
 // are used to carry call information across the real/shadow boundaries.
 
diff --git a/terraform/test-fixtures/apply-cancel-provisioner/main.tf b/terraform/test-fixtures/apply-cancel-provisioner/main.tf
new file mode 100644
index 000000000..dadabd882
--- /dev/null
+++ b/terraform/test-fixtures/apply-cancel-provisioner/main.tf
@@ -0,0 +1,7 @@
+resource "aws_instance" "foo" {
+    num = "2"
+
+    provisioner "shell" {
+        foo = "bar"
+    }
+}

From 2e894c4ef74381a81dc292c0395564f6e6ed36d3 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 11:38:26 -0800
Subject: [PATCH 02/23] plugin: add ResourceProvisioner.Stop API

---
 plugin/resource_provisioner.go      | 28 +++++++++++++++
 plugin/resource_provisioner_test.go | 55 +++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/plugin/resource_provisioner.go b/plugin/resource_provisioner.go
index 982309580..8fce9d8ae 100644
--- a/plugin/resource_provisioner.go
+++ b/plugin/resource_provisioner.go
@@ -77,6 +77,19 @@ func (p *ResourceProvisioner) Apply(
 	return err
 }
 
+func (p *ResourceProvisioner) Stop() error {
+	var resp ResourceProvisionerStopResponse
+	err := p.Client.Call("Plugin.Stop", new(interface{}), &resp)
+	if err != nil {
+		return err
+	}
+	if resp.Error != nil {
+		err = resp.Error
+	}
+
+	return err
+}
+
 func (p *ResourceProvisioner) Close() error {
 	return p.Client.Close()
 }
@@ -100,6 +113,10 @@ type ResourceProvisionerApplyResponse struct {
 	Error *plugin.BasicError
 }
 
+type ResourceProvisionerStopResponse struct {
+	Error *plugin.BasicError
+}
+
 // ResourceProvisionerServer is a net/rpc compatible structure for serving
 // a ResourceProvisioner. This should not be used directly.
 type ResourceProvisionerServer struct {
@@ -143,3 +160,14 @@ func (s *ResourceProvisionerServer) Validate(
 	}
 	return nil
 }
+
+func (s *ResourceProvisionerServer) Stop(
+	_ interface{},
+	reply *ResourceProvisionerStopResponse) error {
+	err := s.Provisioner.Stop()
+	*reply = ResourceProvisionerStopResponse{
+		Error: plugin.NewBasicError(err),
+	}
+
+	return nil
+}
diff --git a/plugin/resource_provisioner_test.go b/plugin/resource_provisioner_test.go
index 073c8d2b7..70ae1ee74 100644
--- a/plugin/resource_provisioner_test.go
+++ b/plugin/resource_provisioner_test.go
@@ -14,6 +14,61 @@ func TestResourceProvisioner_impl(t *testing.T) {
 	var _ terraform.ResourceProvisioner = new(ResourceProvisioner)
 }
 
+func TestResourceProvisioner_stop(t *testing.T) {
+	// Create a mock provider
+	p := new(terraform.MockResourceProvisioner)
+	client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{
+		ProvisionerFunc: testProvisionerFixed(p),
+	}))
+	defer client.Close()
+
+	// Request the provider
+	raw, err := client.Dispense(ProvisionerPluginName)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	provider := raw.(terraform.ResourceProvisioner)
+
+	// Stop
+	e := provider.Stop()
+	if !p.StopCalled {
+		t.Fatal("stop should be called")
+	}
+	if e != nil {
+		t.Fatalf("bad: %#v", e)
+	}
+}
+
+func TestResourceProvisioner_stopErrors(t *testing.T) {
+	p := new(terraform.MockResourceProvisioner)
+	p.StopReturnError = errors.New("foo")
+
+	// Create a mock provider
+	client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{
+		ProvisionerFunc: testProvisionerFixed(p),
+	}))
+	defer client.Close()
+
+	// Request the provider
+	raw, err := client.Dispense(ProvisionerPluginName)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	provider := raw.(terraform.ResourceProvisioner)
+
+	// Stop
+	e := provider.Stop()
+	if !p.StopCalled {
+		t.Fatal("stop should be called")
+	}
+	if e == nil {
+		t.Fatal("should have error")
+	}
+	if e.Error() != "foo" {
+		t.Fatalf("bad: %s", e)
+	}
+}
+
 func TestResourceProvisioner_apply(t *testing.T) {
 	// Create a mock provider
 	p := new(terraform.MockResourceProvisioner)

From 96884ec42d5ccccda6529604eaa7f941728555c9 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 11:38:43 -0800
Subject: [PATCH 03/23] plugin: bump the protocol version due to Provisioner
 change

---
 plugin/serve.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugin/serve.go b/plugin/serve.go
index 932728c97..9e5afbe21 100644
--- a/plugin/serve.go
+++ b/plugin/serve.go
@@ -19,7 +19,7 @@ var Handshake = plugin.HandshakeConfig{
 	// one or the other that makes it so that they can't safely communicate.
 	// This could be adding a new interface value, it could be how
 	// helper/schema computes diffs, etc.
-	ProtocolVersion: 2,
+	ProtocolVersion: 3,
 
 	// The magic cookie values should NEVER be changed.
 	MagicCookieKey:   "TF_PLUGIN_MAGIC_COOKIE",

From b2891bc9ef2e8139f007d71326aab33240ca321f Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 13:57:42 -0800
Subject: [PATCH 04/23] helper/schema: Provisioner support

---
 helper/schema/provisioner.go      | 173 +++++++++++++++++++++
 helper/schema/provisioner_test.go | 241 ++++++++++++++++++++++++++++++
 2 files changed, 414 insertions(+)
 create mode 100644 helper/schema/provisioner.go
 create mode 100644 helper/schema/provisioner_test.go

diff --git a/helper/schema/provisioner.go b/helper/schema/provisioner.go
new file mode 100644
index 000000000..c6d21c199
--- /dev/null
+++ b/helper/schema/provisioner.go
@@ -0,0 +1,173 @@
+package schema
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"sync"
+
+	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/terraform"
+)
+
+// Provisioner represents a resource provisioner in Terraform and properly
+// implements all of the ResourceProvisioner API.
+//
+// This higher level structure makes it much easier to implement a new or
+// custom provisioner for Terraform.
+//
+// The function callbacks for this structure are all passed a context object.
+// This context object has a number of pre-defined values that can be accessed
+// via the global functions defined in context.go.
+type Provisioner struct {
+	// ConnSchema is the schema for the connection settings for this
+	// provisioner.
+	//
+	// The keys of this map are the configuration keys, and the value is
+	// the schema describing the value of the configuration.
+	//
+	// NOTE: The value of connection keys can only be strings for now.
+	ConnSchema map[string]*Schema
+
+	// Schema is the schema for the usage of this provisioner.
+	//
+	// The keys of this map are the configuration keys, and the value is
+	// the schema describing the value of the configuration.
+	Schema map[string]*Schema
+
+	// ApplyFunc is the function for executing the provisioner. This is required.
+	// It is given a context. See the Provisioner struct docs for more
+	// information.
+	ApplyFunc func(ctx context.Context) error
+
+	stopCtx       context.Context
+	stopCtxCancel context.CancelFunc
+	stopOnce      sync.Once
+}
+
+// These constants are the keys that can be used to access data in
+// the context parameters for Provisioners.
+const (
+	connDataInvalid int = iota
+
+	// This returns a *ResourceData for the connection information.
+	// Guaranteed to never be nil.
+	ProvConnDataKey
+
+	// This returns a *ResourceData for the config information.
+	// Guaranteed to never be nil.
+	ProvConfigDataKey
+
+	// This returns a terraform.UIOutput. Guaranteed to never be nil.
+	ProvOutputKey
+)
+
+// InternalValidate should be called to validate the structure
+// of the provisioner.
+//
+// This should be called in a unit test to verify before release that this
+// structure is properly configured for use.
+func (p *Provisioner) InternalValidate() error {
+	if p == nil {
+		return errors.New("provisioner is nil")
+	}
+
+	var validationErrors error
+	{
+		sm := schemaMap(p.ConnSchema)
+		if err := sm.InternalValidate(sm); err != nil {
+			validationErrors = multierror.Append(validationErrors, err)
+		}
+	}
+
+	{
+		sm := schemaMap(p.Schema)
+		if err := sm.InternalValidate(sm); err != nil {
+			validationErrors = multierror.Append(validationErrors, err)
+		}
+	}
+
+	if p.ApplyFunc == nil {
+		validationErrors = multierror.Append(validationErrors, fmt.Errorf(
+			"ApplyFunc must not be nil"))
+	}
+
+	return validationErrors
+}
+
+// StopContext returns a context that checks whether a provisioner is stopped.
+func (p *Provisioner) StopContext() context.Context {
+	p.stopOnce.Do(p.stopInit)
+	return p.stopCtx
+}
+
+func (p *Provisioner) stopInit() {
+	p.stopCtx, p.stopCtxCancel = context.WithCancel(context.Background())
+}
+
+// Stop implementation of terraform.ResourceProvisioner interface.
+func (p *Provisioner) Stop() error {
+	p.stopOnce.Do(p.stopInit)
+	p.stopCtxCancel()
+	return nil
+}
+
+func (p *Provisioner) Validate(c *terraform.ResourceConfig) ([]string, []error) {
+	return schemaMap(p.Schema).Validate(c)
+}
+
+// Apply implementation of terraform.ResourceProvisioner interface.
+func (p *Provisioner) Apply(
+	o terraform.UIOutput,
+	s *terraform.InstanceState,
+	c *terraform.ResourceConfig) error {
+	var connData, configData *ResourceData
+
+	{
+		// We first need to turn the connection information into a
+		// terraform.ResourceConfig so that we can use that type to more
+		// easily build a ResourceData structure. We do this by simply treating
+		// the conn info as configuration input.
+		raw := make(map[string]interface{})
+		for k, v := range s.Ephemeral.ConnInfo {
+			raw[k] = v
+		}
+
+		c, err := config.NewRawConfig(raw)
+		if err != nil {
+			return err
+		}
+
+		sm := schemaMap(p.ConnSchema)
+		diff, err := sm.Diff(nil, terraform.NewResourceConfig(c))
+		if err != nil {
+			return err
+		}
+		connData, err = sm.Data(nil, diff)
+		if err != nil {
+			return err
+		}
+	}
+
+	{
+		// Build the configuration data. Doing this requires making a "diff"
+		// even though that's never used. We use that just to get the correct types.
+		configMap := schemaMap(p.Schema)
+		diff, err := configMap.Diff(nil, c)
+		if err != nil {
+			return err
+		}
+		configData, err = configMap.Data(nil, diff)
+		if err != nil {
+			return err
+		}
+	}
+
+	// Build the context and call the function
+	ctx := p.StopContext()
+	ctx = context.WithValue(ctx, ProvConnDataKey, connData)
+	ctx = context.WithValue(ctx, ProvConfigDataKey, configData)
+	ctx = context.WithValue(ctx, ProvOutputKey, o)
+	return p.ApplyFunc(ctx)
+}
diff --git a/helper/schema/provisioner_test.go b/helper/schema/provisioner_test.go
new file mode 100644
index 000000000..827685e8f
--- /dev/null
+++ b/helper/schema/provisioner_test.go
@@ -0,0 +1,241 @@
+package schema
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func TestProvisioner_impl(t *testing.T) {
+	var _ terraform.ResourceProvisioner = new(Provisioner)
+}
+
+func TestProvisionerValidate(t *testing.T) {
+	cases := []struct {
+		Name   string
+		P      *Provisioner
+		Config map[string]interface{}
+		Err    bool
+	}{
+		{
+			"Basic required field",
+			&Provisioner{
+				Schema: map[string]*Schema{
+					"foo": &Schema{
+						Required: true,
+						Type:     TypeString,
+					},
+				},
+			},
+			nil,
+			true,
+		},
+
+		{
+			"Basic required field set",
+			&Provisioner{
+				Schema: map[string]*Schema{
+					"foo": &Schema{
+						Required: true,
+						Type:     TypeString,
+					},
+				},
+			},
+			map[string]interface{}{
+				"foo": "bar",
+			},
+			false,
+		},
+	}
+
+	for i, tc := range cases {
+		t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) {
+			c, err := config.NewRawConfig(tc.Config)
+			if err != nil {
+				t.Fatalf("err: %s", err)
+			}
+
+			_, es := tc.P.Validate(terraform.NewResourceConfig(c))
+			if len(es) > 0 != tc.Err {
+				t.Fatalf("%d: %#v", i, es)
+			}
+		})
+	}
+}
+
+func TestProvisionerApply(t *testing.T) {
+	cases := []struct {
+		Name   string
+		P      *Provisioner
+		Conn   map[string]string
+		Config map[string]interface{}
+		Err    bool
+	}{
+		{
+			"Basic config",
+			&Provisioner{
+				ConnSchema: map[string]*Schema{
+					"foo": &Schema{
+						Type:     TypeString,
+						Optional: true,
+					},
+				},
+
+				Schema: map[string]*Schema{
+					"foo": &Schema{
+						Type:     TypeInt,
+						Optional: true,
+					},
+				},
+
+				ApplyFunc: func(ctx context.Context) error {
+					cd := ctx.Value(ProvConnDataKey).(*ResourceData)
+					d := ctx.Value(ProvConfigDataKey).(*ResourceData)
+					if d.Get("foo").(int) != 42 {
+						return fmt.Errorf("bad config data")
+					}
+					if cd.Get("foo").(string) != "bar" {
+						return fmt.Errorf("bad conn data")
+					}
+
+					return nil
+				},
+			},
+			map[string]string{
+				"foo": "bar",
+			},
+			map[string]interface{}{
+				"foo": 42,
+			},
+			false,
+		},
+	}
+
+	for i, tc := range cases {
+		t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) {
+			c, err := config.NewRawConfig(tc.Config)
+			if err != nil {
+				t.Fatalf("err: %s", err)
+			}
+
+			state := &terraform.InstanceState{
+				Ephemeral: terraform.EphemeralState{
+					ConnInfo: tc.Conn,
+				},
+			}
+
+			err = tc.P.Apply(
+				nil, state, terraform.NewResourceConfig(c))
+			if err != nil != tc.Err {
+				t.Fatalf("%d: %s", i, err)
+			}
+		})
+	}
+}
+
+func TestProvisionerStop(t *testing.T) {
+	var p Provisioner
+
+	// Verify stopch blocks
+	ch := p.StopContext().Done()
+	select {
+	case <-ch:
+		t.Fatal("should not be stopped")
+	case <-time.After(10 * time.Millisecond):
+	}
+
+	// Stop it
+	if err := p.Stop(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	select {
+	case <-ch:
+	case <-time.After(10 * time.Millisecond):
+		t.Fatal("should be stopped")
+	}
+}
+
+func TestProvisionerStop_apply(t *testing.T) {
+	p := &Provisioner{
+		ConnSchema: map[string]*Schema{
+			"foo": &Schema{
+				Type:     TypeString,
+				Optional: true,
+			},
+		},
+
+		Schema: map[string]*Schema{
+			"foo": &Schema{
+				Type:     TypeInt,
+				Optional: true,
+			},
+		},
+
+		ApplyFunc: func(ctx context.Context) error {
+			<-ctx.Done()
+			return nil
+		},
+	}
+
+	conn := map[string]string{
+		"foo": "bar",
+	}
+
+	conf := map[string]interface{}{
+		"foo": 42,
+	}
+
+	c, err := config.NewRawConfig(conf)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state := &terraform.InstanceState{
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: conn,
+		},
+	}
+
+	// Run the apply in a goroutine
+	doneCh := make(chan struct{})
+	go func() {
+		p.Apply(nil, state, terraform.NewResourceConfig(c))
+		close(doneCh)
+	}()
+
+	// Should block
+	select {
+	case <-doneCh:
+		t.Fatal("should not be done")
+	case <-time.After(10 * time.Millisecond):
+	}
+
+	// Stop!
+	p.Stop()
+
+	select {
+	case <-doneCh:
+	case <-time.After(10 * time.Millisecond):
+		t.Fatal("should be done")
+	}
+}
+
+func TestProvisionerStop_stopFirst(t *testing.T) {
+	var p Provisioner
+
+	// Stop it
+	if err := p.Stop(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	select {
+	case <-p.StopContext().Done():
+	case <-time.After(10 * time.Millisecond):
+		t.Fatal("should be stopped")
+	}
+}

From a1da59a73e4697d0ada1b5be15f8a8803200844b Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 14:11:28 -0800
Subject: [PATCH 05/23] helper/schema: provisioner allows for nil state

---
 helper/schema/provisioner.go      |  6 ++++--
 helper/schema/provisioner_test.go | 36 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/helper/schema/provisioner.go b/helper/schema/provisioner.go
index c6d21c199..d73aaa95e 100644
--- a/helper/schema/provisioner.go
+++ b/helper/schema/provisioner.go
@@ -130,8 +130,10 @@ func (p *Provisioner) Apply(
 		// easily build a ResourceData structure. We do this by simply treating
 		// the conn info as configuration input.
 		raw := make(map[string]interface{})
-		for k, v := range s.Ephemeral.ConnInfo {
-			raw[k] = v
+		if s != nil {
+			for k, v := range s.Ephemeral.ConnInfo {
+				raw[k] = v
+			}
 		}
 
 		c, err := config.NewRawConfig(raw)
diff --git a/helper/schema/provisioner_test.go b/helper/schema/provisioner_test.go
index 827685e8f..d8448acef 100644
--- a/helper/schema/provisioner_test.go
+++ b/helper/schema/provisioner_test.go
@@ -137,6 +137,42 @@ func TestProvisionerApply(t *testing.T) {
 	}
 }
 
+func TestProvisionerApply_nilState(t *testing.T) {
+	p := &Provisioner{
+		ConnSchema: map[string]*Schema{
+			"foo": &Schema{
+				Type:     TypeString,
+				Optional: true,
+			},
+		},
+
+		Schema: map[string]*Schema{
+			"foo": &Schema{
+				Type:     TypeInt,
+				Optional: true,
+			},
+		},
+
+		ApplyFunc: func(ctx context.Context) error {
+			return nil
+		},
+	}
+
+	conf := map[string]interface{}{
+		"foo": 42,
+	}
+
+	c, err := config.NewRawConfig(conf)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	err = p.Apply(nil, nil, terraform.NewResourceConfig(c))
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func TestProvisionerStop(t *testing.T) {
 	var p Provisioner
 

From c5b784c33f363a2accca631a9dd9c69b2294d64b Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 14:11:41 -0800
Subject: [PATCH 06/23] provisioners/local-exec: switch to helper/schema

---
 builtin/bins/provisioner-local-exec/main.go   |  5 +-
 .../local-exec/resource_provisioner.go        | 46 +++++++++----------
 .../local-exec/resource_provisioner_test.go   | 10 ++--
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/builtin/bins/provisioner-local-exec/main.go b/builtin/bins/provisioner-local-exec/main.go
index 87a70c6ce..2e0433ff5 100644
--- a/builtin/bins/provisioner-local-exec/main.go
+++ b/builtin/bins/provisioner-local-exec/main.go
@@ -3,13 +3,10 @@ package main
 import (
 	"github.com/hashicorp/terraform/builtin/provisioners/local-exec"
 	"github.com/hashicorp/terraform/plugin"
-	"github.com/hashicorp/terraform/terraform"
 )
 
 func main() {
 	plugin.Serve(&plugin.ServeOpts{
-		ProvisionerFunc: func() terraform.ResourceProvisioner {
-			return new(localexec.ResourceProvisioner)
-		},
+		ProvisionerFunc: localexec.Provisioner,
 	})
 }
diff --git a/builtin/provisioners/local-exec/resource_provisioner.go b/builtin/provisioners/local-exec/resource_provisioner.go
index 88e5e0045..cfd687010 100644
--- a/builtin/provisioners/local-exec/resource_provisioner.go
+++ b/builtin/provisioners/local-exec/resource_provisioner.go
@@ -1,13 +1,14 @@
 package localexec
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os/exec"
 	"runtime"
 
 	"github.com/armon/circbuf"
-	"github.com/hashicorp/terraform/helper/config"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/go-linereader"
 )
@@ -19,21 +20,26 @@ const (
 	maxBufSize = 8 * 1024
 )
 
-type ResourceProvisioner struct{}
+func Provisioner() terraform.ResourceProvisioner {
+	return &schema.Provisioner{
+		Schema: map[string]*schema.Schema{
+			"command": &schema.Schema{
+				Type:     schema.TypeString,
+				Required: true,
+			},
+		},
 
-func (p *ResourceProvisioner) Apply(
-	o terraform.UIOutput,
-	s *terraform.InstanceState,
-	c *terraform.ResourceConfig) error {
-
-	// Get the command
-	commandRaw, ok := c.Config["command"]
-	if !ok {
-		return fmt.Errorf("local-exec provisioner missing 'command'")
+		ApplyFunc: applyFn,
 	}
-	command, ok := commandRaw.(string)
-	if !ok {
-		return fmt.Errorf("local-exec provisioner command must be a string")
+}
+
+func applyFn(ctx context.Context) error {
+	data := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData)
+	o := ctx.Value(schema.ProvOutputKey).(terraform.UIOutput)
+
+	command := data.Get("command").(string)
+	if command == "" {
+		return fmt.Errorf("local-exec provisioner command must be a non-empty string")
 	}
 
 	// Execute the command using a shell
@@ -49,7 +55,7 @@ func (p *ResourceProvisioner) Apply(
 	// Setup the reader that will read the lines from the command
 	pr, pw := io.Pipe()
 	copyDoneCh := make(chan struct{})
-	go p.copyOutput(o, pr, copyDoneCh)
+	go copyOutput(o, pr, copyDoneCh)
 
 	// Setup the command
 	cmd := exec.Command(shell, flag, command)
@@ -78,15 +84,7 @@ func (p *ResourceProvisioner) Apply(
 	return nil
 }
 
-func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) ([]string, []error) {
-	validator := config.Validator{
-		Required: []string{"command"},
-	}
-	return validator.Validate(c)
-}
-
-func (p *ResourceProvisioner) copyOutput(
-	o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) {
+func copyOutput(o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) {
 	defer close(doneCh)
 	lr := linereader.New(r)
 	for line := range lr.Ch {
diff --git a/builtin/provisioners/local-exec/resource_provisioner_test.go b/builtin/provisioners/local-exec/resource_provisioner_test.go
index 9158c333e..6d04967ad 100644
--- a/builtin/provisioners/local-exec/resource_provisioner_test.go
+++ b/builtin/provisioners/local-exec/resource_provisioner_test.go
@@ -10,10 +10,6 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestResourceProvisioner_impl(t *testing.T) {
-	var _ terraform.ResourceProvisioner = new(ResourceProvisioner)
-}
-
 func TestResourceProvider_Apply(t *testing.T) {
 	defer os.Remove("test_out")
 	c := testConfig(t, map[string]interface{}{
@@ -21,7 +17,7 @@ func TestResourceProvider_Apply(t *testing.T) {
 	})
 
 	output := new(terraform.MockUIOutput)
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	if err := p.Apply(output, nil, c); err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -43,7 +39,7 @@ func TestResourceProvider_Validate_good(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"command": "echo foo",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
@@ -55,7 +51,7 @@ func TestResourceProvider_Validate_good(t *testing.T) {
 
 func TestResourceProvider_Validate_missing(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)

From 0fb87cd96ba17628ebc17d17b89083d285007bae Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 16:06:40 -0800
Subject: [PATCH 07/23] provisioners/local-exec: stoppable

This modifies local-exec to be stoppable with the new Stop API call that
provisioners can listen to.
---
 .../local-exec/resource_provisioner.go        | 20 ++++++++++--
 .../local-exec/resource_provisioner_test.go   | 32 +++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/builtin/provisioners/local-exec/resource_provisioner.go b/builtin/provisioners/local-exec/resource_provisioner.go
index cfd687010..86f27fb35 100644
--- a/builtin/provisioners/local-exec/resource_provisioner.go
+++ b/builtin/provisioners/local-exec/resource_provisioner.go
@@ -68,8 +68,24 @@ func applyFn(ctx context.Context) error {
 		"Executing: %s %s \"%s\"",
 		shell, flag, command))
 
-	// Run the command to completion
-	err := cmd.Run()
+	// Start the command
+	err := cmd.Start()
+	if err == nil {
+		// Wait for the command to complete in a goroutine
+		doneCh := make(chan struct{})
+		go func() {
+			defer close(doneCh)
+			err = cmd.Wait()
+		}()
+
+		// Wait for the command to finish or for us to be interrupted
+		select {
+		case <-doneCh:
+		case <-ctx.Done():
+			cmd.Process.Kill()
+			err = cmd.Wait()
+		}
+	}
 
 	// Close the write-end of the pipe so that the goroutine mirroring output
 	// ends properly.
diff --git a/builtin/provisioners/local-exec/resource_provisioner_test.go b/builtin/provisioners/local-exec/resource_provisioner_test.go
index 6d04967ad..fcc49c01b 100644
--- a/builtin/provisioners/local-exec/resource_provisioner_test.go
+++ b/builtin/provisioners/local-exec/resource_provisioner_test.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/terraform"
@@ -35,6 +36,37 @@ func TestResourceProvider_Apply(t *testing.T) {
 	}
 }
 
+func TestResourceProvider_stop(t *testing.T) {
+	c := testConfig(t, map[string]interface{}{
+		"command": "sleep 60",
+	})
+
+	output := new(terraform.MockUIOutput)
+	p := Provisioner()
+
+	var err error
+	doneCh := make(chan struct{})
+	go func() {
+		defer close(doneCh)
+		err = p.Apply(output, nil, c)
+	}()
+
+	select {
+	case <-doneCh:
+		t.Fatal("should not finish quickly")
+	case <-time.After(10 * time.Millisecond):
+	}
+
+	// Stop it
+	p.Stop()
+
+	select {
+	case <-doneCh:
+	case <-time.After(100 * time.Millisecond):
+		t.Fatal("should finish")
+	}
+}
+
 func TestResourceProvider_Validate_good(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"command": "echo foo",

From 02a4adc07cb2df95230a1ba8f8d68d03adac37e6 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 16:17:34 -0800
Subject: [PATCH 08/23] provisioners/file: convert to helper/schema

---
 .../provisioners/file/resource_provisioner.go | 92 ++++++++-----------
 .../file/resource_provisioner_test.go         | 12 +--
 communicator/communicator.go                  | 16 ++++
 3 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go
index 2cd060b63..012fc768d 100644
--- a/builtin/provisioners/file/resource_provisioner.go
+++ b/builtin/provisioners/file/resource_provisioner.go
@@ -1,6 +1,7 @@
 package file
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"log"
@@ -8,26 +9,48 @@ import (
 	"time"
 
 	"github.com/hashicorp/terraform/communicator"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/go-homedir"
 )
 
-// ResourceProvisioner represents a file provisioner
-type ResourceProvisioner struct{}
+func Provisioner() terraform.ResourceProvisioner {
+	return &schema.Provisioner{
+		Schema: map[string]*schema.Schema{
+			"source": &schema.Schema{
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"content"},
+			},
+
+			"content": &schema.Schema{
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"source"},
+			},
+
+			"destination": &schema.Schema{
+				Type:     schema.TypeString,
+				Required: true,
+			},
+		},
+
+		ApplyFunc: applyFn,
+	}
+}
+
+func applyFn(ctx context.Context) error {
+	connData := ctx.Value(schema.ProvConnDataKey).(*schema.ResourceData)
+	data := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData)
 
-// Apply executes the file provisioner
-func (p *ResourceProvisioner) Apply(
-	o terraform.UIOutput,
-	s *terraform.InstanceState,
-	c *terraform.ResourceConfig) error {
 	// Get a new communicator
-	comm, err := communicator.New(s)
+	comm, err := communicator.NewData(connData)
 	if err != nil {
 		return err
 	}
 
 	// Get the source
-	src, deleteSource, err := p.getSrc(c)
+	src, deleteSource, err := getSrc(data)
 	if err != nil {
 		return err
 	}
@@ -36,57 +59,20 @@ func (p *ResourceProvisioner) Apply(
 	}
 
 	// Get destination
-	dRaw := c.Config["destination"]
-	dst, ok := dRaw.(string)
-	if !ok {
-		return fmt.Errorf("Unsupported 'destination' type! Must be string.")
-	}
-	return p.copyFiles(comm, src, dst)
-}
-
-// Validate checks if the required arguments are configured
-func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) {
-	numDst := 0
-	numSrc := 0
-	for name := range c.Raw {
-		switch name {
-		case "destination":
-			numDst++
-		case "source", "content":
-			numSrc++
-		default:
-			es = append(es, fmt.Errorf("Unknown configuration '%s'", name))
-		}
-	}
-	if numSrc != 1 || numDst != 1 {
-		es = append(es, fmt.Errorf("Must provide one  of 'content' or 'source' and 'destination' to file"))
-	}
-	return
+	dst := data.Get("destination").(string)
+	return copyFiles(comm, src, dst)
 }
 
 // getSrc returns the file to use as source
-func (p *ResourceProvisioner) getSrc(c *terraform.ResourceConfig) (string, bool, error) {
-	var src string
-
-	sRaw, ok := c.Config["source"]
-	if ok {
-		if src, ok = sRaw.(string); !ok {
-			return "", false, fmt.Errorf("Unsupported 'source' type! Must be string.")
-		}
-	}
-
-	content, ok := c.Config["content"]
-	if ok {
+func getSrc(data *schema.ResourceData) (string, bool, error) {
+	src := data.Get("source").(string)
+	if content, ok := data.GetOk("content"); ok {
 		file, err := ioutil.TempFile("", "tf-file-content")
 		if err != nil {
 			return "", true, err
 		}
 
-		contentStr, ok := content.(string)
-		if !ok {
-			return "", true, fmt.Errorf("Unsupported 'content' type! Must be string.")
-		}
-		if _, err = file.WriteString(contentStr); err != nil {
+		if _, err = file.WriteString(content.(string)); err != nil {
 			return "", true, err
 		}
 
@@ -98,7 +84,7 @@ func (p *ResourceProvisioner) getSrc(c *terraform.ResourceConfig) (string, bool,
 }
 
 // copyFiles is used to copy the files from a source to a destination
-func (p *ResourceProvisioner) copyFiles(comm communicator.Communicator, src, dst string) error {
+func copyFiles(comm communicator.Communicator, src, dst string) error {
 	// Wait and retry until we establish the connection
 	err := retryFunc(comm.Timeout(), func() error {
 		err := comm.Connect(nil)
diff --git a/builtin/provisioners/file/resource_provisioner_test.go b/builtin/provisioners/file/resource_provisioner_test.go
index 713c8aa0d..d57dbbaa5 100644
--- a/builtin/provisioners/file/resource_provisioner_test.go
+++ b/builtin/provisioners/file/resource_provisioner_test.go
@@ -7,16 +7,12 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestResourceProvisioner_impl(t *testing.T) {
-	var _ terraform.ResourceProvisioner = new(ResourceProvisioner)
-}
-
 func TestResourceProvider_Validate_good_source(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"source":      "/tmp/foo",
 		"destination": "/tmp/bar",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
@@ -31,7 +27,7 @@ func TestResourceProvider_Validate_good_content(t *testing.T) {
 		"content":     "value to copy",
 		"destination": "/tmp/bar",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
@@ -45,7 +41,7 @@ func TestResourceProvider_Validate_bad_not_destination(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"source": "nope",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
@@ -61,7 +57,7 @@ func TestResourceProvider_Validate_bad_to_many_src(t *testing.T) {
 		"content":     "value to copy",
 		"destination": "/tmp/bar",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
diff --git a/communicator/communicator.go b/communicator/communicator.go
index 5fa2631a4..21f7b6fea 100644
--- a/communicator/communicator.go
+++ b/communicator/communicator.go
@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/terraform/communicator/remote"
 	"github.com/hashicorp/terraform/communicator/ssh"
 	"github.com/hashicorp/terraform/communicator/winrm"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -51,3 +52,18 @@ func New(s *terraform.InstanceState) (Communicator, error) {
 		return nil, fmt.Errorf("connection type '%s' not supported", connType)
 	}
 }
+
+// NewData creates a new Communicator from a ResourceData structure that
+// represents the connection information.
+func NewData(d *schema.ResourceData) (Communicator, error) {
+	// Turn the ResourceData into a legacy-style ConnInfo struct that
+	// is used to instantiate the communicator.
+	raw := d.State()
+	state := &terraform.InstanceState{
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: raw.Attributes,
+		},
+	}
+
+	return New(state)
+}

From a2e044829b8e6094075e80908dfb314734e47ac8 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 16:43:52 -0800
Subject: [PATCH 09/23] provisioners/file: use the old communicator.New just to
 minimize risk

---
 .../provisioners/file/resource_provisioner.go    |  4 ++--
 communicator/communicator.go                     | 16 ----------------
 helper/schema/provisioner.go                     |  5 +++++
 3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go
index 012fc768d..71fde0610 100644
--- a/builtin/provisioners/file/resource_provisioner.go
+++ b/builtin/provisioners/file/resource_provisioner.go
@@ -40,11 +40,11 @@ func Provisioner() terraform.ResourceProvisioner {
 }
 
 func applyFn(ctx context.Context) error {
-	connData := ctx.Value(schema.ProvConnDataKey).(*schema.ResourceData)
+	connState := ctx.Value(schema.ProvRawStateKey).(*terraform.InstanceState)
 	data := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData)
 
 	// Get a new communicator
-	comm, err := communicator.NewData(connData)
+	comm, err := communicator.New(connState)
 	if err != nil {
 		return err
 	}
diff --git a/communicator/communicator.go b/communicator/communicator.go
index 21f7b6fea..5fa2631a4 100644
--- a/communicator/communicator.go
+++ b/communicator/communicator.go
@@ -8,7 +8,6 @@ import (
 	"github.com/hashicorp/terraform/communicator/remote"
 	"github.com/hashicorp/terraform/communicator/ssh"
 	"github.com/hashicorp/terraform/communicator/winrm"
-	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -52,18 +51,3 @@ func New(s *terraform.InstanceState) (Communicator, error) {
 		return nil, fmt.Errorf("connection type '%s' not supported", connType)
 	}
 }
-
-// NewData creates a new Communicator from a ResourceData structure that
-// represents the connection information.
-func NewData(d *schema.ResourceData) (Communicator, error) {
-	// Turn the ResourceData into a legacy-style ConnInfo struct that
-	// is used to instantiate the communicator.
-	raw := d.State()
-	state := &terraform.InstanceState{
-		Ephemeral: terraform.EphemeralState{
-			ConnInfo: raw.Attributes,
-		},
-	}
-
-	return New(state)
-}
diff --git a/helper/schema/provisioner.go b/helper/schema/provisioner.go
index d73aaa95e..6ac3fc1bf 100644
--- a/helper/schema/provisioner.go
+++ b/helper/schema/provisioner.go
@@ -61,6 +61,10 @@ const (
 
 	// This returns a terraform.UIOutput. Guaranteed to never be nil.
 	ProvOutputKey
+
+	// This returns the raw InstanceState passed to Apply. Guaranteed to
+	// be set, but may be nil.
+	ProvRawStateKey
 )
 
 // InternalValidate should be called to validate the structure
@@ -171,5 +175,6 @@ func (p *Provisioner) Apply(
 	ctx = context.WithValue(ctx, ProvConnDataKey, connData)
 	ctx = context.WithValue(ctx, ProvConfigDataKey, configData)
 	ctx = context.WithValue(ctx, ProvOutputKey, o)
+	ctx = context.WithValue(ctx, ProvRawStateKey, s)
 	return p.ApplyFunc(ctx)
 }

From 27c19af9ff85361220d0d6e02b737c80c103ec5e Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 16:47:32 -0800
Subject: [PATCH 10/23] provisioners/file: support Stop

---
 .../provisioners/file/resource_provisioner.go  | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go
index 71fde0610..001e78af5 100644
--- a/builtin/provisioners/file/resource_provisioner.go
+++ b/builtin/provisioners/file/resource_provisioner.go
@@ -58,9 +58,23 @@ func applyFn(ctx context.Context) error {
 		defer os.Remove(src)
 	}
 
-	// Get destination
+	// Begin the file copy
 	dst := data.Get("destination").(string)
-	return copyFiles(comm, src, dst)
+	resultCh := make(chan error, 1)
+	go func() {
+		resultCh <- copyFiles(comm, src, dst)
+	}()
+
+	// Allow the file copy to complete unless there is an interrupt.
+	// If there is an interrupt we make no attempt to cleanly close
+	// the connection currently. We just abruptly exit. Because Terraform
+	// taints the resource, this is fine.
+	select {
+	case err := <-resultCh:
+		return err
+	case <-ctx.Done():
+		return fmt.Errorf("file transfer interrupted")
+	}
 }
 
 // getSrc returns the file to use as source

From 3c0c81957a5064ed8d8592b29ffdc68be563206d Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 17:05:45 -0800
Subject: [PATCH 11/23] provisioners/remote-exec: switch to helper/schema

---
 .../remote-exec/resource_provisioner.go       | 132 +++++++-----------
 .../remote-exec/resource_provisioner_test.go  |  47 ++++---
 helper/schema/testing.go                      |  30 ++++
 3 files changed, 108 insertions(+), 101 deletions(-)
 create mode 100644 helper/schema/testing.go

diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go
index 46d4fd1ac..43ab12b0a 100644
--- a/builtin/provisioners/remote-exec/resource_provisioner.go
+++ b/builtin/provisioners/remote-exec/resource_provisioner.go
@@ -2,6 +2,7 @@ package remoteexec
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -11,26 +12,53 @@ import (
 
 	"github.com/hashicorp/terraform/communicator"
 	"github.com/hashicorp/terraform/communicator/remote"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/go-linereader"
 )
 
-// ResourceProvisioner represents a remote exec provisioner
-type ResourceProvisioner struct{}
+func Provisioner() terraform.ResourceProvisioner {
+	return &schema.Provisioner{
+		Schema: map[string]*schema.Schema{
+			"inline": &schema.Schema{
+				Type:          schema.TypeList,
+				Elem:          &schema.Schema{Type: schema.TypeString},
+				Optional:      true,
+				ConflictsWith: []string{"script", "scripts"},
+			},
+
+			"script": &schema.Schema{
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"inline", "scripts"},
+			},
+
+			"scripts": &schema.Schema{
+				Type:          schema.TypeList,
+				Elem:          &schema.Schema{Type: schema.TypeString},
+				Optional:      true,
+				ConflictsWith: []string{"script", "inline"},
+			},
+		},
+
+		ApplyFunc: applyFn,
+	}
+}
 
 // Apply executes the remote exec provisioner
-func (p *ResourceProvisioner) Apply(
-	o terraform.UIOutput,
-	s *terraform.InstanceState,
-	c *terraform.ResourceConfig) error {
+func applyFn(ctx context.Context) error {
+	connState := ctx.Value(schema.ProvRawStateKey).(*terraform.InstanceState)
+	data := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData)
+	o := ctx.Value(schema.ProvOutputKey).(terraform.UIOutput)
+
 	// Get a new communicator
-	comm, err := communicator.New(s)
+	comm, err := communicator.New(connState)
 	if err != nil {
 		return err
 	}
 
 	// Collect the scripts
-	scripts, err := p.collectScripts(c)
+	scripts, err := collectScripts(data)
 	if err != nil {
 		return err
 	}
@@ -39,67 +67,33 @@ func (p *ResourceProvisioner) Apply(
 	}
 
 	// Copy and execute each script
-	if err := p.runScripts(o, comm, scripts); err != nil {
+	if err := runScripts(o, comm, scripts); err != nil {
 		return err
 	}
+
 	return nil
 }
 
-// Validate checks if the required arguments are configured
-func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) {
-	num := 0
-	for name := range c.Raw {
-		switch name {
-		case "scripts", "script", "inline":
-			num++
-		default:
-			es = append(es, fmt.Errorf("Unknown configuration '%s'", name))
-		}
-	}
-	if num != 1 {
-		es = append(es, fmt.Errorf("Must provide one of 'scripts', 'script' or 'inline' to remote-exec"))
-	}
-	return
-}
-
 // generateScripts takes the configuration and creates a script from each inline config
-func (p *ResourceProvisioner) generateScripts(c *terraform.ResourceConfig) ([]string, error) {
+func generateScripts(d *schema.ResourceData) ([]string, error) {
 	var scripts []string
-	command, ok := c.Config["inline"]
-	if ok {
-		switch cmd := command.(type) {
-		case string:
-			scripts = append(scripts, cmd)
-		case []string:
-			scripts = append(scripts, cmd...)
-		case []interface{}:
-			for _, l := range cmd {
-				lStr, ok := l.(string)
-				if ok {
-					scripts = append(scripts, lStr)
-				} else {
-					return nil, fmt.Errorf("Unsupported 'inline' type! Must be string, or list of strings.")
-				}
-			}
-		default:
-			return nil, fmt.Errorf("Unsupported 'inline' type! Must be string, or list of strings.")
-		}
+	for _, l := range d.Get("inline").([]interface{}) {
+		scripts = append(scripts, l.(string))
 	}
 	return scripts, nil
 }
 
 // collectScripts is used to collect all the scripts we need
 // to execute in preparation for copying them.
-func (p *ResourceProvisioner) collectScripts(c *terraform.ResourceConfig) ([]io.ReadCloser, error) {
+func collectScripts(d *schema.ResourceData) ([]io.ReadCloser, error) {
 	// Check if inline
-	_, ok := c.Config["inline"]
-	if ok {
-		scripts, err := p.generateScripts(c)
+	if _, ok := d.GetOk("inline"); ok {
+		scripts, err := generateScripts(d)
 		if err != nil {
 			return nil, err
 		}
 
-		r := []io.ReadCloser{}
+		var r []io.ReadCloser
 		for _, script := range scripts {
 			r = append(r, ioutil.NopCloser(bytes.NewReader([]byte(script))))
 		}
@@ -109,31 +103,13 @@ func (p *ResourceProvisioner) collectScripts(c *terraform.ResourceConfig) ([]io.
 
 	// Collect scripts
 	var scripts []string
-	s, ok := c.Config["script"]
-	if ok {
-		sStr, ok := s.(string)
-		if !ok {
-			return nil, fmt.Errorf("Unsupported 'script' type! Must be a string.")
-		}
-		scripts = append(scripts, sStr)
+	if script, ok := d.GetOk("script"); ok {
+		scripts = append(scripts, script.(string))
 	}
 
-	sl, ok := c.Config["scripts"]
-	if ok {
-		switch slt := sl.(type) {
-		case []string:
-			scripts = append(scripts, slt...)
-		case []interface{}:
-			for _, l := range slt {
-				lStr, ok := l.(string)
-				if ok {
-					scripts = append(scripts, lStr)
-				} else {
-					return nil, fmt.Errorf("Unsupported 'scripts' type! Must be list of strings.")
-				}
-			}
-		default:
-			return nil, fmt.Errorf("Unsupported 'scripts' type! Must be list of strings.")
+	if scriptList, ok := d.GetOk("scripts"); ok {
+		for _, script := range scriptList.([]interface{}) {
+			scripts = append(scripts, script.(string))
 		}
 	}
 
@@ -155,7 +131,7 @@ func (p *ResourceProvisioner) collectScripts(c *terraform.ResourceConfig) ([]io.
 }
 
 // runScripts is used to copy and execute a set of scripts
-func (p *ResourceProvisioner) runScripts(
+func runScripts(
 	o terraform.UIOutput,
 	comm communicator.Communicator,
 	scripts []io.ReadCloser) error {
@@ -175,8 +151,8 @@ func (p *ResourceProvisioner) runScripts(
 		errR, errW := io.Pipe()
 		outDoneCh := make(chan struct{})
 		errDoneCh := make(chan struct{})
-		go p.copyOutput(o, outR, outDoneCh)
-		go p.copyOutput(o, errR, errDoneCh)
+		go copyOutput(o, outR, outDoneCh)
+		go copyOutput(o, errR, errDoneCh)
 
 		remotePath := comm.ScriptPath()
 		err = retryFunc(comm.Timeout(), func() error {
@@ -225,7 +201,7 @@ func (p *ResourceProvisioner) runScripts(
 	return nil
 }
 
-func (p *ResourceProvisioner) copyOutput(
+func copyOutput(
 	o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) {
 	defer close(doneCh)
 	lr := linereader.New(r)
diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go
index a581301e3..5508e58dc 100644
--- a/builtin/provisioners/remote-exec/resource_provisioner_test.go
+++ b/builtin/provisioners/remote-exec/resource_provisioner_test.go
@@ -9,18 +9,15 @@ import (
 	"reflect"
 
 	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestResourceProvisioner_impl(t *testing.T) {
-	var _ terraform.ResourceProvisioner = new(ResourceProvisioner)
-}
-
 func TestResourceProvider_Validate_good(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"inline": "echo foo",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
@@ -34,7 +31,7 @@ func TestResourceProvider_Validate_bad(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"invalid": "nope",
 	})
-	p := new(ResourceProvisioner)
+	p := Provisioner()
 	warn, errs := p.Validate(c)
 	if len(warn) > 0 {
 		t.Fatalf("Warnings: %v", warn)
@@ -51,16 +48,17 @@ exit 0
 
 var expectedInlineScriptsOut = strings.Split(expectedScriptOut, "\n")
 
-func TestResourceProvider_generateScripts(t *testing.T) {
-	p := new(ResourceProvisioner)
-	conf := testConfig(t, map[string]interface{}{
+func TestResourceProvider_generateScript(t *testing.T) {
+	p := Provisioner().(*schema.Provisioner)
+	conf := map[string]interface{}{
 		"inline": []interface{}{
 			"cd /tmp",
 			"wget http://foobar",
 			"exit 0",
 		},
-	})
-	out, err := p.generateScripts(conf)
+	}
+	out, err := generateScripts(schema.TestResourceDataRaw(
+		t, p.Schema, conf))
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -71,16 +69,17 @@ func TestResourceProvider_generateScripts(t *testing.T) {
 }
 
 func TestResourceProvider_CollectScripts_inline(t *testing.T) {
-	p := new(ResourceProvisioner)
-	conf := testConfig(t, map[string]interface{}{
+	p := Provisioner().(*schema.Provisioner)
+	conf := map[string]interface{}{
 		"inline": []interface{}{
 			"cd /tmp",
 			"wget http://foobar",
 			"exit 0",
 		},
-	})
+	}
 
-	scripts, err := p.collectScripts(conf)
+	scripts, err := collectScripts(schema.TestResourceDataRaw(
+		t, p.Schema, conf))
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -103,12 +102,13 @@ func TestResourceProvider_CollectScripts_inline(t *testing.T) {
 }
 
 func TestResourceProvider_CollectScripts_script(t *testing.T) {
-	p := new(ResourceProvisioner)
-	conf := testConfig(t, map[string]interface{}{
+	p := Provisioner().(*schema.Provisioner)
+	conf := map[string]interface{}{
 		"script": "test-fixtures/script1.sh",
-	})
+	}
 
-	scripts, err := p.collectScripts(conf)
+	scripts, err := collectScripts(schema.TestResourceDataRaw(
+		t, p.Schema, conf))
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -129,16 +129,17 @@ func TestResourceProvider_CollectScripts_script(t *testing.T) {
 }
 
 func TestResourceProvider_CollectScripts_scripts(t *testing.T) {
-	p := new(ResourceProvisioner)
-	conf := testConfig(t, map[string]interface{}{
+	p := Provisioner().(*schema.Provisioner)
+	conf := map[string]interface{}{
 		"scripts": []interface{}{
 			"test-fixtures/script1.sh",
 			"test-fixtures/script1.sh",
 			"test-fixtures/script1.sh",
 		},
-	})
+	}
 
-	scripts, err := p.collectScripts(conf)
+	scripts, err := collectScripts(schema.TestResourceDataRaw(
+		t, p.Schema, conf))
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
diff --git a/helper/schema/testing.go b/helper/schema/testing.go
new file mode 100644
index 000000000..9765bdbc6
--- /dev/null
+++ b/helper/schema/testing.go
@@ -0,0 +1,30 @@
+package schema
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/terraform"
+)
+
+// TestResourceDataRaw creates a ResourceData from a raw configuration map.
+func TestResourceDataRaw(
+	t *testing.T, schema map[string]*Schema, raw map[string]interface{}) *ResourceData {
+	c, err := config.NewRawConfig(raw)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	sm := schemaMap(schema)
+	diff, err := sm.Diff(nil, terraform.NewResourceConfig(c))
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	result, err := sm.Data(nil, diff)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	return result
+}

From f29845e54ed967325ff019b7372f88310ae674af Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 17:11:05 -0800
Subject: [PATCH 12/23] update privisioner bins to use new functions

---
 builtin/bins/provisioner-file/main.go             | 5 +----
 builtin/bins/provisioner-remote-exec/main.go      | 5 +----
 builtin/provisioners/chef/resource_provisioner.go | 5 +++++
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/bins/provisioner-file/main.go b/builtin/bins/provisioner-file/main.go
index 592ff53a6..c0982b0b2 100644
--- a/builtin/bins/provisioner-file/main.go
+++ b/builtin/bins/provisioner-file/main.go
@@ -3,13 +3,10 @@ package main
 import (
 	"github.com/hashicorp/terraform/builtin/provisioners/file"
 	"github.com/hashicorp/terraform/plugin"
-	"github.com/hashicorp/terraform/terraform"
 )
 
 func main() {
 	plugin.Serve(&plugin.ServeOpts{
-		ProvisionerFunc: func() terraform.ResourceProvisioner {
-			return new(file.ResourceProvisioner)
-		},
+		ProvisionerFunc: file.Provisioner,
 	})
 }
diff --git a/builtin/bins/provisioner-remote-exec/main.go b/builtin/bins/provisioner-remote-exec/main.go
index e9874cbbe..83ba43a98 100644
--- a/builtin/bins/provisioner-remote-exec/main.go
+++ b/builtin/bins/provisioner-remote-exec/main.go
@@ -3,13 +3,10 @@ package main
 import (
 	"github.com/hashicorp/terraform/builtin/provisioners/remote-exec"
 	"github.com/hashicorp/terraform/plugin"
-	"github.com/hashicorp/terraform/terraform"
 )
 
 func main() {
 	plugin.Serve(&plugin.ServeOpts{
-		ProvisionerFunc: func() terraform.ResourceProvisioner {
-			return new(remoteexec.ResourceProvisioner)
-		},
+		ProvisionerFunc: remoteexec.Provisioner,
 	})
 }
diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go
index 953b38cf1..2c499e115 100644
--- a/builtin/provisioners/chef/resource_provisioner.go
+++ b/builtin/provisioners/chef/resource_provisioner.go
@@ -132,6 +132,11 @@ type Provisioner struct {
 // ResourceProvisioner represents a generic chef provisioner
 type ResourceProvisioner struct{}
 
+func (r *ResourceProvisioner) Stop() error {
+	// Noop for now. TODO in the future.
+	return nil
+}
+
 // Apply executes the file provisioner
 func (r *ResourceProvisioner) Apply(
 	o terraform.UIOutput,

From 487a37b0dd39f7aa55fae7e35017ddc405e03f69 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 17:55:23 -0800
Subject: [PATCH 13/23] helper/schema: PromoteSingle for legacy support of
 "maybe list" types

---
 .../remote-exec/resource_provisioner.go       |   1 +
 helper/schema/field_reader_config.go          |  25 +++++
 helper/schema/schema.go                       |  21 +++-
 helper/schema/schema_test.go                  | 100 ++++++++++++++++++
 4 files changed, 144 insertions(+), 3 deletions(-)

diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go
index 43ab12b0a..cda3c0b69 100644
--- a/builtin/provisioners/remote-exec/resource_provisioner.go
+++ b/builtin/provisioners/remote-exec/resource_provisioner.go
@@ -23,6 +23,7 @@ func Provisioner() terraform.ResourceProvisioner {
 			"inline": &schema.Schema{
 				Type:          schema.TypeList,
 				Elem:          &schema.Schema{Type: schema.TypeString},
+				PromoteSingle: true,
 				Optional:      true,
 				ConflictsWith: []string{"script", "scripts"},
 			},
diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go
index 53ff5208f..f958bbcb1 100644
--- a/helper/schema/field_reader_config.go
+++ b/helper/schema/field_reader_config.go
@@ -79,10 +79,35 @@ func (r *ConfigFieldReader) readField(
 
 	k := strings.Join(address, ".")
 	schema := schemaList[len(schemaList)-1]
+
+	// If we're getting the single element of a promoted list, then
+	// check to see if we have a single element we need to promote.
+	if address[len(address)-1] == "0" && len(schemaList) > 1 {
+		lastSchema := schemaList[len(schemaList)-2]
+		if lastSchema.Type == TypeList && lastSchema.PromoteSingle {
+			k := strings.Join(address[:len(address)-1], ".")
+			result, err := r.readPrimitive(k, schema)
+			if err == nil {
+				return result, nil
+			}
+		}
+	}
+
 	switch schema.Type {
 	case TypeBool, TypeFloat, TypeInt, TypeString:
 		return r.readPrimitive(k, schema)
 	case TypeList:
+		// If we support promotion then we first check if we have a lone
+		// value that we must promote.
+		// a value that is alone.
+		if schema.PromoteSingle {
+			result, err := r.readPrimitive(k, schema.Elem.(*Schema))
+			if err == nil && result.Exists {
+				result.Value = []interface{}{result.Value}
+				return result, nil
+			}
+		}
+
 		return readListField(&nestedConfigFieldReader{r}, address, schema)
 	case TypeMap:
 		return r.readMap(k, schema)
diff --git a/helper/schema/schema.go b/helper/schema/schema.go
index 25ef5e169..924049d6c 100644
--- a/helper/schema/schema.go
+++ b/helper/schema/schema.go
@@ -118,9 +118,16 @@ type Schema struct {
 	// TypeSet or TypeList. Specific use cases would be if a TypeSet is being
 	// used to wrap a complex structure, however less than one instance would
 	// cause instability.
-	Elem     interface{}
-	MaxItems int
-	MinItems int
+	//
+	// PromoteSingle, if true, will allow single elements to be standalone
+	// and promote them to a list. For example "foo" would be promoted to
+	// ["foo"] automatically. This is primarily for legacy reasons and the
+	// ambiguity is not recommended for new usage. Promotion is only allowed
+	// for primitive element types.
+	Elem          interface{}
+	MaxItems      int
+	MinItems      int
+	PromoteSingle bool
 
 	// The following fields are only valid for a TypeSet type.
 	//
@@ -1140,6 +1147,14 @@ func (m schemaMap) validateList(
 	// We use reflection to verify the slice because you can't
 	// case to []interface{} unless the slice is exactly that type.
 	rawV := reflect.ValueOf(raw)
+
+	// If we support promotion and the raw value isn't a slice, wrap
+	// it in []interface{} and check again.
+	if schema.PromoteSingle && rawV.Kind() != reflect.Slice {
+		raw = []interface{}{raw}
+		rawV = reflect.ValueOf(raw)
+	}
+
 	if rawV.Kind() != reflect.Slice {
 		return nil, []error{fmt.Errorf(
 			"%s: should be a list", k)}
diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index 33ac26f76..dc42215ec 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -582,6 +582,72 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		{
+			Name: "List decode with promotion",
+			Schema: map[string]*Schema{
+				"ports": &Schema{
+					Type:          TypeList,
+					Required:      true,
+					Elem:          &Schema{Type: TypeInt},
+					PromoteSingle: true,
+				},
+			},
+
+			State: nil,
+
+			Config: map[string]interface{}{
+				"ports": "5",
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"ports.#": &terraform.ResourceAttrDiff{
+						Old: "0",
+						New: "1",
+					},
+					"ports.0": &terraform.ResourceAttrDiff{
+						Old: "",
+						New: "5",
+					},
+				},
+			},
+
+			Err: false,
+		},
+
+		{
+			Name: "List decode with promotion with list",
+			Schema: map[string]*Schema{
+				"ports": &Schema{
+					Type:          TypeList,
+					Required:      true,
+					Elem:          &Schema{Type: TypeInt},
+					PromoteSingle: true,
+				},
+			},
+
+			State: nil,
+
+			Config: map[string]interface{}{
+				"ports": []interface{}{"5"},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"ports.#": &terraform.ResourceAttrDiff{
+						Old: "0",
+						New: "1",
+					},
+					"ports.0": &terraform.ResourceAttrDiff{
+						Old: "",
+						New: "5",
+					},
+				},
+			},
+
+			Err: false,
+		},
+
 		{
 			Schema: map[string]*Schema{
 				"ports": &Schema{
@@ -3585,6 +3651,40 @@ func TestSchemaMap_Validate(t *testing.T) {
 			Err: true,
 		},
 
+		"List with promotion": {
+			Schema: map[string]*Schema{
+				"ingress": &Schema{
+					Type:          TypeList,
+					Elem:          &Schema{Type: TypeInt},
+					PromoteSingle: true,
+					Optional:      true,
+				},
+			},
+
+			Config: map[string]interface{}{
+				"ingress": "5",
+			},
+
+			Err: false,
+		},
+
+		"List with promotion set as list": {
+			Schema: map[string]*Schema{
+				"ingress": &Schema{
+					Type:          TypeList,
+					Elem:          &Schema{Type: TypeInt},
+					PromoteSingle: true,
+					Optional:      true,
+				},
+			},
+
+			Config: map[string]interface{}{
+				"ingress": []interface{}{"5"},
+			},
+
+			Err: false,
+		},
+
 		"Optional sub-resource": {
 			Schema: map[string]*Schema{
 				"ingress": &Schema{

From 447a5c8b9e1bfde3dcf5caec156735e7a25ac413 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 18:03:47 -0800
Subject: [PATCH 14/23] scripts: update internal plugin gen to support new
 provisioner

---
 scripts/generate-plugins.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/generate-plugins.go b/scripts/generate-plugins.go
index 0867f9755..a10f1da6e 100644
--- a/scripts/generate-plugins.go
+++ b/scripts/generate-plugins.go
@@ -91,7 +91,7 @@ func makeProviderMap(items []plugin) string {
 func makeProvisionerMap(items []plugin) string {
 	output := ""
 	for _, item := range items {
-		output += fmt.Sprintf("\t\"%s\": func() terraform.ResourceProvisioner { return new(%s.%s) },\n", item.PluginName, item.ImportName, item.TypeName)
+		output += fmt.Sprintf("\t\"%s\":   %s.%s,\n", item.PluginName, item.ImportName, item.TypeName)
 	}
 	return output
 }
@@ -254,8 +254,8 @@ func discoverProviders() ([]plugin, error) {
 
 func discoverProvisioners() ([]plugin, error) {
 	path := "./builtin/provisioners"
-	typeID := "ResourceProvisioner"
-	typeName := ""
+	typeID := "terraform.ResourceProvisioner"
+	typeName := "Provisioner"
 	return discoverTypesInPath(path, typeID, typeName)
 }
 

From cde458d74f39d35381c171f902cfb83570bbe1f5 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 22 Dec 2016 18:17:49 -0800
Subject: [PATCH 15/23] scripts: update tests for generate plugins to pass new
 style

---
 scripts/generate-plugins.go      |  9 +++++++++
 scripts/generate-plugins_test.go | 23 ++++++++++-------------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/scripts/generate-plugins.go b/scripts/generate-plugins.go
index a10f1da6e..07bf33be7 100644
--- a/scripts/generate-plugins.go
+++ b/scripts/generate-plugins.go
@@ -270,6 +270,9 @@ import (
 IMPORTS
 	"github.com/hashicorp/terraform/plugin"
 	"github.com/hashicorp/terraform/terraform"
+
+	// Legacy, will remove once it conforms with new structure
+	chefprovisioner "github.com/hashicorp/terraform/builtin/provisioners/chef"
 )
 
 var InternalProviders = map[string]plugin.ProviderFunc{
@@ -280,4 +283,10 @@ var InternalProvisioners = map[string]plugin.ProvisionerFunc{
 PROVISIONERS
 }
 
+func init() {
+	// Legacy provisioners that don't match our heuristics for auto-finding
+	// built-in provisioners.
+	InternalProvisioners["chef"] = func() terraform.ResourceProvisioner { return new(chefprovisioner.ResourceProvisioner) }
+}
+
 `
diff --git a/scripts/generate-plugins_test.go b/scripts/generate-plugins_test.go
index bbb3fce18..5979ee28f 100644
--- a/scripts/generate-plugins_test.go
+++ b/scripts/generate-plugins_test.go
@@ -7,29 +7,29 @@ func TestMakeProvisionerMap(t *testing.T) {
 		{
 			Package:    "file",
 			PluginName: "file",
-			TypeName:   "ResourceProvisioner",
+			TypeName:   "Provisioner",
 			Path:       "builtin/provisioners/file",
-			ImportName: "fileresourceprovisioner",
+			ImportName: "fileprovisioner",
 		},
 		{
 			Package:    "localexec",
 			PluginName: "local-exec",
-			TypeName:   "ResourceProvisioner",
+			TypeName:   "Provisioner",
 			Path:       "builtin/provisioners/local-exec",
-			ImportName: "localexecresourceprovisioner",
+			ImportName: "localexecprovisioner",
 		},
 		{
 			Package:    "remoteexec",
 			PluginName: "remote-exec",
-			TypeName:   "ResourceProvisioner",
+			TypeName:   "Provisioner",
 			Path:       "builtin/provisioners/remote-exec",
-			ImportName: "remoteexecresourceprovisioner",
+			ImportName: "remoteexecprovisioner",
 		},
 	})
 
-	expected := `	"file": func() terraform.ResourceProvisioner { return new(fileresourceprovisioner.ResourceProvisioner) },
-	"local-exec": func() terraform.ResourceProvisioner { return new(localexecresourceprovisioner.ResourceProvisioner) },
-	"remote-exec": func() terraform.ResourceProvisioner { return new(remoteexecresourceprovisioner.ResourceProvisioner) },
+	expected := `	"file":   fileprovisioner.Provisioner,
+	"local-exec":   localexecprovisioner.Provisioner,
+	"remote-exec":   remoteexecprovisioner.Provisioner,
 `
 
 	if p != expected {
@@ -86,13 +86,10 @@ func TestDiscoverTypesProviders(t *testing.T) {
 }
 
 func TestDiscoverTypesProvisioners(t *testing.T) {
-	plugins, err := discoverTypesInPath("../builtin/provisioners", "ResourceProvisioner", "")
+	plugins, err := discoverTypesInPath("../builtin/provisioners", "terraform.ResourceProvisioner", "Provisioner")
 	if err != nil {
 		t.Fatalf(err.Error())
 	}
-	if !contains(plugins, "chef") {
-		t.Errorf("Expected to find chef provisioner")
-	}
 	if !contains(plugins, "remote-exec") {
 		t.Errorf("Expected to find remote-exec provisioner")
 	}

From b486354a9cd4e2d2c646ede02dc24147dfe4643d Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 26 Dec 2016 16:29:32 -0800
Subject: [PATCH 16/23] communicator/ssh: Disconnect() should also kill the
 actual connection

---
 communicator/ssh/communicator.go | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/communicator/ssh/communicator.go b/communicator/ssh/communicator.go
index 32a881c04..36b698a9a 100644
--- a/communicator/ssh/communicator.go
+++ b/communicator/ssh/communicator.go
@@ -42,6 +42,8 @@ type Communicator struct {
 	config   *sshConfig
 	conn     net.Conn
 	address  string
+
+	lock sync.Mutex
 }
 
 type sshConfig struct {
@@ -96,6 +98,10 @@ func New(s *terraform.InstanceState) (*Communicator, error) {
 
 // Connect implementation of communicator.Communicator interface
 func (c *Communicator) Connect(o terraform.UIOutput) (err error) {
+	// Grab a lock so we can modify our internal attributes
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
 	if c.conn != nil {
 		c.conn.Close()
 	}
@@ -190,8 +196,19 @@ func (c *Communicator) Connect(o terraform.UIOutput) (err error) {
 
 // Disconnect implementation of communicator.Communicator interface
 func (c *Communicator) Disconnect() error {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
 	if c.config.sshAgent != nil {
-		return c.config.sshAgent.Close()
+		if err := c.config.sshAgent.Close(); err != nil {
+			return err
+		}
+	}
+
+	if c.conn != nil {
+		conn := c.conn
+		c.conn = nil
+		return conn.Close()
 	}
 
 	return nil

From 142df657c3597d1d1868ce840b1402a22a912def Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 26 Dec 2016 16:29:44 -0800
Subject: [PATCH 17/23] provisioners/remote-exec: listen to Stop

---
 .../remote-exec/resource_provisioner.go       | 86 +++++++++++++++----
 terraform/context.go                          |  7 ++
 2 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go
index cda3c0b69..042e8544e 100644
--- a/builtin/provisioners/remote-exec/resource_provisioner.go
+++ b/builtin/provisioners/remote-exec/resource_provisioner.go
@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
+	"sync/atomic"
 	"time"
 
 	"github.com/hashicorp/terraform/communicator"
@@ -68,7 +69,7 @@ func applyFn(ctx context.Context) error {
 	}
 
 	// Copy and execute each script
-	if err := runScripts(o, comm, scripts); err != nil {
+	if err := runScripts(ctx, o, comm, scripts); err != nil {
 		return err
 	}
 
@@ -133,18 +134,29 @@ func collectScripts(d *schema.ResourceData) ([]io.ReadCloser, error) {
 
 // runScripts is used to copy and execute a set of scripts
 func runScripts(
+	ctx context.Context,
 	o terraform.UIOutput,
 	comm communicator.Communicator,
 	scripts []io.ReadCloser) error {
+	// Wrap out context in a cancelation function that we use to
+	// kill the connection.
+	ctx, cancelFunc := context.WithCancel(ctx)
+	defer cancelFunc()
+
+	// Wait for the context to end and then disconnect
+	go func() {
+		<-ctx.Done()
+		comm.Disconnect()
+	}()
+
 	// Wait and retry until we establish the connection
-	err := retryFunc(comm.Timeout(), func() error {
+	err := retryFunc(ctx, comm.Timeout(), func() error {
 		err := comm.Connect(o)
 		return err
 	})
 	if err != nil {
 		return err
 	}
-	defer comm.Disconnect()
 
 	for _, script := range scripts {
 		var cmd *remote.Cmd
@@ -156,7 +168,7 @@ func runScripts(
 		go copyOutput(o, errR, errDoneCh)
 
 		remotePath := comm.ScriptPath()
-		err = retryFunc(comm.Timeout(), func() error {
+		err = retryFunc(ctx, comm.Timeout(), func() error {
 			if err := comm.UploadScript(remotePath, script); err != nil {
 				return fmt.Errorf("Failed to upload script: %v", err)
 			}
@@ -179,6 +191,13 @@ func runScripts(
 			}
 		}
 
+		// If we have an error, end our context so the disconnect happens.
+		// This has to happen before the output cleanup below since during
+		// an interrupt this will cause the outputs to end.
+		if err != nil {
+			cancelFunc()
+		}
+
 		// Wait for output to clean up
 		outW.Close()
 		errW.Close()
@@ -212,19 +231,54 @@ func copyOutput(
 }
 
 // retryFunc is used to retry a function for a given duration
-func retryFunc(timeout time.Duration, f func() error) error {
-	finish := time.After(timeout)
-	for {
-		err := f()
-		if err == nil {
-			return nil
-		}
-		log.Printf("Retryable error: %v", err)
+func retryFunc(ctx context.Context, timeout time.Duration, f func() error) error {
+	// Build a new context with the timeout
+	ctx, done := context.WithTimeout(ctx, timeout)
+	defer done()
 
-		select {
-		case <-finish:
-			return err
-		case <-time.After(3 * time.Second):
+	// Try the function in a goroutine
+	var errVal atomic.Value
+	doneCh := make(chan struct{})
+	go func() {
+		defer close(doneCh)
+
+		for {
+			// If our context ended, we want to exit right away.
+			select {
+			case <-ctx.Done():
+				return
+			default:
+			}
+
+			// Try the function call
+			err := f()
+			if err == nil {
+				return
+			}
+
+			log.Printf("Retryable error: %v", err)
+			errVal.Store(err)
 		}
+	}()
+
+	// Wait for completion
+	select {
+	case <-doneCh:
+	case <-ctx.Done():
 	}
+
+	// Check if we have a context error to check if we're interrupted or timeout
+	switch ctx.Err() {
+	case context.Canceled:
+		return fmt.Errorf("interrupted")
+	case context.DeadlineExceeded:
+		return fmt.Errorf("timeout")
+	}
+
+	// Check if we got an error executing
+	if err, ok := errVal.Load().(error); ok {
+		return err
+	}
+
+	return nil
 }
diff --git a/terraform/context.go b/terraform/context.go
index 52b95f058..82e517a86 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -632,10 +632,14 @@ func (c *Context) Refresh() (*State, error) {
 //
 // Stop will block until the task completes.
 func (c *Context) Stop() {
+	log.Printf("[WARN] terraform: Stop called, initiating interrupt sequence")
+
 	c.l.Lock()
 
 	// If we're running, then stop
 	if c.runContextCancel != nil {
+		log.Printf("[WARN] terraform: run context exists, stopping")
+
 		// Tell the hook we want to stop
 		c.sh.Stop()
 
@@ -652,8 +656,11 @@ func (c *Context) Stop() {
 
 	// Wait if we have a context
 	if ctx != nil {
+		log.Printf("[WARN] terraform: stop waiting for context completion")
 		<-ctx.Done()
 	}
+
+	log.Printf("[WARN] terraform: stop complete")
 }
 
 // Validate validates the configuration and returns any warnings or errors.

From a8f64cbceeeeadb6297b400d0c43c351848c752a Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 27 Dec 2016 17:16:14 -0800
Subject: [PATCH 18/23] terraform: make sure Stop blocks until full completion

---
 terraform/context.go                          | 40 +++++-----
 terraform/context_apply_test.go               | 76 +++++++++++++++++++
 .../test-fixtures/apply-cancel-block/main.tf  |  3 +
 3 files changed, 97 insertions(+), 22 deletions(-)
 create mode 100644 terraform/test-fixtures/apply-cancel-block/main.tf

diff --git a/terraform/context.go b/terraform/context.go
index 82e517a86..c85f5f43c 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -93,6 +93,7 @@ type Context struct {
 	parallelSem         Semaphore
 	providerInputConfig map[string]map[string]interface{}
 	runLock             sync.Mutex
+	runCond             *sync.Cond
 	runContext          context.Context
 	runContextCancel    context.CancelFunc
 	shadowErr           error
@@ -648,16 +649,10 @@ func (c *Context) Stop() {
 		c.runContextCancel = nil
 	}
 
-	// Grab the context before we unlock
-	ctx := c.runContext
-
-	// Unlock
-	c.l.Unlock()
-
-	// Wait if we have a context
-	if ctx != nil {
-		log.Printf("[WARN] terraform: stop waiting for context completion")
-		<-ctx.Done()
+	// Grab the condition var before we exit
+	if cond := c.runCond; cond != nil {
+		cond.Wait()
+		c.l.Unlock()
 	}
 
 	log.Printf("[WARN] terraform: stop complete")
@@ -727,23 +722,22 @@ func (c *Context) SetVariable(k string, v interface{}) {
 }
 
 func (c *Context) acquireRun(phase string) func() {
-	// Acquire the runlock first. This is the lock that is held for
-	// the duration of a run to prevent multiple runs.
-	c.runLock.Lock()
-
 	// With the run lock held, grab the context lock to make changes
 	// to the run context.
 	c.l.Lock()
 	defer c.l.Unlock()
 
+	// Wait until we're no longer running
+	for c.runCond != nil {
+		c.runCond.Wait()
+	}
+
+	// Build our lock
+	c.runCond = sync.NewCond(&c.l)
+
 	// Setup debugging
 	dbug.SetPhase(phase)
 
-	// runContext should never be non-nil, check that here
-	if c.runContext != nil {
-		panic("acquireRun called with runContext != nil")
-	}
-
 	// Create a new run context
 	c.runContext, c.runContextCancel = context.WithCancel(context.Background())
 
@@ -772,11 +766,13 @@ func (c *Context) releaseRun() {
 		c.runContextCancel()
 	}
 
+	// Unlock all waiting our condition
+	cond := c.runCond
+	c.runCond = nil
+	cond.Broadcast()
+
 	// Unset the context
 	c.runContext = nil
-
-	// Unlock the run lock
-	c.runLock.Unlock()
 }
 
 func (c *Context) walk(
diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index 20cfe7cd7..605988e25 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -1720,6 +1720,82 @@ func TestContext2Apply_cancel(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_cancelBlock(t *testing.T) {
+	m := testModule(t, "apply-cancel-block")
+	p := testProvider("aws")
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	applyCh := make(chan struct{})
+	p.DiffFn = testDiffFn
+	p.ApplyFn = func(*InstanceInfo, *InstanceState, *InstanceDiff) (*InstanceState, error) {
+		close(applyCh)
+
+		for !ctx.sh.Stopped() {
+			// Wait for stop to be called
+		}
+
+		// Sleep
+		time.Sleep(100 * time.Millisecond)
+
+		return &InstanceState{
+			ID: "foo",
+		}, nil
+	}
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Start the Apply in a goroutine
+	var applyErr error
+	stateCh := make(chan *State)
+	go func() {
+		state, err := ctx.Apply()
+		if err != nil {
+			applyErr = err
+		}
+
+		stateCh <- state
+	}()
+
+	stopDone := make(chan struct{})
+	go func() {
+		defer close(stopDone)
+		<-applyCh
+		ctx.Stop()
+	}()
+
+	// Make sure that stop blocks
+	select {
+	case <-stopDone:
+		t.Fatal("stop should block")
+	case <-time.After(10 * time.Millisecond):
+	}
+
+	// Wait for stop
+	select {
+	case <-stopDone:
+	case <-time.After(500 * time.Millisecond):
+		t.Fatal("stop should be done")
+	}
+
+	// Wait for apply to complete
+	state := <-stateCh
+	if applyErr != nil {
+		t.Fatalf("err: %s", applyErr)
+	}
+
+	checkStateString(t, state, `
+aws_instance.foo:
+  ID = foo
+	`)
+}
+
 func TestContext2Apply_cancelProvisioner(t *testing.T) {
 	m := testModule(t, "apply-cancel-provisioner")
 	p := testProvider("aws")
diff --git a/terraform/test-fixtures/apply-cancel-block/main.tf b/terraform/test-fixtures/apply-cancel-block/main.tf
new file mode 100644
index 000000000..98f5ee87e
--- /dev/null
+++ b/terraform/test-fixtures/apply-cancel-block/main.tf
@@ -0,0 +1,3 @@
+resource "aws_instance" "foo" {
+    num = "2"
+}

From 83cc54bfbeb8df5befb8dcceab06911196f796d8 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 26 Jan 2017 15:11:47 -0800
Subject: [PATCH 19/23] updated generate output

---
 command/internal_plugin_list.go | 23 +++++++++++++++--------
 terraform/graphtype_string.go   |  4 ++--
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/command/internal_plugin_list.go b/command/internal_plugin_list.go
index 06ad2fa80..dab3e3cbc 100644
--- a/command/internal_plugin_list.go
+++ b/command/internal_plugin_list.go
@@ -65,13 +65,15 @@ import (
 	vaultprovider "github.com/hashicorp/terraform/builtin/providers/vault"
 	vcdprovider "github.com/hashicorp/terraform/builtin/providers/vcd"
 	vsphereprovider "github.com/hashicorp/terraform/builtin/providers/vsphere"
-	chefresourceprovisioner "github.com/hashicorp/terraform/builtin/provisioners/chef"
-	fileresourceprovisioner "github.com/hashicorp/terraform/builtin/provisioners/file"
-	localexecresourceprovisioner "github.com/hashicorp/terraform/builtin/provisioners/local-exec"
-	remoteexecresourceprovisioner "github.com/hashicorp/terraform/builtin/provisioners/remote-exec"
+	fileprovisioner "github.com/hashicorp/terraform/builtin/provisioners/file"
+	localexecprovisioner "github.com/hashicorp/terraform/builtin/provisioners/local-exec"
+	remoteexecprovisioner "github.com/hashicorp/terraform/builtin/provisioners/remote-exec"
 
 	"github.com/hashicorp/terraform/plugin"
 	"github.com/hashicorp/terraform/terraform"
+
+	// Legacy, will remove once it conforms with new structure
+	chefprovisioner "github.com/hashicorp/terraform/builtin/provisioners/chef"
 )
 
 var InternalProviders = map[string]plugin.ProviderFunc{
@@ -137,8 +139,13 @@ var InternalProviders = map[string]plugin.ProviderFunc{
 }
 
 var InternalProvisioners = map[string]plugin.ProvisionerFunc{
-	"chef":        func() terraform.ResourceProvisioner { return new(chefresourceprovisioner.ResourceProvisioner) },
-	"file":        func() terraform.ResourceProvisioner { return new(fileresourceprovisioner.ResourceProvisioner) },
-	"local-exec":  func() terraform.ResourceProvisioner { return new(localexecresourceprovisioner.ResourceProvisioner) },
-	"remote-exec": func() terraform.ResourceProvisioner { return new(remoteexecresourceprovisioner.ResourceProvisioner) },
+	"file":        fileprovisioner.Provisioner,
+	"local-exec":  localexecprovisioner.Provisioner,
+	"remote-exec": remoteexecprovisioner.Provisioner,
+}
+
+func init() {
+	// Legacy provisioners that don't match our heuristics for auto-finding
+	// built-in provisioners.
+	InternalProvisioners["chef"] = func() terraform.ResourceProvisioner { return new(chefprovisioner.ResourceProvisioner) }
 }
diff --git a/terraform/graphtype_string.go b/terraform/graphtype_string.go
index ccf9da711..88ecad4f6 100644
--- a/terraform/graphtype_string.go
+++ b/terraform/graphtype_string.go
@@ -4,9 +4,9 @@ package terraform
 
 import "fmt"
 
-const _GraphType_name = "GraphTypeInvalidGraphTypeLegacyGraphTypeRefreshGraphTypePlanGraphTypePlanDestroyGraphTypeApply"
+const _GraphType_name = "GraphTypeInvalidGraphTypeLegacyGraphTypeRefreshGraphTypePlanGraphTypePlanDestroyGraphTypeApplyGraphTypeInputGraphTypeValidate"
 
-var _GraphType_index = [...]uint8{0, 16, 31, 47, 60, 80, 94}
+var _GraphType_index = [...]uint8{0, 16, 31, 47, 60, 80, 94, 108, 125}
 
 func (i GraphType) String() string {
 	if i >= GraphType(len(_GraphType_index)-1) {

From 5b427811177125c555770fbb2968e7e19d0c1b27 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 30 Jan 2017 08:35:10 -0800
Subject: [PATCH 20/23] terraform: defer unlock of lock in Stop to enure it
 always unlocks

---
 terraform/context.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/terraform/context.go b/terraform/context.go
index c85f5f43c..5f8731413 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -636,6 +636,7 @@ func (c *Context) Stop() {
 	log.Printf("[WARN] terraform: Stop called, initiating interrupt sequence")
 
 	c.l.Lock()
+	defer c.l.Unlock()
 
 	// If we're running, then stop
 	if c.runContextCancel != nil {
@@ -652,7 +653,6 @@ func (c *Context) Stop() {
 	// Grab the condition var before we exit
 	if cond := c.runCond; cond != nil {
 		cond.Wait()
-		c.l.Unlock()
 	}
 
 	log.Printf("[WARN] terraform: stop complete")

From 00232f09945fc3987d2f3a287597bc7e19d28f18 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 30 Jan 2017 08:41:38 -0800
Subject: [PATCH 21/23] terraform: acquireRun during test to avoid special case
 logic

---
 terraform/context.go               | 11 ++++-------
 terraform/context_validate_test.go |  1 +
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/terraform/context.go b/terraform/context.go
index 5f8731413..bd9e80aa0 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -907,13 +907,10 @@ func (c *Context) walk(
 }
 
 func (c *Context) watchStop(walker *ContextGraphWalker, doneCh <-chan struct{}) {
-	// Get the stop channel. runContext might be nil only during tests.
-	// If this is called during a proper run operation, this will never
-	// be nil.
-	var stopCh <-chan struct{}
-	if ctx := c.runContext; ctx != nil {
-		stopCh = ctx.Done()
-	}
+	// Get the stop channel. runContext will never be nil since this should
+	// only be called within the context of an operation started with
+	// acquireRun
+	stopCh := c.runContext.Done()
 
 	// Wait for a stop or completion
 	select {
diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go
index 9e434a7be..5bf38eb2c 100644
--- a/terraform/context_validate_test.go
+++ b/terraform/context_validate_test.go
@@ -906,6 +906,7 @@ func TestContext2Validate_PlanGraphBuilder(t *testing.T) {
 		Targets:   c.targets,
 	}).Build(RootModulePath)
 
+	defer c.acquireRun("validate-test")()
 	walker, err := c.walk(graph, graph, walkValidate)
 	if err != nil {
 		t.Fatal(err)

From 3e771a674cdcee508999e98e1cdcc30e1f7ed248 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 30 Jan 2017 08:49:07 -0800
Subject: [PATCH 22/23] terraform: acquire stopCh outside goroutine to ensure
 in lock

---
 terraform/context.go | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/terraform/context.go b/terraform/context.go
index bd9e80aa0..85b7f582d 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -811,7 +811,8 @@ func (c *Context) walk(
 
 	// Watch for a stop so we can call the provider Stop() API.
 	doneCh := make(chan struct{})
-	go c.watchStop(walker, doneCh)
+	stopCh := c.runContext.Done()
+	go c.watchStop(walker, doneCh, stopCh)
 
 	// Walk the real graph, this will block until it completes
 	realErr := graph.Walk(walker)
@@ -906,12 +907,7 @@ func (c *Context) walk(
 	return walker, realErr
 }
 
-func (c *Context) watchStop(walker *ContextGraphWalker, doneCh <-chan struct{}) {
-	// Get the stop channel. runContext will never be nil since this should
-	// only be called within the context of an operation started with
-	// acquireRun
-	stopCh := c.runContext.Done()
-
+func (c *Context) watchStop(walker *ContextGraphWalker, doneCh, stopCh <-chan struct{}) {
 	// Wait for a stop or completion
 	select {
 	case <-stopCh:

From 3776d31d6981493a41a59651bdd1137c16ce3b0f Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 30 Jan 2017 10:21:05 -0800
Subject: [PATCH 23/23] provisioners/local-exec: remove data race by setting
 err only once

---
 builtin/provisioners/local-exec/resource_provisioner.go | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/provisioners/local-exec/resource_provisioner.go b/builtin/provisioners/local-exec/resource_provisioner.go
index 86f27fb35..0ac2f83c0 100644
--- a/builtin/provisioners/local-exec/resource_provisioner.go
+++ b/builtin/provisioners/local-exec/resource_provisioner.go
@@ -72,15 +72,14 @@ func applyFn(ctx context.Context) error {
 	err := cmd.Start()
 	if err == nil {
 		// Wait for the command to complete in a goroutine
-		doneCh := make(chan struct{})
+		doneCh := make(chan error, 1)
 		go func() {
-			defer close(doneCh)
-			err = cmd.Wait()
+			doneCh <- cmd.Wait()
 		}()
 
 		// Wait for the command to finish or for us to be interrupted
 		select {
-		case <-doneCh:
+		case err = <-doneCh:
 		case <-ctx.Done():
 			cmd.Process.Kill()
 			err = cmd.Wait()