From 3edb8599b1eeaa6befc17f8ea1b33bafafcc9bbd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Oct 2016 13:39:02 -0700 Subject: [PATCH] terraform: Shadow interface, properly string through errors at the right time --- terraform/context.go | 26 ++++- terraform/context_apply_test.go | 2 +- terraform/shadow.go | 28 ++++++ terraform/shadow_components.go | 32 +++++++ terraform/shadow_context.go | 14 +-- terraform/shadow_resource_provider.go | 106 ++++++++++++++++----- terraform/shadow_resource_provider_test.go | 4 + 7 files changed, 176 insertions(+), 36 deletions(-) create mode 100644 terraform/shadow.go diff --git a/terraform/context.go b/terraform/context.go index 3e869e43c..7b7f565a1 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -2,7 +2,6 @@ package terraform import ( "fmt" - "io" "log" "sort" "strings" @@ -615,7 +614,7 @@ func (c *Context) walk( // If we have a shadow graph, walk that as well var shadowCtx *Context var shadowCh chan error - var shadowCloser io.Closer + var shadowCloser Shadow if shadow != nil { // Build the shadow context. In the process, override the real context // with the one that is wrapped so that the shadow context can verify @@ -647,13 +646,16 @@ func (c *Context) walk( // If we have a shadow graph, wait for that to complete if shadowCloser != nil { // Notify the shadow that we're done - if err := shadowCloser.Close(); err != nil { + if err := shadowCloser.CloseShadow(); err != nil { c.shadowErr = multierror.Append(c.shadowErr, err) } // Wait for the walk to end log.Printf("[DEBUG] Waiting for shadow graph to complete...") - if err := <-shadowCh; err != nil { + shadowWalkErr := <-shadowCh + + // Get any shadow errors + if err := shadowCloser.ShadowError(); err != nil { c.shadowErr = multierror.Append(c.shadowErr, err) } @@ -662,6 +664,22 @@ func (c *Context) walk( c.shadowErr = multierror.Append(c.shadowErr, err) } + // At this point, if we're supposed to fail on error, then + // we PANIC. Some tests just verify that there is an error, + // so simply appending it to realErr and returning could hide + // shadow problems. + // + // This must be done BEFORE appending shadowWalkErr since the + // shadowWalkErr may include expected errors. + if c.shadowErr != nil && contextFailOnShadowError { + panic(multierror.Prefix(c.shadowErr, "shadow graph:")) + } + + // Now, if we have a walk error, we append that through + if shadowWalkErr != nil { + c.shadowErr = multierror.Append(c.shadowErr, shadowWalkErr) + } + if c.shadowErr == nil { log.Printf("[INFO] Shadow graph success!") } else { diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 6a5de2a34..f11263f52 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -318,10 +318,10 @@ func TestContext2Apply_computedAttrRefTypeMismatch(t *testing.T) { } _, err := ctx.Apply() - if err == nil { t.Fatalf("Expected err, got none!") } + expected := "Expected ami to be string" if !strings.Contains(err.Error(), expected) { t.Fatalf("expected:\n\n%s\n\nto contain:\n\n%s", err, expected) diff --git a/terraform/shadow.go b/terraform/shadow.go new file mode 100644 index 000000000..46325595f --- /dev/null +++ b/terraform/shadow.go @@ -0,0 +1,28 @@ +package terraform + +// Shadow is the interface that any "shadow" structures must implement. +// +// A shadow structure is an interface implementation (typically) that +// shadows a real implementation and verifies that the same behavior occurs +// on both. The semantics of this behavior are up to the interface itself. +// +// A shadow NEVER modifies real values or state. It must always be safe to use. +// +// For example, a ResourceProvider shadow ensures that the same operations +// are done on the same resources with the same configurations. +// +// The typical usage of a shadow following this interface is to complete +// the real operations, then call CloseShadow which tells the shadow that +// the real side is done. Then, once the shadow is also complete, call +// ShadowError to find any errors that may have been caught. +type Shadow interface { + // CloseShadow tells the shadow that the REAL implementation is + // complete. Therefore, any calls that would block should now return + // immediately since no more changes will happen to the real side. + CloseShadow() error + + // ShadowError returns the errors that the shadow has found. + // This should be called AFTER CloseShadow and AFTER the shadow is + // known to be complete (no more calls to it). + ShadowError() error +} diff --git a/terraform/shadow_components.go b/terraform/shadow_components.go index ab491a022..84cf35952 100644 --- a/terraform/shadow_components.go +++ b/terraform/shadow_components.go @@ -112,6 +112,38 @@ func (f *shadowComponentFactory) CloseShadow() error { return result } +func (f *shadowComponentFactory) ShadowError() error { + // If we aren't the shadow, just return + if !f.Shadow { + return nil + } + + // Lock ourselves so we don't modify state + f.lock.Lock() + defer f.lock.Unlock() + + // Grab our shared state + shared := f.shadowComponentFactoryShared + + // If we're not closed, its an error + if !shared.closed { + return fmt.Errorf("component factory must be closed to retrieve errors") + } + + // Close all the providers and provisioners and return the error + var result error + for _, n := range shared.providerKeys { + _, shadow, err := shared.ResourceProvider(n, n) + if err == nil && shadow != nil { + if err := shadow.ShadowError(); err != nil { + result = multierror.Append(result, err) + } + } + } + + return result +} + // shadowComponentFactoryShared is shared data between the two factories. // // It is NOT SAFE to run any function on this struct in parallel. Lock diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index ba3b811c8..3d45e2cec 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -2,7 +2,6 @@ package terraform import ( "fmt" - "io" "reflect" "github.com/hashicorp/go-multierror" @@ -13,14 +12,13 @@ import ( // when walking the graph. The resulting context should be used _only once_ // for a graph walk. // -// The returned io.Closer should be closed after the graph walk with the -// real context is complete. The result of the Close function will be any -// errors caught during the shadowing operation. +// The returned Shadow should be closed after the graph walk with the +// real context is complete. Errors from the shadow can be retrieved there. // // Most importantly, any operations done on the shadow context (the returned // context) will NEVER affect the real context. All structures are deep // copied, no real providers or resources are used, etc. -func newShadowContext(c *Context) (*Context, *Context, io.Closer) { +func newShadowContext(c *Context) (*Context, *Context, Shadow) { // Copy the targets targetRaw, err := copystructure.Copy(c.targets) if err != nil { @@ -99,6 +97,10 @@ type shadowContextCloser struct { } // Close closes the shadow context. -func (c *shadowContextCloser) Close() error { +func (c *shadowContextCloser) CloseShadow() error { return c.Components.CloseShadow() } + +func (c *shadowContextCloser) ShadowError() error { + return c.Components.ShadowError() +} diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 176164996..671b2f60e 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -16,14 +16,7 @@ import ( // be used directly. type shadowResourceProvider interface { ResourceProvider - - // CloseShadow should be called when the _real_ side is complete. - // This will immediately end any blocked calls and return any errors. - // - // Any operations on the shadow provider after this is undefined. It - // could be fine, it could result in crashes, etc. Do not use the - // shadow after this is called. - CloseShadow() error + Shadow } // newShadowResourceProvider creates a new shadowed ResourceProvider. @@ -170,19 +163,20 @@ type shadowResourceProviderShared struct { // NOTE: Anytime a value is added here, be sure to add it to // the Close() method so that it is closed. - CloseErr shadow.Value - Input shadow.Value - Validate shadow.Value - Configure shadow.Value - Apply shadow.KeyedValue - Diff shadow.KeyedValue - Refresh shadow.KeyedValue + CloseErr shadow.Value + Input shadow.Value + Validate shadow.Value + Configure shadow.Value + ValidateResource shadow.KeyedValue + Apply shadow.KeyedValue + Diff shadow.KeyedValue + Refresh shadow.KeyedValue } func (p *shadowResourceProviderShared) Close() error { closers := []io.Closer{ &p.CloseErr, &p.Input, &p.Validate, - &p.Configure, &p.Apply, &p.Diff, + &p.Configure, &p.ValidateResource, &p.Apply, &p.Diff, &p.Refresh, } @@ -199,13 +193,16 @@ func (p *shadowResourceProviderShared) Close() error { } func (p *shadowResourceProviderShadow) CloseShadow() error { - result := p.Error - if err := p.Shared.Close(); err != nil { - result = multierror.Append(result, fmt.Errorf( - "close error: %s", err)) + err := p.Shared.Close() + if err != nil { + err = fmt.Errorf("close error: %s", err) } - return result + return err +} + +func (p *shadowResourceProviderShadow) ShadowError() error { + return p.Error } func (p *shadowResourceProviderShadow) Resources() []ResourceType { @@ -313,6 +310,57 @@ func (p *shadowResourceProviderShadow) Configure(c *ResourceConfig) error { return result.Result } +func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceConfig) ([]string, []error) { + // Unique key + key := t + + // Get the initial value + raw := p.Shared.ValidateResource.Value(key) + + // Find a validation with our configuration + var result *shadowResourceProviderValidateResource + for { + // Get the value + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ValidateResource' call for %q:\n\n%#v", + key, c)) + return nil, nil + } + + wrapper, ok := raw.(*shadowResourceProviderValidateResourceWrapper) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ValidateResource' shadow value: %#v", raw)) + return nil, nil + } + + // Look for the matching call with our configuration + wrapper.RLock() + for _, call := range wrapper.Calls { + if call.Config.Equal(c) { + result = call + break + } + } + wrapper.RUnlock() + + // If we found a result, exit + if result != nil { + break + } + + // Wait for a change so we can get the wrapper again + raw = p.Shared.ValidateResource.WaitForChange(key) + } + + return result.Warns, result.Errors +} + func (p *shadowResourceProviderShadow) Apply( info *InstanceInfo, state *InstanceState, @@ -438,10 +486,6 @@ func (p *shadowResourceProviderShadow) Refresh( // TODO // TODO -func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceConfig) ([]string, []error) { - return nil, nil -} - func (p *shadowResourceProviderShadow) ImportState(info *InstanceInfo, id string) ([]*InstanceState, error) { return nil, nil } @@ -482,6 +526,18 @@ type shadowResourceProviderConfigure struct { Result error } +type shadowResourceProviderValidateResourceWrapper struct { + sync.RWMutex + + Calls []*shadowResourceProviderValidateResource +} + +type shadowResourceProviderValidateResource struct { + Config *ResourceConfig + Warns []string + Errors []error +} + type shadowResourceProviderApply struct { State *InstanceState Diff *InstanceDiff diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 1e25fdd02..933f212b1 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -7,6 +7,10 @@ import ( "time" ) +func TestShadowResourceProvider_impl(t *testing.T) { + var _ Shadow = new(shadowResourceProviderShadow) +} + func TestShadowResourceProvider_cachedValues(t *testing.T) { mock := new(MockResourceProvider) real, shadow := newShadowResourceProvider(mock)