From b3de33cc69abf19cb696e53c250675db61330907 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 30 Jun 2014 11:14:03 -0700 Subject: [PATCH] terraform: failing test but fixes another bug --- terraform/state.go | 9 +- terraform/terraform.go | 23 +++-- terraform/terraform_test.go | 98 ++++++++++++++++++- terraform/test-fixtures/apply-destroy/main.tf | 7 ++ terraform/test-fixtures/plan-destroy/main.tf | 7 ++ 5 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 terraform/test-fixtures/apply-destroy/main.tf create mode 100644 terraform/test-fixtures/plan-destroy/main.tf diff --git a/terraform/state.go b/terraform/state.go index 8fa84b888..933ab264d 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -16,7 +16,8 @@ import ( // can use to keep track of what real world resources it is actually // managing. type State struct { - Resources map[string]*ResourceState + Dependencies map[string][][]string + Resources map[string]*ResourceState once sync.Once } @@ -63,9 +64,13 @@ func (s *State) String() string { for _, k := range names { rs := s.Resources[k] + id := rs.ID + if id == "" { + id = "" + } buf.WriteString(fmt.Sprintf("%s:\n", k)) - buf.WriteString(fmt.Sprintf(" ID = %s\n", rs.ID)) + buf.WriteString(fmt.Sprintf(" ID = %s\n", id)) for ak, av := range rs.Attributes { buf.WriteString(fmt.Sprintf(" %s = %s\n", ak, av)) diff --git a/terraform/terraform.go b/terraform/terraform.go index 978eac343..f16155c61 100644 --- a/terraform/terraform.go +++ b/terraform/terraform.go @@ -103,7 +103,7 @@ func (t *Terraform) apply( g *depgraph.Graph, p *Plan) (*State, error) { s := new(State) - err := g.Walk(t.applyWalkFn(s, p.Vars)) + err := g.Walk(t.applyWalkFn(s, p)) return s, err } @@ -163,17 +163,26 @@ func (t *Terraform) refreshWalkFn(result *State) depgraph.WalkFunc { func (t *Terraform) applyWalkFn( result *State, - vs map[string]string) depgraph.WalkFunc { + p *Plan) depgraph.WalkFunc { var l sync.Mutex // Initialize the result result.init() cb := func(r *Resource) (map[string]string, error) { - // Get the latest diff since there are no computed values anymore - diff, err := r.Provider.Diff(r.State, r.Config) - if err != nil { - return nil, err + diff, ok := p.Diff.Resources[r.Id] + if !ok { + // Skip if there is no diff for a resource + log.Printf("[DEBUG] No diff for %s, skipping.", r.Id) + return nil, nil + } + + if !diff.Destroy { + var err error + diff, err = r.Provider.Diff(r.State, r.Config) + if err != nil { + return nil, err + } } // TODO(mitchellh): we need to verify the diff doesn't change @@ -239,7 +248,7 @@ func (t *Terraform) applyWalkFn( return vars, err } - return t.genericWalkFn(vs, cb) + return t.genericWalkFn(p.Vars, cb) } func (t *Terraform) planWalkFn(result *Plan, opts *PlanOpts) depgraph.WalkFunc { diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 4998b787b..ad1c0ab15 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "reflect" "strings" + "sync" "testing" "github.com/hashicorp/terraform/config" @@ -63,6 +64,57 @@ func TestTerraformApply_compute(t *testing.T) { } } +func TestTerraformApply_destroy(t *testing.T) { + h := new(HookRecordApplyOrder) + + // First, apply the good configuration, build it + c := testConfig(t, "apply-destroy") + tf := testTerraform2(t, &Config{ + Hooks: []Hook{h}, + }) + + p, err := tf.Plan(&PlanOpts{Config: c}) + if err != nil { + t.Fatalf("err: %s", err) + } + + state, err := tf.Apply(p) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Next, plan and apply a destroy operation + p, err = tf.Plan(&PlanOpts{ + Config: new(config.Config), + State: state, + Destroy: true, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + h.Active = true + + state, err = tf.Apply(p) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Test that things were destroyed + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyDestroyStr) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } + + // Test that things were destroyed _in the right order_ + expected2 := []string{"aws_instance.bar", "aws_instance.foo"} + actual2 := h.IDs + if !reflect.DeepEqual(actual2, expected2) { + t.Fatalf("bad: %#v", actual2) + } +} + func TestTerraformApply_hook(t *testing.T) { c := testConfig(t, "apply-good") h := new(MockHook) @@ -189,7 +241,7 @@ func TestTerraformPlan_computed(t *testing.T) { } func TestTerraformPlan_destroy(t *testing.T) { - c := testConfig(t, "plan-good") + c := testConfig(t, "plan-destroy") tf := testTerraform2(t, nil) s := &State{ @@ -436,6 +488,10 @@ func testProviderFunc(n string, rs []string) ResourceProviderFactory { applyFn := func( s *ResourceState, d *ResourceDiff) (*ResourceState, error) { + if d.Destroy { + return nil, nil + } + result := &ResourceState{ ID: "foo", Attributes: make(map[string]string), @@ -571,6 +627,39 @@ func testTerraform2(t *testing.T, c *Config) *Terraform { return tf } +// HookRecordApplyOrder is a test hook that records the order of applies +// by recording the PreApply event. +type HookRecordApplyOrder struct { + NilHook + + Active bool + + IDs []string + States []*ResourceState + Diffs []*ResourceDiff + + l sync.Mutex +} + +func (h *HookRecordApplyOrder) PreApply( + id string, + s *ResourceState, + d *ResourceDiff) (HookAction, error) { + if h.Active { + h.l.Lock() + defer h.l.Unlock() + + h.IDs = append(h.IDs, id) + h.Diffs = append(h.Diffs, d) + h.States = append(h.States, s) + } + + return HookActionContinue, nil +} + +// Below are all the constant strings that are the expected output for +// various tests. + const testTerraformApplyStr = ` aws_instance.bar: ID = foo @@ -594,6 +683,13 @@ aws_instance.foo: id = computed_id ` +const testTerraformApplyDestroyStr = ` +aws_instance.bar: + ID = +aws_instance.foo: + ID = +` + const testTerraformApplyUnknownAttrStr = ` aws_instance.foo: ID = foo diff --git a/terraform/test-fixtures/apply-destroy/main.tf b/terraform/test-fixtures/apply-destroy/main.tf new file mode 100644 index 000000000..e0eeec450 --- /dev/null +++ b/terraform/test-fixtures/apply-destroy/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "foo" { + num = "2" +} + +resource "aws_instance" "bar" { + foo = "{aws_instance.foo.num}" +} diff --git a/terraform/test-fixtures/plan-destroy/main.tf b/terraform/test-fixtures/plan-destroy/main.tf new file mode 100644 index 000000000..94ed55478 --- /dev/null +++ b/terraform/test-fixtures/plan-destroy/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "foo" { + num = "2" +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.num}" +}