From 35f13f9c52626b13e696f2f77a8f28259a08b296 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 24 Sep 2016 12:21:57 -0700 Subject: [PATCH 01/64] terraform: wip on shadow graph, compiles --- terraform/eval_context_shadow.go | 174 ++++++++++++++++++++++++++ terraform/eval_context_shadow_test.go | 10 ++ 2 files changed, 184 insertions(+) create mode 100644 terraform/eval_context_shadow.go create mode 100644 terraform/eval_context_shadow_test.go diff --git a/terraform/eval_context_shadow.go b/terraform/eval_context_shadow.go new file mode 100644 index 000000000..b8f82f1bf --- /dev/null +++ b/terraform/eval_context_shadow.go @@ -0,0 +1,174 @@ +package terraform + +import ( + "sync" + + "github.com/hashicorp/terraform/config" +) + +// ShadowEvalContext is an EvalContext that is used to "shadow" a real +// eval context for comparing whether two separate graph executions result +// in the same output. +// +// This eval context will never communicate with a real provider and will +// never modify real state. +type ShadowEvalContext interface { + EvalContext + + // Close should be called when the _real_ EvalContext operations + // are complete. This will immediately end any blocks calls and record + // any errors. + // + // The returned error is the result of the shadow run. If it is nil, + // then the shadow run seemingly completed successfully. You should + // still compare the resulting states, diffs from both the real and shadow + // contexts to verify equivalent end state. + // + // If the error is non-nil, then an error occurred during the execution + // itself. In this scenario, you should not compare diffs/states since + // they can't be considered accurate since operations during execution + // failed. + Close() error +} + +// NewShadowEvalContext creates a new shadowed EvalContext. This returns +// the real EvalContext that should be used with the real evaluation and +// will communicate with real providers and write real state as well as +// the ShadowEvalContext that should be used with the test graph. +// +// This should be called before the ctx is ever used in order to ensure +// a consistent shadow state. +func NewShadowEvalContext(ctx EvalContext) (EvalContext, ShadowEvalContext) { + real := &shadowEvalContextReal{EvalContext: ctx} + + // Copy the diff. We do this using some weird scoping so that the + // "diff" (real) value never leaks out and can be used. + var diffCopy *Diff + { + diff, lock := ctx.Diff() + lock.RLock() + diffCopy = diff + // TODO: diffCopy = diff.DeepCopy() + lock.RUnlock() + } + + // Copy the state. We do this using some weird scoping so that the + // "state" (real) value never leaks out and can be used. + var stateCopy *State + { + state, lock := ctx.State() + lock.RLock() + stateCopy = state.DeepCopy() + lock.RUnlock() + } + + // Build the shadow copy. For safety, we don't even give the shadow + // copy a reference to the real context. This means that it would be + // very difficult (impossible without some real obvious mistakes) for + // the shadow context to do "real" work. + shadow := &shadowEvalContextShadow{ + PathValue: ctx.Path(), + StateValue: stateCopy, + StateLock: new(sync.RWMutex), + DiffValue: diffCopy, + DiffLock: new(sync.RWMutex), + } + + return real, shadow +} + +// shadowEvalContextReal is the EvalContext that does real work. +type shadowEvalContextReal struct { + EvalContext +} + +// shadowEvalContextShadow is the EvalContext that shadows the real one +// and leans on that for data. +type shadowEvalContextShadow struct { + PathValue []string + Providers map[string]ResourceProvider + DiffValue *Diff + DiffLock *sync.RWMutex + StateValue *State + StateLock *sync.RWMutex + + // Fields relating to closing the context. Closing signals that + // the execution of the real context completed. + closeLock sync.Mutex + closed bool + closeCh chan struct{} +} + +// Shared is the shared state between the shadow and real contexts when +// a shadow context is active. This is used by the real context to setup +// some state, trigger condition variables, etc. +type shadowEvalContextShared struct { + // This lock must be held when modifying just about anything in this + // structure. It is a "big" lock but the work done here is usually very + // fast so we do this. + sync.Mutex + + // Providers is the map of active (initialized) providers. + // + // ProviderWaiters is the condition variable associated with waiting + // for a provider to be initialized. If this is non-nil for a provider, + // then that will be triggered when the provider is initialized. + Providers map[string]struct{} + ProviderWaiters map[string]*sync.Cond +} + +func (c *shadowEvalContextShadow) Close() error { + // TODO + return nil +} + +func (c *shadowEvalContextShadow) Path() []string { + return c.PathValue +} + +func (c *shadowEvalContextShadow) Hook(f func(Hook) (HookAction, error)) error { + // Don't do anything on hooks. Mission critical behavior should not + // depend on hooks and at the time of writing it does not depend on + // hooks. In the future we could also test hooks but not now. + return nil +} + +func (c *shadowEvalContextShadow) Input() UIInput { + // TODO + return nil +} + +func (c *shadowEvalContextShadow) InitProvider(n string) (ResourceProvider, error) { + // Initialize our shadow provider here. We also wait for the + // real context to initialize the same provider. If it doesn't + // before close, then an error is reported. + + // TODO: shadow provider + + return nil, nil +} + +func (c *shadowEvalContextShadow) Diff() (*Diff, *sync.RWMutex) { + return c.DiffValue, c.DiffLock +} + +func (c *shadowEvalContextShadow) State() (*State, *sync.RWMutex) { + return c.StateValue, c.StateLock +} + +func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { return nil } +func (c *shadowEvalContextShadow) CloseProvider(n string) error { return nil } +func (c *shadowEvalContextShadow) ConfigureProvider(string, *ResourceConfig) error { return nil } +func (c *shadowEvalContextShadow) SetProviderConfig(string, *ResourceConfig) error { return nil } +func (c *shadowEvalContextShadow) ParentProviderConfig(string) *ResourceConfig { return nil } +func (c *shadowEvalContextShadow) ProviderInput(string) map[string]interface{} { return nil } +func (c *shadowEvalContextShadow) SetProviderInput(string, map[string]interface{}) {} +func (c *shadowEvalContextShadow) InitProvisioner(string) (ResourceProvisioner, error) { + return nil, nil +} +func (c *shadowEvalContextShadow) Provisioner(string) ResourceProvisioner { return nil } +func (c *shadowEvalContextShadow) CloseProvisioner(string) error { return nil } +func (c *shadowEvalContextShadow) Interpolate(*config.RawConfig, *Resource) (*ResourceConfig, error) { + return nil, nil +} +func (c *shadowEvalContextShadow) SetVariables(string, map[string]interface{}) {} diff --git a/terraform/eval_context_shadow_test.go b/terraform/eval_context_shadow_test.go new file mode 100644 index 000000000..e781653db --- /dev/null +++ b/terraform/eval_context_shadow_test.go @@ -0,0 +1,10 @@ +package terraform + +import ( + "testing" +) + +func TestShadowEvalContext_impl(t *testing.T) { + var _ EvalContext = new(shadowEvalContextReal) + var _ EvalContext = new(shadowEvalContextShadow) +} From 1df3bbdc374672199c6fcedb3227f0f2f07daca2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 26 Sep 2016 10:23:25 -0700 Subject: [PATCH 02/64] terraform: working on the resource provider shadow, not working yet --- helper/shadow/value.go | 51 ++++++ helper/shadow/value_test.go | 43 +++++ terraform/eval_context_shadow.go | 53 +++++-- terraform/shadow_resource_provider.go | 219 ++++++++++++++++++++++++++ 4 files changed, 356 insertions(+), 10 deletions(-) create mode 100644 helper/shadow/value.go create mode 100644 helper/shadow/value_test.go create mode 100644 terraform/shadow_resource_provider.go diff --git a/helper/shadow/value.go b/helper/shadow/value.go new file mode 100644 index 000000000..bf30c12a8 --- /dev/null +++ b/helper/shadow/value.go @@ -0,0 +1,51 @@ +package shadow + +import ( + "sync" +) + +// Value is a struct that coordinates a value between two +// parallel routines. It is similar to atomic.Value except that when +// Value is called if it isn't set it will wait for it. +type Value struct { + lock sync.Mutex + cond *sync.Cond + value interface{} + valueSet bool +} + +// Value returns the value that was set. +func (w *Value) Value() interface{} { + w.lock.Lock() + defer w.lock.Unlock() + + // If we already have a value just return + for !w.valueSet { + // No value, setup the condition variable if we have to + if w.cond == nil { + w.cond = sync.NewCond(&w.lock) + } + + // Wait on it + w.cond.Wait() + } + + // Return the value + return w.value +} + +// SetValue sets the value. +func (w *Value) SetValue(v interface{}) { + w.lock.Lock() + defer w.lock.Unlock() + + // Set the value + w.valueSet = true + w.value = v + + // If we have a condition, clear it + if w.cond != nil { + w.cond.Broadcast() + w.cond = nil + } +} diff --git a/helper/shadow/value_test.go b/helper/shadow/value_test.go new file mode 100644 index 000000000..069b30852 --- /dev/null +++ b/helper/shadow/value_test.go @@ -0,0 +1,43 @@ +package shadow + +import ( + "testing" + "time" +) + +func TestValue(t *testing.T) { + var v Value + + // Start trying to get the value + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value() + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set the value + v.SetValue(42) + val := <-valueCh + + // Verify + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We should be able to ask for the value again immediately + if val := v.Value(); val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We can change the value + v.SetValue(84) + if val := v.Value(); val != 84 { + t.Fatalf("bad: %#v", val) + } +} diff --git a/terraform/eval_context_shadow.go b/terraform/eval_context_shadow.go index b8f82f1bf..66bd49d27 100644 --- a/terraform/eval_context_shadow.go +++ b/terraform/eval_context_shadow.go @@ -1,8 +1,11 @@ package terraform import ( + "errors" + "fmt" "sync" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/config" ) @@ -77,6 +80,14 @@ func NewShadowEvalContext(ctx EvalContext) (EvalContext, ShadowEvalContext) { return real, shadow } +var ( + // errShadow is the error returned by the shadow context when + // things go wrong. This should be ignored and the error result from + // Close should be checked instead since that'll contain more detailed + // error. + errShadow = errors.New("shadow error") +) + // shadowEvalContextReal is the EvalContext that does real work. type shadowEvalContextReal struct { EvalContext @@ -85,6 +96,8 @@ type shadowEvalContextReal struct { // shadowEvalContextShadow is the EvalContext that shadows the real one // and leans on that for data. type shadowEvalContextShadow struct { + Shared *shadowEvalContextShared + PathValue []string Providers map[string]ResourceProvider DiffValue *Diff @@ -92,6 +105,9 @@ type shadowEvalContextShadow struct { StateValue *State StateLock *sync.RWMutex + // The collection of errors that were found during the shadow run + Error error + // Fields relating to closing the context. Closing signals that // the execution of the real context completed. closeLock sync.Mutex @@ -117,11 +133,6 @@ type shadowEvalContextShared struct { ProviderWaiters map[string]*sync.Cond } -func (c *shadowEvalContextShadow) Close() error { - // TODO - return nil -} - func (c *shadowEvalContextShadow) Path() []string { return c.PathValue } @@ -133,16 +144,34 @@ func (c *shadowEvalContextShadow) Hook(f func(Hook) (HookAction, error)) error { return nil } -func (c *shadowEvalContextShadow) Input() UIInput { - // TODO - return nil -} - func (c *shadowEvalContextShadow) InitProvider(n string) (ResourceProvider, error) { // Initialize our shadow provider here. We also wait for the // real context to initialize the same provider. If it doesn't // before close, then an error is reported. + c.Shared.Lock() + defer c.Shared.Unlock() + + // We must only initialize once + if _, ok := c.Providers[n]; ok { + c.Error = multierror.Append(c.Error, fmt.Errorf( + "Provider %q already initialized", n)) + return nil, errShadow + } + + // If we don't have the provider yet, wait for it to initialize + if _, ok := c.Shared.Providers[n]; !ok { + cond := c.Shared.ProviderWaiters[n] + if cond == nil { + cond = sync.NewCond(&c.Shared.Mutex) + c.Shared.ProviderWaiters[n] = cond + } + + // TODO: break on close + cond.Wait() + } + + // We have the provider, set it up if it isn't in our list // TODO: shadow provider return nil, nil @@ -156,6 +185,10 @@ func (c *shadowEvalContextShadow) State() (*State, *sync.RWMutex) { return c.StateValue, c.StateLock } +// TODO: All the functions below are EvalContext functions that must be impl. + +func (c *shadowEvalContextShadow) Close() error { return nil } +func (c *shadowEvalContextShadow) Input() UIInput { return nil } func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { return nil } func (c *shadowEvalContextShadow) CloseProvider(n string) error { return nil } func (c *shadowEvalContextShadow) ConfigureProvider(string, *ResourceConfig) error { return nil } diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go new file mode 100644 index 000000000..7b512146a --- /dev/null +++ b/terraform/shadow_resource_provider.go @@ -0,0 +1,219 @@ +package terraform + +import ( + "fmt" + "sync" + + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform/helper/shadow" +) + +// shadowResourceProvider implements ResourceProvider for the shadow +// eval context defined in eval_context_shadow.go. +// +// This is used to verify behavior with a real provider. This shouldn't +// 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 +} + +// newShadowResourceProvider creates a new shadowed ResourceProvider. +// +// This will assume a well behaved real ResourceProvider. For example, +// it assumes that the `Resources` call underneath doesn't change values +// since once it is called on the real provider, it will be cached and +// returned in the shadow since number of calls to that shouldn't affect +// actual behavior. +// +// However, with calls like Apply, call order is taken into account, +// parameters are checked for equality, etc. +func newShadowResourceProvider(p ResourceProvider) (ResourceProvider, shadowResourceProvider) { + // Create the shared data + shared := shadowResourceProviderShared{} + + // Create the real provider that does actual work + real := &shadowResourceProviderReal{ + ResourceProvider: p, + Shared: &shared, + } + + // Create the shadow that watches the real value + shadow := &shadowResourceProviderShadow{ + Shared: &shared, + } + + return real, shadow +} + +// shadowResourceProviderReal is the real resource provider. Function calls +// to this will perform real work. This records the parameters and return +// values and call order for the shadow to reproduce. +type shadowResourceProviderReal struct { + ResourceProvider + + Shared *shadowResourceProviderShared +} + +func (p *shadowResourceProviderReal) Resources() []ResourceType { + result := p.ResourceProvider.Resources() + p.Shared.Resources.SetValue(result) + return result +} + +func (p *shadowResourceProviderReal) DataSources() []DataSource { + result := p.ResourceProvider.DataSources() + p.Shared.DataSources.SetValue(result) + return result +} + +func (p *shadowResourceProviderReal) Close() error { + var result error + if c, ok := p.ResourceProvider.(ResourceProviderCloser); ok { + result = c.Close() + } + + p.Shared.CloseErr.SetValue(result) + return result +} + +// shadowResourceProviderShadow is the shadow resource provider. Function +// calls never affect real resources. This is paired with the "real" side +// which must be called properly to enable recording. +type shadowResourceProviderShadow struct { + Shared *shadowResourceProviderShared + + Error error // Error is the list of errors from the shadow + ErrorLock sync.Mutex +} + +type shadowResourceProviderShared struct { + CloseErr shadow.Value + Input shadow.Value + Resources shadow.Value + DataSources shadow.Value +} + +func (p *shadowResourceProviderShadow) CloseShadow() error { return nil } + +func (p *shadowResourceProviderShadow) Resources() []ResourceType { + v := p.Shared.Resources.Value() + if v == nil { + return nil + } + + return v.([]ResourceType) +} + +func (p *shadowResourceProviderShadow) DataSources() []DataSource { + v := p.Shared.DataSources.Value() + if v == nil { + return nil + } + + return v.([]DataSource) +} + +func (p *shadowResourceProviderShadow) Close() error { + v := p.Shared.CloseErr.Value() + if v == nil { + return nil + } + + return v.(error) +} + +type shadowResourceProviderInput struct { + Config *ResourceConfig + Result *ResourceConfig + ResultErr error +} + +func (p *shadowResourceProviderShadow) Input( + input UIInput, c *ResourceConfig) (*ResourceConfig, error) { + // Get the result of the input call + raw := p.Shared.Input.Value() + if raw == nil { + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderInput) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'input' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + // TODO + + // Return the results + return result.Result, result.ResultErr +} + +// TODO +// TODO +// TODO +// TODO +// TODO + +func (p *shadowResourceProviderShadow) Validate(c *ResourceConfig) ([]string, []error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceConfig) ([]string, []error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) Configure(c *ResourceConfig) error { + return nil +} + +func (p *shadowResourceProviderShadow) Apply( + info *InstanceInfo, + state *InstanceState, + diff *InstanceDiff) (*InstanceState, error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) Diff( + info *InstanceInfo, + state *InstanceState, + desired *ResourceConfig) (*InstanceDiff, error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) Refresh( + info *InstanceInfo, + s *InstanceState) (*InstanceState, error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) ImportState(info *InstanceInfo, id string) ([]*InstanceState, error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) ValidateDataSource(t string, c *ResourceConfig) ([]string, []error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) ReadDataDiff( + info *InstanceInfo, + desired *ResourceConfig) (*InstanceDiff, error) { + return nil, nil +} + +func (p *shadowResourceProviderShadow) ReadDataApply( + info *InstanceInfo, + d *InstanceDiff) (*InstanceState, error) { + return nil, nil +} From f73dc844c7a9e279bb47552f37fb63d4b532ac81 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 26 Sep 2016 18:01:02 -0700 Subject: [PATCH 03/64] wip --- terraform/resource_test.go | 13 ++++++++++ terraform/shadow_resource_provider.go | 35 +++++++++++++++++++++------ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/terraform/resource_test.go b/terraform/resource_test.go index b6a957726..85792fc2e 100644 --- a/terraform/resource_test.go +++ b/terraform/resource_test.go @@ -239,6 +239,19 @@ func TestResourceConfigGet(t *testing.T) { } } +func TestResourceConfigEqual_nil(t *testing.T) { + var nilRc *ResourceConfig + notNil := NewResourceConfig(nil) + + if nilRc.Equal(notNil) { + t.Fatal("should not be equal") + } + + if notNil.Equal(nilRc) { + t.Fatal("should not be equal") + } +} + func testResourceConfig( t *testing.T, c map[string]interface{}) *ResourceConfig { raw, err := config.NewRawConfig(c) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 7b512146a..84dc349be 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -84,6 +84,18 @@ func (p *shadowResourceProviderReal) Close() error { return result } +func (p *shadowResourceProviderReal) Input( + input UIInput, c *ResourceConfig) (*ResourceConfig, error) { + result, err := p.ResourceProvider.Input(input, c) + p.Shared.Input.SetValue(&shadowResourceProviderInput{ + Config: c, + Result: result, + ResultErr: err, + }) + + return result, err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -130,12 +142,6 @@ func (p *shadowResourceProviderShadow) Close() error { return v.(error) } -type shadowResourceProviderInput struct { - Config *ResourceConfig - Result *ResourceConfig - ResultErr error -} - func (p *shadowResourceProviderShadow) Input( input UIInput, c *ResourceConfig) (*ResourceConfig, error) { // Get the result of the input call @@ -154,7 +160,13 @@ func (p *shadowResourceProviderShadow) Input( } // Compare the parameters, which should be identical - // TODO + if !c.Equal(result.Config) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Input had unequal configurations (real, then shadow):\n\n%#v\n\n%#v", + result.Config, c)) + p.ErrorLock.Unlock() + } // Return the results return result.Result, result.ResultErr @@ -217,3 +229,12 @@ func (p *shadowResourceProviderShadow) ReadDataApply( d *InstanceDiff) (*InstanceState, error) { return nil, nil } + +// The structs for the various function calls are put below. These structs +// are used to carry call information across the real/shadow boundaries. + +type shadowResourceProviderInput struct { + Config *ResourceConfig + Result *ResourceConfig + ResultErr error +} From 37f5c6ae2685a22a84a8dd736013893a3f0ff788 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 28 Sep 2016 16:47:58 -0700 Subject: [PATCH 04/64] terraform: ResourceConfig.Equal handles nil case --- terraform/resource.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/terraform/resource.go b/terraform/resource.go index ba4251b0f..db6db9ffd 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -115,6 +115,11 @@ func (c *ResourceConfig) DeepCopy() *ResourceConfig { // Equal checks the equality of two resource configs. func (c *ResourceConfig) Equal(c2 *ResourceConfig) bool { + // If either are nil, then they're only equal if they're both nil + if c == nil || c2 == nil { + return c == c2 + } + // Two resource configs if their exported properties are equal. // We don't compare "raw" because it is never used again after // initialization and for all intents and purposes they are equal From d37bb87bf240e26997a83f96244c8caba7914cc8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 11:29:59 -0700 Subject: [PATCH 05/64] terraform: ResourceConfig.DeepCopy should handle the nil case --- terraform/resource.go | 5 +++++ terraform/resource_test.go | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/terraform/resource.go b/terraform/resource.go index db6db9ffd..937efdbe4 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -98,6 +98,11 @@ func NewResourceConfig(c *config.RawConfig) *ResourceConfig { // to modify any of the structures that are part of the resource config without // affecting the original configuration. func (c *ResourceConfig) DeepCopy() *ResourceConfig { + // DeepCopying a nil should return a nil to avoid panics + if c == nil { + return nil + } + // Copy, this will copy all the exported attributes copy, err := copystructure.Config{Lock: true}.Copy(c) if err != nil { diff --git a/terraform/resource_test.go b/terraform/resource_test.go index 85792fc2e..ab6a2273b 100644 --- a/terraform/resource_test.go +++ b/terraform/resource_test.go @@ -239,6 +239,14 @@ func TestResourceConfigGet(t *testing.T) { } } +func TestResourceConfigDeepCopy_nil(t *testing.T) { + var nilRc *ResourceConfig + actual := nilRc.DeepCopy() + if actual != nil { + t.Fatalf("bad: %#v", actual) + } +} + func TestResourceConfigEqual_nil(t *testing.T) { var nilRc *ResourceConfig notNil := NewResourceConfig(nil) From bb5f116cec397831cc4aa9044b8fdb5efaa4b2ce Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 11:33:36 -0700 Subject: [PATCH 06/64] terraform: shadow resource provider tests begin --- terraform/shadow_resource_provider.go | 50 ++++----- terraform/shadow_resource_provider_test.go | 115 +++++++++++++++++++++ 2 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 terraform/shadow_resource_provider_test.go diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 84dc349be..d5ab83afc 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -48,6 +48,9 @@ func newShadowResourceProvider(p ResourceProvider) (ResourceProvider, shadowReso // Create the shadow that watches the real value shadow := &shadowResourceProviderShadow{ Shared: &shared, + + resources: p.Resources(), + dataSources: p.DataSources(), } return real, shadow @@ -62,18 +65,6 @@ type shadowResourceProviderReal struct { Shared *shadowResourceProviderShared } -func (p *shadowResourceProviderReal) Resources() []ResourceType { - result := p.ResourceProvider.Resources() - p.Shared.Resources.SetValue(result) - return result -} - -func (p *shadowResourceProviderReal) DataSources() []DataSource { - result := p.ResourceProvider.DataSources() - p.Shared.DataSources.SetValue(result) - return result -} - func (p *shadowResourceProviderReal) Close() error { var result error if c, ok := p.ResourceProvider.(ResourceProviderCloser); ok { @@ -88,8 +79,8 @@ func (p *shadowResourceProviderReal) Input( input UIInput, c *ResourceConfig) (*ResourceConfig, error) { result, err := p.ResourceProvider.Input(input, c) p.Shared.Input.SetValue(&shadowResourceProviderInput{ - Config: c, - Result: result, + Config: c.DeepCopy(), + Result: result.DeepCopy(), ResultErr: err, }) @@ -102,35 +93,32 @@ func (p *shadowResourceProviderReal) Input( type shadowResourceProviderShadow struct { Shared *shadowResourceProviderShared + // Cached values that are expected to not change + resources []ResourceType + dataSources []DataSource + Error error // Error is the list of errors from the shadow ErrorLock sync.Mutex } type shadowResourceProviderShared struct { - CloseErr shadow.Value - Input shadow.Value - Resources shadow.Value - DataSources shadow.Value + CloseErr shadow.Value + Input shadow.Value } -func (p *shadowResourceProviderShadow) CloseShadow() error { return nil } +func (p *shadowResourceProviderShadow) CloseShadow() error { + // For now, just return the error. What we need to do in the future + // is marked the provider as "closed" so that any subsequent calls + // will fail out. + return p.Error +} func (p *shadowResourceProviderShadow) Resources() []ResourceType { - v := p.Shared.Resources.Value() - if v == nil { - return nil - } - - return v.([]ResourceType) + return p.resources } func (p *shadowResourceProviderShadow) DataSources() []DataSource { - v := p.Shared.DataSources.Value() - if v == nil { - return nil - } - - return v.([]DataSource) + return p.dataSources } func (p *shadowResourceProviderShadow) Close() error { diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go new file mode 100644 index 000000000..2acb4181c --- /dev/null +++ b/terraform/shadow_resource_provider_test.go @@ -0,0 +1,115 @@ +package terraform + +import ( + "reflect" + "testing" + "time" +) + +func TestShadowResourceProvider_cachedValues(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Resources + { + actual := shadow.Resources() + expected := real.Resources() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad:\n\n%#v\n\n%#v", actual, expected) + } + } + + // DataSources + { + actual := shadow.DataSources() + expected := real.DataSources() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad:\n\n%#v\n\n%#v", actual, expected) + } + } +} + +func TestShadowResourceProviderInput(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + ui := new(MockUIInput) + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + returnConfig := testResourceConfig(t, map[string]interface{}{ + "bar": "baz", + }) + + // Configure the mock + mock.InputReturnConfig = returnConfig + + // Verify that it blocks until the real input is called + var actual *ResourceConfig + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + actual, err = shadow.Input(ui, config) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real input + realResult, realErr := real.Input(ui, config) + if !realResult.Equal(returnConfig) { + t.Fatalf("bad: %#v", realResult) + } + if realErr != nil { + t.Fatalf("bad: %s", realErr) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !actual.Equal(returnConfig) { + t.Fatalf("bad: %#v", actual) + } + if err != nil { + t.Fatalf("bad: %s", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowResourceProviderInput_badInput(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + ui := new(MockUIInput) + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + configBad := testResourceConfig(t, map[string]interface{}{ + "foo": "nope", + }) + + // Call the real with one + real.Input(ui, config) + + // Call the shadow with another + _, err := shadow.Input(ui, configBad) + if err != nil { + t.Fatalf("bad: %s", err) + } + + // Verify we have an error + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should have error") + } +} From 23204d24140e0b26f79adac782875ed9f26ffc93 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 15:39:20 -0700 Subject: [PATCH 07/64] terraform: ResourceProvider.Validate for shadow --- terraform/shadow_resource_provider.go | 57 +++++++++++++-- terraform/shadow_resource_provider_test.go | 81 ++++++++++++++++++++++ 2 files changed, 131 insertions(+), 7 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index d5ab83afc..9d7caafb6 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -87,6 +87,17 @@ func (p *shadowResourceProviderReal) Input( return result, err } +func (p *shadowResourceProviderReal) Validate(c *ResourceConfig) ([]string, []error) { + warns, errs := p.ResourceProvider.Validate(c) + p.Shared.Validate.SetValue(&shadowResourceProviderValidate{ + Config: c.DeepCopy(), + ResultWarn: warns, + ResultErr: errs, + }) + + return warns, errs +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -104,6 +115,7 @@ type shadowResourceProviderShadow struct { type shadowResourceProviderShared struct { CloseErr shadow.Value Input shadow.Value + Validate shadow.Value } func (p *shadowResourceProviderShadow) CloseShadow() error { @@ -160,16 +172,41 @@ func (p *shadowResourceProviderShadow) Input( return result.Result, result.ResultErr } -// TODO -// TODO -// TODO -// TODO -// TODO - func (p *shadowResourceProviderShadow) Validate(c *ResourceConfig) ([]string, []error) { - return nil, nil + // Get the result of the validate call + raw := p.Shared.Validate.Value() + if raw == nil { + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderValidate) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'validate' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + if !c.Equal(result.Config) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Validate had unequal configurations (real, then shadow):\n\n%#v\n\n%#v", + result.Config, c)) + p.ErrorLock.Unlock() + } + + // Return the results + return result.ResultWarn, result.ResultErr } +// TODO +// TODO +// TODO +// TODO +// TODO + func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceConfig) ([]string, []error) { return nil, nil } @@ -226,3 +263,9 @@ type shadowResourceProviderInput struct { Result *ResourceConfig ResultErr error } + +type shadowResourceProviderValidate struct { + Config *ResourceConfig + ResultWarn []string + ResultErr []error +} diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 2acb4181c..b2811aa9a 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -1,6 +1,7 @@ package terraform import ( + "fmt" "reflect" "testing" "time" @@ -113,3 +114,83 @@ func TestShadowResourceProviderInput_badInput(t *testing.T) { t.Fatal("should have error") } } + +func TestShadowResourceProviderValidate(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + returnWarns := []string{"foo"} + returnErrs := []error{fmt.Errorf("bar")} + + // Configure the mock + mock.ValidateReturnWarns = returnWarns + mock.ValidateReturnErrors = returnErrs + + // Verify that it blocks until the real func is called + var warns []string + var errs []error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + warns, errs = shadow.Validate(config) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realWarns, realErrs := real.Validate(config) + if !reflect.DeepEqual(realWarns, returnWarns) { + t.Fatalf("bad: %#v", realWarns) + } + if !reflect.DeepEqual(realErrs, returnErrs) { + t.Fatalf("bad: %#v", realWarns) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !reflect.DeepEqual(warns, returnWarns) { + t.Fatalf("bad: %#v", warns) + } + if !reflect.DeepEqual(errs, returnErrs) { + t.Fatalf("bad: %#v", errs) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowResourceProviderValidate_badInput(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + configBad := testResourceConfig(t, map[string]interface{}{ + "foo": "nope", + }) + + // Call the real with one + real.Validate(config) + + // Call the shadow with another + shadow.Validate(configBad) + + // Verify we have an error + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should have error") + } +} From 3522b07b7526e2020d0169445731b41dc9cb4ea4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 15:44:06 -0700 Subject: [PATCH 08/64] terraform: Shadow resource provider Configure --- terraform/shadow_resource_provider.go | 55 ++++++++++++++--- terraform/shadow_resource_provider_test.go | 71 ++++++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 9d7caafb6..a9a1b2dc7 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -98,6 +98,16 @@ func (p *shadowResourceProviderReal) Validate(c *ResourceConfig) ([]string, []er return warns, errs } +func (p *shadowResourceProviderReal) Configure(c *ResourceConfig) error { + err := p.ResourceProvider.Configure(c) + p.Shared.Configure.SetValue(&shadowResourceProviderConfigure{ + Config: c.DeepCopy(), + Result: err, + }) + + return err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -113,9 +123,10 @@ type shadowResourceProviderShadow struct { } type shadowResourceProviderShared struct { - CloseErr shadow.Value - Input shadow.Value - Validate shadow.Value + CloseErr shadow.Value + Input shadow.Value + Validate shadow.Value + Configure shadow.Value } func (p *shadowResourceProviderShadow) CloseShadow() error { @@ -201,6 +212,35 @@ func (p *shadowResourceProviderShadow) Validate(c *ResourceConfig) ([]string, [] return result.ResultWarn, result.ResultErr } +func (p *shadowResourceProviderShadow) Configure(c *ResourceConfig) error { + // Get the result of the call + raw := p.Shared.Configure.Value() + if raw == nil { + return nil + } + + result, ok := raw.(*shadowResourceProviderConfigure) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'configure' shadow value: %#v", raw)) + return nil + } + + // Compare the parameters, which should be identical + if !c.Equal(result.Config) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Configure had unequal configurations (real, then shadow):\n\n%#v\n\n%#v", + result.Config, c)) + p.ErrorLock.Unlock() + } + + // Return the results + return result.Result +} + // TODO // TODO // TODO @@ -211,10 +251,6 @@ func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceCon return nil, nil } -func (p *shadowResourceProviderShadow) Configure(c *ResourceConfig) error { - return nil -} - func (p *shadowResourceProviderShadow) Apply( info *InstanceInfo, state *InstanceState, @@ -269,3 +305,8 @@ type shadowResourceProviderValidate struct { ResultWarn []string ResultErr []error } + +type shadowResourceProviderConfigure struct { + Config *ResourceConfig + Result error +} diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index b2811aa9a..9c36fd546 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -194,3 +194,74 @@ func TestShadowResourceProviderValidate_badInput(t *testing.T) { t.Fatal("should have error") } } + +func TestShadowResourceProviderConfigure(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + returnErr := fmt.Errorf("bar") + + // Configure the mock + mock.ConfigureReturnError = returnErr + + // Verify that it blocks until the real func is called + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + err = shadow.Configure(config) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realErr := real.Configure(config) + if !reflect.DeepEqual(realErr, returnErr) { + t.Fatalf("bad: %#v", realErr) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !reflect.DeepEqual(err, returnErr) { + t.Fatalf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowResourceProviderConfigure_badInput(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + configBad := testResourceConfig(t, map[string]interface{}{ + "foo": "nope", + }) + + // Call the real with one + real.Configure(config) + + // Call the shadow with another + shadow.Configure(configBad) + + // Verify we have an error + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should have error") + } +} From 8426cea6b0e9682c3ca72f741db08f0b2bb79dc7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 17:00:01 -0700 Subject: [PATCH 09/64] helper/shadow: OrderedValue --- helper/shadow/keyed_value.go | 5 ++ helper/shadow/ordered_value.go | 66 +++++++++++++++++++++++++ helper/shadow/ordered_value_test.go | 76 +++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 helper/shadow/keyed_value.go create mode 100644 helper/shadow/ordered_value.go create mode 100644 helper/shadow/ordered_value_test.go diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go new file mode 100644 index 000000000..6e31dfa1a --- /dev/null +++ b/helper/shadow/keyed_value.go @@ -0,0 +1,5 @@ +package shadow + +// KeyedValue is a struct that coordinates a value by key. +type KeyedValue struct { +} diff --git a/helper/shadow/ordered_value.go b/helper/shadow/ordered_value.go new file mode 100644 index 000000000..0a43d4d4d --- /dev/null +++ b/helper/shadow/ordered_value.go @@ -0,0 +1,66 @@ +package shadow + +import ( + "container/list" + "sync" +) + +// OrderedValue is a struct that keeps track of a value in the order +// it is set. Each time Value() is called, it will return the most recent +// calls value then discard it. +// +// This is unlike Value that returns the same value once it is set. +type OrderedValue struct { + lock sync.Mutex + values *list.List + waiters *list.List +} + +// Value returns the last value that was set, or blocks until one +// is received. +func (w *OrderedValue) Value() interface{} { + w.lock.Lock() + + // If we have a pending value already, use it + if w.values != nil && w.values.Len() > 0 { + front := w.values.Front() + w.values.Remove(front) + w.lock.Unlock() + return front.Value + } + + // No pending value, create a waiter + if w.waiters == nil { + w.waiters = list.New() + } + + var val Value + w.waiters.PushBack(&val) + w.lock.Unlock() + + // Return the value once we have it + return val.Value() +} + +// SetValue sets the latest value. +func (w *OrderedValue) SetValue(v interface{}) { + w.lock.Lock() + defer w.lock.Unlock() + + // If we have a waiter, notify it + if w.waiters != nil && w.waiters.Len() > 0 { + front := w.waiters.Front() + w.waiters.Remove(front) + + val := front.Value.(*Value) + val.SetValue(v) + return + } + + // Add it to the list of values + if w.values == nil { + w.values = list.New() + } + + w.values.PushBack(v) +} diff --git a/helper/shadow/ordered_value_test.go b/helper/shadow/ordered_value_test.go new file mode 100644 index 000000000..53cda35f6 --- /dev/null +++ b/helper/shadow/ordered_value_test.go @@ -0,0 +1,76 @@ +package shadow + +import ( + "testing" + "time" +) + +func TestOrderedValue(t *testing.T) { + var v OrderedValue + + // Start trying to get the value + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value() + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set the value + v.SetValue(42) + val := <-valueCh + + // Verify + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We should not get the value again + go func() { + valueCh <- v.Value() + }() + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // We should get the next value + v.SetValue(21) + val = <-valueCh + if val != 21 { + t.Fatalf("bad: %#v", val) + } +} + +func TestOrderedValue_setFirst(t *testing.T) { + var v OrderedValue + + // Set the value + v.SetValue(42) + val := v.Value() + + // Verify + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We should not get the value again + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value() + }() + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set a value so the goroutine doesn't hang around + v.SetValue(1) +} From bd69e41c14fe9f30693936581804b72f9167784d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 18:48:53 -0700 Subject: [PATCH 10/64] helper/shadow: KeyedValue --- helper/shadow/keyed_value.go | 56 ++++++++++++++++++++++++++++++- helper/shadow/keyed_value_test.go | 51 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 helper/shadow/keyed_value_test.go diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index 6e31dfa1a..0c03c7f6f 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -1,5 +1,59 @@ package shadow -// KeyedValue is a struct that coordinates a value by key. +import ( + "sync" +) + +// KeyedValue is a struct that coordinates a value by key. If a value is +// not available for a give key, it'll block until it is available. type KeyedValue struct { + lock sync.Mutex + once sync.Once + values map[string]interface{} + waiters map[string]*Value +} + +// Value returns the value that was set for the given key, or blocks +// until one is available. +func (w *KeyedValue) Value(k string) interface{} { + w.lock.Lock() + w.once.Do(w.init) + + // If we have this value already, return it + if v, ok := w.values[k]; ok { + w.lock.Unlock() + return v + } + + // No pending value, check for a waiter + val := w.waiters[k] + if val == nil { + val = new(Value) + w.waiters[k] = val + } + w.lock.Unlock() + + // Return the value once we have it + return val.Value() +} + +func (w *KeyedValue) SetValue(k string, v interface{}) { + w.lock.Lock() + defer w.lock.Unlock() + w.once.Do(w.init) + + // Set the value, always + w.values[k] = v + + // If we have a waiter, set it + if val, ok := w.waiters[k]; ok { + val.SetValue(v) + w.waiters[k] = nil + } +} + +// Must be called with w.lock held. +func (w *KeyedValue) init() { + w.values = make(map[string]interface{}) + w.waiters = make(map[string]*Value) } diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go new file mode 100644 index 000000000..cfdc053e4 --- /dev/null +++ b/helper/shadow/keyed_value_test.go @@ -0,0 +1,51 @@ +package shadow + +import ( + "testing" + "time" +) + +func TestKeyedValue(t *testing.T) { + var v KeyedValue + + // Start trying to get the value + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set the value + v.SetValue("foo", 42) + val := <-valueCh + + // Verify + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We should get the next value + val = v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } +} + +func TestKeyedValue_setFirst(t *testing.T) { + var v KeyedValue + + // Set the value + v.SetValue("foo", 42) + val := v.Value("foo") + + // Verify + if val != 42 { + t.Fatalf("bad: %#v", val) + } +} From cbbd492bcec2f28b7ea977cb255bd46a32e89af3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 19:00:09 -0700 Subject: [PATCH 11/64] terraform: shadow resource provider Apply --- terraform/shadow_resource_provider.go | 69 +++++++++++++++++++--- terraform/shadow_resource_provider_test.go | 54 +++++++++++++++++ 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index a9a1b2dc7..63df15527 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -108,6 +108,21 @@ func (p *shadowResourceProviderReal) Configure(c *ResourceConfig) error { return err } +func (p *shadowResourceProviderReal) Apply( + info *InstanceInfo, + state *InstanceState, + diff *InstanceDiff) (*InstanceState, error) { + result, err := p.ResourceProvider.Apply(info, state, diff) + p.Shared.Apply.SetValue(info.HumanId(), &shadowResourceProviderApply{ + State: state, + Diff: diff, + Result: result, + ResultErr: err, + }) + + return result, err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -127,6 +142,7 @@ type shadowResourceProviderShared struct { Input shadow.Value Validate shadow.Value Configure shadow.Value + Apply shadow.KeyedValue } func (p *shadowResourceProviderShadow) CloseShadow() error { @@ -241,6 +257,45 @@ func (p *shadowResourceProviderShadow) Configure(c *ResourceConfig) error { return result.Result } +func (p *shadowResourceProviderShadow) Apply( + info *InstanceInfo, + state *InstanceState, + diff *InstanceDiff) (*InstanceState, error) { + // Unique key + key := info.HumanId() + raw := p.Shared.Apply.Value(key) + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'apply' call for %q:\n\n%#v\n\n%#v", + key, state, diff)) + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderApply) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'apply' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + if !state.Equal(result.State) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "State had unequal states (real, then shadow):\n\n%#v\n\n%#v", + result.State, state)) + p.ErrorLock.Unlock() + } + + // TODO: compare diffs + + return result.Result, result.ResultErr +} + // TODO // TODO // TODO @@ -251,13 +306,6 @@ func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceCon return nil, nil } -func (p *shadowResourceProviderShadow) Apply( - info *InstanceInfo, - state *InstanceState, - diff *InstanceDiff) (*InstanceState, error) { - return nil, nil -} - func (p *shadowResourceProviderShadow) Diff( info *InstanceInfo, state *InstanceState, @@ -310,3 +358,10 @@ type shadowResourceProviderConfigure struct { Config *ResourceConfig Result error } + +type shadowResourceProviderApply struct { + State *InstanceState + Diff *InstanceDiff + Result *InstanceState + ResultErr error +} diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 9c36fd546..4c185f3ce 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -265,3 +265,57 @@ func TestShadowResourceProviderConfigure_badInput(t *testing.T) { t.Fatal("should have error") } } + +func TestShadowResourceProviderApply(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + info := &InstanceInfo{Id: "foo"} + state := &InstanceState{ID: "foo"} + diff := &InstanceDiff{Destroy: true} + mockResult := &InstanceState{ID: "bar"} + + // Configure the mock + mock.ApplyReturn = mockResult + + // Verify that it blocks until the real func is called + var result *InstanceState + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + result, err = shadow.Apply(info, state, diff) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realResult, realErr := real.Apply(info, state, diff) + if !realResult.Equal(mockResult) { + t.Fatalf("bad: %#v", realResult) + } + if realErr != nil { + t.Fatalf("bad: %#v", realErr) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !result.Equal(mockResult) { + t.Fatalf("bad: %#v", result) + } + if err != nil { + t.Fatalf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} From 82a1158f5533cff0b9253ba20cdc0df8be097ecd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 19:04:58 -0700 Subject: [PATCH 12/64] terraform: ResourceProvider.Diff shadow --- terraform/shadow_resource_provider.go | 74 ++++++++++++++++++++-- terraform/shadow_resource_provider_test.go | 54 ++++++++++++++++ 2 files changed, 121 insertions(+), 7 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 63df15527..d3b74b29e 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -123,6 +123,21 @@ func (p *shadowResourceProviderReal) Apply( return result, err } +func (p *shadowResourceProviderReal) Diff( + info *InstanceInfo, + state *InstanceState, + desired *ResourceConfig) (*InstanceDiff, error) { + result, err := p.ResourceProvider.Diff(info, state, desired) + p.Shared.Diff.SetValue(info.HumanId(), &shadowResourceProviderDiff{ + State: state, + Desired: desired, + Result: result, + ResultErr: err, + }) + + return result, err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -143,6 +158,7 @@ type shadowResourceProviderShared struct { Validate shadow.Value Configure shadow.Value Apply shadow.KeyedValue + Diff shadow.KeyedValue } func (p *shadowResourceProviderShadow) CloseShadow() error { @@ -296,6 +312,50 @@ func (p *shadowResourceProviderShadow) Apply( return result.Result, result.ResultErr } +func (p *shadowResourceProviderShadow) Diff( + info *InstanceInfo, + state *InstanceState, + desired *ResourceConfig) (*InstanceDiff, error) { + // Unique key + key := info.HumanId() + raw := p.Shared.Diff.Value(key) + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'diff' call for %q:\n\n%#v\n\n%#v", + key, state, desired)) + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderDiff) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'diff' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + if !state.Equal(result.State) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Diff %q had unequal states (real, then shadow):\n\n%#v\n\n%#v", + key, result.State, state)) + p.ErrorLock.Unlock() + } + if !desired.Equal(result.Desired) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Diff %q had unequal states (real, then shadow):\n\n%#v\n\n%#v", + key, result.Desired, desired)) + p.ErrorLock.Unlock() + } + + return result.Result, result.ResultErr +} + // TODO // TODO // TODO @@ -306,13 +366,6 @@ func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceCon return nil, nil } -func (p *shadowResourceProviderShadow) Diff( - info *InstanceInfo, - state *InstanceState, - desired *ResourceConfig) (*InstanceDiff, error) { - return nil, nil -} - func (p *shadowResourceProviderShadow) Refresh( info *InstanceInfo, s *InstanceState) (*InstanceState, error) { @@ -365,3 +418,10 @@ type shadowResourceProviderApply struct { Result *InstanceState ResultErr error } + +type shadowResourceProviderDiff struct { + State *InstanceState + Desired *ResourceConfig + Result *InstanceDiff + ResultErr error +} diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 4c185f3ce..622a92631 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -319,3 +319,57 @@ func TestShadowResourceProviderApply(t *testing.T) { t.Fatalf("bad: %s", err) } } + +func TestShadowResourceProviderDiff(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + info := &InstanceInfo{Id: "foo"} + state := &InstanceState{ID: "foo"} + desired := testResourceConfig(t, map[string]interface{}{"foo": "bar"}) + mockResult := &InstanceDiff{Destroy: true} + + // Configure the mock + mock.DiffReturn = mockResult + + // Verify that it blocks until the real func is called + var result *InstanceDiff + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + result, err = shadow.Diff(info, state, desired) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realResult, realErr := real.Diff(info, state, desired) + if !reflect.DeepEqual(realResult, mockResult) { + t.Fatalf("bad: %#v", realResult) + } + if realErr != nil { + t.Fatalf("bad: %#v", realErr) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !reflect.DeepEqual(result, mockResult) { + t.Fatalf("bad: %#v", result) + } + if err != nil { + t.Fatalf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} From 51ac3c5969ed4e86d62e03adee80066ba480c3d4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 19:08:10 -0700 Subject: [PATCH 13/64] terraform: ResourceProvider.Refresh (shadow) --- terraform/shadow_resource_provider.go | 62 +++++++++++++++++++--- terraform/shadow_resource_provider_test.go | 53 ++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index d3b74b29e..429f77fb7 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -138,6 +138,19 @@ func (p *shadowResourceProviderReal) Diff( return result, err } +func (p *shadowResourceProviderReal) Refresh( + info *InstanceInfo, + state *InstanceState) (*InstanceState, error) { + result, err := p.ResourceProvider.Refresh(info, state) + p.Shared.Refresh.SetValue(info.HumanId(), &shadowResourceProviderRefresh{ + State: state, + Result: result, + ResultErr: err, + }) + + return result, err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -159,6 +172,7 @@ type shadowResourceProviderShared struct { Configure shadow.Value Apply shadow.KeyedValue Diff shadow.KeyedValue + Refresh shadow.KeyedValue } func (p *shadowResourceProviderShadow) CloseShadow() error { @@ -356,6 +370,42 @@ func (p *shadowResourceProviderShadow) Diff( return result.Result, result.ResultErr } +func (p *shadowResourceProviderShadow) Refresh( + info *InstanceInfo, + state *InstanceState) (*InstanceState, error) { + // Unique key + key := info.HumanId() + raw := p.Shared.Refresh.Value(key) + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'refresh' call for %q:\n\n%#v", + key, state)) + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderRefresh) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'refresh' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + if !state.Equal(result.State) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Refresh %q had unequal states (real, then shadow):\n\n%#v\n\n%#v", + key, result.State, state)) + p.ErrorLock.Unlock() + } + + return result.Result, result.ResultErr +} + // TODO // TODO // TODO @@ -366,12 +416,6 @@ func (p *shadowResourceProviderShadow) ValidateResource(t string, c *ResourceCon return nil, nil } -func (p *shadowResourceProviderShadow) Refresh( - info *InstanceInfo, - s *InstanceState) (*InstanceState, error) { - return nil, nil -} - func (p *shadowResourceProviderShadow) ImportState(info *InstanceInfo, id string) ([]*InstanceState, error) { return nil, nil } @@ -425,3 +469,9 @@ type shadowResourceProviderDiff struct { Result *InstanceDiff ResultErr error } + +type shadowResourceProviderRefresh struct { + State *InstanceState + Result *InstanceState + ResultErr error +} diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 622a92631..1e25fdd02 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -373,3 +373,56 @@ func TestShadowResourceProviderDiff(t *testing.T) { t.Fatalf("bad: %s", err) } } + +func TestShadowResourceProviderRefresh(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + info := &InstanceInfo{Id: "foo"} + state := &InstanceState{ID: "foo"} + mockResult := &InstanceState{ID: "bar"} + + // Configure the mock + mock.RefreshReturn = mockResult + + // Verify that it blocks until the real func is called + var result *InstanceState + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + result, err = shadow.Refresh(info, state) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realResult, realErr := real.Refresh(info, state) + if !realResult.Equal(mockResult) { + t.Fatalf("bad: %#v", realResult) + } + if realErr != nil { + t.Fatalf("bad: %#v", realErr) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !result.Equal(mockResult) { + t.Fatalf("bad: %#v", result) + } + if err != nil { + t.Fatalf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} From fb96b0c4220dcdd50f781b35a1bb0533200a7c5d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 21:26:07 -0700 Subject: [PATCH 14/64] terraform: EvalContext.initProvider shadow --- ...ntext_shadow.go => shadow_eval_context.go} | 101 +++++++++++------- ...ow_test.go => shadow_eval_context_test.go} | 0 2 files changed, 63 insertions(+), 38 deletions(-) rename terraform/{eval_context_shadow.go => shadow_eval_context.go} (77%) rename terraform/{eval_context_shadow_test.go => shadow_eval_context_test.go} (100%) diff --git a/terraform/eval_context_shadow.go b/terraform/shadow_eval_context.go similarity index 77% rename from terraform/eval_context_shadow.go rename to terraform/shadow_eval_context.go index 66bd49d27..4f0c5ee53 100644 --- a/terraform/eval_context_shadow.go +++ b/terraform/shadow_eval_context.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/helper/shadow" ) // ShadowEvalContext is an EvalContext that is used to "shadow" a real @@ -91,6 +92,28 @@ var ( // shadowEvalContextReal is the EvalContext that does real work. type shadowEvalContextReal struct { EvalContext + + Shared *shadowEvalContextShared +} + +func (c *shadowEvalContextReal) InitProvider(n string) (ResourceProvider, error) { + // Initialize the real provider + p, err := c.EvalContext.InitProvider(n) + + // Create the shadow + var real ResourceProvider + var shadow shadowResourceProvider + if err == nil { + real, shadow = newShadowResourceProvider(p) + } + + // Store the result + c.Shared.Providers.SetValue(n, &shadowEvalContextInitProvider{ + Shadow: shadow, + ResultErr: err, + }) + + return real, err } // shadowEvalContextShadow is the EvalContext that shadows the real one @@ -106,7 +129,8 @@ type shadowEvalContextShadow struct { StateLock *sync.RWMutex // The collection of errors that were found during the shadow run - Error error + Error error + ErrorLock sync.Mutex // Fields relating to closing the context. Closing signals that // the execution of the real context completed. @@ -119,18 +143,7 @@ type shadowEvalContextShadow struct { // a shadow context is active. This is used by the real context to setup // some state, trigger condition variables, etc. type shadowEvalContextShared struct { - // This lock must be held when modifying just about anything in this - // structure. It is a "big" lock but the work done here is usually very - // fast so we do this. - sync.Mutex - - // Providers is the map of active (initialized) providers. - // - // ProviderWaiters is the condition variable associated with waiting - // for a provider to be initialized. If this is non-nil for a provider, - // then that will be triggered when the provider is initialized. - Providers map[string]struct{} - ProviderWaiters map[string]*sync.Cond + Providers shadow.KeyedValue } func (c *shadowEvalContextShadow) Path() []string { @@ -145,36 +158,30 @@ func (c *shadowEvalContextShadow) Hook(f func(Hook) (HookAction, error)) error { } func (c *shadowEvalContextShadow) InitProvider(n string) (ResourceProvider, error) { - // Initialize our shadow provider here. We also wait for the - // real context to initialize the same provider. If it doesn't - // before close, then an error is reported. - - c.Shared.Lock() - defer c.Shared.Unlock() - - // We must only initialize once - if _, ok := c.Providers[n]; ok { - c.Error = multierror.Append(c.Error, fmt.Errorf( - "Provider %q already initialized", n)) - return nil, errShadow + // Wait for the provider value + raw := c.Shared.Providers.Value(n) + if raw == nil { + return nil, c.err(fmt.Errorf( + "Unknown 'InitProvider' call for %q", n)) } - // If we don't have the provider yet, wait for it to initialize - if _, ok := c.Shared.Providers[n]; !ok { - cond := c.Shared.ProviderWaiters[n] - if cond == nil { - cond = sync.NewCond(&c.Shared.Mutex) - c.Shared.ProviderWaiters[n] = cond - } - - // TODO: break on close - cond.Wait() + result, ok := raw.(*shadowEvalContextInitProvider) + if !ok { + return nil, c.err(fmt.Errorf( + "Unknown 'InitProvider' shadow value: %#v", raw)) } - // We have the provider, set it up if it isn't in our list - // TODO: shadow provider + result.Lock() + defer result.Unlock() - return nil, nil + if result.Init { + // Record the error but continue... + c.err(fmt.Errorf( + "InitProvider: provider %q already initialized", n)) + } + + result.Init = true + return result.Shadow, result.ResultErr } func (c *shadowEvalContextShadow) Diff() (*Diff, *sync.RWMutex) { @@ -185,6 +192,13 @@ func (c *shadowEvalContextShadow) State() (*State, *sync.RWMutex) { return c.StateValue, c.StateLock } +func (c *shadowEvalContextShadow) err(err error) error { + c.ErrorLock.Lock() + defer c.ErrorLock.Unlock() + c.Error = multierror.Append(c.Error, err) + return err +} + // TODO: All the functions below are EvalContext functions that must be impl. func (c *shadowEvalContextShadow) Close() error { return nil } @@ -205,3 +219,14 @@ func (c *shadowEvalContextShadow) Interpolate(*config.RawConfig, *Resource) (*Re return nil, nil } func (c *shadowEvalContextShadow) SetVariables(string, map[string]interface{}) {} + +// The structs for the various function calls are put below. These structs +// are used to carry call information across the real/shadow boundaries. + +type shadowEvalContextInitProvider struct { + Shadow shadowResourceProvider + ResultErr error + + sync.Mutex // Must be held to modify the field below + Init bool // Keeps track of whether it has been initialized in the shadow +} diff --git a/terraform/eval_context_shadow_test.go b/terraform/shadow_eval_context_test.go similarity index 100% rename from terraform/eval_context_shadow_test.go rename to terraform/shadow_eval_context_test.go From 792a9f1de4369e815fcb1ac79c8a6809d0b581d3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 21:56:07 -0700 Subject: [PATCH 15/64] terraform: EvalContext.InitProvider (shadow) tests --- terraform/shadow_eval_context.go | 26 +++++++---- terraform/shadow_eval_context_test.go | 64 +++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/terraform/shadow_eval_context.go b/terraform/shadow_eval_context.go index 4f0c5ee53..48b8812fe 100644 --- a/terraform/shadow_eval_context.go +++ b/terraform/shadow_eval_context.go @@ -43,17 +43,23 @@ type ShadowEvalContext interface { // This should be called before the ctx is ever used in order to ensure // a consistent shadow state. func NewShadowEvalContext(ctx EvalContext) (EvalContext, ShadowEvalContext) { - real := &shadowEvalContextReal{EvalContext: ctx} + var shared shadowEvalContextShared + real := &shadowEvalContextReal{ + EvalContext: ctx, + Shared: &shared, + } // Copy the diff. We do this using some weird scoping so that the // "diff" (real) value never leaks out and can be used. var diffCopy *Diff { diff, lock := ctx.Diff() - lock.RLock() - diffCopy = diff - // TODO: diffCopy = diff.DeepCopy() - lock.RUnlock() + if lock != nil { + lock.RLock() + diffCopy = diff + // TODO: diffCopy = diff.DeepCopy() + lock.RUnlock() + } } // Copy the state. We do this using some weird scoping so that the @@ -61,9 +67,11 @@ func NewShadowEvalContext(ctx EvalContext) (EvalContext, ShadowEvalContext) { var stateCopy *State { state, lock := ctx.State() - lock.RLock() - stateCopy = state.DeepCopy() - lock.RUnlock() + if lock != nil { + lock.RLock() + stateCopy = state.DeepCopy() + lock.RUnlock() + } } // Build the shadow copy. For safety, we don't even give the shadow @@ -71,6 +79,8 @@ func NewShadowEvalContext(ctx EvalContext) (EvalContext, ShadowEvalContext) { // very difficult (impossible without some real obvious mistakes) for // the shadow context to do "real" work. shadow := &shadowEvalContextShadow{ + Shared: &shared, + PathValue: ctx.Path(), StateValue: stateCopy, StateLock: new(sync.RWMutex), diff --git a/terraform/shadow_eval_context_test.go b/terraform/shadow_eval_context_test.go index e781653db..2123e4816 100644 --- a/terraform/shadow_eval_context_test.go +++ b/terraform/shadow_eval_context_test.go @@ -1,10 +1,74 @@ package terraform import ( + "fmt" + "reflect" "testing" + "time" ) func TestShadowEvalContext_impl(t *testing.T) { var _ EvalContext = new(shadowEvalContextReal) var _ EvalContext = new(shadowEvalContextShadow) } + +func TestShadowEvalContextInitProvider(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Verify that it blocks until the real func is called + var result ResourceProvider + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + result, err = shadow.InitProvider(name) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realResult, realErr := real.InitProvider(name) + if realErr != nil { + t.Fatalf("bad: %#v", realErr) + } + realResult.Configure(nil) + if !mockResult.ConfigureCalled { + t.Fatalf("bad: %#v", realResult) + } + mockResult.ConfigureCalled = false + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if err != nil { + t.Fatalf("bad: %#v", err) + } + + // Verify that the returned value is a shadow. Calling one function + // shouldn't affect the other. + result.Configure(nil) + if mockResult.ConfigureCalled { + t.Fatal("close should not be called") + } + + // And doing some work should result in that value + mockErr := fmt.Errorf("yo") + mockResult.ConfigureReturnError = mockErr + realResult.Configure(nil) + if err := result.Configure(nil); !reflect.DeepEqual(err, mockErr) { + t.Fatalf("bad: %#v", err) + } +} From ce56712473aa3aa8a2b7bf7d7bbd37fb112fc80f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 30 Sep 2016 22:02:35 -0700 Subject: [PATCH 16/64] terraform: EvalContext.InitProvider(shadow) test double init --- terraform/shadow_eval_context.go | 8 +++++-- terraform/shadow_eval_context_test.go | 34 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/terraform/shadow_eval_context.go b/terraform/shadow_eval_context.go index 48b8812fe..d1fc90272 100644 --- a/terraform/shadow_eval_context.go +++ b/terraform/shadow_eval_context.go @@ -32,7 +32,7 @@ type ShadowEvalContext interface { // itself. In this scenario, you should not compare diffs/states since // they can't be considered accurate since operations during execution // failed. - Close() error + CloseShadow() error } // NewShadowEvalContext creates a new shadowed EvalContext. This returns @@ -156,6 +156,11 @@ type shadowEvalContextShared struct { Providers shadow.KeyedValue } +func (c *shadowEvalContextShadow) CloseShadow() error { + // TODO: somehow shut this thing down + return c.Error +} + func (c *shadowEvalContextShadow) Path() []string { return c.PathValue } @@ -211,7 +216,6 @@ func (c *shadowEvalContextShadow) err(err error) error { // TODO: All the functions below are EvalContext functions that must be impl. -func (c *shadowEvalContextShadow) Close() error { return nil } func (c *shadowEvalContextShadow) Input() UIInput { return nil } func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { return nil } func (c *shadowEvalContextShadow) CloseProvider(n string) error { return nil } diff --git a/terraform/shadow_eval_context_test.go b/terraform/shadow_eval_context_test.go index 2123e4816..cbcd1db17 100644 --- a/terraform/shadow_eval_context_test.go +++ b/terraform/shadow_eval_context_test.go @@ -71,4 +71,38 @@ func TestShadowEvalContextInitProvider(t *testing.T) { if err := result.Configure(nil); !reflect.DeepEqual(err, mockErr) { t.Fatalf("bad: %#v", err) } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowEvalContextInitProvider_doubleInit(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Call the real func + real.InitProvider(name) + + // Get the provider twice + shadow.InitProvider(name) + p, err := shadow.InitProvider(name) + if err != nil { + t.Fatalf("err: %s", err) + } + if p == nil { + t.Fatal("should return provider") + } + + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should error") + } } From ea8e7659e2bd285435896298e55851927aaf7592 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 1 Oct 2016 12:57:53 -0700 Subject: [PATCH 17/64] terraform: EvalContext.Provider (shadow) --- terraform/shadow_eval_context.go | 29 ++++++++++++++- terraform/shadow_eval_context_test.go | 51 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/terraform/shadow_eval_context.go b/terraform/shadow_eval_context.go index d1fc90272..26c29a68c 100644 --- a/terraform/shadow_eval_context.go +++ b/terraform/shadow_eval_context.go @@ -199,6 +199,34 @@ func (c *shadowEvalContextShadow) InitProvider(n string) (ResourceProvider, erro return result.Shadow, result.ResultErr } +func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { + // Wait for the provider value + raw := c.Shared.Providers.Value(n) + if raw == nil { + c.err(fmt.Errorf( + "Unknown 'Provider' call for %q", n)) + return nil + } + + result, ok := raw.(*shadowEvalContextInitProvider) + if !ok { + c.err(fmt.Errorf( + "Unknown 'Provider' shadow value: %#v", raw)) + return nil + } + + result.Lock() + defer result.Unlock() + + if !result.Init { + // Record the error but continue... + c.err(fmt.Errorf( + "Provider: provider %q requested but not initialized", n)) + } + + return result.Shadow +} + func (c *shadowEvalContextShadow) Diff() (*Diff, *sync.RWMutex) { return c.DiffValue, c.DiffLock } @@ -217,7 +245,6 @@ func (c *shadowEvalContextShadow) err(err error) error { // TODO: All the functions below are EvalContext functions that must be impl. func (c *shadowEvalContextShadow) Input() UIInput { return nil } -func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { return nil } func (c *shadowEvalContextShadow) CloseProvider(n string) error { return nil } func (c *shadowEvalContextShadow) ConfigureProvider(string, *ResourceConfig) error { return nil } func (c *shadowEvalContextShadow) SetProviderConfig(string, *ResourceConfig) error { return nil } diff --git a/terraform/shadow_eval_context_test.go b/terraform/shadow_eval_context_test.go index cbcd1db17..30e7fdf14 100644 --- a/terraform/shadow_eval_context_test.go +++ b/terraform/shadow_eval_context_test.go @@ -106,3 +106,54 @@ func TestShadowEvalContextInitProvider_doubleInit(t *testing.T) { t.Fatal("should error") } } + +func TestShadowEvalContextProvider(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Call the real func + real.InitProvider(name) + shadow.InitProvider(name) + + // Get the provider twice + p := shadow.Provider(name) + if p == nil { + t.Fatal("should return provider") + } + + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowEvalContextProvider_noInit(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Call the real func + real.InitProvider(name) + + // Get the provider w/o calling init + p := shadow.Provider(name) + if p == nil { + t.Fatal("should return provider") + } + + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should error") + } +} From 17b909a59b97f003950bd06a3d8ee1f4c2f5a6df Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 1 Oct 2016 13:05:43 -0700 Subject: [PATCH 18/64] terraform: EvalContext.CloseProvider (shadow) --- terraform/shadow_eval_context.go | 36 ++++++++++++- terraform/shadow_eval_context_test.go | 77 +++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/terraform/shadow_eval_context.go b/terraform/shadow_eval_context.go index 26c29a68c..e39fc14db 100644 --- a/terraform/shadow_eval_context.go +++ b/terraform/shadow_eval_context.go @@ -227,6 +227,38 @@ func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { return result.Shadow } +func (c *shadowEvalContextShadow) CloseProvider(n string) error { + // Wait for the provider value + raw := c.Shared.Providers.Value(n) + if raw == nil { + c.err(fmt.Errorf( + "Unknown 'CloseProvider' call for %q", n)) + return nil + } + + result, ok := raw.(*shadowEvalContextInitProvider) + if !ok { + c.err(fmt.Errorf( + "Unknown 'CloseProvider' shadow value: %#v", raw)) + return nil + } + + result.Lock() + defer result.Unlock() + + if !result.Init { + // Record the error but continue... + c.err(fmt.Errorf( + "CloseProvider: provider %q requested but not initialized", n)) + } else if result.Closed { + c.err(fmt.Errorf( + "CloseProvider: provider %q requested but already closed", n)) + } + + result.Closed = true + return nil +} + func (c *shadowEvalContextShadow) Diff() (*Diff, *sync.RWMutex) { return c.DiffValue, c.DiffLock } @@ -245,17 +277,18 @@ func (c *shadowEvalContextShadow) err(err error) error { // TODO: All the functions below are EvalContext functions that must be impl. func (c *shadowEvalContextShadow) Input() UIInput { return nil } -func (c *shadowEvalContextShadow) CloseProvider(n string) error { return nil } func (c *shadowEvalContextShadow) ConfigureProvider(string, *ResourceConfig) error { return nil } func (c *shadowEvalContextShadow) SetProviderConfig(string, *ResourceConfig) error { return nil } func (c *shadowEvalContextShadow) ParentProviderConfig(string) *ResourceConfig { return nil } func (c *shadowEvalContextShadow) ProviderInput(string) map[string]interface{} { return nil } func (c *shadowEvalContextShadow) SetProviderInput(string, map[string]interface{}) {} + func (c *shadowEvalContextShadow) InitProvisioner(string) (ResourceProvisioner, error) { return nil, nil } func (c *shadowEvalContextShadow) Provisioner(string) ResourceProvisioner { return nil } func (c *shadowEvalContextShadow) CloseProvisioner(string) error { return nil } + func (c *shadowEvalContextShadow) Interpolate(*config.RawConfig, *Resource) (*ResourceConfig, error) { return nil, nil } @@ -270,4 +303,5 @@ type shadowEvalContextInitProvider struct { sync.Mutex // Must be held to modify the field below Init bool // Keeps track of whether it has been initialized in the shadow + Closed bool // Keeps track of whether this provider is closed } diff --git a/terraform/shadow_eval_context_test.go b/terraform/shadow_eval_context_test.go index 30e7fdf14..2ef5e2c09 100644 --- a/terraform/shadow_eval_context_test.go +++ b/terraform/shadow_eval_context_test.go @@ -157,3 +157,80 @@ func TestShadowEvalContextProvider_noInit(t *testing.T) { t.Fatal("should error") } } + +func TestShadowEvalContextCloseProvider(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Call the real func + real.InitProvider(name) + shadow.InitProvider(name) + + // Get the provider twice + if err := shadow.CloseProvider(name); err != nil { + t.Fatalf("err: %s", err) + } + + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowEvalContextCloseProvider_doubleClose(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Call the real func + real.InitProvider(name) + shadow.InitProvider(name) + + // Close the provider twice + if err := shadow.CloseProvider(name); err != nil { + t.Fatalf("err: %s", err) + } + if err := shadow.CloseProvider(name); err != nil { + t.Fatalf("err: %s", err) + } + + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should error") + } +} + +func TestShadowEvalContextCloseProvider_noInitClose(t *testing.T) { + mock := new(MockEvalContext) + real, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Call the real func + real.InitProvider(name) + + // Close the provider + if err := shadow.CloseProvider(name); err != nil { + t.Fatalf("err: %s", err) + } + + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should error") + } +} From d6168edc501a084407d1309dbdbbfccd96a933dc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 1 Oct 2016 13:10:07 -0700 Subject: [PATCH 19/64] helper/shadow: KeyedValue.ValueOk --- helper/shadow/keyed_value.go | 46 +++++++++++++++++++++---------- helper/shadow/keyed_value_test.go | 24 ++++++++++++++++ 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index 0c03c7f6f..9c6d576e9 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -16,27 +16,21 @@ type KeyedValue struct { // Value returns the value that was set for the given key, or blocks // until one is available. func (w *KeyedValue) Value(k string) interface{} { - w.lock.Lock() - w.once.Do(w.init) - - // If we have this value already, return it - if v, ok := w.values[k]; ok { - w.lock.Unlock() + v, val := w.valueWaiter(k) + if val == nil { return v } - // No pending value, check for a waiter - val := w.waiters[k] - if val == nil { - val = new(Value) - w.waiters[k] = val - } - w.lock.Unlock() - - // Return the value once we have it return val.Value() } +// ValueOk gets the value for the given key, returning immediately if the +// value doesn't exist. The second return argument is true if the value exists. +func (w *KeyedValue) ValueOk(k string) (interface{}, bool) { + v, val := w.valueWaiter(k) + return v, val == nil +} + func (w *KeyedValue) SetValue(k string, v interface{}) { w.lock.Lock() defer w.lock.Unlock() @@ -57,3 +51,25 @@ func (w *KeyedValue) init() { w.values = make(map[string]interface{}) w.waiters = make(map[string]*Value) } + +func (w *KeyedValue) valueWaiter(k string) (interface{}, *Value) { + w.lock.Lock() + w.once.Do(w.init) + + // If we have this value already, return it + if v, ok := w.values[k]; ok { + w.lock.Unlock() + return v, nil + } + + // No pending value, check for a waiter + val := w.waiters[k] + if val == nil { + val = new(Value) + w.waiters[k] = val + } + w.lock.Unlock() + + // Return the waiter + return nil, val +} diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go index cfdc053e4..ff59fe8de 100644 --- a/helper/shadow/keyed_value_test.go +++ b/helper/shadow/keyed_value_test.go @@ -49,3 +49,27 @@ func TestKeyedValue_setFirst(t *testing.T) { t.Fatalf("bad: %#v", val) } } + +func TestKeyedValueOk(t *testing.T) { + var v KeyedValue + + // Try + val, ok := v.ValueOk("foo") + if ok { + t.Fatal("should not be ok") + } + + // Set + v.SetValue("foo", 42) + + // Try again + val, ok = v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != 42 { + t.Fatalf("bad: %#v", val) + } +} From 3504054b1e517f56c03a3f0afe35c6eea8714638 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 1 Oct 2016 13:11:32 -0700 Subject: [PATCH 20/64] terraform: EvalContext.CloseProvider (shadow) works if never init --- terraform/shadow_eval_context.go | 7 ++++++- terraform/shadow_eval_context_test.go | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/terraform/shadow_eval_context.go b/terraform/shadow_eval_context.go index e39fc14db..0c962c433 100644 --- a/terraform/shadow_eval_context.go +++ b/terraform/shadow_eval_context.go @@ -229,7 +229,12 @@ func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { func (c *shadowEvalContextShadow) CloseProvider(n string) error { // Wait for the provider value - raw := c.Shared.Providers.Value(n) + raw, ok := c.Shared.Providers.ValueOk(n) + if !ok { + c.err(fmt.Errorf( + "CloseProvider called for uninitialized provider %q", n)) + return nil + } if raw == nil { c.err(fmt.Errorf( "Unknown 'CloseProvider' call for %q", n)) diff --git a/terraform/shadow_eval_context_test.go b/terraform/shadow_eval_context_test.go index 2ef5e2c09..08db93a9f 100644 --- a/terraform/shadow_eval_context_test.go +++ b/terraform/shadow_eval_context_test.go @@ -234,3 +234,24 @@ func TestShadowEvalContextCloseProvider_noInitClose(t *testing.T) { t.Fatal("should error") } } + +func TestShadowEvalContextCloseProvider_noCreate(t *testing.T) { + mock := new(MockEvalContext) + _, shadow := NewShadowEvalContext(mock) + + // Args, results + name := "foo" + mockResult := new(MockResourceProvider) + + // Configure the mock + mock.InitProviderProvider = mockResult + + // Close the provider + if err := shadow.CloseProvider(name); err != nil { + t.Fatalf("err: %s", err) + } + + if err := shadow.CloseShadow(); err == nil { + t.Fatal("should error") + } +} From 9ae9f208d1aa1c1481de974399acf1e7285f1809 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 1 Oct 2016 16:51:00 -0700 Subject: [PATCH 21/64] terraform: Context knows how to walk a shadow graph and report errors --- terraform/context.go | 111 ++++++++++++++++++++++++++++++++---- terraform/context_import.go | 2 +- terraform/shadow_context.go | 20 +++++++ terraform/terraform_test.go | 4 ++ 4 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 terraform/shadow_context.go diff --git a/terraform/context.go b/terraform/context.go index 5940dd3f4..fc7e4c994 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "io" "log" "sort" "strings" @@ -32,6 +33,12 @@ const ( InputModeStd = InputModeVar | InputModeProvider ) +var ( + // contextFailOnShadowError will cause Context operations to return + // errors when shadow operations fail. This is only used for testing. + contextFailOnShadowError = false +) + // ContextOpts are the user-configurable options to create a context with // NewContext. type ContextOpts struct { @@ -74,6 +81,7 @@ type Context struct { parallelSem Semaphore providerInputConfig map[string]map[string]interface{} runCh <-chan struct{} + shadowErr error } // NewContext creates a new Context structure. @@ -190,6 +198,33 @@ func (c *Context) graphBuilder(g *ContextGraphOpts) GraphBuilder { } } +// ShadowError returns any errors caught during a shadow operation. +// +// A shadow operation is an operation run in parallel to a real operation +// that performs the same tasks using new logic on copied state. The results +// are compared to ensure that the new logic works the same as the old logic. +// The shadow never affects the real operation or return values. +// +// The result of the shadow operation are only available through this function +// call after a real operation is complete. +// +// For API consumers of Context, you can safely ignore this function +// completely if you have no interest in helping report experimental feature +// errors to Terraform maintainers. Otherwise, please call this function +// after every operation and report this to the user. +// +// IMPORTANT: Shadow errors are _never_ critical: they _never_ affect +// the real state or result of a real operation. They are purely informational +// to assist in future Terraform versions being more stable. Please message +// this effectively to the end user. +// +// This must be called only when no other operation is running (refresh, +// plan, etc.). The result can be used in parallel to any other operation +// running. +func (c *Context) ShadowError() error { + return c.shadowErr +} + // Input asks for input to fill variables and provider configurations. // This modifies the configuration in-place, so asking for Input twice // may result in different UI output showing different current values. @@ -300,7 +335,7 @@ func (c *Context) Input(mode InputMode) error { } // Do the walk - if _, err := c.walk(graph, walkInput); err != nil { + if _, err := c.walk(graph, nil, walkInput); err != nil { return err } } @@ -329,9 +364,9 @@ func (c *Context) Apply() (*State, error) { // Do the walk var walker *ContextGraphWalker if c.destroy { - walker, err = c.walk(graph, walkDestroy) + walker, err = c.walk(graph, nil, walkDestroy) } else { - walker, err = c.walk(graph, walkApply) + walker, err = c.walk(graph, nil, walkApply) } if len(walker.ValidationErrors) > 0 { @@ -396,7 +431,7 @@ func (c *Context) Plan() (*Plan, error) { } // Do the walk - walker, err := c.walk(graph, operation) + walker, err := c.walk(graph, nil, operation) if err != nil { return nil, err } @@ -434,7 +469,7 @@ func (c *Context) Refresh() (*State, error) { } // Do the walk - if _, err := c.walk(graph, walkRefresh); err != nil { + if _, err := c.walk(graph, nil, walkRefresh); err != nil { return nil, err } @@ -502,7 +537,7 @@ func (c *Context) Validate() ([]string, []error) { } // Walk - walker, err := c.walk(graph, walkValidate) + walker, err := c.walk(graph, nil, walkValidate) if err != nil { return nil, multierror.Append(errs, err).Errors } @@ -541,8 +576,13 @@ func (c *Context) acquireRun() chan<- struct{} { c.l.Lock() } + // Create the new channel ch := make(chan struct{}) c.runCh = ch + + // Reset the shadow errors + c.shadowErr = nil + return ch } @@ -556,11 +596,62 @@ func (c *Context) releaseRun(ch chan<- struct{}) { } func (c *Context) walk( - graph *Graph, operation walkOperation) (*ContextGraphWalker, error) { - // Walk the graph + graph, shadow *Graph, operation walkOperation) (*ContextGraphWalker, error) { + // Keep track of the "real" context which is the context that does + // the real work: talking to real providers, modifying real state, etc. + realCtx := c + + // If we have a shadow graph, walk that as well + var shadowCh chan error + var shadowCloser io.Closer + 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 + // the results of the real. + var shadowCtx *Context + realCtx, shadowCtx, shadowCloser = newShadowContext(c) + + // Build the graph walker for the shadow. + shadowWalker := &ContextGraphWalker{ + Context: shadowCtx, + Operation: operation, + } + + // Kick off the shadow walk. This will block on any operations + // on the real walk so it is fine to start first. + shadowCh = make(chan error) + go func() { + shadowCh <- shadow.Walk(shadowWalker) + }() + } + + // Build the real graph walker log.Printf("[DEBUG] Starting graph walk: %s", operation.String()) - walker := &ContextGraphWalker{Context: c, Operation: operation} - return walker, graph.Walk(walker) + walker := &ContextGraphWalker{Context: realCtx, Operation: operation} + + // Walk the real graph, this will block until it completes + realErr := graph.Walk(walker) + + // 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 { + c.shadowErr = multierror.Append(c.shadowErr, err) + } + + // Wait for the walk to end + if err := <-shadowCh; err != nil { + c.shadowErr = multierror.Append(c.shadowErr, err) + } + + // If we're supposed to fail on shadow errors, then report it + if contextFailOnShadowError && c.shadowErr != nil { + realErr = multierror.Append(realErr, multierror.Prefix( + c.shadowErr, "shadow graph:")) + } + } + + return walker, realErr } // parseVariableAsHCL parses the value of a single variable as would have been specified diff --git a/terraform/context_import.go b/terraform/context_import.go index 20969ae00..eb9534c1a 100644 --- a/terraform/context_import.go +++ b/terraform/context_import.go @@ -63,7 +63,7 @@ func (c *Context) Import(opts *ImportOpts) (*State, error) { } // Walk it - if _, err := c.walk(graph, walkImport); err != nil { + if _, err := c.walk(graph, nil, walkImport); err != nil { return c.state, err } diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go new file mode 100644 index 000000000..6e02eb3ec --- /dev/null +++ b/terraform/shadow_context.go @@ -0,0 +1,20 @@ +package terraform + +import ( + "io" +) + +// newShadowContext creates a new context that will shadow the given context +// 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. +// +// 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) { + return c, nil, nil +} diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 880de448a..910005d2a 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -23,6 +23,7 @@ const fixtureDir = "./test-fixtures" func TestMain(m *testing.M) { flag.Parse() + if testing.Verbose() { // if we're verbose, use the logging requested by TF_LOG logging.SetOutput() @@ -31,6 +32,9 @@ func TestMain(m *testing.M) { log.SetOutput(ioutil.Discard) } + // Make sure shadow operations fail our real tests + contextFailOnShadowError = true + os.Exit(m.Run()) } From f7134d95e4b38d6b61599f1bf60b0a0657f0ff4a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 1 Oct 2016 21:30:09 -0700 Subject: [PATCH 22/64] terraform: Diff.DeepCopy --- terraform/context.go | 16 ++++++++++++++++ terraform/diff.go | 13 +++++++++++++ terraform/terraform_test.go | 3 +++ 3 files changed, 32 insertions(+) diff --git a/terraform/context.go b/terraform/context.go index fc7e4c994..58119f78a 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -37,6 +37,11 @@ var ( // contextFailOnShadowError will cause Context operations to return // errors when shadow operations fail. This is only used for testing. contextFailOnShadowError = false + + // contextTestDeepCopyOnPlan will perform a Diff DeepCopy on every + // Plan operation, effectively testing the Diff DeepCopy whenever + // a Plan occurs. This is enabled for tests. + contextTestDeepCopyOnPlan = false ) // ContextOpts are the user-configurable options to create a context with @@ -437,6 +442,17 @@ func (c *Context) Plan() (*Plan, error) { } p.Diff = c.diff + // If this is true, it means we're running unit tests. In this case, + // we perform a deep copy just to ensure that all context tests also + // test that a diff is copy-able. This will panic if it fails. This + // is enabled during unit tests. + // + // This should never be true during production usage, but even if it is, + // it can't do any real harm. + if contextTestDeepCopyOnPlan { + p.Diff.DeepCopy() + } + // Now that we have a diff, we can build the exact graph that Apply will use // and catch any possible cycles during the Plan phase. if _, err := c.Graph(&ContextGraphOpts{Validate: true}); err != nil { diff --git a/terraform/diff.go b/terraform/diff.go index 351a3c48d..b43e16ee8 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -9,6 +9,8 @@ import ( "sort" "strings" "sync" + + "github.com/mitchellh/copystructure" ) // DiffChangeType is an enum with the kind of changes a diff has planned. @@ -79,6 +81,17 @@ func (d *Diff) Empty() bool { return true } +// DeepCopy performs a deep copy of all parts of the Diff, making the +// resulting Diff safe to use without modifying this one. +func (d *Diff) DeepCopy() *Diff { + copy, err := copystructure.Config{Lock: true}.Copy(d) + if err != nil { + panic(err) + } + + return copy.(*Diff) +} + func (d *Diff) String() string { var buf bytes.Buffer diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 910005d2a..dd6185613 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -35,6 +35,9 @@ func TestMain(m *testing.M) { // Make sure shadow operations fail our real tests contextFailOnShadowError = true + // Always DeepCopy the Diff on every Plan during a test + contextTestDeepCopyOnPlan = true + os.Exit(m.Run()) } From 02e93f592043aa7787bc0f38104d5e7992d924ac Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 00:01:11 -0700 Subject: [PATCH 23/64] terraform: shadowResourceProviderFactory This helper helps create the factory maps for the context. --- terraform/context.go | 4 + terraform/shadow_context.go | 39 ++++++- terraform/shadow_resource_provider_factory.go | 101 ++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 terraform/shadow_resource_provider_factory.go diff --git a/terraform/context.go b/terraform/context.go index 58119f78a..c1e52f03a 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -68,6 +68,10 @@ type ContextOpts struct { // // Extra functions on Context can be found in context_*.go files. type Context struct { + // Maintainer note: Anytime this struct is changed, please verify + // that newShadowContext still does the right thing. Tests should + // fail regardless but putting this note here as well. + destroy bool diff *Diff diffLock sync.RWMutex diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 6e02eb3ec..8bbebc00d 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -2,6 +2,8 @@ package terraform import ( "io" + + "github.com/mitchellh/copystructure" ) // newShadowContext creates a new context that will shadow the given context @@ -16,5 +18,40 @@ import ( // 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) { - return c, nil, nil + // Copy the targets + targetRaw, err := copystructure.Config{Lock: true}.Copy(c.targets) + if err != nil { + panic(err) + } + + // Copy the variables + varRaw, err := copystructure.Config{Lock: true}.Copy(c.variables) + if err != nil { + panic(err) + } + + // The factories + providerFactory := &shadowResourceProviderFactory{Original: c.providers} + + // Create the shadow + shadow := &Context{ + destroy: c.destroy, + diff: c.diff.DeepCopy(), + hooks: nil, // TODO: do we need to copy? stop hook? + module: c.module, + providers: providerFactory.ShadowMap(), + provisioners: nil, //TODO + state: c.state.DeepCopy(), + targets: targetRaw.([]string), + uiInput: nil, // TODO + variables: varRaw.(map[string]interface{}), + } + + // Create the real context. This is effectively just a copy of + // the context given except we need to modify some of the values + // to point to the real side of a shadow so the shadow can compare values. + real := *c + real.providers = providerFactory.RealMap() + + return &real, shadow, nil } diff --git a/terraform/shadow_resource_provider_factory.go b/terraform/shadow_resource_provider_factory.go new file mode 100644 index 000000000..05407606a --- /dev/null +++ b/terraform/shadow_resource_provider_factory.go @@ -0,0 +1,101 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/helper/shadow" +) + +// shadowResourceProviderFactory is a helper that takes an actual, original +// map of ResourceProvider factories and provides methods to create mappings +// for shadowed resource providers. +type shadowResourceProviderFactory struct { + // Original is the original factory map + Original map[string]ResourceProviderFactory + + shadows shadow.KeyedValue +} + +type shadowResourceProviderFactoryEntry struct { + Real ResourceProvider + Shadow shadowResourceProvider + Err error +} + +// RealMap returns the factory map for the "real" side of the shadow. This +// is the side that does actual work. +// TODO: test +func (f *shadowResourceProviderFactory) RealMap() map[string]ResourceProviderFactory { + m := make(map[string]ResourceProviderFactory) + for k, _ := range f.Original { + m[k] = f.realFactory(k) + } + + return m +} + +// ShadowMap returns the factory map for the "shadow" side of the shadow. This +// is the side that doesn't do any actual work but does compare results +// with the real side. +// TODO: test +func (f *shadowResourceProviderFactory) ShadowMap() map[string]ResourceProviderFactory { + m := make(map[string]ResourceProviderFactory) + for k, _ := range f.Original { + m[k] = f.shadowFactory(k) + } + + return m +} + +func (f *shadowResourceProviderFactory) realFactory(n string) ResourceProviderFactory { + return func() (ResourceProvider, error) { + // Get the original factory function + originalF, ok := f.Original[n] + if !ok { + return nil, fmt.Errorf("unknown provider initialized: %s", n) + } + + // Build the entry + var entry shadowResourceProviderFactoryEntry + + // Initialize it + p, err := originalF() + if err != nil { + entry.Err = err + p = nil // Just to be sure + } + + if p != nil { + // Create the shadow + real, shadow := newShadowResourceProvider(p) + entry.Real = real + entry.Shadow = shadow + } + + // Store the value + f.shadows.SetValue(n, &entry) + + // Return + return entry.Real, entry.Err + } +} + +func (f *shadowResourceProviderFactory) shadowFactory(n string) ResourceProviderFactory { + return func() (ResourceProvider, error) { + // Get the value + raw := f.shadows.Value(n) + if raw == nil { + return nil, fmt.Errorf( + "Nil shadow value for provider %q. Please report this bug.", + n) + } + + entry, ok := raw.(*shadowResourceProviderFactoryEntry) + if !ok { + return nil, fmt.Errorf("Unknown value for shadow provider: %#v", raw) + } + + // Return + return entry.Shadow, entry.Err + } +} From 742af8752b9b66c9c33ca995aa7942473bfb7570 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 09:45:18 -0700 Subject: [PATCH 24/64] terraform: run the shadow graph for Apply operations (everything fails) --- terraform/context.go | 18 +++++++++++++----- terraform/context_apply_test.go | 2 +- terraform/shadow_context.go | 25 ++++++++++++++++++++++--- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index c1e52f03a..110ba4772 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -375,7 +375,7 @@ func (c *Context) Apply() (*State, error) { if c.destroy { walker, err = c.walk(graph, nil, walkDestroy) } else { - walker, err = c.walk(graph, nil, walkApply) + walker, err = c.walk(graph, graph, walkApply) } if len(walker.ValidationErrors) > 0 { @@ -641,6 +641,7 @@ func (c *Context) walk( // on the real walk so it is fine to start first. shadowCh = make(chan error) go func() { + log.Printf("[INFO] Starting shadow graph walk: %s", operation.String()) shadowCh <- shadow.Walk(shadowWalker) }() } @@ -660,14 +661,21 @@ func (c *Context) walk( } // Wait for the walk to end + log.Printf("[DEBUG] Waiting for shadow graph to complete...") if err := <-shadowCh; err != nil { c.shadowErr = multierror.Append(c.shadowErr, err) } - // If we're supposed to fail on shadow errors, then report it - if contextFailOnShadowError && c.shadowErr != nil { - realErr = multierror.Append(realErr, multierror.Prefix( - c.shadowErr, "shadow graph:")) + if c.shadowErr == nil { + log.Printf("[INFO] Shadow graph success!") + } else { + log.Printf("[ERROR] Shadow graph error: %s", c.shadowErr) + + // If we're supposed to fail on shadow errors, then report it + if contextFailOnShadowError { + realErr = multierror.Append(realErr, multierror.Prefix( + c.shadowErr, "shadow graph:")) + } } } diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 0296ef487..6a5de2a34 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/terraform/config/module" ) -func TestContext2Apply(t *testing.T) { +func TestContext2Apply_basic(t *testing.T) { m := testModule(t, "apply-good") p := testProvider("aws") p.ApplyFn = testApplyFn diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 8bbebc00d..05a0f9c67 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -19,13 +19,13 @@ import ( // copied, no real providers or resources are used, etc. func newShadowContext(c *Context) (*Context, *Context, io.Closer) { // Copy the targets - targetRaw, err := copystructure.Config{Lock: true}.Copy(c.targets) + targetRaw, err := copystructure.Copy(c.targets) if err != nil { panic(err) } // Copy the variables - varRaw, err := copystructure.Config{Lock: true}.Copy(c.variables) + varRaw, err := copystructure.Copy(c.variables) if err != nil { panic(err) } @@ -45,6 +45,12 @@ func newShadowContext(c *Context) (*Context, *Context, io.Closer) { targets: targetRaw.([]string), uiInput: nil, // TODO variables: varRaw.(map[string]interface{}), + + // Hardcoded to 4 since parallelism in the shadow doesn't matter + // a ton since we're doing far less compared to the real side + // and our operations are MUCH faster. + parallelSem: NewSemaphore(4), + providerInputConfig: make(map[string]map[string]interface{}), } // Create the real context. This is effectively just a copy of @@ -53,5 +59,18 @@ func newShadowContext(c *Context) (*Context, *Context, io.Closer) { real := *c real.providers = providerFactory.RealMap() - return &real, shadow, nil + return &real, shadow, &shadowContextCloser{ + Providers: providerFactory, + } +} + +// shadowContextCloser is the io.Closer returned by newShadowContext that +// closes all the shadows and returns the results. +type shadowContextCloser struct { + Providers interface{} +} + +// Close closes the shadow context. +func (c *shadowContextCloser) Close() error { + return nil } From 5053872e82dcbf813d6a302cbe75e861ff15068e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 11:13:49 -0700 Subject: [PATCH 25/64] terraform: Diff.DeepCopy test to catch a bug that in copystructure This was fixed upstream but keeping the test around to prevent regressions. --- terraform/diff.go | 4 ++++ terraform/diff_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/terraform/diff.go b/terraform/diff.go index b43e16ee8..33d27f714 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -292,6 +292,10 @@ func (d *ModuleDiff) String() string { return buf.String() } +func (d *ModuleDiff) GoString() string { + return fmt.Sprintf("*%#v", *d) +} + // InstanceDiff is the diff of a resource from some state to another. type InstanceDiff struct { mu sync.Mutex diff --git a/terraform/diff_test.go b/terraform/diff_test.go index faffcbbcc..6239bc413 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -115,6 +115,39 @@ func TestModuleDiff_ChangeType(t *testing.T) { } } +func TestDiff_DeepCopy(t *testing.T) { + cases := map[string]*Diff{ + "empty": &Diff{}, + + "basic diff": &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + "aws_instance.foo": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "num": &ResourceAttrDiff{ + Old: "0", + New: "2", + }, + }, + }, + }, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + dup := tc.DeepCopy() + if !reflect.DeepEqual(dup, tc) { + t.Fatalf("\n%#v\n\n%#v", dup, tc) + } + }) + } +} + func TestModuleDiff_Empty(t *testing.T) { diff := new(ModuleDiff) if !diff.Empty() { From 0b00bbde4e54097c1be014164a922c980219a102 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 18:23:37 -0700 Subject: [PATCH 26/64] terraform: switch to a component factory This is necessary so that the shadow version can actually keep track of what provider is used for what. Before, providers for different alises were just initialized but the factory had no idea. Arguably this is fine but when trying to build a shadow graph this presents challenges. With these changes, we now pass an opaque "uid" through that is used to keep track of the providers and what real maps to what shadow. --- terraform/context.go | 64 +++---- terraform/context_components.go | 65 +++++++ terraform/context_import.go | 8 +- terraform/eval_context_builtin.go | 23 +-- terraform/graph_walk_context.go | 3 +- terraform/shadow_components.go | 169 ++++++++++++++++++ terraform/shadow_context.go | 27 ++- terraform/shadow_resource_provider_factory.go | 101 ----------- 8 files changed, 280 insertions(+), 180 deletions(-) create mode 100644 terraform/context_components.go create mode 100644 terraform/shadow_components.go delete mode 100644 terraform/shadow_resource_provider_factory.go diff --git a/terraform/context.go b/terraform/context.go index 110ba4772..cc69305d0 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -72,19 +72,18 @@ type Context struct { // that newShadowContext still does the right thing. Tests should // fail regardless but putting this note here as well. - destroy bool - diff *Diff - diffLock sync.RWMutex - hooks []Hook - module *module.Tree - providers map[string]ResourceProviderFactory - provisioners map[string]ResourceProvisionerFactory - sh *stopHook - state *State - stateLock sync.RWMutex - targets []string - uiInput UIInput - variables map[string]interface{} + components contextComponentFactory + destroy bool + diff *Diff + diffLock sync.RWMutex + hooks []Hook + module *module.Tree + sh *stopHook + state *State + stateLock sync.RWMutex + targets []string + uiInput UIInput + variables map[string]interface{} l sync.Mutex // Lock acquired during any task parallelSem Semaphore @@ -153,16 +152,18 @@ func NewContext(opts *ContextOpts) (*Context, error) { } return &Context{ - destroy: opts.Destroy, - diff: opts.Diff, - hooks: hooks, - module: opts.Module, - providers: opts.Providers, - provisioners: opts.Provisioners, - state: state, - targets: opts.Targets, - uiInput: opts.UIInput, - variables: variables, + components: &basicComponentFactory{ + providers: opts.Providers, + provisioners: opts.Provisioners, + }, + destroy: opts.Destroy, + diff: opts.Diff, + hooks: hooks, + module: opts.Module, + state: state, + targets: opts.Targets, + uiInput: opts.UIInput, + variables: variables, parallelSem: NewSemaphore(par), providerInputConfig: make(map[string]map[string]interface{}), @@ -183,22 +184,11 @@ func (c *Context) Graph(g *ContextGraphOpts) (*Graph, error) { // GraphBuilder returns the GraphBuilder that will be used to create // the graphs for this context. func (c *Context) graphBuilder(g *ContextGraphOpts) GraphBuilder { - // TODO test - providers := make([]string, 0, len(c.providers)) - for k, _ := range c.providers { - providers = append(providers, k) - } - - provisioners := make([]string, 0, len(c.provisioners)) - for k, _ := range c.provisioners { - provisioners = append(provisioners, k) - } - return &BuiltinGraphBuilder{ Root: c.module, Diff: c.diff, - Providers: providers, - Provisioners: provisioners, + Providers: c.components.ResourceProviders(), + Provisioners: c.components.ResourceProvisioners(), State: c.state, Targets: c.targets, Destroy: c.destroy, @@ -375,7 +365,7 @@ func (c *Context) Apply() (*State, error) { if c.destroy { walker, err = c.walk(graph, nil, walkDestroy) } else { - walker, err = c.walk(graph, graph, walkApply) + walker, err = c.walk(graph, nil, walkApply) } if len(walker.ValidationErrors) > 0 { diff --git a/terraform/context_components.go b/terraform/context_components.go new file mode 100644 index 000000000..6f507445c --- /dev/null +++ b/terraform/context_components.go @@ -0,0 +1,65 @@ +package terraform + +import ( + "fmt" +) + +// contextComponentFactory is the interface that Context uses +// to initialize various components such as providers and provisioners. +// This factory gets more information than the raw maps using to initialize +// a Context. This information is used for debugging. +type contextComponentFactory interface { + // ResourceProvider creates a new ResourceProvider with the given + // type. The "uid" is a unique identifier for this provider being + // initialized that can be used for internal tracking. + ResourceProvider(typ, uid string) (ResourceProvider, error) + ResourceProviders() []string + + // ResourceProvisioner creates a new ResourceProvisioner with the + // given type. The "uid" is a unique identifier for this provisioner + // being initialized that can be used for internal tracking. + ResourceProvisioner(typ, uid string) (ResourceProvisioner, error) + ResourceProvisioners() []string +} + +// basicComponentFactory just calls a factory from a map directly. +type basicComponentFactory struct { + providers map[string]ResourceProviderFactory + provisioners map[string]ResourceProvisionerFactory +} + +func (c *basicComponentFactory) ResourceProviders() []string { + result := make([]string, len(c.providers)) + for k, _ := range c.providers { + result = append(result, k) + } + + return result +} + +func (c *basicComponentFactory) ResourceProvisioners() []string { + result := make([]string, len(c.provisioners)) + for k, _ := range c.provisioners { + result = append(result, k) + } + + return result +} + +func (c *basicComponentFactory) ResourceProvider(typ, uid string) (ResourceProvider, error) { + f, ok := c.providers[typ] + if !ok { + return nil, fmt.Errorf("unknown provider %q", typ) + } + + return f() +} + +func (c *basicComponentFactory) ResourceProvisioner(typ, uid string) (ResourceProvisioner, error) { + f, ok := c.provisioners[typ] + if !ok { + return nil, fmt.Errorf("unknown provisioner %q", typ) + } + + return f() +} diff --git a/terraform/context_import.go b/terraform/context_import.go index eb9534c1a..8ac0e4e14 100644 --- a/terraform/context_import.go +++ b/terraform/context_import.go @@ -43,17 +43,11 @@ func (c *Context) Import(opts *ImportOpts) (*State, error) { // Copy our own state c.state = c.state.DeepCopy() - // Get supported providers (for the graph builder) - providers := make([]string, 0, len(c.providers)) - for k, _ := range c.providers { - providers = append(providers, k) - } - // Initialize our graph builder builder := &ImportGraphBuilder{ ImportTargets: opts.Targets, Module: opts.Module, - Providers: providers, + Providers: c.components.ResourceProviders(), } // Build the graph! diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index d85cb77fc..8316cb3e8 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -26,14 +26,13 @@ type BuiltinEvalContext struct { InterpolaterVars map[string]map[string]interface{} InterpolaterVarLock *sync.Mutex + Components contextComponentFactory Hooks []Hook InputValue UIInput - Providers map[string]ResourceProviderFactory ProviderCache map[string]ResourceProvider ProviderConfigCache map[string]*ResourceConfig ProviderInputConfig map[string]map[string]interface{} ProviderLock *sync.Mutex - Provisioners map[string]ResourceProvisionerFactory ProvisionerCache map[string]ResourceProvisioner ProvisionerLock *sync.Mutex DiffValue *Diff @@ -82,13 +81,9 @@ func (ctx *BuiltinEvalContext) InitProvider(n string) (ResourceProvider, error) defer ctx.ProviderLock.Unlock() typeName := strings.SplitN(n, ".", 2)[0] + uid := n - f, ok := ctx.Providers[typeName] - if !ok { - return nil, fmt.Errorf("Provider '%s' not found", typeName) - } - - p, err := f() + p, err := ctx.Components.ResourceProvider(typeName, uid) if err != nil { return nil, err } @@ -231,12 +226,7 @@ func (ctx *BuiltinEvalContext) InitProvisioner( ctx.ProvisionerLock.Lock() defer ctx.ProvisionerLock.Unlock() - f, ok := ctx.Provisioners[n] - if !ok { - return nil, fmt.Errorf("Provisioner '%s' not found", n) - } - - p, err := f() + p, err := ctx.Components.ResourceProvisioner(n, n) if err != nil { return nil, err } @@ -341,9 +331,4 @@ func (ctx *BuiltinEvalContext) State() (*State, *sync.RWMutex) { } func (ctx *BuiltinEvalContext) init() { - // We nil-check the things below because they're meant to be configured, - // and we just default them to non-nil. - if ctx.Providers == nil { - ctx.Providers = make(map[string]ResourceProviderFactory) - } } diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 7424fdbbd..459fcdec9 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -68,12 +68,11 @@ func (w *ContextGraphWalker) EnterPath(path []string) EvalContext { PathValue: path, Hooks: w.Context.hooks, InputValue: w.Context.uiInput, - Providers: w.Context.providers, + Components: w.Context.components, ProviderCache: w.providerCache, ProviderConfigCache: w.providerConfigCache, ProviderInputConfig: w.Context.providerInputConfig, ProviderLock: &w.providerLock, - Provisioners: w.Context.provisioners, ProvisionerCache: w.provisionerCache, ProvisionerLock: &w.provisionerLock, DiffValue: w.Context.diff, diff --git a/terraform/shadow_components.go b/terraform/shadow_components.go new file mode 100644 index 000000000..b4a444d40 --- /dev/null +++ b/terraform/shadow_components.go @@ -0,0 +1,169 @@ +package terraform + +import ( + "fmt" + "sync" + + "github.com/hashicorp/terraform/helper/shadow" +) + +// newShadowComponentFactory creates a shadowed contextComponentFactory +// so that requests to create new components result in both a real and +// shadow side. +func newShadowComponentFactory( + f contextComponentFactory) (contextComponentFactory, *shadowComponentFactory) { + // Create the shared data + shared := &shadowComponentFactoryShared{contextComponentFactory: f} + + // Create the real side + real := &shadowComponentFactoryReal{ + shadowComponentFactoryShared: shared, + } + + // Create the shadow + shadow := &shadowComponentFactory{ + shadowComponentFactoryShared: shared, + } + + return real, shadow +} + +// shadowComponentFactory is the shadow side. Any components created +// with this factory are fake and will not cause real work to happen. +type shadowComponentFactory struct { + *shadowComponentFactoryShared +} + +func (f *shadowComponentFactory) ResourceProvider( + n, uid string) (ResourceProvider, error) { + _, shadow, err := f.shadowComponentFactoryShared.ResourceProvider(n, uid) + return shadow, err +} + +func (f *shadowComponentFactory) ResourceProvisioner( + n, uid string) (ResourceProvisioner, error) { + _, shadow, err := f.shadowComponentFactoryShared.ResourceProvisioner(n, uid) + return shadow, err +} + +// shadowComponentFactoryReal is the real side of the component factory. +// Operations here result in real components that do real work. +type shadowComponentFactoryReal struct { + *shadowComponentFactoryShared +} + +func (f *shadowComponentFactoryReal) ResourceProvider( + n, uid string) (ResourceProvider, error) { + real, _, err := f.shadowComponentFactoryShared.ResourceProvider(n, uid) + return real, err +} + +func (f *shadowComponentFactoryReal) ResourceProvisioner( + n, uid string) (ResourceProvisioner, error) { + real, _, err := f.shadowComponentFactoryShared.ResourceProvisioner(n, uid) + return real, err +} + +// shadowComponentFactoryShared is shared data between the two factories. +type shadowComponentFactoryShared struct { + contextComponentFactory + + providers shadow.KeyedValue + provisioners shadow.KeyedValue + lock sync.Mutex +} + +// shadowResourceProviderFactoryEntry is the entry that is stored in +// the Shadows key/value for a provider. +type shadowComponentFactoryProviderEntry struct { + Real ResourceProvider + Shadow shadowResourceProvider + Err error +} + +type shadowComponentFactoryProvisionerEntry struct { + Real ResourceProvisioner + Shadow ResourceProvisioner + Err error +} + +func (f *shadowComponentFactoryShared) ResourceProvider( + n, uid string) (ResourceProvider, shadowResourceProvider, error) { + f.lock.Lock() + defer f.lock.Unlock() + + // Determine if we already have a value + raw, ok := f.providers.ValueOk(uid) + if !ok { + // Build the entry + var entry shadowComponentFactoryProviderEntry + + // No value, initialize. Create the original + p, err := f.contextComponentFactory.ResourceProvider(n, uid) + if err != nil { + entry.Err = err + p = nil // Just to be sure + } + + if p != nil { + // Create the shadow + real, shadow := newShadowResourceProvider(p) + entry.Real = real + entry.Shadow = shadow + } + + // Store the value + f.providers.SetValue(uid, &entry) + raw = &entry + } + + // Read the entry + entry, ok := raw.(*shadowComponentFactoryProviderEntry) + if !ok { + return nil, nil, fmt.Errorf("Unknown value for shadow provider: %#v", raw) + } + + // Return + return entry.Real, entry.Shadow, entry.Err +} + +func (f *shadowComponentFactoryShared) ResourceProvisioner( + n, uid string) (ResourceProvisioner, ResourceProvisioner, error) { + f.lock.Lock() + defer f.lock.Unlock() + + // Determine if we already have a value + raw, ok := f.provisioners.ValueOk(uid) + if !ok { + // Build the entry + var entry shadowComponentFactoryProvisionerEntry + + // No value, initialize. Create the original + p, err := f.contextComponentFactory.ResourceProvisioner(n, uid) + if err != nil { + entry.Err = err + p = nil // Just to be sure + } + + if p != nil { + // For now, just create a mock since we don't support provisioners yet + real := p + shadow := new(MockResourceProvisioner) + entry.Real = real + entry.Shadow = shadow + } + + // Store the value + f.provisioners.SetValue(uid, &entry) + raw = &entry + } + + // Read the entry + entry, ok := raw.(*shadowComponentFactoryProvisionerEntry) + if !ok { + return nil, nil, fmt.Errorf("Unknown value for shadow provisioner: %#v", raw) + } + + // Return + return entry.Real, entry.Shadow, entry.Err +} diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 05a0f9c67..d73f737b4 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -31,20 +31,19 @@ func newShadowContext(c *Context) (*Context, *Context, io.Closer) { } // The factories - providerFactory := &shadowResourceProviderFactory{Original: c.providers} + componentsReal, componentsShadow := newShadowComponentFactory(c.components) // Create the shadow shadow := &Context{ - destroy: c.destroy, - diff: c.diff.DeepCopy(), - hooks: nil, // TODO: do we need to copy? stop hook? - module: c.module, - providers: providerFactory.ShadowMap(), - provisioners: nil, //TODO - state: c.state.DeepCopy(), - targets: targetRaw.([]string), - uiInput: nil, // TODO - variables: varRaw.(map[string]interface{}), + components: componentsShadow, + destroy: c.destroy, + diff: c.diff.DeepCopy(), + hooks: nil, // TODO: do we need to copy? stop hook? + module: c.module, + state: c.state.DeepCopy(), + targets: targetRaw.([]string), + uiInput: nil, // TODO + variables: varRaw.(map[string]interface{}), // Hardcoded to 4 since parallelism in the shadow doesn't matter // a ton since we're doing far less compared to the real side @@ -57,17 +56,17 @@ func newShadowContext(c *Context) (*Context, *Context, io.Closer) { // the context given except we need to modify some of the values // to point to the real side of a shadow so the shadow can compare values. real := *c - real.providers = providerFactory.RealMap() + real.components = componentsReal return &real, shadow, &shadowContextCloser{ - Providers: providerFactory, + Components: componentsShadow, } } // shadowContextCloser is the io.Closer returned by newShadowContext that // closes all the shadows and returns the results. type shadowContextCloser struct { - Providers interface{} + Components interface{} } // Close closes the shadow context. diff --git a/terraform/shadow_resource_provider_factory.go b/terraform/shadow_resource_provider_factory.go deleted file mode 100644 index 05407606a..000000000 --- a/terraform/shadow_resource_provider_factory.go +++ /dev/null @@ -1,101 +0,0 @@ -package terraform - -import ( - "fmt" - - "github.com/hashicorp/terraform/helper/shadow" -) - -// shadowResourceProviderFactory is a helper that takes an actual, original -// map of ResourceProvider factories and provides methods to create mappings -// for shadowed resource providers. -type shadowResourceProviderFactory struct { - // Original is the original factory map - Original map[string]ResourceProviderFactory - - shadows shadow.KeyedValue -} - -type shadowResourceProviderFactoryEntry struct { - Real ResourceProvider - Shadow shadowResourceProvider - Err error -} - -// RealMap returns the factory map for the "real" side of the shadow. This -// is the side that does actual work. -// TODO: test -func (f *shadowResourceProviderFactory) RealMap() map[string]ResourceProviderFactory { - m := make(map[string]ResourceProviderFactory) - for k, _ := range f.Original { - m[k] = f.realFactory(k) - } - - return m -} - -// ShadowMap returns the factory map for the "shadow" side of the shadow. This -// is the side that doesn't do any actual work but does compare results -// with the real side. -// TODO: test -func (f *shadowResourceProviderFactory) ShadowMap() map[string]ResourceProviderFactory { - m := make(map[string]ResourceProviderFactory) - for k, _ := range f.Original { - m[k] = f.shadowFactory(k) - } - - return m -} - -func (f *shadowResourceProviderFactory) realFactory(n string) ResourceProviderFactory { - return func() (ResourceProvider, error) { - // Get the original factory function - originalF, ok := f.Original[n] - if !ok { - return nil, fmt.Errorf("unknown provider initialized: %s", n) - } - - // Build the entry - var entry shadowResourceProviderFactoryEntry - - // Initialize it - p, err := originalF() - if err != nil { - entry.Err = err - p = nil // Just to be sure - } - - if p != nil { - // Create the shadow - real, shadow := newShadowResourceProvider(p) - entry.Real = real - entry.Shadow = shadow - } - - // Store the value - f.shadows.SetValue(n, &entry) - - // Return - return entry.Real, entry.Err - } -} - -func (f *shadowResourceProviderFactory) shadowFactory(n string) ResourceProviderFactory { - return func() (ResourceProvider, error) { - // Get the value - raw := f.shadows.Value(n) - if raw == nil { - return nil, fmt.Errorf( - "Nil shadow value for provider %q. Please report this bug.", - n) - } - - entry, ok := raw.(*shadowResourceProviderFactoryEntry) - if !ok { - return nil, fmt.Errorf("Unknown value for shadow provider: %#v", raw) - } - - // Return - return entry.Shadow, entry.Err - } -} From 89e8656c6b9dc7fc08e14c820ce2c67104ea5501 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 19:34:07 -0700 Subject: [PATCH 27/64] terraform: component uid includes the path --- terraform/eval_context_builtin.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 8316cb3e8..032f79f9d 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -80,19 +80,18 @@ func (ctx *BuiltinEvalContext) InitProvider(n string) (ResourceProvider, error) ctx.ProviderLock.Lock() defer ctx.ProviderLock.Unlock() - typeName := strings.SplitN(n, ".", 2)[0] - uid := n + providerPath := make([]string, len(ctx.Path())+1) + copy(providerPath, ctx.Path()) + providerPath[len(providerPath)-1] = n + key := PathCacheKey(providerPath) - p, err := ctx.Components.ResourceProvider(typeName, uid) + typeName := strings.SplitN(n, ".", 2)[0] + p, err := ctx.Components.ResourceProvider(typeName, key) if err != nil { return nil, err } - providerPath := make([]string, len(ctx.Path())+1) - copy(providerPath, ctx.Path()) - providerPath[len(providerPath)-1] = n - - ctx.ProviderCache[PathCacheKey(providerPath)] = p + ctx.ProviderCache[key] = p return p, nil } @@ -226,16 +225,17 @@ func (ctx *BuiltinEvalContext) InitProvisioner( ctx.ProvisionerLock.Lock() defer ctx.ProvisionerLock.Unlock() - p, err := ctx.Components.ResourceProvisioner(n, n) + provPath := make([]string, len(ctx.Path())+1) + copy(provPath, ctx.Path()) + provPath[len(provPath)-1] = n + key := PathCacheKey(provPath) + + p, err := ctx.Components.ResourceProvisioner(n, key) if err != nil { return nil, err } - provPath := make([]string, len(ctx.Path())+1) - copy(provPath, ctx.Path()) - provPath[len(provPath)-1] = n - - ctx.ProvisionerCache[PathCacheKey(provPath)] = p + ctx.ProvisionerCache[key] = p return p, nil } From 8ef35d756197e03b4a3aac4ab52782ff7ca0f9e9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 19:39:27 -0700 Subject: [PATCH 28/64] terraform: simplify the shadow component factory This unifies shadow/real under one since it was really just a basic switch of what to return. --- terraform/shadow_components.go | 39 +++++++++++++++------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/terraform/shadow_components.go b/terraform/shadow_components.go index b4a444d40..8aacfb15e 100644 --- a/terraform/shadow_components.go +++ b/terraform/shadow_components.go @@ -16,13 +16,14 @@ func newShadowComponentFactory( shared := &shadowComponentFactoryShared{contextComponentFactory: f} // Create the real side - real := &shadowComponentFactoryReal{ + real := &shadowComponentFactory{ shadowComponentFactoryShared: shared, } // Create the shadow shadow := &shadowComponentFactory{ shadowComponentFactoryShared: shared, + Shadow: true, } return real, shadow @@ -32,36 +33,30 @@ func newShadowComponentFactory( // with this factory are fake and will not cause real work to happen. type shadowComponentFactory struct { *shadowComponentFactoryShared + + Shadow bool // True if this should return the shadow } func (f *shadowComponentFactory) ResourceProvider( n, uid string) (ResourceProvider, error) { - _, shadow, err := f.shadowComponentFactoryShared.ResourceProvider(n, uid) - return shadow, err + real, shadow, err := f.shadowComponentFactoryShared.ResourceProvider(n, uid) + var result ResourceProvider = real + if f.Shadow { + result = shadow + } + + return result, err } func (f *shadowComponentFactory) ResourceProvisioner( n, uid string) (ResourceProvisioner, error) { - _, shadow, err := f.shadowComponentFactoryShared.ResourceProvisioner(n, uid) - return shadow, err -} + real, shadow, err := f.shadowComponentFactoryShared.ResourceProvisioner(n, uid) + var result ResourceProvisioner = real + if f.Shadow { + result = shadow + } -// shadowComponentFactoryReal is the real side of the component factory. -// Operations here result in real components that do real work. -type shadowComponentFactoryReal struct { - *shadowComponentFactoryShared -} - -func (f *shadowComponentFactoryReal) ResourceProvider( - n, uid string) (ResourceProvider, error) { - real, _, err := f.shadowComponentFactoryShared.ResourceProvider(n, uid) - return real, err -} - -func (f *shadowComponentFactoryReal) ResourceProvisioner( - n, uid string) (ResourceProvisioner, error) { - real, _, err := f.shadowComponentFactoryShared.ResourceProvisioner(n, uid) - return real, err + return result, err } // shadowComponentFactoryShared is shared data between the two factories. From 3e7665db094df0c39c04d504ef40542a6bef2773 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Oct 2016 21:19:59 -0700 Subject: [PATCH 29/64] terraform: shadow component factory supports closing --- terraform/context.go | 3 +- terraform/shadow_components.go | 72 +++++++++++++++++++++++++++++----- terraform/shadow_context.go | 4 +- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index cc69305d0..27aa71d43 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -365,7 +365,8 @@ func (c *Context) Apply() (*State, error) { if c.destroy { walker, err = c.walk(graph, nil, walkDestroy) } else { - walker, err = c.walk(graph, nil, walkApply) + //walker, err = c.walk(graph, nil, walkApply) + walker, err = c.walk(graph, graph, walkApply) } if len(walker.ValidationErrors) > 0 { diff --git a/terraform/shadow_components.go b/terraform/shadow_components.go index 8aacfb15e..ab491a022 100644 --- a/terraform/shadow_components.go +++ b/terraform/shadow_components.go @@ -4,6 +4,7 @@ import ( "fmt" "sync" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/helper/shadow" ) @@ -31,14 +32,23 @@ func newShadowComponentFactory( // shadowComponentFactory is the shadow side. Any components created // with this factory are fake and will not cause real work to happen. +// +// Unlike other shadowers, the shadow component factory will allow the +// shadow to create _any_ component even if it is never requested on the +// real side. This is because errors will happen later downstream as function +// calls are made to the shadows that are never matched on the real side. type shadowComponentFactory struct { *shadowComponentFactoryShared Shadow bool // True if this should return the shadow + lock sync.Mutex } func (f *shadowComponentFactory) ResourceProvider( n, uid string) (ResourceProvider, error) { + f.lock.Lock() + defer f.lock.Unlock() + real, shadow, err := f.shadowComponentFactoryShared.ResourceProvider(n, uid) var result ResourceProvider = real if f.Shadow { @@ -50,6 +60,9 @@ func (f *shadowComponentFactory) ResourceProvider( func (f *shadowComponentFactory) ResourceProvisioner( n, uid string) (ResourceProvisioner, error) { + f.lock.Lock() + defer f.lock.Unlock() + real, shadow, err := f.shadowComponentFactoryShared.ResourceProvisioner(n, uid) var result ResourceProvisioner = real if f.Shadow { @@ -59,13 +72,58 @@ func (f *shadowComponentFactory) ResourceProvisioner( return result, err } +// CloseShadow is called when the _real_ side is complete. This will cause +// all future blocking operations to return immediately on the shadow to +// ensure the shadow also completes. +func (f *shadowComponentFactory) CloseShadow() 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 already closed, its an error + if shared.closed { + return fmt.Errorf("component factory shadow already closed") + } + + // 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.CloseShadow(); err != nil { + result = multierror.Append(result, err) + } + } + } + + // TODO: provisioners once they're done + + // Mark ourselves as closed + shared.closed = true + + return result +} + // shadowComponentFactoryShared is shared data between the two factories. +// +// It is NOT SAFE to run any function on this struct in parallel. Lock +// access to this struct. type shadowComponentFactoryShared struct { contextComponentFactory - providers shadow.KeyedValue - provisioners shadow.KeyedValue - lock sync.Mutex + closed bool + providers shadow.KeyedValue + providerKeys []string + provisioners shadow.KeyedValue + provisionerKeys []string } // shadowResourceProviderFactoryEntry is the entry that is stored in @@ -84,9 +142,6 @@ type shadowComponentFactoryProvisionerEntry struct { func (f *shadowComponentFactoryShared) ResourceProvider( n, uid string) (ResourceProvider, shadowResourceProvider, error) { - f.lock.Lock() - defer f.lock.Unlock() - // Determine if we already have a value raw, ok := f.providers.ValueOk(uid) if !ok { @@ -109,6 +164,7 @@ func (f *shadowComponentFactoryShared) ResourceProvider( // Store the value f.providers.SetValue(uid, &entry) + f.providerKeys = append(f.providerKeys, uid) raw = &entry } @@ -124,9 +180,6 @@ func (f *shadowComponentFactoryShared) ResourceProvider( func (f *shadowComponentFactoryShared) ResourceProvisioner( n, uid string) (ResourceProvisioner, ResourceProvisioner, error) { - f.lock.Lock() - defer f.lock.Unlock() - // Determine if we already have a value raw, ok := f.provisioners.ValueOk(uid) if !ok { @@ -150,6 +203,7 @@ func (f *shadowComponentFactoryShared) ResourceProvisioner( // Store the value f.provisioners.SetValue(uid, &entry) + f.provisionerKeys = append(f.provisionerKeys, uid) raw = &entry } diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index d73f737b4..60c6e1680 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -66,10 +66,10 @@ func newShadowContext(c *Context) (*Context, *Context, io.Closer) { // shadowContextCloser is the io.Closer returned by newShadowContext that // closes all the shadows and returns the results. type shadowContextCloser struct { - Components interface{} + Components *shadowComponentFactory } // Close closes the shadow context. func (c *shadowContextCloser) Close() error { - return nil + return c.Components.CloseShadow() } From d2fb630df81f1c7c49430915c204ffa54fcd6687 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 4 Oct 2016 20:14:45 -0700 Subject: [PATCH 30/64] helper/shadow: Value.Close --- helper/shadow/value.go | 28 ++++++++++++++++++++++++++++ helper/shadow/value_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/helper/shadow/value.go b/helper/shadow/value.go index bf30c12a8..2413335b8 100644 --- a/helper/shadow/value.go +++ b/helper/shadow/value.go @@ -1,12 +1,24 @@ package shadow import ( + "errors" "sync" ) +// ErrClosed is returned by any closed values. +// +// A "closed value" is when the shadow has been notified that the real +// side is complete and any blocking values will _never_ be satisfied +// in the future. In this case, this error is returned. If a value is already +// available, that is still returned. +var ErrClosed = errors.New("shadow closed") + // Value is a struct that coordinates a value between two // parallel routines. It is similar to atomic.Value except that when // Value is called if it isn't set it will wait for it. +// +// The Value can be closed with Close, which will cause any future +// blocking operations to return immediately with ErrClosed. type Value struct { lock sync.Mutex cond *sync.Cond @@ -14,6 +26,22 @@ type Value struct { valueSet bool } +// Close closes the value. This can never fail. For a definition of +// "close" see the struct docs. +func (w *Value) Close() error { + w.lock.Lock() + set := w.valueSet + w.lock.Unlock() + + // If we haven't set the value, set it + if !set { + w.SetValue(ErrClosed) + } + + // Done + return nil +} + // Value returns the value that was set. func (w *Value) Value() interface{} { w.lock.Lock() diff --git a/helper/shadow/value_test.go b/helper/shadow/value_test.go index 069b30852..ad8a64aa9 100644 --- a/helper/shadow/value_test.go +++ b/helper/shadow/value_test.go @@ -41,3 +41,32 @@ func TestValue(t *testing.T) { t.Fatalf("bad: %#v", val) } } + +func TestValueClose(t *testing.T) { + var v Value + + // Close + v.Close() + + // Verify + val := v.Value() + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} + +func TestValueClose_existing(t *testing.T) { + var v Value + + // Set the value + v.SetValue(42) + + // Close + v.Close() + + // Verify + val := v.Value() + if val != 42 { + t.Fatalf("bad: %#v", val) + } +} From 136ac4728de82531e2642c8ae75a4c6c248f2c62 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 4 Oct 2016 20:20:07 -0700 Subject: [PATCH 31/64] helper/shadow: KeyedValue.Close --- helper/shadow/keyed_value.go | 23 +++++++++++ helper/shadow/keyed_value_test.go | 65 +++++++++++++++++++++++++++++++ helper/shadow/value_test.go | 31 +++++++++++++++ 3 files changed, 119 insertions(+) diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index 9c6d576e9..c0f6b58f0 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -11,6 +11,24 @@ type KeyedValue struct { once sync.Once values map[string]interface{} waiters map[string]*Value + closed bool +} + +// Close closes the value. This can never fail. For a definition of +// "close" see the ErrClosed docs. +func (w *KeyedValue) Close() error { + w.lock.Lock() + defer w.lock.Unlock() + + // Set closed to true always + w.closed = true + + // For all waiters, complete with ErrClosed + for _, w := range w.waiters { + w.SetValue(ErrClosed) + } + + return nil } // Value returns the value that was set for the given key, or blocks @@ -62,6 +80,11 @@ func (w *KeyedValue) valueWaiter(k string) (interface{}, *Value) { return v, nil } + // If we're closed, return that + if w.closed { + return ErrClosed, nil + } + // No pending value, check for a waiter val := w.waiters[k] if val == nil { diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go index ff59fe8de..a141606a2 100644 --- a/helper/shadow/keyed_value_test.go +++ b/helper/shadow/keyed_value_test.go @@ -73,3 +73,68 @@ func TestKeyedValueOk(t *testing.T) { t.Fatalf("bad: %#v", val) } } + +func TestKeyedValueClose(t *testing.T) { + var v KeyedValue + + // Close + v.Close() + + // Try again + val, ok := v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} + +func TestKeyedValueClose_blocked(t *testing.T) { + var v KeyedValue + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Close + v.Close() + + // Verify + val := <-valueCh + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} + +func TestKeyedValueClose_existing(t *testing.T) { + var v KeyedValue + + // Set a value + v.SetValue("foo", "bar") + + // Close + v.Close() + + // Try again + val, ok := v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != "bar" { + t.Fatalf("bad: %#v", val) + } +} diff --git a/helper/shadow/value_test.go b/helper/shadow/value_test.go index ad8a64aa9..8bc957939 100644 --- a/helper/shadow/value_test.go +++ b/helper/shadow/value_test.go @@ -55,6 +55,37 @@ func TestValueClose(t *testing.T) { } } +func TestValueClose_blocked(t *testing.T) { + var v Value + + // Start trying to get the value + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value() + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set the value + v.Close() + val := <-valueCh + + // Verify + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } + + // We should be able to ask for the value again immediately + if val := v.Value(); val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} + func TestValueClose_existing(t *testing.T) { var v Value From 47f4343bf57e744e48713a4e24300f7b46ba68fa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 4 Oct 2016 20:30:28 -0700 Subject: [PATCH 32/64] helper/shadow: KeyedValue add test case to avoid panic --- helper/shadow/keyed_value.go | 2 +- helper/shadow/keyed_value_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index c0f6b58f0..ed26bb72b 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -60,7 +60,7 @@ func (w *KeyedValue) SetValue(k string, v interface{}) { // If we have a waiter, set it if val, ok := w.waiters[k]; ok { val.SetValue(v) - w.waiters[k] = nil + delete(w.waiters, k) } } diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go index a141606a2..92b26917e 100644 --- a/helper/shadow/keyed_value_test.go +++ b/helper/shadow/keyed_value_test.go @@ -138,3 +138,33 @@ func TestKeyedValueClose_existing(t *testing.T) { t.Fatalf("bad: %#v", val) } } + +func TestKeyedValueClose_existingBlocked(t *testing.T) { + var v KeyedValue + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value("foo") + }() + + // Wait + time.Sleep(10 * time.Millisecond) + + // Set a value + v.SetValue("foo", "bar") + + // Close + v.Close() + + // Try again + val, ok := v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != "bar" { + t.Fatalf("bad: %#v", val) + } +} From 62162427f41bb75809eeeef3a5bf9930faea271d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 4 Oct 2016 20:31:21 -0700 Subject: [PATCH 33/64] terraform: ResourceProvider (shadow) CloseShadow closes all shadow values --- terraform/shadow_resource_provider.go | 31 +++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 429f77fb7..b534b0d3e 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "io" "sync" "github.com/hashicorp/go-multierror" @@ -175,11 +176,33 @@ type shadowResourceProviderShared struct { Refresh shadow.KeyedValue } +func (p *shadowResourceProviderShared) Close() error { + closers := []io.Closer{ + &p.CloseErr, &p.Input, &p.Validate, + &p.Configure, &p.Apply, &p.Diff, + &p.Refresh, + } + + for _, c := range closers { + // This should never happen, but we don't panic because a panic + // could affect the real behavior of Terraform and a shadow should + // never be able to do that. + if err := c.Close(); err != nil { + return err + } + } + + return nil +} + func (p *shadowResourceProviderShadow) CloseShadow() error { - // For now, just return the error. What we need to do in the future - // is marked the provider as "closed" so that any subsequent calls - // will fail out. - return p.Error + result := p.Error + if err := p.Shared.Close(); err != nil { + result = multierror.Append(result, fmt.Errorf( + "close error: %s", err)) + } + + return result } func (p *shadowResourceProviderShadow) Resources() []ResourceType { From 184b4a8b099dd845db7e22636521af31f07d2c4b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 4 Oct 2016 20:52:14 -0700 Subject: [PATCH 34/64] terraform: context verifies real and shadow state/diff match --- terraform/context.go | 7 ++++++- terraform/shadow_context.go | 29 +++++++++++++++++++++++++++ terraform/shadow_resource_provider.go | 3 +++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 27aa71d43..3e869e43c 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -613,13 +613,13 @@ func (c *Context) walk( realCtx := c // If we have a shadow graph, walk that as well + var shadowCtx *Context var shadowCh chan error var shadowCloser io.Closer 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 // the results of the real. - var shadowCtx *Context realCtx, shadowCtx, shadowCloser = newShadowContext(c) // Build the graph walker for the shadow. @@ -657,6 +657,11 @@ func (c *Context) walk( c.shadowErr = multierror.Append(c.shadowErr, err) } + // Verify the contexts (compare) + if err := shadowContextVerify(realCtx, shadowCtx); err != nil { + c.shadowErr = multierror.Append(c.shadowErr, err) + } + if c.shadowErr == nil { log.Printf("[INFO] Shadow graph success!") } else { diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 60c6e1680..ba3b811c8 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -1,8 +1,11 @@ package terraform import ( + "fmt" "io" + "reflect" + "github.com/hashicorp/go-multierror" "github.com/mitchellh/copystructure" ) @@ -63,6 +66,32 @@ func newShadowContext(c *Context) (*Context, *Context, io.Closer) { } } +// shadowContextVerify takes the real and shadow context and verifies they +// have equal diffs and states. +func shadowContextVerify(real, shadow *Context) error { + var result error + + // Compare the states + if !real.state.Equal(shadow.state) { + result = multierror.Append(result, fmt.Errorf( + "Real and shadow states do not match! "+ + "Real state:\n\n%s\n\n"+ + "Shadow state:\n\n%s\n\n", + real.state, shadow.state)) + } + + // Compare the diffs + if !reflect.DeepEqual(real.diff, shadow.diff) { + result = multierror.Append(result, fmt.Errorf( + "Real and shadow diffs do not match! "+ + "Real diff:\n\n%s\n\n"+ + "Shadow diff:\n\n%s\n\n", + real.diff, shadow.diff)) + } + + return result +} + // shadowContextCloser is the io.Closer returned by newShadowContext that // closes all the shadows and returns the results. type shadowContextCloser struct { diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index b534b0d3e..176164996 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -167,6 +167,9 @@ type shadowResourceProviderShadow struct { } 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 From 0408c2dfb2e4fdcc0d06861ee179c15cdaaf3a65 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Oct 2016 12:54:53 -0700 Subject: [PATCH 35/64] helper/shadow: KeyedValue.WaitForChange --- helper/shadow/keyed_value.go | 20 +++++++++++ helper/shadow/keyed_value_test.go | 55 +++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index ed26bb72b..e685e6c3d 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -42,6 +42,26 @@ func (w *KeyedValue) Value(k string) interface{} { return val.Value() } +// WaitForChange waits for the value with the given key to be set again. +// If the key isn't set, it'll wait for an initial value. Note that while +// it is called "WaitForChange", the value isn't guaranteed to _change_; +// this will return when a SetValue is called for the given k. +func (w *KeyedValue) WaitForChange(k string) interface{} { + w.lock.Lock() + w.once.Do(w.init) + + // Check for an active waiter. If there isn't one, make it + val := w.waiters[k] + if val == nil { + val = new(Value) + w.waiters[k] = val + } + w.lock.Unlock() + + // And wait + return val.Value() +} + // ValueOk gets the value for the given key, returning immediately if the // value doesn't exist. The second return argument is true if the value exists. func (w *KeyedValue) ValueOk(k string) (interface{}, bool) { diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go index 92b26917e..56d368190 100644 --- a/helper/shadow/keyed_value_test.go +++ b/helper/shadow/keyed_value_test.go @@ -168,3 +168,58 @@ func TestKeyedValueClose_existingBlocked(t *testing.T) { t.Fatalf("bad: %#v", val) } } + +func TestKeyedValueWaitForChange(t *testing.T) { + var v KeyedValue + + // Set a value + v.SetValue("foo", 42) + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.WaitForChange("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set a new value + v.SetValue("foo", 84) + + // Verify + val := <-valueCh + if val != 84 { + t.Fatalf("bad: %#v", val) + } +} + +func TestKeyedValueWaitForChange_initial(t *testing.T) { + var v KeyedValue + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.WaitForChange("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set a new value + v.SetValue("foo", 84) + + // Verify + val := <-valueCh + if val != 84 { + t.Fatalf("bad: %#v", val) + } +} From c92ee5a8bda87bbb7158475d1b0afa4859d68c41 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Oct 2016 13:00:44 -0700 Subject: [PATCH 36/64] helper/shadow: KeyedValue.WaitForChange returns immediately if closed --- helper/shadow/keyed_value.go | 10 +++++++-- helper/shadow/keyed_value_test.go | 35 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index e685e6c3d..fe48cc761 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -24,8 +24,9 @@ func (w *KeyedValue) Close() error { w.closed = true // For all waiters, complete with ErrClosed - for _, w := range w.waiters { - w.SetValue(ErrClosed) + for k, val := range w.waiters { + val.SetValue(ErrClosed) + delete(w.waiters, k) } return nil @@ -50,6 +51,11 @@ func (w *KeyedValue) WaitForChange(k string) interface{} { w.lock.Lock() w.once.Do(w.init) + // If we're closed, we're closed + if w.closed { + return ErrClosed + } + // Check for an active waiter. If there isn't one, make it val := w.waiters[k] if val == nil { diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go index 56d368190..098c54a59 100644 --- a/helper/shadow/keyed_value_test.go +++ b/helper/shadow/keyed_value_test.go @@ -223,3 +223,38 @@ func TestKeyedValueWaitForChange_initial(t *testing.T) { t.Fatalf("bad: %#v", val) } } + +func TestKeyedValueWaitForChange_closed(t *testing.T) { + var v KeyedValue + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.WaitForChange("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Close + v.Close() + + // Verify + val := <-valueCh + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } + + // Set a value + v.SetValue("foo", 42) + + // Try again + val = v.WaitForChange("foo") + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} From 3edb8599b1eeaa6befc17f8ea1b33bafafcc9bbd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Oct 2016 13:39:02 -0700 Subject: [PATCH 37/64] 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) From 24456c042af85f3da3e638adc434e98e1386cb5b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Oct 2016 19:05:08 -0700 Subject: [PATCH 38/64] terraform: ResourceProvider (shadow) ValidateResource --- terraform/shadow_resource_provider.go | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 671b2f60e..9fb87e1d8 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "io" + "log" "sync" "github.com/hashicorp/go-multierror" @@ -102,6 +103,45 @@ func (p *shadowResourceProviderReal) Configure(c *ResourceConfig) error { return err } +func (p *shadowResourceProviderReal) ValidateResource( + t string, c *ResourceConfig) ([]string, []error) { + key := t + + // Real operation + warns, errs := p.ResourceProvider.ValidateResource(t, c) + + // Get the result + raw, ok := p.Shared.ValidateResource.ValueOk(key) + if !ok { + raw = new(shadowResourceProviderValidateResourceWrapper) + } + + wrapper, ok := raw.(*shadowResourceProviderValidateResourceWrapper) + if !ok { + // If this fails then we just continue with our day... the shadow + // will fail to but there isn't much we can do. + log.Printf( + "[ERROR] unknown value in ValidateResource shadow value: %#v", raw) + return warns, errs + } + + // Lock the wrapper for writing and record our call + wrapper.Lock() + defer wrapper.Unlock() + + wrapper.Calls = append(wrapper.Calls, &shadowResourceProviderValidateResource{ + Config: c, + Warns: warns, + Errors: errs, + }) + + // Set it + p.Shared.ValidateResource.SetValue(key, wrapper) + + // Return the result + return warns, errs +} + func (p *shadowResourceProviderReal) Apply( info *InstanceInfo, state *InstanceState, From a355d4ad5550c985ab16df5bf3943231c0ba6baf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 10:48:59 -0700 Subject: [PATCH 39/64] terraform: deposed should have a unique ID for instance info --- terraform/transform_deposed.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/transform_deposed.go b/terraform/transform_deposed.go index fa3143c3c..635fc99eb 100644 --- a/terraform/transform_deposed.go +++ b/terraform/transform_deposed.go @@ -72,7 +72,7 @@ func (n *graphNodeDeposedResource) EvalTree() EvalNode { seq := &EvalSequence{Nodes: make([]EvalNode, 0, 5)} // Build instance info - info := &InstanceInfo{Id: n.ResourceName, Type: n.ResourceType} + info := &InstanceInfo{Id: n.Name(), Type: n.ResourceType} seq.Nodes = append(seq.Nodes, &EvalInstanceInfo{Info: info}) // Refresh the resource From 61c789aace6e085b5ca1a4b0f949cf3a927273be Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 10:49:52 -0700 Subject: [PATCH 40/64] terraform: shadow graph runs in sequence after the real graph --- terraform/context.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 7b7f565a1..f25ba50ed 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -613,27 +613,12 @@ func (c *Context) walk( // If we have a shadow graph, walk that as well var shadowCtx *Context - var shadowCh chan error 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 // the results of the real. realCtx, shadowCtx, shadowCloser = newShadowContext(c) - - // Build the graph walker for the shadow. - shadowWalker := &ContextGraphWalker{ - Context: shadowCtx, - Operation: operation, - } - - // Kick off the shadow walk. This will block on any operations - // on the real walk so it is fine to start first. - shadowCh = make(chan error) - go func() { - log.Printf("[INFO] Starting shadow graph walk: %s", operation.String()) - shadowCh <- shadow.Walk(shadowWalker) - }() } // Build the real graph walker @@ -645,6 +630,20 @@ func (c *Context) walk( // If we have a shadow graph, wait for that to complete if shadowCloser != nil { + // Build the graph walker for the shadow. + shadowWalker := &ContextGraphWalker{ + Context: shadowCtx, + Operation: operation, + } + + // Kick off the shadow walk. This will block on any operations + // on the real walk so it is fine to start first. + shadowCh := make(chan error) + go func() { + log.Printf("[INFO] Starting shadow graph walk: %s", operation.String()) + shadowCh <- shadow.Walk(shadowWalker) + }() + // Notify the shadow that we're done if err := shadowCloser.CloseShadow(); err != nil { c.shadowErr = multierror.Append(c.shadowErr, err) From 5c1af55711bc29f55d0d8ff5d8194adf35df4d90 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 11:00:28 -0700 Subject: [PATCH 41/64] terraform: don't run the shadow graph on interrupt --- terraform/context.go | 18 ++++++++++++++++-- terraform/shadow_context.go | 2 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index f25ba50ed..af5f9e911 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -590,6 +590,9 @@ func (c *Context) acquireRun() chan<- struct{} { ch := make(chan struct{}) c.runCh = ch + // Reset the stop hook so we're not stopped + c.sh.Reset() + // Reset the shadow errors c.shadowErr = nil @@ -602,7 +605,6 @@ func (c *Context) releaseRun(ch chan<- struct{}) { close(ch) c.runCh = nil - c.sh.Reset() } func (c *Context) walk( @@ -628,7 +630,19 @@ func (c *Context) walk( // Walk the real graph, this will block until it completes realErr := graph.Walk(walker) - // If we have a shadow graph, wait for that to complete + // 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 + // isn't supported with shadows at the moment. + if shadowCloser != nil && c.sh.Stopped() { + // Ignore the error result, there is nothing we could care about + shadowCloser.CloseShadow() + + // Set it to nil so we don't do anything + shadowCloser = nil + } + + // If we have a shadow graph, wait for that to complete. if shadowCloser != nil { // Build the graph walker for the shadow. shadowWalker := &ContextGraphWalker{ diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 3d45e2cec..5eb31cf3d 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -39,7 +39,7 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) { components: componentsShadow, destroy: c.destroy, diff: c.diff.DeepCopy(), - hooks: nil, // TODO: do we need to copy? stop hook? + hooks: nil, module: c.module, state: c.state.DeepCopy(), targets: targetRaw.([]string), From 548a585762f732b2b29418ba8106ef67d61d12fb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 11:54:09 -0700 Subject: [PATCH 42/64] terraform: unique ID for destroying resources --- terraform/transform_resource.go | 1 + 1 file changed, 1 insertion(+) diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 23a23f3c4..7a7fc223f 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -862,6 +862,7 @@ func (n *graphNodeExpandedResourceDestroy) ConfigType() GraphNodeConfigType { // GraphNodeEvalable impl. func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode { info := n.instanceInfo() + info.Id += " (destroy)" var diffApply *InstanceDiff var provider ResourceProvider From 50e0647c533c8eab053a642a1e79a040739ed8d0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 12:15:41 -0700 Subject: [PATCH 43/64] helper/shadow: ComparedValue --- helper/shadow/compared_value.go | 128 +++++++++++++++++++ helper/shadow/compared_value_test.go | 178 +++++++++++++++++++++++++++ helper/shadow/keyed_value.go | 1 + 3 files changed, 307 insertions(+) create mode 100644 helper/shadow/compared_value.go create mode 100644 helper/shadow/compared_value_test.go diff --git a/helper/shadow/compared_value.go b/helper/shadow/compared_value.go new file mode 100644 index 000000000..4223e9255 --- /dev/null +++ b/helper/shadow/compared_value.go @@ -0,0 +1,128 @@ +package shadow + +import ( + "sync" +) + +// ComparedValue is a struct that finds a value by comparing some key +// to the list of stored values. This is useful when there is no easy +// uniquely identifying key that works in a map (for that, use KeyedValue). +// +// ComparedValue is very expensive, relative to other Value types. Try to +// limit the number of values stored in a ComparedValue by potentially +// nesting it within a KeyedValue (a keyed value points to a compared value, +// for example). +type ComparedValue struct { + // Func is a function that is given the lookup key and a single + // stored value. If it matches, it returns true. + Func func(k, v interface{}) bool + + lock sync.Mutex + once sync.Once + closed bool + values []interface{} + waiters map[interface{}]*Value +} + +// Close closes the value. This can never fail. For a definition of +// "close" see the ErrClosed docs. +func (w *ComparedValue) Close() error { + w.lock.Lock() + defer w.lock.Unlock() + + // Set closed to true always + w.closed = true + + // For all waiters, complete with ErrClosed + for k, val := range w.waiters { + val.SetValue(ErrClosed) + delete(w.waiters, k) + } + + return nil +} + +// Value returns the value that was set for the given key, or blocks +// until one is available. +func (w *ComparedValue) Value(k interface{}) interface{} { + v, val := w.valueWaiter(k) + if val == nil { + return v + } + + return val.Value() +} + +// ValueOk gets the value for the given key, returning immediately if the +// value doesn't exist. The second return argument is true if the value exists. +func (w *ComparedValue) ValueOk(k interface{}) (interface{}, bool) { + v, val := w.valueWaiter(k) + return v, val == nil +} + +func (w *ComparedValue) SetValue(v interface{}) { + w.lock.Lock() + defer w.lock.Unlock() + w.once.Do(w.init) + + // Check if we already have this exact value (by simply comparing + // with == directly). If we do, then we don't insert it again. + found := false + for _, v2 := range w.values { + if v == v2 { + found = true + break + } + } + + if !found { + // Set the value, always + w.values = append(w.values, v) + } + + // Go through the waiters + for k, val := range w.waiters { + if w.Func(k, v) { + val.SetValue(v) + delete(w.waiters, k) + } + } +} + +func (w *ComparedValue) valueWaiter(k interface{}) (interface{}, *Value) { + w.lock.Lock() + w.once.Do(w.init) + + // Look for a pre-existing value + for _, v := range w.values { + if w.Func(k, v) { + w.lock.Unlock() + return v, nil + } + } + + // If we're closed, return that + if w.closed { + w.lock.Unlock() + return ErrClosed, nil + } + + // Pre-existing value doesn't exist, create a waiter + val := w.waiters[k] + if val == nil { + val = new(Value) + w.waiters[k] = val + } + w.lock.Unlock() + + // Return the waiter + return nil, val +} + +// Must be called with w.lock held. +func (w *ComparedValue) init() { + w.waiters = make(map[interface{}]*Value) + if w.Func == nil { + w.Func = func(k, v interface{}) bool { return k == v } + } +} diff --git a/helper/shadow/compared_value_test.go b/helper/shadow/compared_value_test.go new file mode 100644 index 000000000..2d98d444d --- /dev/null +++ b/helper/shadow/compared_value_test.go @@ -0,0 +1,178 @@ +package shadow + +import ( + "testing" + "time" +) + +func TestComparedValue(t *testing.T) { + v := &ComparedValue{ + Func: func(k, v interface{}) bool { return k == v }, + } + + // Start trying to get the value + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Set the value + v.SetValue("foo") + val := <-valueCh + + // Verify + if val != "foo" { + t.Fatalf("bad: %#v", val) + } + + // We should get the next value + val = v.Value("foo") + if val != "foo" { + t.Fatalf("bad: %#v", val) + } +} + +func TestComparedValue_setFirst(t *testing.T) { + v := &ComparedValue{ + Func: func(k, v interface{}) bool { return k == v }, + } + + // Set the value + v.SetValue("foo") + val := v.Value("foo") + + // Verify + if val != "foo" { + t.Fatalf("bad: %#v", val) + } +} + +func TestComparedValueOk(t *testing.T) { + v := &ComparedValue{ + Func: func(k, v interface{}) bool { return k == v }, + } + + // Try + val, ok := v.ValueOk("foo") + if ok { + t.Fatal("should not be ok") + } + + // Set + v.SetValue("foo") + + // Try again + val, ok = v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != "foo" { + t.Fatalf("bad: %#v", val) + } +} + +func TestComparedValueClose(t *testing.T) { + v := &ComparedValue{ + Func: func(k, v interface{}) bool { return k == v }, + } + + // Close + v.Close() + + // Try again + val, ok := v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} + +func TestComparedValueClose_blocked(t *testing.T) { + var v ComparedValue + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value("foo") + }() + + // We should not get the value + select { + case <-valueCh: + t.Fatal("shouldn't receive value") + case <-time.After(10 * time.Millisecond): + } + + // Close + v.Close() + + // Verify + val := <-valueCh + if val != ErrClosed { + t.Fatalf("bad: %#v", val) + } +} + +func TestComparedValueClose_existing(t *testing.T) { + var v ComparedValue + + // Set a value + v.SetValue("foo") + + // Close + v.Close() + + // Try again + val, ok := v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != "foo" { + t.Fatalf("bad: %#v", val) + } +} + +func TestComparedValueClose_existingBlocked(t *testing.T) { + var v ComparedValue + + // Start reading this should be blocking + valueCh := make(chan interface{}) + go func() { + valueCh <- v.Value("foo") + }() + + // Wait + time.Sleep(10 * time.Millisecond) + + // Set a value + v.SetValue("foo") + + // Close + v.Close() + + // Try again + val, ok := v.ValueOk("foo") + if !ok { + t.Fatal("should be ok") + } + + // Verify + if val != "foo" { + t.Fatalf("bad: %#v", val) + } +} diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index fe48cc761..e31920fe8 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -108,6 +108,7 @@ func (w *KeyedValue) valueWaiter(k string) (interface{}, *Value) { // If we're closed, return that if w.closed { + w.lock.Unlock() return ErrClosed, nil } From 4de803622df6c66804aa912d5d4d0a673e3c9ff0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 14:26:51 -0700 Subject: [PATCH 44/64] terraform: ResourceProvisioner shadow --- terraform/shadow_resource_provisioner.go | 271 ++++++++++++++++++ terraform/shadow_resource_provisioner_test.go | 178 ++++++++++++ 2 files changed, 449 insertions(+) create mode 100644 terraform/shadow_resource_provisioner.go create mode 100644 terraform/shadow_resource_provisioner_test.go diff --git a/terraform/shadow_resource_provisioner.go b/terraform/shadow_resource_provisioner.go new file mode 100644 index 000000000..6e405c09d --- /dev/null +++ b/terraform/shadow_resource_provisioner.go @@ -0,0 +1,271 @@ +package terraform + +import ( + "fmt" + "io" + "log" + "sync" + + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform/helper/shadow" +) + +// shadowResourceProvisioner implements ResourceProvisioner for the shadow +// eval context defined in eval_context_shadow.go. +// +// This is used to verify behavior with a real provisioner. This shouldn't +// be used directly. +type shadowResourceProvisioner interface { + ResourceProvisioner + Shadow +} + +// newShadowResourceProvisioner creates a new shadowed ResourceProvisioner. +func newShadowResourceProvisioner( + p ResourceProvisioner) (ResourceProvisioner, shadowResourceProvisioner) { + // Create the shared data + shared := shadowResourceProvisionerShared{ + Validate: shadow.ComparedValue{ + Func: shadowResourceProvisionerValidateCompare, + }, + } + + // Create the real provisioner that does actual work + real := &shadowResourceProvisionerReal{ + ResourceProvisioner: p, + Shared: &shared, + } + + // Create the shadow that watches the real value + shadow := &shadowResourceProvisionerShadow{ + Shared: &shared, + } + + return real, shadow +} + +// shadowResourceProvisionerReal is the real resource provisioner. Function calls +// to this will perform real work. This records the parameters and return +// values and call order for the shadow to reproduce. +type shadowResourceProvisionerReal struct { + ResourceProvisioner + + Shared *shadowResourceProvisionerShared +} + +func (p *shadowResourceProvisionerReal) Close() error { + var result error + if c, ok := p.ResourceProvisioner.(ResourceProvisionerCloser); ok { + result = c.Close() + } + + p.Shared.CloseErr.SetValue(result) + return result +} + +func (p *shadowResourceProvisionerReal) Validate(c *ResourceConfig) ([]string, []error) { + warns, errs := p.ResourceProvisioner.Validate(c) + p.Shared.Validate.SetValue(&shadowResourceProvisionerValidate{ + Config: c, + ResultWarn: warns, + ResultErr: errs, + }) + + return warns, errs +} + +func (p *shadowResourceProvisionerReal) Apply( + output UIOutput, s *InstanceState, c *ResourceConfig) error { + err := p.ResourceProvisioner.Apply(output, s, c) + + // Write the result, grab a lock for writing. This should nver + // block long since the operations below don't block. + p.Shared.ApplyLock.Lock() + defer p.Shared.ApplyLock.Unlock() + + key := s.ID + raw, ok := p.Shared.Apply.ValueOk(key) + if !ok { + // Setup a new value + raw = &shadow.ComparedValue{ + Func: shadowResourceProvisionerApplyCompare, + } + + // Set it + p.Shared.Apply.SetValue(key, raw) + } + + compareVal, ok := raw.(*shadow.ComparedValue) + if !ok { + // Just log and return so that we don't cause the real side + // any side effects. + log.Printf("[ERROR] unknown value in 'apply': %#v", raw) + return err + } + + // Write the resulting value + compareVal.SetValue(&shadowResourceProvisionerApply{ + Config: c, + ResultErr: err, + }) + + return err +} + +// 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. +type shadowResourceProvisionerShadow struct { + Shared *shadowResourceProvisionerShared + + Error error // Error is the list of errors from the shadow + ErrorLock sync.Mutex +} + +type shadowResourceProvisionerShared 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 + Validate shadow.ComparedValue + Apply shadow.KeyedValue + ApplyLock sync.Mutex // For writing only +} + +func (p *shadowResourceProvisionerShared) Close() error { + closers := []io.Closer{ + &p.CloseErr, + } + + for _, c := range closers { + // This should never happen, but we don't panic because a panic + // could affect the real behavior of Terraform and a shadow should + // never be able to do that. + if err := c.Close(); err != nil { + return err + } + } + + return nil +} + +func (p *shadowResourceProvisionerShadow) CloseShadow() error { + err := p.Shared.Close() + if err != nil { + err = fmt.Errorf("close error: %s", err) + } + + return err +} + +func (p *shadowResourceProvisionerShadow) ShadowError() error { + return p.Error +} + +func (p *shadowResourceProvisionerShadow) Close() error { + v := p.Shared.CloseErr.Value() + if v == nil { + return nil + } + + return v.(error) +} + +func (p *shadowResourceProvisionerShadow) Validate(c *ResourceConfig) ([]string, []error) { + // Get the result of the validate call + raw := p.Shared.Validate.Value(c) + if raw == nil { + return nil, nil + } + + result, ok := raw.(*shadowResourceProvisionerValidate) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'validate' shadow value: %#v", raw)) + return nil, nil + } + + // We don't need to compare configurations because we key on the + // configuration so just return right away. + return result.ResultWarn, result.ResultErr +} + +func (p *shadowResourceProvisionerShadow) Apply( + output UIOutput, s *InstanceState, c *ResourceConfig) error { + // Get the value based on the key + key := s.ID + raw := p.Shared.Apply.Value(key) + if raw == nil { + return nil + } + + compareVal, ok := raw.(*shadow.ComparedValue) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'apply' shadow value: %#v", raw)) + return nil + } + + // With the compared value, we compare against our config + raw = compareVal.Value(c) + if raw == nil { + return nil + } + + result, ok := raw.(*shadowResourceProvisionerApply) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'apply' shadow value: %#v", raw)) + return nil + } + + return result.ResultErr +} + +// The structs for the various function calls are put below. These structs +// are used to carry call information across the real/shadow boundaries. + +type shadowResourceProvisionerValidate struct { + Config *ResourceConfig + ResultWarn []string + ResultErr []error +} + +type shadowResourceProvisionerApply struct { + Config *ResourceConfig + ResultErr error +} + +func shadowResourceProvisionerValidateCompare(k, v interface{}) bool { + c, ok := k.(*ResourceConfig) + if !ok { + return false + } + + result, ok := v.(*shadowResourceProvisionerValidate) + if !ok { + return false + } + + return c.Equal(result.Config) +} + +func shadowResourceProvisionerApplyCompare(k, v interface{}) bool { + c, ok := k.(*ResourceConfig) + if !ok { + return false + } + + result, ok := v.(*shadowResourceProvisionerApply) + if !ok { + return false + } + + return c.Equal(result.Config) +} diff --git a/terraform/shadow_resource_provisioner_test.go b/terraform/shadow_resource_provisioner_test.go new file mode 100644 index 000000000..7e37d264a --- /dev/null +++ b/terraform/shadow_resource_provisioner_test.go @@ -0,0 +1,178 @@ +package terraform + +import ( + "errors" + "fmt" + "reflect" + "testing" + "time" +) + +func TestShadowResourceProvisioner_impl(t *testing.T) { + var _ Shadow = new(shadowResourceProvisionerShadow) +} + +func TestShadowResourceProvisionerValidate(t *testing.T) { + mock := new(MockResourceProvisioner) + real, shadow := newShadowResourceProvisioner(mock) + + // Test values + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + returnWarns := []string{"foo"} + returnErrs := []error{fmt.Errorf("bar")} + + // Configure the mock + mock.ValidateReturnWarns = returnWarns + mock.ValidateReturnErrors = returnErrs + + // Verify that it blocks until the real func is called + var warns []string + var errs []error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + warns, errs = shadow.Validate(config) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realWarns, realErrs := real.Validate(config) + if !reflect.DeepEqual(realWarns, returnWarns) { + t.Fatalf("bad: %#v", realWarns) + } + if !reflect.DeepEqual(realErrs, returnErrs) { + t.Fatalf("bad: %#v", realWarns) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !reflect.DeepEqual(warns, returnWarns) { + t.Fatalf("bad: %#v", warns) + } + if !reflect.DeepEqual(errs, returnErrs) { + t.Fatalf("bad: %#v", errs) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowResourceProvisionerValidate_diff(t *testing.T) { + mock := new(MockResourceProvisioner) + real, shadow := newShadowResourceProvisioner(mock) + + // Test values + config := testResourceConfig(t, map[string]interface{}{ + "foo": "bar", + }) + returnWarns := []string{"foo"} + returnErrs := []error{fmt.Errorf("bar")} + + // Configure the mock + mock.ValidateReturnWarns = returnWarns + mock.ValidateReturnErrors = returnErrs + + // Run a real validation with a config + real.Validate(testResourceConfig(t, map[string]interface{}{"bar": "baz"})) + + // Verify that it blocks until the real func is called + var warns []string + var errs []error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + warns, errs = shadow.Validate(config) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realWarns, realErrs := real.Validate(config) + if !reflect.DeepEqual(realWarns, returnWarns) { + t.Fatalf("bad: %#v", realWarns) + } + if !reflect.DeepEqual(realErrs, returnErrs) { + t.Fatalf("bad: %#v", realWarns) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if !reflect.DeepEqual(warns, returnWarns) { + t.Fatalf("bad: %#v", warns) + } + if !reflect.DeepEqual(errs, returnErrs) { + t.Fatalf("bad: %#v", errs) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowResourceProvisionerApply(t *testing.T) { + mock := new(MockResourceProvisioner) + real, shadow := newShadowResourceProvisioner(mock) + + // Test values + output := new(MockUIOutput) + state := &InstanceState{ID: "foo"} + config := testResourceConfig(t, map[string]interface{}{"foo": "bar"}) + mockReturn := errors.New("err") + + // Configure the mock + mock.ApplyReturnError = mockReturn + + // Verify that it blocks until the real func is called + var err error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + err = shadow.Apply(output, state, config) + }() + + select { + case <-doneCh: + t.Fatal("should block until finished") + case <-time.After(10 * time.Millisecond): + } + + // Call the real func + realErr := real.Apply(output, state, config) + if realErr != mockReturn { + t.Fatalf("bad: %#v", realErr) + } + + // The shadow should finish now + <-doneCh + + // Verify the shadow returned the same values + if err != mockReturn { + t.Errorf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } + if err := shadow.ShadowError(); err != nil { + t.Fatalf("bad: %s", err) + } +} From fdeb4656c96aaac46e15273fbf69a7350696ff21 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Oct 2016 14:56:02 -0700 Subject: [PATCH 45/64] terraform: deep copy shadow arguments to avoid state modifications The arguments passed into Apply, Refresh, Diff could be modified which caused the shadow comparison later to cause errors. Also, the result should be deep copied so that it isn't modified. --- terraform/diff.go | 10 +++ terraform/shadow_components.go | 25 ++++-- terraform/shadow_resource_provider.go | 26 +++++-- terraform/shadow_resource_provider_test.go | 90 ++++++++++++++++++++++ 4 files changed, 138 insertions(+), 13 deletions(-) diff --git a/terraform/diff.go b/terraform/diff.go index 33d27f714..4b810092b 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -381,6 +381,16 @@ func (d *InstanceDiff) Empty() bool { return !d.Destroy && !d.DestroyTainted && len(d.Attributes) == 0 } +// DeepCopy performs a deep copy of all parts of the InstanceDiff +func (d *InstanceDiff) DeepCopy() *InstanceDiff { + copy, err := copystructure.Config{Lock: true}.Copy(d) + if err != nil { + panic(err) + } + + return copy.(*InstanceDiff) +} + func (d *InstanceDiff) GoString() string { return fmt.Sprintf("*%#v", InstanceDiff{ Attributes: d.Attributes, diff --git a/terraform/shadow_components.go b/terraform/shadow_components.go index 84cf35952..141493df2 100644 --- a/terraform/shadow_components.go +++ b/terraform/shadow_components.go @@ -104,7 +104,14 @@ func (f *shadowComponentFactory) CloseShadow() error { } } - // TODO: provisioners once they're done + for _, n := range shared.provisionerKeys { + _, shadow, err := shared.ResourceProvisioner(n, n) + if err == nil && shadow != nil { + if err := shadow.CloseShadow(); err != nil { + result = multierror.Append(result, err) + } + } + } // Mark ourselves as closed shared.closed = true @@ -141,6 +148,15 @@ func (f *shadowComponentFactory) ShadowError() error { } } + for _, n := range shared.provisionerKeys { + _, shadow, err := shared.ResourceProvisioner(n, n) + if err == nil && shadow != nil { + if err := shadow.ShadowError(); err != nil { + result = multierror.Append(result, err) + } + } + } + return result } @@ -168,7 +184,7 @@ type shadowComponentFactoryProviderEntry struct { type shadowComponentFactoryProvisionerEntry struct { Real ResourceProvisioner - Shadow ResourceProvisioner + Shadow shadowResourceProvisioner Err error } @@ -211,7 +227,7 @@ func (f *shadowComponentFactoryShared) ResourceProvider( } func (f *shadowComponentFactoryShared) ResourceProvisioner( - n, uid string) (ResourceProvisioner, ResourceProvisioner, error) { + n, uid string) (ResourceProvisioner, shadowResourceProvisioner, error) { // Determine if we already have a value raw, ok := f.provisioners.ValueOk(uid) if !ok { @@ -227,8 +243,7 @@ func (f *shadowComponentFactoryShared) ResourceProvisioner( if p != nil { // For now, just create a mock since we don't support provisioners yet - real := p - shadow := new(MockResourceProvisioner) + real, shadow := newShadowResourceProvisioner(p) entry.Real = real entry.Shadow = shadow } diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 9fb87e1d8..b6713ce8f 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -146,11 +146,15 @@ func (p *shadowResourceProviderReal) Apply( info *InstanceInfo, state *InstanceState, diff *InstanceDiff) (*InstanceState, error) { + // Thse have to be copied before the call since call can modify + stateCopy := state.DeepCopy() + diffCopy := diff.DeepCopy() + result, err := p.ResourceProvider.Apply(info, state, diff) p.Shared.Apply.SetValue(info.HumanId(), &shadowResourceProviderApply{ - State: state, - Diff: diff, - Result: result, + State: stateCopy, + Diff: diffCopy, + Result: result.DeepCopy(), ResultErr: err, }) @@ -161,11 +165,14 @@ func (p *shadowResourceProviderReal) Diff( info *InstanceInfo, state *InstanceState, desired *ResourceConfig) (*InstanceDiff, error) { + // Thse have to be copied before the call since call can modify + stateCopy := state.DeepCopy() + result, err := p.ResourceProvider.Diff(info, state, desired) p.Shared.Diff.SetValue(info.HumanId(), &shadowResourceProviderDiff{ - State: state, + State: stateCopy, Desired: desired, - Result: result, + Result: result.DeepCopy(), ResultErr: err, }) @@ -175,10 +182,13 @@ func (p *shadowResourceProviderReal) Diff( func (p *shadowResourceProviderReal) Refresh( info *InstanceInfo, state *InstanceState) (*InstanceState, error) { + // Thse have to be copied before the call since call can modify + stateCopy := state.DeepCopy() + result, err := p.ResourceProvider.Refresh(info, state) p.Shared.Refresh.SetValue(info.HumanId(), &shadowResourceProviderRefresh{ - State: state, - Result: result, + State: stateCopy, + Result: result.DeepCopy(), ResultErr: err, }) @@ -430,7 +440,7 @@ func (p *shadowResourceProviderShadow) Apply( if !state.Equal(result.State) { p.ErrorLock.Lock() p.Error = multierror.Append(p.Error, fmt.Errorf( - "State had unequal states (real, then shadow):\n\n%#v\n\n%#v", + "Apply: state had unequal states (real, then shadow):\n\n%#v\n\n%#v", result.State, state)) p.ErrorLock.Unlock() } diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 933f212b1..1b04184de 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -324,6 +324,96 @@ func TestShadowResourceProviderApply(t *testing.T) { } } +func TestShadowResourceProviderApply_modifyDiff(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + info := &InstanceInfo{Id: "foo"} + state := &InstanceState{ID: "foo"} + diff := &InstanceDiff{} + mockResult := &InstanceState{ID: "foo"} + + // Configure the mock + mock.ApplyFn = func( + info *InstanceInfo, + s *InstanceState, d *InstanceDiff) (*InstanceState, error) { + d.Destroy = true + return s, nil + } + + // Call the real func + realResult, realErr := real.Apply(info, state.DeepCopy(), diff.DeepCopy()) + if !realResult.Equal(mockResult) { + t.Fatalf("bad: %#v", realResult) + } + if realErr != nil { + t.Fatalf("bad: %#v", realErr) + } + + // Verify the shadow returned the same values + result, err := shadow.Apply(info, state.DeepCopy(), diff.DeepCopy()) + if !result.Equal(mockResult) { + t.Fatalf("bad: %#v", result) + } + if err != nil { + t.Fatalf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } + if err := shadow.ShadowError(); err != nil { + t.Fatalf("bad: %s", err) + } +} + +func TestShadowResourceProviderApply_modifyState(t *testing.T) { + mock := new(MockResourceProvider) + real, shadow := newShadowResourceProvider(mock) + + // Test values + info := &InstanceInfo{Id: "foo"} + state := &InstanceState{ID: ""} + diff := &InstanceDiff{} + mockResult := &InstanceState{ID: "foo"} + + // Configure the mock + mock.ApplyFn = func( + info *InstanceInfo, + s *InstanceState, d *InstanceDiff) (*InstanceState, error) { + s.ID = "foo" + return s, nil + } + + // Call the real func + realResult, realErr := real.Apply(info, state.DeepCopy(), diff) + if !realResult.Equal(mockResult) { + t.Fatalf("bad: %#v", realResult) + } + if realErr != nil { + t.Fatalf("bad: %#v", realErr) + } + + // Verify the shadow returned the same values + result, err := shadow.Apply(info, state.DeepCopy(), diff) + if !result.Equal(mockResult) { + t.Fatalf("bad: %#v", result) + } + if err != nil { + t.Fatalf("bad: %#v", err) + } + + // Verify we have no errors + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } + if err := shadow.ShadowError(); err != nil { + t.Fatalf("bad: %s", err) + } +} + func TestShadowResourceProviderDiff(t *testing.T) { mock := new(MockResourceProvider) real, shadow := newShadowResourceProvider(mock) From a014b098b00dbf29922ab2d1b56988b665681710 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 8 Oct 2016 16:51:41 +0800 Subject: [PATCH 46/64] terraform: copy the provider input configs for the shadow context --- terraform/shadow_context.go | 8 +++++++- terraform/shadow_resource_provider.go | 8 ++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 5eb31cf3d..eba8640fb 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -31,6 +31,12 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) { panic(err) } + // Copy the provider inputs + providerInputRaw, err := copystructure.Copy(c.providerInputConfig) + if err != nil { + panic(err) + } + // The factories componentsReal, componentsShadow := newShadowComponentFactory(c.components) @@ -50,7 +56,7 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) { // a ton since we're doing far less compared to the real side // and our operations are MUCH faster. parallelSem: NewSemaphore(4), - providerInputConfig: make(map[string]map[string]interface{}), + providerInputConfig: providerInputRaw.(map[string]map[string]interface{}), } // Create the real context. This is effectively just a copy of diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index b6713ce8f..0cf1813a5 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -72,9 +72,11 @@ func (p *shadowResourceProviderReal) Close() error { func (p *shadowResourceProviderReal) Input( input UIInput, c *ResourceConfig) (*ResourceConfig, error) { + cCopy := c.DeepCopy() + result, err := p.ResourceProvider.Input(input, c) p.Shared.Input.SetValue(&shadowResourceProviderInput{ - Config: c.DeepCopy(), + Config: cCopy, Result: result.DeepCopy(), ResultErr: err, }) @@ -94,9 +96,11 @@ func (p *shadowResourceProviderReal) Validate(c *ResourceConfig) ([]string, []er } func (p *shadowResourceProviderReal) Configure(c *ResourceConfig) error { + cCopy := c.DeepCopy() + err := p.ResourceProvider.Configure(c) p.Shared.Configure.SetValue(&shadowResourceProviderConfigure{ - Config: c.DeepCopy(), + Config: cCopy, Result: err, }) From d30cfef4d2d5c1f34ed6fbe9898beac696c0cbb3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 8 Oct 2016 16:54:20 +0800 Subject: [PATCH 47/64] terraform: remove shadow eval context since we're not shadowing that We allow the built in context to work as expected and shadow just the components now. This is better since it allows us to use much more of the REAL structures. --- terraform/shadow_eval_context.go | 312 -------------------------- terraform/shadow_eval_context_test.go | 257 --------------------- 2 files changed, 569 deletions(-) delete mode 100644 terraform/shadow_eval_context.go delete mode 100644 terraform/shadow_eval_context_test.go diff --git a/terraform/shadow_eval_context.go b/terraform/shadow_eval_context.go deleted file mode 100644 index 0c962c433..000000000 --- a/terraform/shadow_eval_context.go +++ /dev/null @@ -1,312 +0,0 @@ -package terraform - -import ( - "errors" - "fmt" - "sync" - - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/terraform/config" - "github.com/hashicorp/terraform/helper/shadow" -) - -// ShadowEvalContext is an EvalContext that is used to "shadow" a real -// eval context for comparing whether two separate graph executions result -// in the same output. -// -// This eval context will never communicate with a real provider and will -// never modify real state. -type ShadowEvalContext interface { - EvalContext - - // Close should be called when the _real_ EvalContext operations - // are complete. This will immediately end any blocks calls and record - // any errors. - // - // The returned error is the result of the shadow run. If it is nil, - // then the shadow run seemingly completed successfully. You should - // still compare the resulting states, diffs from both the real and shadow - // contexts to verify equivalent end state. - // - // If the error is non-nil, then an error occurred during the execution - // itself. In this scenario, you should not compare diffs/states since - // they can't be considered accurate since operations during execution - // failed. - CloseShadow() error -} - -// NewShadowEvalContext creates a new shadowed EvalContext. This returns -// the real EvalContext that should be used with the real evaluation and -// will communicate with real providers and write real state as well as -// the ShadowEvalContext that should be used with the test graph. -// -// This should be called before the ctx is ever used in order to ensure -// a consistent shadow state. -func NewShadowEvalContext(ctx EvalContext) (EvalContext, ShadowEvalContext) { - var shared shadowEvalContextShared - real := &shadowEvalContextReal{ - EvalContext: ctx, - Shared: &shared, - } - - // Copy the diff. We do this using some weird scoping so that the - // "diff" (real) value never leaks out and can be used. - var diffCopy *Diff - { - diff, lock := ctx.Diff() - if lock != nil { - lock.RLock() - diffCopy = diff - // TODO: diffCopy = diff.DeepCopy() - lock.RUnlock() - } - } - - // Copy the state. We do this using some weird scoping so that the - // "state" (real) value never leaks out and can be used. - var stateCopy *State - { - state, lock := ctx.State() - if lock != nil { - lock.RLock() - stateCopy = state.DeepCopy() - lock.RUnlock() - } - } - - // Build the shadow copy. For safety, we don't even give the shadow - // copy a reference to the real context. This means that it would be - // very difficult (impossible without some real obvious mistakes) for - // the shadow context to do "real" work. - shadow := &shadowEvalContextShadow{ - Shared: &shared, - - PathValue: ctx.Path(), - StateValue: stateCopy, - StateLock: new(sync.RWMutex), - DiffValue: diffCopy, - DiffLock: new(sync.RWMutex), - } - - return real, shadow -} - -var ( - // errShadow is the error returned by the shadow context when - // things go wrong. This should be ignored and the error result from - // Close should be checked instead since that'll contain more detailed - // error. - errShadow = errors.New("shadow error") -) - -// shadowEvalContextReal is the EvalContext that does real work. -type shadowEvalContextReal struct { - EvalContext - - Shared *shadowEvalContextShared -} - -func (c *shadowEvalContextReal) InitProvider(n string) (ResourceProvider, error) { - // Initialize the real provider - p, err := c.EvalContext.InitProvider(n) - - // Create the shadow - var real ResourceProvider - var shadow shadowResourceProvider - if err == nil { - real, shadow = newShadowResourceProvider(p) - } - - // Store the result - c.Shared.Providers.SetValue(n, &shadowEvalContextInitProvider{ - Shadow: shadow, - ResultErr: err, - }) - - return real, err -} - -// shadowEvalContextShadow is the EvalContext that shadows the real one -// and leans on that for data. -type shadowEvalContextShadow struct { - Shared *shadowEvalContextShared - - PathValue []string - Providers map[string]ResourceProvider - DiffValue *Diff - DiffLock *sync.RWMutex - StateValue *State - StateLock *sync.RWMutex - - // The collection of errors that were found during the shadow run - Error error - ErrorLock sync.Mutex - - // Fields relating to closing the context. Closing signals that - // the execution of the real context completed. - closeLock sync.Mutex - closed bool - closeCh chan struct{} -} - -// Shared is the shared state between the shadow and real contexts when -// a shadow context is active. This is used by the real context to setup -// some state, trigger condition variables, etc. -type shadowEvalContextShared struct { - Providers shadow.KeyedValue -} - -func (c *shadowEvalContextShadow) CloseShadow() error { - // TODO: somehow shut this thing down - return c.Error -} - -func (c *shadowEvalContextShadow) Path() []string { - return c.PathValue -} - -func (c *shadowEvalContextShadow) Hook(f func(Hook) (HookAction, error)) error { - // Don't do anything on hooks. Mission critical behavior should not - // depend on hooks and at the time of writing it does not depend on - // hooks. In the future we could also test hooks but not now. - return nil -} - -func (c *shadowEvalContextShadow) InitProvider(n string) (ResourceProvider, error) { - // Wait for the provider value - raw := c.Shared.Providers.Value(n) - if raw == nil { - return nil, c.err(fmt.Errorf( - "Unknown 'InitProvider' call for %q", n)) - } - - result, ok := raw.(*shadowEvalContextInitProvider) - if !ok { - return nil, c.err(fmt.Errorf( - "Unknown 'InitProvider' shadow value: %#v", raw)) - } - - result.Lock() - defer result.Unlock() - - if result.Init { - // Record the error but continue... - c.err(fmt.Errorf( - "InitProvider: provider %q already initialized", n)) - } - - result.Init = true - return result.Shadow, result.ResultErr -} - -func (c *shadowEvalContextShadow) Provider(n string) ResourceProvider { - // Wait for the provider value - raw := c.Shared.Providers.Value(n) - if raw == nil { - c.err(fmt.Errorf( - "Unknown 'Provider' call for %q", n)) - return nil - } - - result, ok := raw.(*shadowEvalContextInitProvider) - if !ok { - c.err(fmt.Errorf( - "Unknown 'Provider' shadow value: %#v", raw)) - return nil - } - - result.Lock() - defer result.Unlock() - - if !result.Init { - // Record the error but continue... - c.err(fmt.Errorf( - "Provider: provider %q requested but not initialized", n)) - } - - return result.Shadow -} - -func (c *shadowEvalContextShadow) CloseProvider(n string) error { - // Wait for the provider value - raw, ok := c.Shared.Providers.ValueOk(n) - if !ok { - c.err(fmt.Errorf( - "CloseProvider called for uninitialized provider %q", n)) - return nil - } - if raw == nil { - c.err(fmt.Errorf( - "Unknown 'CloseProvider' call for %q", n)) - return nil - } - - result, ok := raw.(*shadowEvalContextInitProvider) - if !ok { - c.err(fmt.Errorf( - "Unknown 'CloseProvider' shadow value: %#v", raw)) - return nil - } - - result.Lock() - defer result.Unlock() - - if !result.Init { - // Record the error but continue... - c.err(fmt.Errorf( - "CloseProvider: provider %q requested but not initialized", n)) - } else if result.Closed { - c.err(fmt.Errorf( - "CloseProvider: provider %q requested but already closed", n)) - } - - result.Closed = true - return nil -} - -func (c *shadowEvalContextShadow) Diff() (*Diff, *sync.RWMutex) { - return c.DiffValue, c.DiffLock -} - -func (c *shadowEvalContextShadow) State() (*State, *sync.RWMutex) { - return c.StateValue, c.StateLock -} - -func (c *shadowEvalContextShadow) err(err error) error { - c.ErrorLock.Lock() - defer c.ErrorLock.Unlock() - c.Error = multierror.Append(c.Error, err) - return err -} - -// TODO: All the functions below are EvalContext functions that must be impl. - -func (c *shadowEvalContextShadow) Input() UIInput { return nil } -func (c *shadowEvalContextShadow) ConfigureProvider(string, *ResourceConfig) error { return nil } -func (c *shadowEvalContextShadow) SetProviderConfig(string, *ResourceConfig) error { return nil } -func (c *shadowEvalContextShadow) ParentProviderConfig(string) *ResourceConfig { return nil } -func (c *shadowEvalContextShadow) ProviderInput(string) map[string]interface{} { return nil } -func (c *shadowEvalContextShadow) SetProviderInput(string, map[string]interface{}) {} - -func (c *shadowEvalContextShadow) InitProvisioner(string) (ResourceProvisioner, error) { - return nil, nil -} -func (c *shadowEvalContextShadow) Provisioner(string) ResourceProvisioner { return nil } -func (c *shadowEvalContextShadow) CloseProvisioner(string) error { return nil } - -func (c *shadowEvalContextShadow) Interpolate(*config.RawConfig, *Resource) (*ResourceConfig, error) { - return nil, nil -} -func (c *shadowEvalContextShadow) SetVariables(string, map[string]interface{}) {} - -// The structs for the various function calls are put below. These structs -// are used to carry call information across the real/shadow boundaries. - -type shadowEvalContextInitProvider struct { - Shadow shadowResourceProvider - ResultErr error - - sync.Mutex // Must be held to modify the field below - Init bool // Keeps track of whether it has been initialized in the shadow - Closed bool // Keeps track of whether this provider is closed -} diff --git a/terraform/shadow_eval_context_test.go b/terraform/shadow_eval_context_test.go deleted file mode 100644 index 08db93a9f..000000000 --- a/terraform/shadow_eval_context_test.go +++ /dev/null @@ -1,257 +0,0 @@ -package terraform - -import ( - "fmt" - "reflect" - "testing" - "time" -) - -func TestShadowEvalContext_impl(t *testing.T) { - var _ EvalContext = new(shadowEvalContextReal) - var _ EvalContext = new(shadowEvalContextShadow) -} - -func TestShadowEvalContextInitProvider(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Verify that it blocks until the real func is called - var result ResourceProvider - var err error - doneCh := make(chan struct{}) - go func() { - defer close(doneCh) - result, err = shadow.InitProvider(name) - }() - - select { - case <-doneCh: - t.Fatal("should block until finished") - case <-time.After(10 * time.Millisecond): - } - - // Call the real func - realResult, realErr := real.InitProvider(name) - if realErr != nil { - t.Fatalf("bad: %#v", realErr) - } - realResult.Configure(nil) - if !mockResult.ConfigureCalled { - t.Fatalf("bad: %#v", realResult) - } - mockResult.ConfigureCalled = false - - // The shadow should finish now - <-doneCh - - // Verify the shadow returned the same values - if err != nil { - t.Fatalf("bad: %#v", err) - } - - // Verify that the returned value is a shadow. Calling one function - // shouldn't affect the other. - result.Configure(nil) - if mockResult.ConfigureCalled { - t.Fatal("close should not be called") - } - - // And doing some work should result in that value - mockErr := fmt.Errorf("yo") - mockResult.ConfigureReturnError = mockErr - realResult.Configure(nil) - if err := result.Configure(nil); !reflect.DeepEqual(err, mockErr) { - t.Fatalf("bad: %#v", err) - } - - // Verify we have no errors - if err := shadow.CloseShadow(); err != nil { - t.Fatalf("bad: %s", err) - } -} - -func TestShadowEvalContextInitProvider_doubleInit(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Call the real func - real.InitProvider(name) - - // Get the provider twice - shadow.InitProvider(name) - p, err := shadow.InitProvider(name) - if err != nil { - t.Fatalf("err: %s", err) - } - if p == nil { - t.Fatal("should return provider") - } - - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should error") - } -} - -func TestShadowEvalContextProvider(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Call the real func - real.InitProvider(name) - shadow.InitProvider(name) - - // Get the provider twice - p := shadow.Provider(name) - if p == nil { - t.Fatal("should return provider") - } - - if err := shadow.CloseShadow(); err != nil { - t.Fatalf("bad: %s", err) - } -} - -func TestShadowEvalContextProvider_noInit(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Call the real func - real.InitProvider(name) - - // Get the provider w/o calling init - p := shadow.Provider(name) - if p == nil { - t.Fatal("should return provider") - } - - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should error") - } -} - -func TestShadowEvalContextCloseProvider(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Call the real func - real.InitProvider(name) - shadow.InitProvider(name) - - // Get the provider twice - if err := shadow.CloseProvider(name); err != nil { - t.Fatalf("err: %s", err) - } - - if err := shadow.CloseShadow(); err != nil { - t.Fatalf("bad: %s", err) - } -} - -func TestShadowEvalContextCloseProvider_doubleClose(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Call the real func - real.InitProvider(name) - shadow.InitProvider(name) - - // Close the provider twice - if err := shadow.CloseProvider(name); err != nil { - t.Fatalf("err: %s", err) - } - if err := shadow.CloseProvider(name); err != nil { - t.Fatalf("err: %s", err) - } - - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should error") - } -} - -func TestShadowEvalContextCloseProvider_noInitClose(t *testing.T) { - mock := new(MockEvalContext) - real, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Call the real func - real.InitProvider(name) - - // Close the provider - if err := shadow.CloseProvider(name); err != nil { - t.Fatalf("err: %s", err) - } - - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should error") - } -} - -func TestShadowEvalContextCloseProvider_noCreate(t *testing.T) { - mock := new(MockEvalContext) - _, shadow := NewShadowEvalContext(mock) - - // Args, results - name := "foo" - mockResult := new(MockResourceProvider) - - // Configure the mock - mock.InitProviderProvider = mockResult - - // Close the provider - if err := shadow.CloseProvider(name); err != nil { - t.Fatalf("err: %s", err) - } - - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should error") - } -} From e2fc0b518fd8728ec822afb6cbd15b184aa10e28 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 8 Oct 2016 16:58:00 +0800 Subject: [PATCH 48/64] terraform: ShadowError returns errors, not the close operation --- terraform/shadow_resource_provider_test.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/terraform/shadow_resource_provider_test.go b/terraform/shadow_resource_provider_test.go index 1b04184de..be2fba0ea 100644 --- a/terraform/shadow_resource_provider_test.go +++ b/terraform/shadow_resource_provider_test.go @@ -114,8 +114,11 @@ func TestShadowResourceProviderInput_badInput(t *testing.T) { } // Verify we have an error - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should have error") + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } + if err := shadow.ShadowError(); err == nil { + t.Fatal("should error") } } @@ -194,8 +197,11 @@ func TestShadowResourceProviderValidate_badInput(t *testing.T) { shadow.Validate(configBad) // Verify we have an error - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should have error") + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } + if err := shadow.ShadowError(); err == nil { + t.Fatal("should error") } } @@ -265,8 +271,11 @@ func TestShadowResourceProviderConfigure_badInput(t *testing.T) { shadow.Configure(configBad) // Verify we have an error - if err := shadow.CloseShadow(); err == nil { - t.Fatal("should have error") + if err := shadow.CloseShadow(); err != nil { + t.Fatalf("bad: %s", err) + } + if err := shadow.ShadowError(); err == nil { + t.Fatal("should error") } } From d7a5cc5b35d4918d78785c0aed043a60361e42f8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 8 Oct 2016 17:33:49 +0800 Subject: [PATCH 49/64] terraform: InstanceInfo.uniqueId This adds a new function to get a unique identifier scoped to the graph walk in order to identify operations against the same instance. This is used by the shadow to namespace provider function calls. --- terraform/resource.go | 15 +++++++++++++++ terraform/shadow_resource_provider.go | 12 ++++++------ terraform/transform_resource.go | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/terraform/resource.go b/terraform/resource.go index 937efdbe4..7f1ec3ca4 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -62,6 +62,12 @@ type InstanceInfo struct { // Type is the resource type of this instance Type string + + // uniqueExtra is an internal field that can be populated to supply + // extra metadata that is used to identify a unique instance in + // the graph walk. This will be appended to HumanID when uniqueId + // is called. + uniqueExtra string } // HumanId is a unique Id that is human-friendly and useful for UI elements. @@ -76,6 +82,15 @@ func (i *InstanceInfo) HumanId() string { i.Id) } +func (i *InstanceInfo) uniqueId() string { + prefix := i.HumanId() + if v := i.uniqueExtra; v != "" { + prefix += " " + v + } + + return prefix +} + // ResourceConfig holds the configuration given for a resource. This is // done instead of a raw `map[string]interface{}` type so that rich // methods can be added to it to make dealing with it easier. diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 0cf1813a5..862c77515 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -155,7 +155,7 @@ func (p *shadowResourceProviderReal) Apply( diffCopy := diff.DeepCopy() result, err := p.ResourceProvider.Apply(info, state, diff) - p.Shared.Apply.SetValue(info.HumanId(), &shadowResourceProviderApply{ + p.Shared.Apply.SetValue(info.uniqueId(), &shadowResourceProviderApply{ State: stateCopy, Diff: diffCopy, Result: result.DeepCopy(), @@ -173,7 +173,7 @@ func (p *shadowResourceProviderReal) Diff( stateCopy := state.DeepCopy() result, err := p.ResourceProvider.Diff(info, state, desired) - p.Shared.Diff.SetValue(info.HumanId(), &shadowResourceProviderDiff{ + p.Shared.Diff.SetValue(info.uniqueId(), &shadowResourceProviderDiff{ State: stateCopy, Desired: desired, Result: result.DeepCopy(), @@ -190,7 +190,7 @@ func (p *shadowResourceProviderReal) Refresh( stateCopy := state.DeepCopy() result, err := p.ResourceProvider.Refresh(info, state) - p.Shared.Refresh.SetValue(info.HumanId(), &shadowResourceProviderRefresh{ + p.Shared.Refresh.SetValue(info.uniqueId(), &shadowResourceProviderRefresh{ State: stateCopy, Result: result.DeepCopy(), ResultErr: err, @@ -420,7 +420,7 @@ func (p *shadowResourceProviderShadow) Apply( state *InstanceState, diff *InstanceDiff) (*InstanceState, error) { // Unique key - key := info.HumanId() + key := info.uniqueId() raw := p.Shared.Apply.Value(key) if raw == nil { p.ErrorLock.Lock() @@ -459,7 +459,7 @@ func (p *shadowResourceProviderShadow) Diff( state *InstanceState, desired *ResourceConfig) (*InstanceDiff, error) { // Unique key - key := info.HumanId() + key := info.uniqueId() raw := p.Shared.Diff.Value(key) if raw == nil { p.ErrorLock.Lock() @@ -502,7 +502,7 @@ func (p *shadowResourceProviderShadow) Refresh( info *InstanceInfo, state *InstanceState) (*InstanceState, error) { // Unique key - key := info.HumanId() + key := info.uniqueId() raw := p.Shared.Refresh.Value(key) if raw == nil { p.ErrorLock.Lock() diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 7a7fc223f..d53e9f951 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -862,7 +862,7 @@ func (n *graphNodeExpandedResourceDestroy) ConfigType() GraphNodeConfigType { // GraphNodeEvalable impl. func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode { info := n.instanceInfo() - info.Id += " (destroy)" + info.uniqueExtra = "destroy" var diffApply *InstanceDiff var provider ResourceProvider From 817979c56d5e54e745dda34fe43b4ae6583c844e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 8 Oct 2016 18:04:04 +0800 Subject: [PATCH 50/64] terraform: ResourceProvider.ValidateResource (shadow) config deep copy --- terraform/shadow_resource_provider.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 862c77515..4535660e3 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -110,6 +110,7 @@ func (p *shadowResourceProviderReal) Configure(c *ResourceConfig) error { func (p *shadowResourceProviderReal) ValidateResource( t string, c *ResourceConfig) ([]string, []error) { key := t + configCopy := c.DeepCopy() // Real operation warns, errs := p.ResourceProvider.ValidateResource(t, c) @@ -134,7 +135,7 @@ func (p *shadowResourceProviderReal) ValidateResource( defer wrapper.Unlock() wrapper.Calls = append(wrapper.Calls, &shadowResourceProviderValidateResource{ - Config: c, + Config: configCopy, Warns: warns, Errors: errs, }) From c2dd9a7338f35c8bb7e0a741251a2a01726915c5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 9 Oct 2016 10:02:04 +0800 Subject: [PATCH 51/64] terraform: Provier.Diff (shadow) deep copy the config before call --- terraform/shadow_resource_provider.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 4535660e3..65e5617ef 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -172,11 +172,12 @@ func (p *shadowResourceProviderReal) Diff( desired *ResourceConfig) (*InstanceDiff, error) { // Thse have to be copied before the call since call can modify stateCopy := state.DeepCopy() + desiredCopy := desired.DeepCopy() result, err := p.ResourceProvider.Diff(info, state, desired) p.Shared.Diff.SetValue(info.uniqueId(), &shadowResourceProviderDiff{ State: stateCopy, - Desired: desired, + Desired: desiredCopy, Result: result.DeepCopy(), ResultErr: err, }) From 30596ca3716b4b9b54699bc80f4e2544138d5f41 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 9 Oct 2016 10:05:58 +0800 Subject: [PATCH 52/64] terraform: sanity test (passes, always passed) --- config/raw_config.go | 4 ++++ terraform/resource_test.go | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/config/raw_config.go b/config/raw_config.go index cf3bdf9bd..260e315bb 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -62,6 +62,10 @@ func (r *RawConfig) RawMap() map[string]interface{} { // Copy returns a copy of this RawConfig, uninterpolated. func (r *RawConfig) Copy() *RawConfig { + if r == nil { + return nil + } + r.lock.Lock() defer r.lock.Unlock() diff --git a/terraform/resource_test.go b/terraform/resource_test.go index ab6a2273b..0916af09d 100644 --- a/terraform/resource_test.go +++ b/terraform/resource_test.go @@ -247,6 +247,14 @@ func TestResourceConfigDeepCopy_nil(t *testing.T) { } } +func TestResourceConfigDeepCopy_nilComputed(t *testing.T) { + rc := &ResourceConfig{} + actual := rc.DeepCopy() + if actual.ComputedKeys != nil { + t.Fatalf("bad: %#v", actual) + } +} + func TestResourceConfigEqual_nil(t *testing.T) { var nilRc *ResourceConfig notNil := NewResourceConfig(nil) From 31f8d13678a4de3264009e6f22332846a702b3a6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 9 Oct 2016 10:18:38 +0800 Subject: [PATCH 53/64] terraform: Diff.Equal and tests --- terraform/diff.go | 37 +++++++++++++++++++++++++++++++++++ terraform/diff_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/terraform/diff.go b/terraform/diff.go index 4b810092b..d566d2582 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -81,6 +81,25 @@ func (d *Diff) Empty() bool { return true } +// Equal compares two diffs for exact equality. +// +// This is different from the Same comparison that is supported which +// checks for operation equality taking into account computed values. Equal +// instead checks for exact equality. +func (d *Diff) Equal(d2 *Diff) bool { + // If one is nil, they must both be nil + if d == nil || d2 == nil { + return d == d2 + } + + // Sort the modules + sort.Sort(moduleDiffSort(d.Modules)) + sort.Sort(moduleDiffSort(d2.Modules)) + + // Use DeepEqual + return reflect.DeepEqual(d, d2) +} + // DeepCopy performs a deep copy of all parts of the Diff, making the // resulting Diff safe to use without modifying this one. func (d *Diff) DeepCopy() *Diff { @@ -659,3 +678,21 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) { return true, "" } + +// moduleDiffSort implements sort.Interface to sort module diffs by path. +type moduleDiffSort []*ModuleDiff + +func (s moduleDiffSort) Len() int { return len(s) } +func (s moduleDiffSort) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s moduleDiffSort) Less(i, j int) bool { + a := s[i] + b := s[j] + + // If the lengths are different, then the shorter one always wins + if len(a.Path) != len(b.Path) { + return len(a.Path) < len(b.Path) + } + + // Otherwise, compare lexically + return strings.Join(a.Path, ".") < strings.Join(b.Path, ".") +} diff --git a/terraform/diff_test.go b/terraform/diff_test.go index 6239bc413..5234c6aaf 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -40,6 +40,50 @@ func TestDiffEmpty_taintedIsNotEmpty(t *testing.T) { } } +func TestDiffEqual(t *testing.T) { + cases := map[string]struct { + D1, D2 *Diff + Equal bool + }{ + "nil": { + nil, + new(Diff), + false, + }, + + "empty": { + new(Diff), + new(Diff), + true, + }, + + "different module order": { + &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{Path: []string{"root", "foo"}}, + &ModuleDiff{Path: []string{"root", "bar"}}, + }, + }, + &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{Path: []string{"root", "bar"}}, + &ModuleDiff{Path: []string{"root", "foo"}}, + }, + }, + true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + actual := tc.D1.Equal(tc.D2) + if actual != tc.Equal { + t.Fatalf("expected: %v\n\n%#v\n\n%#v", tc.Equal, tc.D1, tc.D2) + } + }) + } +} + func TestModuleDiff_ChangeType(t *testing.T) { cases := []struct { Diff *ModuleDiff From b0801bf125e4dafb79b2ee5b0c21c3ea91b983ea Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 9 Oct 2016 12:12:18 +0800 Subject: [PATCH 54/64] terraform: ResourceProvider.ReadDataDiff (shadow) --- terraform/shadow_context.go | 3 +- terraform/shadow_resource_provider.go | 192 +++++++++++++++++++++++--- 2 files changed, 174 insertions(+), 21 deletions(-) diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index eba8640fb..1a289793b 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -2,7 +2,6 @@ package terraform import ( "fmt" - "reflect" "github.com/hashicorp/go-multierror" "github.com/mitchellh/copystructure" @@ -85,7 +84,7 @@ func shadowContextVerify(real, shadow *Context) error { } // Compare the diffs - if !reflect.DeepEqual(real.diff, shadow.diff) { + if !real.diff.Equal(shadow.diff) { result = multierror.Append(result, fmt.Errorf( "Real and shadow diffs do not match! "+ "Real diff:\n\n%s\n\n"+ diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 65e5617ef..d156f273e 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -201,6 +201,62 @@ func (p *shadowResourceProviderReal) Refresh( return result, err } +func (p *shadowResourceProviderReal) ValidateDataSource( + t string, c *ResourceConfig) ([]string, []error) { + key := t + configCopy := c.DeepCopy() + + // Real operation + warns, errs := p.ResourceProvider.ValidateDataSource(t, c) + + // Get the result + raw, ok := p.Shared.ValidateDataSource.ValueOk(key) + if !ok { + raw = new(shadowResourceProviderValidateDataSourceWrapper) + } + + wrapper, ok := raw.(*shadowResourceProviderValidateDataSourceWrapper) + if !ok { + // If this fails then we just continue with our day... the shadow + // will fail to but there isn't much we can do. + log.Printf( + "[ERROR] unknown value in ValidateDataSource shadow value: %#v", raw) + return warns, errs + } + + // Lock the wrapper for writing and record our call + wrapper.Lock() + defer wrapper.Unlock() + + wrapper.Calls = append(wrapper.Calls, &shadowResourceProviderValidateDataSource{ + Config: configCopy, + Warns: warns, + Errors: errs, + }) + + // Set it + p.Shared.ValidateDataSource.SetValue(key, wrapper) + + // Return the result + return warns, errs +} + +func (p *shadowResourceProviderReal) ReadDataDiff( + info *InstanceInfo, + desired *ResourceConfig) (*InstanceDiff, error) { + // These have to be copied before the call since call can modify + desiredCopy := desired.DeepCopy() + + result, err := p.ResourceProvider.ReadDataDiff(info, desired) + p.Shared.ReadDataDiff.SetValue(info.uniqueId(), &shadowResourceProviderReadDataDiff{ + Desired: desiredCopy, + Result: result.DeepCopy(), + ResultErr: err, + }) + + return result, err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -219,21 +275,23 @@ 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 - ValidateResource shadow.KeyedValue - 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 + ValidateDataSource shadow.KeyedValue + ReadDataDiff shadow.KeyedValue } func (p *shadowResourceProviderShared) Close() error { closers := []io.Closer{ &p.CloseErr, &p.Input, &p.Validate, &p.Configure, &p.ValidateResource, &p.Apply, &p.Diff, - &p.Refresh, + &p.Refresh, &p.ValidateDataSource, &p.ReadDataDiff, } for _, c := range closers { @@ -536,6 +594,94 @@ func (p *shadowResourceProviderShadow) Refresh( return result.Result, result.ResultErr } +func (p *shadowResourceProviderShadow) ValidateDataSource( + t string, c *ResourceConfig) ([]string, []error) { + // Unique key + key := t + + // Get the initial value + raw := p.Shared.ValidateDataSource.Value(key) + + // Find a validation with our configuration + var result *shadowResourceProviderValidateDataSource + for { + // Get the value + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ValidateDataSource' call for %q:\n\n%#v", + key, c)) + return nil, nil + } + + wrapper, ok := raw.(*shadowResourceProviderValidateDataSourceWrapper) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ValidateDataSource' 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.ValidateDataSource.WaitForChange(key) + } + + return result.Warns, result.Errors +} + +func (p *shadowResourceProviderShadow) ReadDataDiff( + info *InstanceInfo, + desired *ResourceConfig) (*InstanceDiff, error) { + // Unique key + key := info.uniqueId() + raw := p.Shared.ReadDataDiff.Value(key) + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ReadDataDiff' call for %q:\n\n%#v", + key, desired)) + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderReadDataDiff) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ReadDataDiff' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + if !desired.Equal(result.Desired) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "ReadDataDiff %q had unequal configs (real, then shadow):\n\n%#v\n\n%#v", + key, result.Desired, desired)) + p.ErrorLock.Unlock() + } + + return result.Result, result.ResultErr +} + // TODO // TODO // TODO @@ -546,16 +692,6 @@ func (p *shadowResourceProviderShadow) ImportState(info *InstanceInfo, id string return nil, nil } -func (p *shadowResourceProviderShadow) ValidateDataSource(t string, c *ResourceConfig) ([]string, []error) { - return nil, nil -} - -func (p *shadowResourceProviderShadow) ReadDataDiff( - info *InstanceInfo, - desired *ResourceConfig) (*InstanceDiff, error) { - return nil, nil -} - func (p *shadowResourceProviderShadow) ReadDataApply( info *InstanceInfo, d *InstanceDiff) (*InstanceState, error) { @@ -613,3 +749,21 @@ type shadowResourceProviderRefresh struct { Result *InstanceState ResultErr error } + +type shadowResourceProviderValidateDataSourceWrapper struct { + sync.RWMutex + + Calls []*shadowResourceProviderValidateDataSource +} + +type shadowResourceProviderValidateDataSource struct { + Config *ResourceConfig + Warns []string + Errors []error +} + +type shadowResourceProviderReadDataDiff struct { + Desired *ResourceConfig + Result *InstanceDiff + ResultErr error +} From 10bcdd04d4a8a38c773ad004b66b39395f60af4c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 18:45:40 +0800 Subject: [PATCH 55/64] helper/shadow: KeyedValue.Init --- helper/shadow/keyed_value.go | 45 ++++++++++++++++++++------ helper/shadow/keyed_value_test.go | 54 +++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/helper/shadow/keyed_value.go b/helper/shadow/keyed_value.go index e31920fe8..5a0b608e4 100644 --- a/helper/shadow/keyed_value.go +++ b/helper/shadow/keyed_value.go @@ -35,11 +35,16 @@ func (w *KeyedValue) Close() error { // Value returns the value that was set for the given key, or blocks // until one is available. func (w *KeyedValue) Value(k string) interface{} { + w.lock.Lock() v, val := w.valueWaiter(k) + w.lock.Unlock() + + // If we have no waiter, then return the value if val == nil { return v } + // We have a waiter, so wait return val.Value() } @@ -71,6 +76,9 @@ func (w *KeyedValue) WaitForChange(k string) interface{} { // ValueOk gets the value for the given key, returning immediately if the // value doesn't exist. The second return argument is true if the value exists. func (w *KeyedValue) ValueOk(k string) (interface{}, bool) { + w.lock.Lock() + defer w.lock.Unlock() + v, val := w.valueWaiter(k) return v, val == nil } @@ -78,6 +86,30 @@ func (w *KeyedValue) ValueOk(k string) (interface{}, bool) { func (w *KeyedValue) SetValue(k string, v interface{}) { w.lock.Lock() defer w.lock.Unlock() + w.setValue(k, v) +} + +// Init will initialize the key to a given value only if the key has +// not been set before. This is safe to call multiple times and in parallel. +func (w *KeyedValue) Init(k string, v interface{}) { + w.lock.Lock() + defer w.lock.Unlock() + + // If we have a waiter, set the value. + _, val := w.valueWaiter(k) + if val != nil { + w.setValue(k, v) + } +} + +// Must be called with w.lock held. +func (w *KeyedValue) init() { + w.values = make(map[string]interface{}) + w.waiters = make(map[string]*Value) +} + +// setValue is like SetValue but assumes the lock is held. +func (w *KeyedValue) setValue(k string, v interface{}) { w.once.Do(w.init) // Set the value, always @@ -90,25 +122,19 @@ func (w *KeyedValue) SetValue(k string, v interface{}) { } } -// Must be called with w.lock held. -func (w *KeyedValue) init() { - w.values = make(map[string]interface{}) - w.waiters = make(map[string]*Value) -} - +// valueWaiter gets the value or the Value waiter for a given key. +// +// This must be called with lock held. func (w *KeyedValue) valueWaiter(k string) (interface{}, *Value) { - w.lock.Lock() w.once.Do(w.init) // If we have this value already, return it if v, ok := w.values[k]; ok { - w.lock.Unlock() return v, nil } // If we're closed, return that if w.closed { - w.lock.Unlock() return ErrClosed, nil } @@ -118,7 +144,6 @@ func (w *KeyedValue) valueWaiter(k string) (interface{}, *Value) { val = new(Value) w.waiters[k] = val } - w.lock.Unlock() // Return the waiter return nil, val diff --git a/helper/shadow/keyed_value_test.go b/helper/shadow/keyed_value_test.go index 098c54a59..ee4b639eb 100644 --- a/helper/shadow/keyed_value_test.go +++ b/helper/shadow/keyed_value_test.go @@ -169,6 +169,60 @@ func TestKeyedValueClose_existingBlocked(t *testing.T) { } } +func TestKeyedValueInit(t *testing.T) { + var v KeyedValue + + v.Init("foo", 42) + + // We should get the value + val := v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We should get the value + val = v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // This should do nothing + v.Init("foo", 84) + + // We should get the value + val = v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } +} + +func TestKeyedValueInit_set(t *testing.T) { + var v KeyedValue + + v.SetValue("foo", 42) + + // We should get the value + val := v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // We should get the value + val = v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } + + // This should do nothing + v.Init("foo", 84) + + // We should get the value + val = v.Value("foo") + if val != 42 { + t.Fatalf("bad: %#v", val) + } +} + func TestKeyedValueWaitForChange(t *testing.T) { var v KeyedValue From a9f11665839c735c7a489d11931d4a67b7abe013 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 18:47:52 +0800 Subject: [PATCH 56/64] terraform: use KeyedValue.Init to avoid initialization race There were races with ValidateResource in the provider initializing the data which resulting in lost data for the shadow. A new "Init" function has been added to the shadow structs to support safe concurrent initialization. --- terraform/shadow_resource_provider.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index d156f273e..3ced39fcc 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -115,12 +115,12 @@ func (p *shadowResourceProviderReal) ValidateResource( // Real operation warns, errs := p.ResourceProvider.ValidateResource(t, c) - // Get the result - raw, ok := p.Shared.ValidateResource.ValueOk(key) - if !ok { - raw = new(shadowResourceProviderValidateResourceWrapper) - } + // Initialize to ensure we always have a wrapper with a lock + p.Shared.ValidateResource.Init( + key, &shadowResourceProviderValidateResourceWrapper{}) + // Get the result + raw := p.Shared.ValidateResource.Value(key) wrapper, ok := raw.(*shadowResourceProviderValidateResourceWrapper) if !ok { // If this fails then we just continue with our day... the shadow @@ -140,9 +140,6 @@ func (p *shadowResourceProviderReal) ValidateResource( Errors: errs, }) - // Set it - p.Shared.ValidateResource.SetValue(key, wrapper) - // Return the result return warns, errs } @@ -209,12 +206,12 @@ func (p *shadowResourceProviderReal) ValidateDataSource( // Real operation warns, errs := p.ResourceProvider.ValidateDataSource(t, c) - // Get the result - raw, ok := p.Shared.ValidateDataSource.ValueOk(key) - if !ok { - raw = new(shadowResourceProviderValidateDataSourceWrapper) - } + // Initialize + p.Shared.ValidateDataSource.Init( + key, &shadowResourceProviderValidateDataSourceWrapper{}) + // Get the result + raw := p.Shared.ValidateDataSource.Value(key) wrapper, ok := raw.(*shadowResourceProviderValidateDataSourceWrapper) if !ok { // If this fails then we just continue with our day... the shadow From 4c951428d740551e700c85bc8caa076eba1df767 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 18:50:41 +0800 Subject: [PATCH 57/64] terraform: enable shadow on destroy and Plan --- terraform/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index af5f9e911..aa8f13806 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -362,7 +362,7 @@ func (c *Context) Apply() (*State, error) { // Do the walk var walker *ContextGraphWalker if c.destroy { - walker, err = c.walk(graph, nil, walkDestroy) + walker, err = c.walk(graph, graph, walkDestroy) } else { //walker, err = c.walk(graph, nil, walkApply) walker, err = c.walk(graph, graph, walkApply) @@ -430,7 +430,7 @@ func (c *Context) Plan() (*Plan, error) { } // Do the walk - walker, err := c.walk(graph, nil, operation) + walker, err := c.walk(graph, graph, operation) if err != nil { return nil, err } From 98fa7d92a40552fa437da112c7cb7872291dffc0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 18:56:57 +0800 Subject: [PATCH 58/64] terraform: support data source apply for shadows --- terraform/diff.go | 15 +++++++ terraform/shadow_resource_provider.go | 65 ++++++++++++++++++++++++--- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/terraform/diff.go b/terraform/diff.go index d566d2582..33b4a9896 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -400,6 +400,21 @@ func (d *InstanceDiff) Empty() bool { return !d.Destroy && !d.DestroyTainted && len(d.Attributes) == 0 } +// Equal compares two diffs for exact equality. +// +// This is different from the Same comparison that is supported which +// checks for operation equality taking into account computed values. Equal +// instead checks for exact equality. +func (d *InstanceDiff) Equal(d2 *InstanceDiff) bool { + // If one is nil, they must both be nil + if d == nil || d2 == nil { + return d == d2 + } + + // Use DeepEqual + return reflect.DeepEqual(d, d2) +} + // DeepCopy performs a deep copy of all parts of the InstanceDiff func (d *InstanceDiff) DeepCopy() *InstanceDiff { copy, err := copystructure.Config{Lock: true}.Copy(d) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 3ced39fcc..0355bd78e 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -254,6 +254,22 @@ func (p *shadowResourceProviderReal) ReadDataDiff( return result, err } +func (p *shadowResourceProviderReal) ReadDataApply( + info *InstanceInfo, + diff *InstanceDiff) (*InstanceState, error) { + // Thse have to be copied before the call since call can modify + diffCopy := diff.DeepCopy() + + result, err := p.ResourceProvider.ReadDataApply(info, diff) + p.Shared.ReadDataApply.SetValue(info.uniqueId(), &shadowResourceProviderReadDataApply{ + Diff: diffCopy, + Result: result.DeepCopy(), + ResultErr: err, + }) + + return result, err +} + // shadowResourceProviderShadow is the shadow resource provider. Function // calls never affect real resources. This is paired with the "real" side // which must be called properly to enable recording. @@ -282,6 +298,7 @@ type shadowResourceProviderShared struct { Refresh shadow.KeyedValue ValidateDataSource shadow.KeyedValue ReadDataDiff shadow.KeyedValue + ReadDataApply shadow.KeyedValue } func (p *shadowResourceProviderShared) Close() error { @@ -679,6 +696,42 @@ func (p *shadowResourceProviderShadow) ReadDataDiff( return result.Result, result.ResultErr } +func (p *shadowResourceProviderShadow) ReadDataApply( + info *InstanceInfo, + d *InstanceDiff) (*InstanceState, error) { + // Unique key + key := info.uniqueId() + raw := p.Shared.ReadDataApply.Value(key) + if raw == nil { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ReadDataApply' call for %q:\n\n%#v", + key, d)) + return nil, nil + } + + result, ok := raw.(*shadowResourceProviderReadDataApply) + if !ok { + p.ErrorLock.Lock() + defer p.ErrorLock.Unlock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Unknown 'ReadDataApply' shadow value: %#v", raw)) + return nil, nil + } + + // Compare the parameters, which should be identical + if !d.Equal(result.Diff) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "ReadDataApply: unequal diffs (real, then shadow):\n\n%#v\n\n%#v", + result.Diff, d)) + p.ErrorLock.Unlock() + } + + return result.Result, result.ResultErr +} + // TODO // TODO // TODO @@ -689,12 +742,6 @@ func (p *shadowResourceProviderShadow) ImportState(info *InstanceInfo, id string return nil, nil } -func (p *shadowResourceProviderShadow) ReadDataApply( - info *InstanceInfo, - d *InstanceDiff) (*InstanceState, error) { - return nil, nil -} - // The structs for the various function calls are put below. These structs // are used to carry call information across the real/shadow boundaries. @@ -764,3 +811,9 @@ type shadowResourceProviderReadDataDiff struct { Result *InstanceDiff ResultErr error } + +type shadowResourceProviderReadDataApply struct { + Diff *InstanceDiff + Result *InstanceState + ResultErr error +} From 7f04b33d3d1fc7e30d640d127c35a502193acaf9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 18:57:52 +0800 Subject: [PATCH 59/64] terraform: enable shadow walking on Refresh and Validate --- terraform/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index aa8f13806..3e2e8aa3e 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -479,7 +479,7 @@ func (c *Context) Refresh() (*State, error) { } // Do the walk - if _, err := c.walk(graph, nil, walkRefresh); err != nil { + if _, err := c.walk(graph, graph, walkRefresh); err != nil { return nil, err } @@ -547,7 +547,7 @@ func (c *Context) Validate() ([]string, []error) { } // Walk - walker, err := c.walk(graph, nil, walkValidate) + walker, err := c.walk(graph, graph, walkValidate) if err != nil { return nil, multierror.Append(errs, err).Errors } From 9a876f65bad16eb8683b2b74cf53655e64d57aeb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 19:01:02 +0800 Subject: [PATCH 60/64] terraform: compare diffs on shadow Apply --- terraform/shadow_resource_provider.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 0355bd78e..967817597 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -523,7 +523,13 @@ func (p *shadowResourceProviderShadow) Apply( p.ErrorLock.Unlock() } - // TODO: compare diffs + if !diff.Equal(result.Diff) { + p.ErrorLock.Lock() + p.Error = multierror.Append(p.Error, fmt.Errorf( + "Apply: unequal diffs (real, then shadow):\n\n%#v\n\n%#v", + result.Diff, diff)) + p.ErrorLock.Unlock() + } return result.Result, result.ResultErr } From 70cee9c1c634a1a2518b8c733f2e704c483d1be3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 19:03:41 +0800 Subject: [PATCH 61/64] terraform: clean up any final TODOs with comments and placeholders --- terraform/shadow_context.go | 8 +++++++- terraform/shadow_resource_provider.go | 8 +------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 1a289793b..fdadea370 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -48,9 +48,15 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) { module: c.module, state: c.state.DeepCopy(), targets: targetRaw.([]string), - uiInput: nil, // TODO variables: varRaw.(map[string]interface{}), + // NOTE(mitchellh): This is not going to work for shadows that are + // testing that input results in the proper end state. At the time + // of writing, input is not used in any state-changing graph + // walks anyways, so this checks nothing. We set it to this to avoid + // any panics but even a "nil" value worked here. + uiInput: new(MockUIInput), + // Hardcoded to 4 since parallelism in the shadow doesn't matter // a ton since we're doing far less compared to the real side // and our operations are MUCH faster. diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 967817597..4d7643438 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -738,14 +738,8 @@ func (p *shadowResourceProviderShadow) ReadDataApply( return result.Result, result.ResultErr } -// TODO -// TODO -// TODO -// TODO -// TODO - func (p *shadowResourceProviderShadow) ImportState(info *InstanceInfo, id string) ([]*InstanceState, error) { - return nil, nil + panic("import not supported by shadow graph") } // The structs for the various function calls are put below. These structs From baa59ff75d810221be190bc9036663924ea79322 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 19:47:21 +0800 Subject: [PATCH 62/64] terraform: fix go vet issues by not using *c to copy --- terraform/shadow_context.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index fdadea370..0e213d019 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -67,10 +67,31 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) { // Create the real context. This is effectively just a copy of // the context given except we need to modify some of the values // to point to the real side of a shadow so the shadow can compare values. - real := *c - real.components = componentsReal + real := &Context{ + // The fields below are changed. + components: componentsReal, - return &real, shadow, &shadowContextCloser{ + // The fields below are direct copies + destroy: c.destroy, + diff: c.diff, + // diffLock - no copy + hooks: c.hooks, + module: c.module, + sh: c.sh, + state: c.state, + // stateLock - no copy + targets: c.targets, + uiInput: c.uiInput, + variables: c.variables, + + // l - no copy + parallelSem: c.parallelSem, + providerInputConfig: c.providerInputConfig, + runCh: c.runCh, + shadowErr: c.shadowErr, + } + + return real, shadow, &shadowContextCloser{ Components: componentsShadow, } } From e2c415a87eff10ee120f3cf8fa4008e89e3daa24 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 17 Oct 2016 16:29:24 -0700 Subject: [PATCH 63/64] terraform: resource provider must never return pointers to same data This is a requirement for the parallelism of Terraform to work sanely. We could deep copy every result but I think this would be unrealistic and impose a performance cost when it isn't necessary in most cases. --- terraform/context_plan_test.go | 4 ++++ terraform/resource_provider.go | 6 ++++++ terraform/resource_provider_mock.go | 20 ++++++++++++++------ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 8778e4221..e58dbdf9f 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1088,6 +1088,10 @@ func TestContext2Plan_dataResourceBecomesComputed(t *testing.T) { t.Fatalf("missing diff for data.aws_data_resource.foo") } + // This is added by the diff but we want to verify that we got + // the same diff as above minus the dynamic stuff. + delete(iDiff.Attributes, "id") + if same, _ := p.ReadDataDiffReturn.Same(iDiff); !same { t.Fatalf( "incorrect diff for data.data_resource.foo\ngot: %#v\nwant: %#v", diff --git a/terraform/resource_provider.go b/terraform/resource_provider.go index 37cd1d5c3..542f14a61 100644 --- a/terraform/resource_provider.go +++ b/terraform/resource_provider.go @@ -3,6 +3,12 @@ package terraform // ResourceProvider is an interface that must be implemented by any // resource provider: the thing that creates and manages the resources in // a Terraform configuration. +// +// Important implementation note: All returned pointers, such as +// *ResourceConfig, *InstanceState, *InstanceDiff, etc. must not point to +// shared data. Terraform is highly parallel and assumes that this data is safe +// to read/write in parallel so it must be unique references. Note that it is +// safe to return arguments as results, however. type ResourceProvider interface { /********************************************************************* * Functions related to the provider diff --git a/terraform/resource_provider_mock.go b/terraform/resource_provider_mock.go index 8389fd0ae..f8acfafa9 100644 --- a/terraform/resource_provider_mock.go +++ b/terraform/resource_provider_mock.go @@ -157,7 +157,7 @@ func (p *MockResourceProvider) Apply( return p.ApplyFn(info, state, diff) } - return p.ApplyReturn, p.ApplyReturnError + return p.ApplyReturn.DeepCopy(), p.ApplyReturnError } func (p *MockResourceProvider) Diff( @@ -175,7 +175,7 @@ func (p *MockResourceProvider) Diff( return p.DiffFn(info, state, desired) } - return p.DiffReturn, p.DiffReturnError + return p.DiffReturn.DeepCopy(), p.DiffReturnError } func (p *MockResourceProvider) Refresh( @@ -192,7 +192,7 @@ func (p *MockResourceProvider) Refresh( return p.RefreshFn(info, s) } - return p.RefreshReturn, p.RefreshReturnError + return p.RefreshReturn.DeepCopy(), p.RefreshReturnError } func (p *MockResourceProvider) Resources() []ResourceType { @@ -214,7 +214,15 @@ func (p *MockResourceProvider) ImportState(info *InstanceInfo, id string) ([]*In return p.ImportStateFn(info, id) } - return p.ImportStateReturn, p.ImportStateReturnError + var result []*InstanceState + if p.ImportStateReturn != nil { + result = make([]*InstanceState, len(p.ImportStateReturn)) + for i, v := range p.ImportStateReturn { + result[i] = v.DeepCopy() + } + } + + return result, p.ImportStateReturnError } func (p *MockResourceProvider) ValidateDataSource(t string, c *ResourceConfig) ([]string, []error) { @@ -245,7 +253,7 @@ func (p *MockResourceProvider) ReadDataDiff( return p.ReadDataDiffFn(info, desired) } - return p.ReadDataDiffReturn, p.ReadDataDiffReturnError + return p.ReadDataDiffReturn.DeepCopy(), p.ReadDataDiffReturnError } func (p *MockResourceProvider) ReadDataApply( @@ -262,7 +270,7 @@ func (p *MockResourceProvider) ReadDataApply( return p.ReadDataApplyFn(info, d) } - return p.ReadDataApplyReturn, p.ReadDataApplyReturnError + return p.ReadDataApplyReturn.DeepCopy(), p.ReadDataApplyReturnError } func (p *MockResourceProvider) DataSources() []DataSource { From 35d18686183c88afe6a5455e3ded65781536f41c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 10:07:47 -0700 Subject: [PATCH 64/64] terraform: remove ModuleDiff.GoString To address comments by @jbardin re: if we had a mutex this will fail vet. --- terraform/diff.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/terraform/diff.go b/terraform/diff.go index 33b4a9896..e0e097e80 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -311,10 +311,6 @@ func (d *ModuleDiff) String() string { return buf.String() } -func (d *ModuleDiff) GoString() string { - return fmt.Sprintf("*%#v", *d) -} - // InstanceDiff is the diff of a resource from some state to another. type InstanceDiff struct { mu sync.Mutex