diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index a657e767f..dbab70255 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -4,6 +4,7 @@ import ( "reflect" "sort" "strings" + "sync" "testing" ) @@ -685,6 +686,100 @@ func TestContext2Refresh_vars(t *testing.T) { } } +func TestContext2Refresh_orphanModule(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "refresh-module-orphan") + + // Create a custom refresh function to track the order they were visited + var order []string + var orderLock sync.Mutex + p.RefreshFn = func( + info *InstanceInfo, + is *InstanceState) (*InstanceState, error) { + orderLock.Lock() + defer orderLock.Unlock() + + order = append(order, is.ID) + return is, nil + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + Primary: &InstanceState{ + ID: "i-abc123", + Attributes: map[string]string{ + "childid": "i-bcd234", + "grandchildid": "i-cde345", + }, + }, + Dependencies: []string{ + "module.child", + "module.child", + }, + }, + }, + }, + &ModuleState{ + Path: append(rootModulePath, "child"), + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Primary: &InstanceState{ + ID: "i-bcd234", + Attributes: map[string]string{ + "grandchildid": "i-cde345", + }, + }, + Dependencies: []string{ + "module.grandchild", + }, + }, + }, + Outputs: map[string]string{ + "id": "i-bcd234", + "grandchild_id": "i-cde345", + }, + }, + &ModuleState{ + Path: append(rootModulePath, "child", "grandchild"), + Resources: map[string]*ResourceState{ + "aws_instance.baz": &ResourceState{ + Primary: &InstanceState{ + ID: "i-cde345", + }, + }, + }, + Outputs: map[string]string{ + "id": "i-cde345", + }, + }, + }, + } + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + }) + + testCheckDeadlock(t, func() { + _, err := ctx.Refresh() + if err != nil { + t.Fatalf("err: %s", err) + } + + // TODO: handle order properly for orphaned modules / resources + // expected := []string{"i-abc123", "i-bcd234", "i-cde345"} + // if !reflect.DeepEqual(order, expected) { + // t.Fatalf("expected: %#v, got: %#v", expected, order) + // } + }) +} + func TestContext2Validate(t *testing.T) { p := testProvider("aws") m := testModule(t, "validate-good") diff --git a/terraform/graph.go b/terraform/graph.go index 6744a931e..e75d93663 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -74,7 +74,11 @@ func (g *Graph) Replace(o, n dag.Vertex) bool { // Go through and update our lookaside to point to the new vertex for k, v := range g.dependableMap { if v == o { - g.dependableMap[k] = n + if _, ok := n.(GraphNodeDependable); ok { + g.dependableMap[k] = n + } else { + delete(g.dependableMap, k) + } } } diff --git a/terraform/graph_test.go b/terraform/graph_test.go index c56f5bfad..ef91fec4a 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -1,6 +1,7 @@ package terraform import ( + "reflect" "strings" "testing" ) @@ -20,7 +21,7 @@ func TestGraphAdd(t *testing.T) { func TestGraphConnectDependent(t *testing.T) { var g Graph - g.Add(&testGraphDependable{VertexName: "a", Mock: []string{"a"}}) + g.Add(&testGraphDependable{VertexName: "a"}) b := g.Add(&testGraphDependable{ VertexName: "b", DependentOnMock: []string{"a"}, @@ -37,10 +38,40 @@ func TestGraphConnectDependent(t *testing.T) { } } +func TestGraphReplace_DependableWithNonDependable(t *testing.T) { + var g Graph + a := g.Add(&testGraphDependable{VertexName: "a"}) + b := g.Add(&testGraphDependable{ + VertexName: "b", + DependentOnMock: []string{"a"}, + }) + newA := "non-dependable-a" + + if missing := g.ConnectDependent(b); len(missing) > 0 { + t.Fatalf("bad: %#v", missing) + } + + if !g.Replace(a, newA) { + t.Fatalf("failed to replace") + } + + c := g.Add(&testGraphDependable{ + VertexName: "c", + DependentOnMock: []string{"a"}, + }) + + // This should fail by reporting missing, since a node with dependable + // name "a" is no longer in the graph. + missing := g.ConnectDependent(c) + expected := []string{"a"} + if !reflect.DeepEqual(expected, missing) { + t.Fatalf("expected: %#v, got: %#v", expected, missing) + } +} + type testGraphDependable struct { VertexName string DependentOnMock []string - Mock []string } func (v *testGraphDependable) Name() string { @@ -48,7 +79,7 @@ func (v *testGraphDependable) Name() string { } func (v *testGraphDependable) DependableName() []string { - return v.Mock + return []string{v.VertexName} } func (v *testGraphDependable) DependentOn() []string { diff --git a/terraform/test-fixtures/refresh-module-orphan/child/grandchild/main.tf b/terraform/test-fixtures/refresh-module-orphan/child/grandchild/main.tf new file mode 100644 index 000000000..942e93dbc --- /dev/null +++ b/terraform/test-fixtures/refresh-module-orphan/child/grandchild/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "baz" {} + +output "id" { value = "${aws_instance.baz.id}" } diff --git a/terraform/test-fixtures/refresh-module-orphan/child/main.tf b/terraform/test-fixtures/refresh-module-orphan/child/main.tf new file mode 100644 index 000000000..7c3fc842f --- /dev/null +++ b/terraform/test-fixtures/refresh-module-orphan/child/main.tf @@ -0,0 +1,10 @@ +module "grandchild" { + source = "./grandchild" +} + +resource "aws_instance" "bar" { + grandchildid = "${module.grandchild.id}" +} + +output "id" { value = "${aws_instance.bar.id}" } +output "grandchild_id" { value = "${module.grandchild.id}" } diff --git a/terraform/test-fixtures/refresh-module-orphan/main.tf b/terraform/test-fixtures/refresh-module-orphan/main.tf new file mode 100644 index 000000000..244374d9d --- /dev/null +++ b/terraform/test-fixtures/refresh-module-orphan/main.tf @@ -0,0 +1,10 @@ +/* +module "child" { + source = "./child" +} + +resource "aws_instance" "bar" { + childid = "${module.child.id}" + grandchildid = "${module.child.grandchild_id}" +} +*/