From d77a72ba8452d3dc31f209b08296d074fc4def96 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 5 Jun 2014 07:27:01 -0700 Subject: [PATCH] terraform: take into account dependency variables in diffs --- terraform/resource_provider.go | 2 +- terraform/resource_provider_mock.go | 6 ++-- terraform/state.go | 17 +++++++-- terraform/terraform.go | 53 +++++++++++++++++++++++++---- terraform/terraform_test.go | 4 +-- 5 files changed, 67 insertions(+), 15 deletions(-) diff --git a/terraform/resource_provider.go b/terraform/resource_provider.go index 2bdceafa6..879c75ea2 100644 --- a/terraform/resource_provider.go +++ b/terraform/resource_provider.go @@ -24,7 +24,7 @@ type ResourceProvider interface { // Diff diffs a resource versus a desired state and returns // a diff. Diff( - ResourceState, + *ResourceState, map[string]interface{}) (ResourceDiff, error) } diff --git a/terraform/resource_provider_mock.go b/terraform/resource_provider_mock.go index 6cb08448b..6854dc732 100644 --- a/terraform/resource_provider_mock.go +++ b/terraform/resource_provider_mock.go @@ -11,9 +11,9 @@ type MockResourceProvider struct { ConfigureReturnWarnings []string ConfigureReturnError error DiffCalled bool - DiffState ResourceState + DiffState *ResourceState DiffDesired map[string]interface{} - DiffFn func(ResourceState, map[string]interface{}) (ResourceDiff, error) + DiffFn func(*ResourceState, map[string]interface{}) (ResourceDiff, error) DiffReturn ResourceDiff DiffReturnError error ResourcesCalled bool @@ -27,7 +27,7 @@ func (p *MockResourceProvider) Configure(c map[string]interface{}) ([]string, er } func (p *MockResourceProvider) Diff( - state ResourceState, + state *ResourceState, desired map[string]interface{}) (ResourceDiff, error) { p.DiffCalled = true p.DiffState = state diff --git a/terraform/state.go b/terraform/state.go index 4c65c16e3..e40e13b05 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -1,10 +1,21 @@ package terraform +import ( + "sync" +) + // State keeps track of a snapshot state-of-the-world that Terraform // can use to keep track of what real world resources it is actually // managing. type State struct { - resources map[string]ResourceState + resources map[string]*ResourceState + once sync.Once +} + +func (s *State) init() { + s.once.Do(func() { + s.resources = make(map[string]*ResourceState) + }) } // ResourceState holds the state of a resource that is used so that @@ -33,7 +44,7 @@ type ResourceState struct { // computeID. func (s *ResourceState) MergeDiff( d map[string]*ResourceAttrDiff, - computedID string) ResourceState { + computedID string) *ResourceState { var result ResourceState if s != nil { result = *s @@ -54,5 +65,5 @@ func (s *ResourceState) MergeDiff( result.Attributes[k] = diff.New } - return result + return &result } diff --git a/terraform/terraform.go b/terraform/terraform.go index d8201687a..f1430fe4b 100644 --- a/terraform/terraform.go +++ b/terraform/terraform.go @@ -141,7 +141,19 @@ func (t *Terraform) Refresh(*State) (*State, error) { func (t *Terraform) diffWalkFn( state *State, result *Diff) depgraph.WalkFunc { - var resultLock sync.Mutex + var l sync.RWMutex + + // Initialize the state since we always read it + if state == nil { + state = new(State) + state.init() + } + + // Initialize the result diff so we can write to it + result.init() + + // This is the value that will be used for computed properties + computedId := "computed" return func(n *depgraph.Noun) error { // If it is the root node, ignore @@ -155,7 +167,14 @@ func (t *Terraform) diffWalkFn( panic(fmt.Sprintf("No provider for resource: %s", r.Id())) } - var rs ResourceState + l.RLock() + rs := state.resources[r.Id()] + vs := t.replaceVariables(r, state) + if len(vs) > 0 { + r = r.ReplaceVariables(vs) + } + l.RUnlock() + diff, err := p.Diff(rs, r.Config) if err != nil { return err @@ -166,16 +185,38 @@ func (t *Terraform) diffWalkFn( return nil } - // Acquire a lock and modify the resulting diff - resultLock.Lock() - defer resultLock.Unlock() - result.init() + // Acquire a lock since this function is called in parallel + l.Lock() + defer l.Unlock() + + // Update the resulting diff result.Resources[r.Id()] = diff.Attributes + // Update the state for child dependencies + state.resources[r.Id()] = rs.MergeDiff(diff.Attributes, computedId) + return nil } } +// replaceVariables will return the mapping of variable replacements to +// apply for a given resource with a given state. +func (t *Terraform) replaceVariables( + r *config.Resource, + s *State) map[string]string { + result := make(map[string]string) + for k, v := range t.variables { + result[k] = v + } + for n, rs := range s.resources { + for attrK, attrV := range rs.Attributes { + result[fmt.Sprintf("%s.%s", n, attrK)] = attrV + } + } + + return result +} + // matchingPrefixes takes a resource type and a set of resource // providers we know about by prefix and returns a list of prefixes // that might be valid for that resource. diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index f60a6ae46..09f157484 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -141,7 +141,7 @@ func testProviderFunc(n string, rs []string) ResourceProviderFactory { return func() (ResourceProvider, error) { diffFn := func( - _ ResourceState, + _ *ResourceState, c map[string]interface{}) (ResourceDiff, error) { var diff ResourceDiff diff.Attributes = make(map[string]*ResourceAttrDiff) @@ -205,7 +205,7 @@ func testTerraform(t *testing.T, name string) *Terraform { const testTerraformDiffStr = ` aws_instance.bar - foo: "" => "${aws_instance.foo.num}" + foo: "" => "2" aws_instance.foo num: "" => "2" `