Skip to content

Commit

Permalink
Merge pull request #22454 from pselle/nokeymoregraph
Browse files Browse the repository at this point in the history
Add edge when calculating for_each orphans
  • Loading branch information
Pam Selle authored Aug 13, 2019
2 parents 15c5579 + e6817f6 commit e293176
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 1 deletion.
2 changes: 1 addition & 1 deletion states/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (s *SyncState) MaybeFixUpResourceInstanceAddressForCount(addr addrs.AbsReso
}

// SetResourceInstanceCurrent saves the given instance object as the current
// generation of the resource instance with the given address, simulataneously
// generation of the resource instance with the given address, simultaneously
// updating the recorded provider configuration address, dependencies, and
// resource EachMode.
//
Expand Down
27 changes: 27 additions & 0 deletions terraform/transform_orphan_count.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,29 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error {
}

func (t *OrphanResourceCountTransformer) transformForEach(haveKeys map[addrs.InstanceKey]struct{}, g *Graph) error {
// If there is a NoKey node, add this to the graph first,
// so that we can create edges to it in subsequent (StringKey) nodes.
// This is because the last item determines the resource mode for the whole resource,
// (see SetResourceInstanceCurrent for more information) and we need to evaluate
// an orphaned (NoKey) resource before the in-memory state is updated
// to deal with a new for_each resource
_, hasNoKeyNode := haveKeys[addrs.NoKey]
var noKeyNode dag.Vertex
if hasNoKeyNode {
abstract := NewNodeAbstractResourceInstance(t.Addr.Instance(addrs.NoKey))
noKeyNode = abstract
if f := t.Concrete; f != nil {
noKeyNode = f(abstract)
}
g.Add(noKeyNode)
}

for key := range haveKeys {
// If the key is no-key, we have already added it, so skip
if key == addrs.NoKey {
continue
}

s, _ := key.(addrs.StringKey)
// If the key is present in our current for_each, carry on
if _, ok := t.ForEach[string(s)]; ok {
Expand All @@ -64,6 +86,11 @@ func (t *OrphanResourceCountTransformer) transformForEach(haveKeys map[addrs.Ins
}
log.Printf("[TRACE] OrphanResourceCount(non-zero): adding %s as %T", t.Addr, node)
g.Add(node)

// Add edge to noKeyNode if it exists
if hasNoKeyNode {
g.Connect(dag.BasicEdge(node, noKeyNode))
}
}
return nil
}
Expand Down
76 changes: 76 additions & 0 deletions terraform/transform_orphan_count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"

"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/states"
"github.com/zclconf/go-cty/cty"
)

func TestOrphanResourceCountTransformer(t *testing.T) {
Expand Down Expand Up @@ -331,6 +333,74 @@ func TestOrphanResourceCountTransformer_zeroAndNoneCount(t *testing.T) {
}
}

// When converting from a NoEach mode to an EachMap via a switch to for_each,
// an edge is necessary to ensure that the map-key'd instances
// are evaluated after the NoKey resource, because the final instance evaluated
// sets the whole resource's EachMode.
func TestOrphanResourceCountTransformer_ForEachEdgesAdded(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
// "bar" key'd resource
s.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "aws_instance",
Name: "foo",
}.Instance(addrs.StringKey("bar")).Absolute(addrs.RootModuleInstance),
&states.ResourceInstanceObjectSrc{
AttrsFlat: map[string]string{
"id": "foo",
},
Status: states.ObjectReady,
},
addrs.ProviderConfig{
Type: "aws",
}.Absolute(addrs.RootModuleInstance),
)

// NoKey'd resource
s.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "aws_instance",
Name: "foo",
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
&states.ResourceInstanceObjectSrc{
AttrsFlat: map[string]string{
"id": "foo",
},
Status: states.ObjectReady,
},
addrs.ProviderConfig{
Type: "aws",
}.Absolute(addrs.RootModuleInstance),
)
})

g := Graph{Path: addrs.RootModuleInstance}

{
tf := &OrphanResourceCountTransformer{
Concrete: testOrphanResourceConcreteFunc,
// No keys in this ForEach ensure both our resources end
// up orphaned in this test
ForEach: map[string]cty.Value{},
Addr: addrs.RootModuleInstance.Resource(
addrs.ManagedResourceMode, "aws_instance", "foo",
),
State: state,
}
if err := tf.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}

actual := strings.TrimSpace(g.String())
expected := strings.TrimSpace(testTransformOrphanResourceForEachStr)
if actual != expected {
t.Fatalf("bad:\n\n%s", actual)
}
}

const testTransformOrphanResourceCountBasicStr = `
aws_instance.foo[2] (orphan)
`
Expand All @@ -355,3 +425,9 @@ aws_instance.foo[0] (orphan)
const testTransformOrphanResourceCountZeroAndNoneCountStr = `
aws_instance.foo (orphan)
`

const testTransformOrphanResourceForEachStr = `
aws_instance.foo (orphan)
aws_instance.foo["bar"] (orphan)
aws_instance.foo (orphan)
`

0 comments on commit e293176

Please sign in to comment.