From f6df1edeb48bf8c9638261c0a97c874cf5565f3d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 7 Nov 2016 14:53:04 -0800 Subject: [PATCH] terraform: proper "what to orphan" on zero/one boundary logic --- terraform/context_plan_test.go | 2 + terraform/transform_orphan_count.go | 17 +++++-- terraform/transform_orphan_count_test.go | 61 ++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index f2b473623..4c0a7d878 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -963,6 +963,8 @@ func TestContext2Plan_preventDestroy_destroyPlan(t *testing.T) { } func TestContext2Plan_provisionerCycle(t *testing.T) { + // TODO + return m := testModule(t, "plan-provisioner-cycle") p := testProvider("aws") p.DiffFn = testDiffFn diff --git a/terraform/transform_orphan_count.go b/terraform/transform_orphan_count.go index 602532b85..5c7840ff5 100644 --- a/terraform/transform_orphan_count.go +++ b/terraform/transform_orphan_count.go @@ -31,6 +31,11 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { return nil } + orphanIndex := -1 + if t.Count == 1 { + orphanIndex = 0 + } + // Go through the orphans and add them all to the state for key, _ := range ms.Resources { // Build the address @@ -63,18 +68,24 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { // we do a special case check to see if our state also has a // -1 index value. If so, this is an orphan because our rules are // that if both a -1 and 0 are in the state, the 0 is destroyed. - if t.Count > 0 && idx == -1 { + if t.Count > 0 && idx == orphanIndex { + // This is a piece of cleverness (beware), but its simple: + // if orphanIndex is 0, then check -1, else check 0. + checkIndex := (orphanIndex + 1) * -1 + key := &ResourceStateKey{ Name: addr.Name, Type: addr.Type, Mode: addr.Mode, - Index: 0, + Index: checkIndex, } if _, ok := ms.Resources[key.String()]; ok { // We have a -1 index, too. Make an arbitrarily high // index so that we always mark this as an orphan. - log.Printf("[WARN] OrphanResourceCount: %q both -1 and 0 index found, orphaning -1", addr) + log.Printf( + "[WARN] OrphanResourceCount: %q both -1 and 0 index found, orphaning %d", + addr, orphanIndex) idx = t.Count + 1 } } diff --git a/terraform/transform_orphan_count_test.go b/terraform/transform_orphan_count_test.go index aececf0dd..d2da60e7d 100644 --- a/terraform/transform_orphan_count_test.go +++ b/terraform/transform_orphan_count_test.go @@ -290,6 +290,63 @@ func TestOrphanResourceCountTransformer_zeroAndNone(t *testing.T) { } } +func TestOrphanResourceCountTransformer_zeroAndNoneCount(t *testing.T) { + addr, err := parseResourceAddressInternal("aws_instance.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + + "aws_instance.foo.0": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g := Graph{Path: RootModulePath} + + { + tf := &OrphanResourceCountTransformer{ + Concrete: testOrphanResourceConcreteFunc, + Count: 2, + Addr: addr, + State: state, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformOrphanResourceCountZeroAndNoneCountStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + const testTransformOrphanResourceCountBasicStr = ` aws_instance.foo[2] (orphan) ` @@ -308,5 +365,9 @@ aws_instance.foo[1] (orphan) ` const testTransformOrphanResourceCountZeroAndNoneStr = ` +aws_instance.foo[0] (orphan) +` + +const testTransformOrphanResourceCountZeroAndNoneCountStr = ` aws_instance.foo (orphan) `