Skip to content

Commit

Permalink
refresh should not modify state deps
Browse files Browse the repository at this point in the history
Don't modify dependencies or depends_on during refresh, just restore
what was already in the instance state.
  • Loading branch information
jbardin committed Nov 7, 2019
1 parent ba047ca commit af12cfe
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 86 deletions.
62 changes: 27 additions & 35 deletions terraform/context_refresh_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package terraform

import (
"fmt"
"reflect"
"regexp"
"sort"
Expand Down Expand Up @@ -1965,24 +1966,10 @@ func TestContext2Refresh_dataResourceDependsOn(t *testing.T) {
}
}

// verify that dependencies are updated in the state during refresh
func TestRefresh_updateDependencies(t *testing.T) {
// verify that dependencies are maintained in the state during refresh
func TestRefresh_maintainDependencies(t *testing.T) {
state := states.NewState()
root := state.EnsureModule(addrs.RootModuleInstance)
root.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "aws_instance",
Name: "foo",
}.Instance(addrs.NoKey),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"foo"}`),
},
addrs.ProviderConfig{
Type: "aws",
}.Absolute(addrs.RootModuleInstance),
)
root.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Expand All @@ -1993,7 +1980,7 @@ func TestRefresh_updateDependencies(t *testing.T) {
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"bar","foo":"foo"}`),
Dependencies: []addrs.AbsResource{
// Existing dependencies should not be removed during refresh
// Existing dependencies should not be changed during refresh
{
Module: addrs.RootModuleInstance,
Resource: addrs.Resource{
Expand All @@ -2003,6 +1990,15 @@ func TestRefresh_updateDependencies(t *testing.T) {
},
},
},
DependsOn: []addrs.Referenceable{
addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "aws_instance",
Name: "zab",
},
},
},
},
addrs.ProviderConfig{
Type: "aws",
Expand All @@ -2012,10 +2008,6 @@ func TestRefresh_updateDependencies(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "aws_instance" "bar" {
foo = aws_instance.foo.id
}
resource "aws_instance" "foo" {
}`,
})

Expand All @@ -2038,19 +2030,19 @@ resource "aws_instance" "foo" {
t.Fatalf("plan errors: %s", diags.Err())
}

expect := strings.TrimSpace(`
aws_instance.bar:
ID = bar
provider = provider.aws
foo = foo
Dependencies:
aws_instance.baz
aws_instance.foo
aws_instance.foo:
ID = foo
provider = provider.aws
`)
bar := result.RootModule().ResourceInstance(addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "aws_instance",
Name: "bar",
}.Instance(addrs.NoKey))

checkStateString(t, result, expect)
depString := fmt.Sprintf("%v", bar.Current.Dependencies)
doString := fmt.Sprintf("%v", bar.Current.DependsOn)
if depString != "[aws_instance.baz]" {
t.Fatal("expected Dependencies of [aws_instance.baz], got:", depString)
}

if doString != "[aws_instance.zab]" {
t.Fatal("expected DependsOn of [aws_isntance.zab], got:", doString)
}
}
1 change: 1 addition & 0 deletions terraform/eval_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) {
newState.Value = resp.NewState
newState.Private = resp.Private
newState.Dependencies = state.Dependencies
newState.DependsOn = state.DependsOn

// Call post-refresh hook
err = ctx.Hook(func(h Hook) (HookAction, error) {
Expand Down
45 changes: 3 additions & 42 deletions terraform/eval_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package terraform
import (
"fmt"
"log"
"sort"

"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/configs"
Expand Down Expand Up @@ -232,6 +231,9 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) {
if n.Dependencies != nil {
log.Printf("[TRACE] EvalWriteState: recording %d dependencies for %s", len(*n.Dependencies), absAddr)
obj.Dependencies = *n.Dependencies

// If we have Dependencies, then we need to replace any old DependsOn values
obj.DependsOn = nil
}

if n.ProviderSchema == nil || *n.ProviderSchema == nil {
Expand Down Expand Up @@ -484,44 +486,3 @@ func (n *EvalForgetResourceState) Eval(ctx EvalContext) (interface{}, error) {

return nil, nil
}

// EvalRefreshDependencies is an EvalNode implementation that appends any newly
// found dependencies to those saved in the state. The existing dependencies
// are retained, as they may be missing from the config, and will be required
// for the updates and destroys during the next apply.
type EvalRefreshDependencies struct {
// Prior State
State **states.ResourceInstanceObject
// Dependencies to write to the new state
Dependencies *[]addrs.AbsResource
}

func (n *EvalRefreshDependencies) Eval(ctx EvalContext) (interface{}, error) {
state := *n.State
if state == nil {
// no existing state to append
return nil, nil
}

depMap := make(map[string]addrs.AbsResource)
for _, d := range *n.Dependencies {
depMap[d.String()] = d
}

for _, d := range state.Dependencies {
depMap[d.String()] = d
}

deps := make([]addrs.AbsResource, 0, len(depMap))
for _, d := range depMap {
deps = append(deps, d)
}

sort.Slice(deps, func(i, j int) bool {
return deps[i].String() < deps[j].String()
})

*n.Dependencies = deps

return nil, nil
}
1 change: 0 additions & 1 deletion terraform/graph_builder_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer {
// Connect so that the references are ready for targeting. We'll
// have to connect again later for providers and so on.
&ReferenceTransformer{},
&AttachDependenciesTransformer{},

// Target
&TargetsTransformer{
Expand Down
11 changes: 3 additions & 8 deletions terraform/node_resource_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN
var provider providers.Interface
var providerSchema *ProviderSchema
var state *states.ResourceInstanceObject
//var dependencies []addrs.AbsResource
//var dependsOn []addrs.Referenceable

// This happened during initial development. All known cases were
// fixed and tested but as a sanity check let's assert here.
Expand All @@ -210,13 +212,7 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN
Addr: addr.Resource,
Provider: &provider,
ProviderSchema: &providerSchema,

Output: &state,
},

&EvalRefreshDependencies{
State: &state,
Dependencies: &n.Dependencies,
Output: &state,
},

&EvalRefresh{
Expand All @@ -233,7 +229,6 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN
ProviderAddr: n.ResolvedProvider,
ProviderSchema: &providerSchema,
State: &state,
Dependencies: &n.Dependencies,
},
},
}
Expand Down

0 comments on commit af12cfe

Please sign in to comment.