diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 6d2deb9e53..c50d6d1204 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -22,6 +22,13 @@ import ( const TestEnvVar = "TF_ACC" +// TestProvider can be implemented by any ResourceProvider to provide custom +// reset functionality at the start of an acceptance test. +// The helper/schema Provider implements this interface. +type TestProvider interface { + TestReset() error +} + // TestCheckFunc is the callback type used with acceptance tests to check // the state of a resource. The state passed in is the latest state known, // or in the case of being after a destroy, it is the last known state when @@ -216,13 +223,9 @@ func Test(t TestT, c TestCase) { c.PreCheck() } - // Build our context options that we can - ctxProviders := c.ProviderFactories - if ctxProviders == nil { - ctxProviders = make(map[string]terraform.ResourceProviderFactory) - for k, p := range c.Providers { - ctxProviders[k] = terraform.ResourceProviderFactoryFixed(p) - } + ctxProviders, err := testProviderFactories(c) + if err != nil { + t.Fatal(err) } opts := terraform.ContextOpts{Providers: ctxProviders} @@ -333,6 +336,43 @@ func Test(t TestT, c TestCase) { } } +// testProviderFactories is a helper to build the ResourceProviderFactory map +// with pre instantiated ResourceProviders, so that we can reset them for the +// test, while only calling the factory function once. +// Any errors are stored so that they can be returned by the factory in +// terraform to match non-test behavior. +func testProviderFactories(c TestCase) (map[string]terraform.ResourceProviderFactory, error) { + ctxProviders := make(map[string]terraform.ResourceProviderFactory) + + // add any fixed providers + for k, p := range c.Providers { + ctxProviders[k] = terraform.ResourceProviderFactoryFixed(p) + } + + // call any factory functions and store the result. + for k, pf := range c.ProviderFactories { + p, err := pf() + ctxProviders[k] = func() (terraform.ResourceProvider, error) { + return p, err + } + } + + // reset the providers if needed + for k, pf := range ctxProviders { + // we can ignore any errors here, if we don't have a provider to reset + // the error will be handled later + p, _ := pf() + if p, ok := p.(TestProvider); ok { + err := p.TestReset() + if err != nil { + return nil, fmt.Errorf("[ERROR] failed to reset provider %q: %s", k, err) + } + } + } + + return ctxProviders, nil +} + // UnitTest is a helper to force the acceptance testing harness to run in the // normal unit test suite. This should only be used for resource that don't // have any external dependencies. diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index d2e05c0c52..634f307734 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -4,7 +4,9 @@ import ( "errors" "fmt" "os" + "regexp" "strings" + "sync" "sync/atomic" "testing" @@ -25,8 +27,26 @@ func init() { } } +// wrap the mock provider to implement TestProvider +type resetProvider struct { + *terraform.MockResourceProvider + mu sync.Mutex + TestResetCalled bool + TestResetError error +} + +func (p *resetProvider) TestReset() error { + p.mu.Lock() + defer p.mu.Unlock() + p.TestResetCalled = true + return p.TestResetError +} + func TestTest(t *testing.T) { - mp := testProvider() + mp := &resetProvider{ + MockResourceProvider: testProvider(), + } + mp.DiffReturn = nil mp.ApplyFn = func( @@ -95,6 +115,9 @@ func TestTest(t *testing.T) { if !checkDestroy { t.Fatal("didn't call check for destroy") } + if !mp.TestResetCalled { + t.Fatal("didn't call TestReset") + } } func TestTest_idRefresh(t *testing.T) { @@ -355,6 +378,53 @@ func TestTest_stepError(t *testing.T) { } } +func TestTest_factoryError(t *testing.T) { + resourceFactoryError := fmt.Errorf("resource factory error") + + factory := func() (terraform.ResourceProvider, error) { + return nil, resourceFactoryError + } + + mt := new(mockT) + Test(mt, TestCase{ + ProviderFactories: map[string]terraform.ResourceProviderFactory{ + "test": factory, + }, + Steps: []TestStep{ + TestStep{ + ExpectError: regexp.MustCompile("resource factory error"), + }, + }, + }) + + if !mt.failed() { + t.Fatal("test should've failed") + } +} + +func TestTest_resetError(t *testing.T) { + mp := &resetProvider{ + MockResourceProvider: testProvider(), + TestResetError: fmt.Errorf("provider reset error"), + } + + mt := new(mockT) + Test(mt, TestCase{ + Providers: map[string]terraform.ResourceProvider{ + "test": mp, + }, + Steps: []TestStep{ + TestStep{ + ExpectError: regexp.MustCompile("provider reset error"), + }, + }, + }) + + if !mt.failed() { + t.Fatal("test should've failed") + } +} + func TestComposeAggregateTestCheckFunc(t *testing.T) { check1 := func(s *terraform.State) error { return errors.New("Error 1") diff --git a/helper/schema/provider.go b/helper/schema/provider.go index 5b50d54a1f..d52d2f5f06 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -50,8 +50,15 @@ type Provider struct { // See the ConfigureFunc documentation for more information. ConfigureFunc ConfigureFunc + // MetaReset is called by TestReset to reset any state stored in the meta + // interface. This is especially important if the StopContext is stored by + // the provider. + MetaReset func() error + meta interface{} + // a mutex is required because TestReset can directly repalce the stopCtx + stopMu sync.Mutex stopCtx context.Context stopCtxCancel context.CancelFunc stopOnce sync.Once @@ -124,20 +131,43 @@ func (p *Provider) Stopped() bool { // StopCh returns a channel that is closed once the provider is stopped. func (p *Provider) StopContext() context.Context { p.stopOnce.Do(p.stopInit) + + p.stopMu.Lock() + defer p.stopMu.Unlock() + return p.stopCtx } func (p *Provider) stopInit() { + p.stopMu.Lock() + defer p.stopMu.Unlock() + 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.stopMu.Lock() + defer p.stopMu.Unlock() + p.stopCtxCancel() return nil } +// TestReset resets any state stored in the Provider, and will call TestReset +// on Meta if it implements the TestProvider interface. +// This may be used to reset the schema.Provider at the start of a test, and is +// automatically called by resource.Test. +func (p *Provider) TestReset() error { + p.stopInit() + if p.MetaReset != nil { + return p.MetaReset() + } + return nil +} + // Input implementation of terraform.ResourceProvider interface. func (p *Provider) Input( input terraform.UIInput, diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index ed5918844b..5b06c5e576 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -381,3 +381,29 @@ func TestProviderStop_stopFirst(t *testing.T) { t.Fatal("should be stopped") } } + +func TestProviderReset(t *testing.T) { + var p Provider + stopCtx := p.StopContext() + p.MetaReset = func() error { + stopCtx = p.StopContext() + return nil + } + + // cancel the current context + p.Stop() + + if err := p.TestReset(); err != nil { + t.Fatal(err) + } + + // the first context should have been replaced + if err := stopCtx.Err(); err != nil { + t.Fatal(err) + } + + // we should not get a canceled context here either + if err := p.StopContext().Err(); err != nil { + t.Fatal(err) + } +} diff --git a/terraform/context.go b/terraform/context.go index 3c4e4b62ed..beb0804479 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -781,15 +781,14 @@ func (c *Context) walk( } // Watch for a stop so we can call the provider Stop() API. - doneCh := make(chan struct{}) - stopCh := c.runContext.Done() - go c.watchStop(walker, doneCh, stopCh) + watchStop, watchWait := c.watchStop(walker) // Walk the real graph, this will block until it completes realErr := graph.Walk(walker) - // Close the done channel so the watcher stops - close(doneCh) + // Close the channel so the watcher stops, and wait for it to return. + close(watchStop) + <-watchWait // 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 @@ -878,52 +877,74 @@ func (c *Context) walk( return walker, realErr } -func (c *Context) watchStop(walker *ContextGraphWalker, doneCh, stopCh <-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) +// watchStop immediately returns a `stop` and a `wait` chan after dispatching +// the watchStop goroutine. This will watch the runContext for cancellation and +// stop the providers accordingly. When the watch is no longer needed, the +// `stop` chan should be closed before waiting on the `wait` chan. +// The `wait` chan is important, because without synchronizing with the end of +// the watchStop goroutine, the runContext may also be closed during the select +// incorrectly causing providers to be stopped. Even if the graph walk is done +// at that point, stopping a provider permanently cancels its StopContext which +// can cause later actions to fail. +func (c *Context) watchStop(walker *ContextGraphWalker) (chan struct{}, <-chan struct{}) { + stop := make(chan struct{}) + wait := make(chan struct{}) + + // get the runContext cancellation channel now, because releaseRun will + // write to the runContext field. + done := c.runContext.Done() + + go func() { + defer close(wait) + // Wait for a stop or completion + select { + case <-done: + // done means the context was canceled, so we need to try and stop + // providers. + case <-stop: + // our own stop channel was closed. + return } - 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() - } - } + // If we're here, we're stopped, trigger the call. - { - // 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) + { + // 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() + } } - 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() + { + // 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() + } } - } + }() + + return stop, wait } // parseVariableAsHCL parses the value of a single variable as would have been specified