From 3b2282ca5728955059c4aeba6671144456820f9c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 29 Nov 2016 09:16:18 -0800 Subject: [PATCH] terraform: Destroy node should only include deposed for specific index Fixes #10338 The destruction step for a resource was included the deposed resources for _all_ resources with that name (ignoring the "index"). For example: `aws_instance.foo.0` was including destroying deposed for `aws_instance.foo.1`. This changes the config to the deposed transformer to properly include that index. This change includes a larger change of changing `stateId` to include the index. This affected more parts but was ultimately the issue in question. --- terraform/node_resource_apply.go | 3 -- terraform/node_resource_destroy.go | 7 +-- terraform/node_resource_destroy_test.go | 67 ++++++++++++++++++++++++ terraform/node_resource_plan_destroy.go | 7 --- terraform/node_resource_plan_instance.go | 3 -- terraform/node_resource_plan_orphan.go | 7 --- terraform/resource_address.go | 3 ++ terraform/resource_address_test.go | 4 +- 8 files changed, 74 insertions(+), 27 deletions(-) create mode 100644 terraform/node_resource_destroy_test.go diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 87bc205d0..811aa55eb 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -23,9 +23,6 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { // stateId is the ID to put into the state stateId := addr.stateId() - if addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) - } // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index b15a0eb46..23652200d 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -48,7 +48,7 @@ func (n *NodeDestroyResource) References() []string { // GraphNodeDynamicExpandable func (n *NodeDestroyResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // If we have no config we do nothing - if n.Config == nil { + if n.Addr == nil { return nil, nil } @@ -62,7 +62,7 @@ func (n *NodeDestroyResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // We want deposed resources in the state to be destroyed steps = append(steps, &DeposedTransformer{ State: state, - View: n.Config.Id(), + View: n.Addr.stateId(), }) // Target @@ -85,9 +85,6 @@ func (n *NodeDestroyResource) DynamicExpand(ctx EvalContext) (*Graph, error) { func (n *NodeDestroyResource) EvalTree() EvalNode { // stateId is the ID to put into the state stateId := n.Addr.stateId() - if n.Addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, n.Addr.Index) - } // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ diff --git a/terraform/node_resource_destroy_test.go b/terraform/node_resource_destroy_test.go new file mode 100644 index 000000000..869105fa2 --- /dev/null +++ b/terraform/node_resource_destroy_test.go @@ -0,0 +1,67 @@ +package terraform + +import ( + "strings" + "sync" + "testing" +) + +func TestNodeDestroyResourceDynamicExpand_deposedCount(t *testing.T) { + var stateLock sync.RWMutex + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar.0": &ResourceState{ + Type: "aws_instance", + Deposed: []*InstanceState{ + &InstanceState{ + ID: "foo", + }, + }, + }, + "aws_instance.bar.1": &ResourceState{ + Type: "aws_instance", + Deposed: []*InstanceState{ + &InstanceState{ + ID: "bar", + }, + }, + }, + }, + }, + }, + } + + addr, err := parseResourceAddressInternal("aws_instance.bar.0") + if err != nil { + t.Fatalf("err: %s", err) + } + + m := testModule(t, "apply-cbd-count") + n := &NodeDestroyResource{ + NodeAbstractResource: &NodeAbstractResource{ + Addr: addr, + ResourceState: state.Modules[0].Resources["aws_instance.bar.0"], + Config: m.Config().Resources[0], + }, + } + + g, err := n.DynamicExpand(&MockEvalContext{ + PathPath: []string{"root"}, + StateState: state, + StateLock: &stateLock, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(` +aws_instance.bar.0 (deposed #0) +`) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} diff --git a/terraform/node_resource_plan_destroy.go b/terraform/node_resource_plan_destroy.go index 041c78655..9b02362b6 100644 --- a/terraform/node_resource_plan_destroy.go +++ b/terraform/node_resource_plan_destroy.go @@ -1,9 +1,5 @@ package terraform -import ( - "fmt" -) - // NodePlanDestroyableResource represents a resource that is "applyable": // it is ready to be applied and is represented by a diff. type NodePlanDestroyableResource struct { @@ -21,9 +17,6 @@ func (n *NodePlanDestroyableResource) EvalTree() EvalNode { // stateId is the ID to put into the state stateId := addr.stateId() - if addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) - } // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 9dafd22ba..340c3ae1b 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -19,9 +19,6 @@ func (n *NodePlannableResourceInstance) EvalTree() EvalNode { // stateId is the ID to put into the state stateId := addr.stateId() - if addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) - } // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index 90641ca0f..73d6e41f5 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -1,9 +1,5 @@ package terraform -import ( - "fmt" -) - // NodePlannableResourceOrphan represents a resource that is "applyable": // it is ready to be applied and is represented by a diff. type NodePlannableResourceOrphan struct { @@ -20,9 +16,6 @@ func (n *NodePlannableResourceOrphan) EvalTree() EvalNode { // stateId is the ID to put into the state stateId := addr.stateId() - if addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) - } // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 2dc2058f5..aa35dbb41 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -102,6 +102,9 @@ func (r *ResourceAddress) stateId() string { default: panic(fmt.Errorf("unknown resource mode: %s", r.Mode)) } + if r.Index >= 0 { + result += fmt.Sprintf(".%d", r.Index) + } return result } diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index 2604441fe..76c356129 100644 --- a/terraform/resource_address_test.go +++ b/terraform/resource_address_test.go @@ -571,7 +571,7 @@ func TestResourceAddressStateId(t *testing.T) { "aws_instance.foo", }, - "basic resource ignores count": { + "basic resource with index": { &ResourceAddress{ Mode: config.ManagedResourceMode, Type: "aws_instance", @@ -579,7 +579,7 @@ func TestResourceAddressStateId(t *testing.T) { InstanceType: TypePrimary, Index: 2, }, - "aws_instance.foo", + "aws_instance.foo.2", }, "data resource": {