From 19350d617d8ba96a5b52bd803340e296995e5cca Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 8 Nov 2016 09:35:57 -0800 Subject: [PATCH] terraform: references can have backups terraform: more specific resource references terraform: outputs need to know about the new reference format terraform: resources w/o a config still have a referencable name --- terraform/context_apply_test.go | 12 +++- terraform/node_output.go | 8 ++- terraform/node_resource_abstract.go | 38 +++++++++- terraform/resource_address.go | 4 ++ terraform/transform_destroy_edge.go | 88 +++++++++++++++++------- terraform/transform_destroy_edge_test.go | 2 +- terraform/transform_reference.go | 66 ++++++++++++------ terraform/transform_reference_test.go | 63 +++++++++++++++++ 8 files changed, 229 insertions(+), 52 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 25e37f4b0..ece2cd792 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3489,6 +3489,8 @@ func TestContext2Apply_destroyOrder(t *testing.T) { t.Fatalf("err: %s", err) } + t.Logf("State 1: %s", state) + // Next, plan and apply config-less to force a destroy with "apply" h.Active = true ctx = testContext2(t, &ContextOpts{ @@ -3698,8 +3700,10 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { }) // First plan and apply a create operation - if _, err := ctx.Plan(); err != nil { + if p, err := ctx.Plan(); err != nil { t.Fatalf("plan err: %s", err) + } else { + t.Logf("Step 1 plan: %s", p) } state, err = ctx.Apply() @@ -3733,6 +3737,8 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { t.Fatalf("destroy plan err: %s", err) } + t.Logf("Step 2 plan: %s", plan) + var buf bytes.Buffer if err := WritePlan(plan, &buf); err != nil { t.Fatalf("plan write err: %s", err) @@ -3756,6 +3762,8 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { if err != nil { t.Fatalf("destroy apply err: %s", err) } + + t.Logf("Step 2 state: %s", state) } //Test that things were destroyed @@ -3766,7 +3774,7 @@ module.child: `) if actual != expected { - t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual) + t.Fatalf("expected:\n\n%s\n\nactual:\n\n%s", expected, actual) } } diff --git a/terraform/node_output.go b/terraform/node_output.go index c10c6e4f8..3e6001618 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "strings" "github.com/hashicorp/terraform/config" ) @@ -38,7 +39,12 @@ func (n *NodeApplyableOutput) References() []string { var result []string result = append(result, ReferencesFromConfig(n.Config.RawConfig)...) for _, v := range result { - result = append(result, v+".destroy") + split := strings.Split(v, "/") + for i, s := range split { + split[i] = s + ".destroy" + } + + result = append(result, strings.Join(split, "/")) } return result diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 9ba303a6e..f502547d7 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -1,6 +1,8 @@ package terraform import ( + "fmt" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" ) @@ -43,11 +45,43 @@ func (n *NodeAbstractResource) Path() []string { // GraphNodeReferenceable func (n *NodeAbstractResource) ReferenceableName() []string { - if n.Config == nil { + // We always are referenceable as "type.name" as long as + // we have a config or address. Determine what that value is. + var id string + if n.Config != nil { + id = n.Config.Id() + } else if n.Addr != nil { + addrCopy := n.Addr.Copy() + addrCopy.Index = -1 + id = addrCopy.String() + } else { + // No way to determine our type.name, just return return nil } - return []string{n.Config.Id()} + var result []string + + // Always include our own ID. This is primarily for backwards + // compatibility with states that didn't yet support the more + // specific dep string. + result = append(result, id) + + // We represent all multi-access + result = append(result, fmt.Sprintf("%s.*", id)) + + // We represent either a specific number, or all numbers + suffix := "N" + if n.Addr != nil { + idx := n.Addr.Index + if idx == -1 { + idx = 0 + } + + suffix = fmt.Sprintf("%d", idx) + } + result = append(result, fmt.Sprintf("%s.%s", id, suffix)) + + return result } // GraphNodeReferencer diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 06e943f9e..2dc2058f5 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -29,6 +29,10 @@ type ResourceAddress struct { // Copy returns a copy of this ResourceAddress func (r *ResourceAddress) Copy() *ResourceAddress { + if r == nil { + return nil + } + n := &ResourceAddress{ Path: make([]string, 0, len(r.Path)), Index: r.Index, diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 972d9b6a2..dd6ed114c 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -120,10 +120,14 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { &AttachStateTransformer{State: t.State}, } - // Go through the all destroyers and find what they're destroying. - // Use this to find the dependencies, look up if any of them are being - // destroyed, and to make the proper edge. - for d, dns := range destroyers { + // Go through all the nodes being destroyed and create a graph. + // The resulting graph is only of things being CREATED. For example, + // following our example, the resulting graph would be: + // + // A, B (with no edges) + // + var tempG Graph + for d, _ := range destroyers { // d is what is being destroyed. We parse the resource address // which it came from it is a panic if this fails. addr, err := ParseResourceAddress(d) @@ -135,27 +139,48 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // find the dependencies we need to: build a graph and use the // attach config and state transformers then ask for references. node := &NodeAbstractResource{Addr: addr} - { - var g Graph - g.Add(node) - for _, s := range steps { - if err := s.Transform(&g); err != nil { - return err - } - } - } + tempG.Add(node) + } - // Get the references of the creation node. If it has none, - // then there are no edges to make here. - prefix := modulePrefixStr(normalizeModulePath(addr.Path)) - deps := modulePrefixList(node.References(), prefix) + // Run the graph transforms so we have the information we need to + // build references. + for _, s := range steps { + if err := s.Transform(&tempG); err != nil { + return err + } + } + + // Create a reference map for easy lookup + refMap := NewReferenceMap(tempG.Vertices()) + + // Go through all the nodes in the graph and determine what they + // depend on. + for _, v := range tempG.Vertices() { + // Find all the references + refs, _ := refMap.References(v) log.Printf( - "[TRACE] DestroyEdgeTransformer: creation of %q depends on %#v", - d, deps) - if len(deps) == 0 { + "[TRACE] DestroyEdgeTransformer: creation node %q references %v", + dag.VertexName(v), refs) + + // If we have no references, then we won't need to do anything + if len(refs) == 0 { continue } + // Get the destroy node for this. In the example of our struct, + // we are currently at B and we're looking for B_d. + rn, ok := v.(GraphNodeResource) + if !ok { + continue + } + + addr := rn.ResourceAddr() + if addr == nil { + continue + } + + dns := destroyers[addr.String()] + // We have dependencies, check if any are being destroyed // to build the list of things that we must depend on! // @@ -163,17 +188,28 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // // B_d => A_d => A => B // - // Then at this point in the algorithm we started with A_d, - // we built A (to get dependencies), and we found B. We're now looking - // to see if B_d exists. + // Then at this point in the algorithm we started with B_d, + // we built B (to get dependencies), and we found A. We're now looking + // to see if A_d exists. var depDestroyers []dag.Vertex - for _, d := range deps { - if ds, ok := destroyers[d]; ok { + for _, v := range refs { + rn, ok := v.(GraphNodeResource) + if !ok { + continue + } + + addr := rn.ResourceAddr() + if addr == nil { + continue + } + + key := addr.String() + if ds, ok := destroyers[key]; ok { for _, d := range ds { depDestroyers = append(depDestroyers, d.(dag.Vertex)) log.Printf( "[TRACE] DestroyEdgeTransformer: destruction of %q depends on %s", - addr.String(), dag.VertexName(d)) + key, dag.VertexName(d)) } } } diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go index 9489b5a5c..9475e5165 100644 --- a/terraform/transform_destroy_edge_test.go +++ b/terraform/transform_destroy_edge_test.go @@ -5,7 +5,7 @@ import ( "testing" ) -func TestDestroyEdgeTransformer(t *testing.T) { +func TestDestroyEdgeTransformer_basic(t *testing.T) { g := Graph{Path: RootModulePath} g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index d5bcdac33..1da835fbd 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "log" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" @@ -90,27 +91,37 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { var matches []dag.Vertex var missing []string prefix := m.prefix(v) - for _, n := range rn.References() { - n = prefix + n - parents, ok := m.references[n] - if !ok { - missing = append(missing, n) - continue - } - - // Make sure this isn't a self reference, which isn't included - selfRef := false - for _, p := range parents { - if p == v { - selfRef = true - break + for _, ns := range rn.References() { + found := false + for _, n := range strings.Split(ns, "/") { + n = prefix + n + parents, ok := m.references[n] + if !ok { + continue } - } - if selfRef { - continue + + // Mark that we found a match + found = true + + // Make sure this isn't a self reference, which isn't included + selfRef := false + for _, p := range parents { + if p == v { + selfRef = true + break + } + } + if selfRef { + continue + } + + matches = append(matches, parents...) + break } - matches = append(matches, parents...) + if !found { + missing = append(missing, ns) + } } return matches, missing @@ -233,8 +244,23 @@ func ReferenceFromInterpolatedVar(v config.InterpolatedVariable) []string { case *config.ModuleVariable: return []string{fmt.Sprintf("module.%s.output.%s", v.Name, v.Field)} case *config.ResourceVariable: - result := []string{v.ResourceId()} - return result + id := v.ResourceId() + + // If we have a multi-reference (splat), then we depend on ALL + // resources with this type/name. + if v.Multi && v.Index == -1 { + return []string{fmt.Sprintf("%s.*", id)} + } + + // Otherwise, we depend on a specific index. + idx := v.Index + if !v.Multi || v.Index == -1 { + idx = 0 + } + + // Depend on the index, as well as "N" which represents the + // un-expanded set of resources. + return []string{fmt.Sprintf("%s.%d/%s.N", id, idx, id)} case *config.UserVariable: return []string{fmt.Sprintf("var.%s", v.Name)} default: diff --git a/terraform/transform_reference_test.go b/terraform/transform_reference_test.go index 31cf4664d..544af6b04 100644 --- a/terraform/transform_reference_test.go +++ b/terraform/transform_reference_test.go @@ -88,6 +88,56 @@ func TestReferenceTransformer_path(t *testing.T) { } } +func TestReferenceTransformer_backup(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"C/A"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefBackupStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestReferenceTransformer_backupPrimary(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"C/A"}, + }) + g.Add(&graphNodeRefParentTest{ + NameValue: "C", + Names: []string{"C"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefBackupPrimaryStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestReferenceMapReferences(t *testing.T) { cases := map[string]struct { Nodes []dag.Vertex @@ -202,6 +252,19 @@ B A ` +const testTransformRefBackupStr = ` +A +B + A +` + +const testTransformRefBackupPrimaryStr = ` +A +B + C +C +` + const testTransformRefPathStr = ` A B