Merge pull request #9607 from hashicorp/f-provider-stop-redo

terraform: ResourceProvider.Stop (redo)

builtin/providers/azurerm/config.go

@@ -1,6 +1,7 @@
 package azurerm
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"net/http"
@@ -30,6 +31,8 @@ type ArmClient struct {
 	tenantId       string
 	subscriptionId string
 
+	StopContext context.Context
+
 	rivieraClient *riviera.Client
 
 	availSetClient         compute.AvailabilitySetsClient

builtin/providers/azurerm/provider.go

@@ -17,7 +17,8 @@ import (
 
 // Provider returns a terraform.ResourceProvider.
 func Provider() terraform.ResourceProvider {
-	return &schema.Provider{
+	var p *schema.Provider
+	p = &schema.Provider{
 		Schema: map[string]*schema.Schema{
 			"subscription_id": {
 				Type:        schema.TypeString,
@@ -105,8 +106,10 @@ func Provider() terraform.ResourceProvider {
 			"azurerm_sql_firewall_rule": resourceArmSqlFirewallRule(),
 			"azurerm_sql_server":        resourceArmSqlServer(),
 		},
-		ConfigureFunc: providerConfigure,
+		ConfigureFunc: providerConfigure(p),
 	}
+
+	return p
 }
 
 // Config is the configuration structure used to instantiate a
@@ -141,29 +144,33 @@ func (c *Config) validate() error {
 	return err.ErrorOrNil()
 }
 
-func providerConfigure(d *schema.ResourceData) (interface{}, error) {
+func providerConfigure(p *schema.Provider) schema.ConfigureFunc {
-	config := &Config{
+	return func(d *schema.ResourceData) (interface{}, error) {
-		SubscriptionID: d.Get("subscription_id").(string),
+		config := &Config{
-		ClientID:       d.Get("client_id").(string),
+			SubscriptionID: d.Get("subscription_id").(string),
-		ClientSecret:   d.Get("client_secret").(string),
+			ClientID:       d.Get("client_id").(string),
-		TenantID:       d.Get("tenant_id").(string),
+			ClientSecret:   d.Get("client_secret").(string),
-	}
+			TenantID:       d.Get("tenant_id").(string),
+		}
 
-	if err := config.validate(); err != nil {
+		if err := config.validate(); err != nil {
-		return nil, err
+			return nil, err
-	}
+		}
 
-	client, err := config.getArmClient()
+		client, err := config.getArmClient()
-	if err != nil {
+		if err != nil {
-		return nil, err
+			return nil, err
-	}
+		}
 
-	err = registerAzureResourceProvidersWithSubscription(client.rivieraClient)
+		client.StopContext = p.StopContext()
-	if err != nil {
-		return nil, err
-	}
 
-	return client, nil
+		err = registerAzureResourceProvidersWithSubscription(client.rivieraClient)
+		if err != nil {
+			return nil, err
+		}
+
+		return client, nil
+	}
 }
 
 func registerProviderWithSubscription(providerName string, client *riviera.Client) error {

builtin/providers/azurerm/resource_arm_storage_account.go

@@ -1,6 +1,7 @@
 package azurerm
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"net/http"
@@ -11,7 +12,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/arm/storage"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
-	"github.com/hashicorp/terraform/helper/signalwrapper"
 	"github.com/hashicorp/terraform/helper/validation"
 )
 
@@ -192,24 +192,11 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e
 		opts.Properties.AccessTier = storage.AccessTier(accessTier.(string))
 	}
 
-	// Create the storage account. We wrap this so that it is cancellable
+	// Create
-	// with a Ctrl-C since this can take a LONG time.
+	cancelCtx, cancelFunc := context.WithTimeout(client.StopContext, 1*time.Hour)
-	wrap := signalwrapper.Run(func(cancelCh <-chan struct{}) error {
+	_, createErr := storageClient.Create(
-		_, err := storageClient.Create(resourceGroupName, storageAccountName, opts, cancelCh)
+		resourceGroupName, storageAccountName, opts, cancelCtx.Done())
-		return err
+	cancelFunc()
-	})
-
-	// Check the result of the wrapped function.
-	var createErr error
-	select {
-	case <-time.After(1 * time.Hour):
-		// An hour is way above the expected P99 for this API call so
-		// we premature cancel and error here.
-		createErr = wrap.Cancel()
-	case createErr = <-wrap.ErrCh:
-		// Successfully ran (but perhaps not successfully completed)
-		// the function.
-	}
 
 	// The only way to get the ID back apparently is to read the resource again
 	read, err := storageClient.GetProperties(resourceGroupName, storageAccountName)

helper/schema/provider.go

@@ -1,9 +1,11 @@
 package schema
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"sort"
+	"sync"
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/terraform"
@@ -49,6 +51,10 @@ type Provider struct {
 	ConfigureFunc ConfigureFunc
 
 	meta interface{}
+
+	stopCtx       context.Context
+	stopCtxCancel context.CancelFunc
+	stopOnce      sync.Once
 }
 
 // ConfigureFunc is the function used to configure a Provider.
@@ -104,6 +110,34 @@ func (p *Provider) SetMeta(v interface{}) {
 	p.meta = v
 }
 
+// Stopped reports whether the provider has been stopped or not.
+func (p *Provider) Stopped() bool {
+	ctx := p.StopContext()
+	select {
+	case <-ctx.Done():
+		return true
+	default:
+		return false
+	}
+}
+
+// StopCh returns a channel that is closed once the provider is stopped.
+func (p *Provider) StopContext() context.Context {
+	p.stopOnce.Do(p.stopInit)
+	return p.stopCtx
+}
+
+func (p *Provider) stopInit() {
+	p.stopCtx, p.stopCtxCancel = context.WithCancel(context.Background())
+}
+
+// Stop implementation of terraform.ResourceProvider interface.
+func (p *Provider) Stop() error {
+	p.stopOnce.Do(p.stopInit)
+	p.stopCtxCancel()
+	return nil
+}
+
 // Input implementation of terraform.ResourceProvider interface.
 func (p *Provider) Input(
 	input terraform.UIInput,

helper/schema/provider_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/terraform"
@@ -328,3 +329,55 @@ func TestProviderMeta(t *testing.T) {
 		t.Fatalf("bad: %#v", v)
 	}
 }
+
+func TestProviderStop(t *testing.T) {
+	var p Provider
+
+	if p.Stopped() {
+		t.Fatal("should not be stopped")
+	}
+
+	// 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)
+	}
+
+	// Verify
+	if !p.Stopped() {
+		t.Fatal("should be stopped")
+	}
+
+	select {
+	case <-ch:
+	case <-time.After(10 * time.Millisecond):
+		t.Fatal("should be stopped")
+	}
+}
+
+func TestProviderStop_stopFirst(t *testing.T) {
+	var p Provider
+
+	// Stop it
+	if err := p.Stop(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Verify
+	if !p.Stopped() {
+		t.Fatal("should be stopped")
+	}
+
+	select {
+	case <-p.StopContext().Done():
+	case <-time.After(10 * time.Millisecond):
+		t.Fatal("should be stopped")
+	}
+}

plugin/resource_provider.go

@@ -28,6 +28,19 @@ type ResourceProvider struct {
 	Client *rpc.Client
 }
 
+func (p *ResourceProvider) Stop() error {
+	var resp ResourceProviderStopResponse
+	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 *ResourceProvider) Input(
 	input terraform.UIInput,
 	c *terraform.ResourceConfig) (*terraform.ResourceConfig, error) {
@@ -295,6 +308,10 @@ type ResourceProviderServer struct {
 	Provider terraform.ResourceProvider
 }
 
+type ResourceProviderStopResponse struct {
+	Error *plugin.BasicError
+}
+
 type ResourceProviderConfigureResponse struct {
 	Error *plugin.BasicError
 }
@@ -390,6 +407,17 @@ type ResourceProviderValidateResourceResponse struct {
 	Errors   []*plugin.BasicError
 }
 
+func (s *ResourceProviderServer) Stop(
+	_ interface{},
+	reply *ResourceProviderStopResponse) error {
+	err := s.Provider.Stop()
+	*reply = ResourceProviderStopResponse{
+		Error: plugin.NewBasicError(err),
+	}
+
+	return nil
+}
+
 func (s *ResourceProviderServer) Input(
 	args *ResourceProviderInputArgs,
 	reply *ResourceProviderInputResponse) error {

plugin/resource_provider_test.go

@@ -14,6 +14,61 @@ func TestResourceProvider_impl(t *testing.T) {
 	var _ terraform.ResourceProvider = new(ResourceProvider)
 }
 
+func TestResourceProvider_stop(t *testing.T) {
+	// Create a mock provider
+	p := new(terraform.MockResourceProvider)
+	client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{
+		ProviderFunc: testProviderFixed(p),
+	}))
+	defer client.Close()
+
+	// Request the provider
+	raw, err := client.Dispense(ProviderPluginName)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	provider := raw.(terraform.ResourceProvider)
+
+	// Stop
+	e := provider.Stop()
+	if !p.StopCalled {
+		t.Fatal("stop should be called")
+	}
+	if e != nil {
+		t.Fatalf("bad: %#v", e)
+	}
+}
+
+func TestResourceProvider_stopErrors(t *testing.T) {
+	p := new(terraform.MockResourceProvider)
+	p.StopReturnError = errors.New("foo")
+
+	// Create a mock provider
+	client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{
+		ProviderFunc: testProviderFixed(p),
+	}))
+	defer client.Close()
+
+	// Request the provider
+	raw, err := client.Dispense(ProviderPluginName)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	provider := raw.(terraform.ResourceProvider)
+
+	// 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 TestResourceProvider_input(t *testing.T) {
 	// Create a mock provider
 	p := new(terraform.MockResourceProvider)

terraform/context.go

@@ -92,6 +92,7 @@ type Context struct {
 	parallelSem         Semaphore
 	providerInputConfig map[string]map[string]interface{}
 	runCh               <-chan struct{}
+	stopCh              chan struct{}
 	shadowErr           error
 }
 
@@ -588,6 +589,9 @@ func (c *Context) Stop() {
 	// Tell the hook we want to stop
 	c.sh.Stop()
 
+	// Close the stop channel
+	close(c.stopCh)
+
 	// Wait for us to stop
 	c.l.Unlock()
 	<-ch
@@ -675,6 +679,9 @@ func (c *Context) acquireRun(phase string) chan<- struct{} {
 	ch := make(chan struct{})
 	c.runCh = ch
 
+	// Reset the stop channel so we can watch that
+	c.stopCh = make(chan struct{})
+
 	// Reset the stop hook so we're not stopped
 	c.sh.Reset()
 
@@ -695,6 +702,7 @@ func (c *Context) releaseRun(ch chan<- struct{}) {
 
 	close(ch)
 	c.runCh = nil
+	c.stopCh = nil
 }
 
 func (c *Context) walk(
@@ -732,9 +740,16 @@ func (c *Context) walk(
 		DebugGraph: dg,
 	}
 
+	// Watch for a stop so we can call the provider Stop() API.
+	doneCh := make(chan struct{})
+	go c.watchStop(walker, c.stopCh, doneCh)
+
 	// Walk the real graph, this will block until it completes
 	realErr := graph.Walk(walker)
 
+	// Close the done channel so the watcher stops
+	close(doneCh)
+
 	// If we have a shadow graph and we interrupted the real graph, then
 	// we just close the shadow and never verify it. It is non-trivial to
 	// recreate the exact execution state up until an interruption so this
@@ -824,6 +839,35 @@ func (c *Context) walk(
 	return walker, realErr
 }
 
+func (c *Context) watchStop(walker *ContextGraphWalker, stopCh, doneCh <-chan struct{}) {
+	// Wait for a stop or completion
+	select {
+	case <-stopCh:
+		// Stop was triggered. Fall out of the select
+	case <-doneCh:
+		// Done, just exit completely
+		return
+	}
+
+	// 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()
+
+	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()
+	}
+}
+
 // parseVariableAsHCL parses the value of a single variable as would have been specified
 // on the command line via -var or in an environment variable named TF_VAR_x, where x is
 // the name of the variable. In order to get around the restriction of HCL requiring a

terraform/context_apply_test.go

@@ -1157,6 +1157,10 @@ func TestContext2Apply_cancel(t *testing.T) {
 	if actual != expected {
 		t.Fatalf("bad: \n%s", actual)
 	}
+
+	if !p.StopCalled {
+		t.Fatal("stop should be called")
+	}
 }
 
 func TestContext2Apply_compute(t *testing.T) {

terraform/resource_provider.go

@@ -47,6 +47,26 @@ type ResourceProvider interface {
 	// knows how to manage.
 	Resources() []ResourceType
 
+	// Stop is called when the provider 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
+
 	/*********************************************************************
 	* Functions related to individual resources
 	*********************************************************************/

terraform/resource_provider_mock.go

@@ -56,6 +56,9 @@ type MockResourceProvider struct {
 	ReadDataDiffFn                 func(*InstanceInfo, *ResourceConfig) (*InstanceDiff, error)
 	ReadDataDiffReturn             *InstanceDiff
 	ReadDataDiffReturnError        error
+	StopCalled                     bool
+	StopFn                         func() error
+	StopReturnError                error
 	DataSourcesCalled              bool
 	DataSourcesReturn              []DataSource
 	ValidateCalled                 bool
@@ -141,6 +144,18 @@ func (p *MockResourceProvider) Configure(c *ResourceConfig) error {
 	return p.ConfigureReturnError
 }
 
+func (p *MockResourceProvider) Stop() error {
+	p.Lock()
+	defer p.Unlock()
+
+	p.StopCalled = true
+	if p.StopFn != nil {
+		return p.StopFn()
+	}
+
+	return p.StopReturnError
+}
+
 func (p *MockResourceProvider) Apply(
 	info *InstanceInfo,
 	state *InstanceState,

terraform/shadow_resource_provider.go

@@ -107,6 +107,10 @@ func (p *shadowResourceProviderReal) Configure(c *ResourceConfig) error {
 	return err
 }
 
+func (p *shadowResourceProviderReal) Stop() error {
+	return p.ResourceProvider.Stop()
+}
+
 func (p *shadowResourceProviderReal) ValidateResource(
 	t string, c *ResourceConfig) ([]string, []error) {
 	key := t
@@ -441,6 +445,11 @@ func (p *shadowResourceProviderShadow) Configure(c *ResourceConfig) error {
 	return result.Result
 }
 
+// Stop returns immediately.
+func (p *shadowResourceProviderShadow) Stop() error {
+	return nil
+}
+
 func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceConfig) ([]string, []error) {
 	// Unique key
 	key := t