From bfb770ee89aeed066c99e40fd525757bb1b37488 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Wed, 11 Nov 2015 12:35:35 -0500 Subject: [PATCH 1/2] Add failing test for targeted destroy on orphan This replicates the issue reported in #3852. --- terraform/context_plan_test.go | 47 +++++++++++++++++++ .../plan-targeted-orphan/main.tf | 6 +++ 2 files changed, 53 insertions(+) create mode 100644 terraform/test-fixtures/plan-targeted-orphan/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index db6f2457725d..e91fc77472e8 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1627,6 +1627,53 @@ STATE: } } +func TestContext2Plan_targetedOrphan(t *testing.T) { + m := testModule(t, "plan-targeted-orphan") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.orphan": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-789xyz", + }, + }, + }, + }, + }, + }, + Destroy: true, + Targets: []string{"aws_instance.orphan"}, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(`DIFF: + +DESTROY: aws_instance.orphan + +STATE: + +aws_instance.orphan: + ID = i-789xyz`) + if actual != expected { + t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) + } +} + func TestContext2Plan_provider(t *testing.T) { m := testModule(t, "plan-provider") p := testProvider("aws") diff --git a/terraform/test-fixtures/plan-targeted-orphan/main.tf b/terraform/test-fixtures/plan-targeted-orphan/main.tf new file mode 100644 index 000000000000..f2020858b148 --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-orphan/main.tf @@ -0,0 +1,6 @@ +# This resource was previously "created" and the fixture represents +# it being destroyed subsequently + +/*resource "aws_instance" "orphan" {*/ + /*foo = "bar"*/ +/*}*/ From 38bb9f24161bf503f43c21949b367a986153248f Mon Sep 17 00:00:00 2001 From: James Nugent Date: Fri, 13 Nov 2015 13:17:14 -0600 Subject: [PATCH 2/2] Allow targeting of orphan nodes Fixes #3852. We now run the OrphanTransformer even when targeting, and pass it the list of targets following resource expansion. --- terraform/graph_builder.go | 5 ++-- terraform/graph_config_node_resource.go | 6 ++--- terraform/transform_orphan.go | 33 +++++++++++++++++-------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index ca99667016ae..7963bcbf4f7a 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -105,9 +105,8 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { // Create all our resources from the configuration and state &ConfigTransformer{Module: b.Root}, &OrphanTransformer{ - State: b.State, - Module: b.Root, - Targeting: len(b.Targets) > 0, + State: b.State, + Module: b.Root, }, // Output-related transformations diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 2bf0e4568a46..9fc696c2a3aa 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -163,9 +163,9 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) // expand orphans, which have all the same semantics in a destroy // as a primary. steps = append(steps, &OrphanTransformer{ - State: state, - View: n.Resource.Id(), - Targeting: len(n.Targets) > 0, + State: state, + View: n.Resource.Id(), + Targets: n.Targets, }) steps = append(steps, &DeposedTransformer{ diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index 45ea050ba37c..13e8fbf941c2 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -2,7 +2,7 @@ package terraform import ( "fmt" - "log" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" @@ -29,7 +29,7 @@ type OrphanTransformer struct { // Targets are user-specified resources to target. We need to be aware of // these so we don't improperly identify orphans when they've just been // filtered out of the graph via targeting. - Targeting bool + Targets []ResourceAddress // View, if non-nil will set a view on the module state. View string @@ -41,13 +41,6 @@ func (t *OrphanTransformer) Transform(g *Graph) error { return nil } - if t.Targeting { - log.Printf("Skipping orphan transformer because we have targets.") - // If we are in a run where we are targeting nodes, we won't process - // orphans for this run. - return nil - } - // Build up all our state representatives resourceRep := make(map[string]struct{}) for _, v := range g.Vertices() { @@ -74,8 +67,24 @@ func (t *OrphanTransformer) Transform(g *Graph) error { state = state.View(t.View) } - // Go over each resource orphan and add it to the graph. resourceOrphans := state.Orphans(config) + if len(t.Targets) > 0 { + var targetedOrphans []string + for _, o := range resourceOrphans { + targeted := false + for _, t := range t.Targets { + prefix := fmt.Sprintf("%s.%s.%d", t.Type, t.Name, t.Index) + if strings.HasPrefix(o, prefix) { + targeted = true + } + } + if targeted { + targetedOrphans = append(targetedOrphans, o) + } + } + resourceOrphans = targetedOrphans + } + resourceVertexes = make([]dag.Vertex, len(resourceOrphans)) for i, k := range resourceOrphans { // If this orphan is represented by some other node somehow, @@ -173,6 +182,10 @@ type graphNodeOrphanResource struct { dependentOn []string } +func (n *graphNodeOrphanResource) ResourceAddress() *ResourceAddress { + return n.ResourceAddress() +} + func (n *graphNodeOrphanResource) DependableName() []string { return []string{n.dependableName()} }