terraform: node referenceable name from state shuldn't contain path

Fixes #11749

I'm **really** surprised this didn't come up earlier.

When only the state is available for a node, the advertised
referenceable name (the name used for dependency connections) included
the module path. This module path is automatically prepended to the
name. This means that probably every non-root resource for state-only
operations (destroys) didn't order properly.

This fixes that by omitting the path properly.

Multiple tests added to verify both graph correctness as well as a
higher level context test.

Will backport to 0.8.x
This commit is contained in:
Mitchell Hashimoto 2017-02-07 20:14:38 -08:00
parent af61d566c2
commit 8ed9bdfedc
No known key found for this signature in database
GPG Key ID: 744E147AA52F5B0A
3 changed files with 148 additions and 1 deletions

View File

@ -1329,6 +1329,82 @@ func testContext2Apply_destroyDependsOnStateOnly(t *testing.T) {
} }
} }
// Test that destroy ordering is correct with dependencies only
// in the state within a module (GH-11749)
func TestContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T) {
// It is possible for this to be racy, so we loop a number of times
// just to check.
for i := 0; i < 10; i++ {
testContext2Apply_destroyDependsOnStateOnlyModule(t)
}
}
func testContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T) {
m := testModule(t, "empty")
p := testProvider("aws")
p.ApplyFn = testApplyFn
p.DiffFn = testDiffFn
state := &State{
Modules: []*ModuleState{
&ModuleState{
Path: []string{"root", "child"},
Resources: map[string]*ResourceState{
"aws_instance.foo": &ResourceState{
Type: "aws_instance",
Primary: &InstanceState{
ID: "foo",
Attributes: map[string]string{},
},
},
"aws_instance.bar": &ResourceState{
Type: "aws_instance",
Primary: &InstanceState{
ID: "bar",
Attributes: map[string]string{},
},
Dependencies: []string{"aws_instance.foo"},
},
},
},
},
}
// Record the order we see Apply
var actual []string
var actualLock sync.Mutex
p.ApplyFn = func(
info *InstanceInfo, _ *InstanceState, _ *InstanceDiff) (*InstanceState, error) {
actualLock.Lock()
defer actualLock.Unlock()
actual = append(actual, info.Id)
return nil, nil
}
ctx := testContext2(t, &ContextOpts{
Module: m,
Providers: map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p),
},
State: state,
Destroy: true,
Parallelism: 1, // To check ordering
})
if _, err := ctx.Plan(); err != nil {
t.Fatalf("err: %s", err)
}
if _, err := ctx.Apply(); err != nil {
t.Fatalf("err: %s", err)
}
expected := []string{"aws_instance.bar", "aws_instance.foo"}
if !reflect.DeepEqual(actual, expected) {
t.Fatalf("bad: %#v", actual)
}
}
func TestContext2Apply_dataBasic(t *testing.T) { func TestContext2Apply_dataBasic(t *testing.T) {
m := testModule(t, "apply-data-basic") m := testModule(t, "apply-data-basic")
p := testProvider("null") p := testProvider("null")

View File

@ -210,6 +210,76 @@ func TestApplyGraphBuilder_doubleCBD(t *testing.T) {
} }
} }
// This tests the ordering of two resources being destroyed that depend
// on each other from only state. GH-11749
func TestApplyGraphBuilder_destroyStateOnly(t *testing.T) {
diff := &Diff{
Modules: []*ModuleDiff{
&ModuleDiff{
Path: []string{"root", "child"},
Resources: map[string]*InstanceDiff{
"aws_instance.A": &InstanceDiff{
Destroy: true,
},
"aws_instance.B": &InstanceDiff{
Destroy: true,
},
},
},
},
}
state := &State{
Modules: []*ModuleState{
&ModuleState{
Path: []string{"root", "child"},
Resources: map[string]*ResourceState{
"aws_instance.A": &ResourceState{
Type: "aws_instance",
Primary: &InstanceState{
ID: "foo",
Attributes: map[string]string{},
},
},
"aws_instance.B": &ResourceState{
Type: "aws_instance",
Primary: &InstanceState{
ID: "bar",
Attributes: map[string]string{},
},
Dependencies: []string{"aws_instance.A"},
},
},
},
},
}
b := &ApplyGraphBuilder{
Module: testModule(t, "empty"),
Diff: diff,
State: state,
Providers: []string{"aws"},
DisableReduce: true,
}
g, err := b.Build(RootModulePath)
if err != nil {
t.Fatalf("err: %s", err)
}
t.Logf("Graph: %s", g.String())
if !reflect.DeepEqual(g.Path, RootModulePath) {
t.Fatalf("bad: %#v", g.Path)
}
testGraphHappensBefore(
t, g,
"module.child.aws_instance.B (destroy)",
"module.child.aws_instance.A (destroy)")
}
// This tests the ordering of destroying a single count of a resource. // This tests the ordering of destroying a single count of a resource.
func TestApplyGraphBuilder_destroyCount(t *testing.T) { func TestApplyGraphBuilder_destroyCount(t *testing.T) {
diff := &Diff{ diff := &Diff{

View File

@ -53,7 +53,8 @@ func (n *NodeAbstractResource) ReferenceableName() []string {
id = n.Config.Id() id = n.Config.Id()
} else if n.Addr != nil { } else if n.Addr != nil {
addrCopy := n.Addr.Copy() addrCopy := n.Addr.Copy()
addrCopy.Index = -1 addrCopy.Path = nil // ReferenceTransformer handles paths
addrCopy.Index = -1 // We handle indexes below
id = addrCopy.String() id = addrCopy.String()
} else { } else {
// No way to determine our type.name, just return // No way to determine our type.name, just return