From a14ea76c84e47d7d9ca1f7863b4898bb9e0965cd Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 13:31:20 -0700 Subject: [PATCH 01/14] config: Support create_before_destroy config --- config/config.go | 13 +++--- config/loader_hcl.go | 28 +++++++++--- config/loader_test.go | 44 +++++++++++++++++++ config/test-fixtures/create-before-destroy.tf | 10 +++++ 4 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 config/test-fixtures/create-before-destroy.tf diff --git a/config/config.go b/config/config.go index 45fb3028c..b7a1fef6b 100644 --- a/config/config.go +++ b/config/config.go @@ -54,12 +54,13 @@ type ProviderConfig struct { // A Terraform resource is something that represents some component that // can be created and managed, and has some properties associated with it. type Resource struct { - Name string - Type string - Count int - RawConfig *RawConfig - Provisioners []*Provisioner - DependsOn []string + Name string + Type string + Count int + RawConfig *RawConfig + Provisioners []*Provisioner + DependsOn []string + CreateBeforeDestroy bool } // Provisioner is a configured provisioner step on a resource. diff --git a/config/loader_hcl.go b/config/loader_hcl.go index babc34ca6..4bf34c376 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -394,6 +394,7 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { delete(config, "count") delete(config, "depends_on") delete(config, "provisioner") + delete(config, "create_before_destroy") rawConfig, err := NewRawConfig(config) if err != nil { @@ -457,13 +458,28 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { } } + // Check if the resource should be re-created before + // destroying the existing instance + var createBeforeDestroy bool + if o := obj.Get("create_before_destroy", false); o != nil { + err = hcl.DecodeObject(&createBeforeDestroy, o) + if err != nil { + return nil, fmt.Errorf( + "Error parsing create_before_destroy for %s[%s]: %s", + t.Key, + k, + err) + } + } + result = append(result, &Resource{ - Name: k, - Type: t.Key, - Count: count, - RawConfig: rawConfig, - Provisioners: provisioners, - DependsOn: dependsOn, + Name: k, + Type: t.Key, + Count: count, + RawConfig: rawConfig, + Provisioners: provisioners, + DependsOn: dependsOn, + CreateBeforeDestroy: createBeforeDestroy, }) } } diff --git a/config/loader_test.go b/config/loader_test.go index 70dba0a16..d8ab2b6c2 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -346,6 +346,43 @@ func TestLoad_connections(t *testing.T) { } } +func TestLoad_createBeforeDestroy(t *testing.T) { + c, err := Load(filepath.Join(fixtureDir, "create-before-destroy.tf")) + if err != nil { + t.Fatalf("err: %s", err) + } + + if c == nil { + t.Fatal("config should not be nil") + } + + actual := resourcesStr(c.Resources) + if actual != strings.TrimSpace(createBeforeDestroyResourcesStr) { + t.Fatalf("bad:\n%s", actual) + } + + // Check for the flag value + r := c.Resources[0] + if r.Name != "web" && r.Type != "aws_instance" { + t.Fatalf("Bad: %#v", r) + } + + // Should enable create before destroy + if !r.CreateBeforeDestroy { + t.Fatalf("Bad: %#v", r) + } + + r = c.Resources[1] + if r.Name != "bar" && r.Type != "aws_instance" { + t.Fatalf("Bad: %#v", r) + } + + // Should not enable create before destroy + if r.CreateBeforeDestroy { + t.Fatalf("Bad: %#v", r) + } +} + const basicOutputsStr = ` web_ip vars @@ -523,3 +560,10 @@ foo (required) <> <> ` + +const createBeforeDestroyResourcesStr = ` +aws_instance[bar] (x1) + ami +aws_instance[web] (x1) + ami +` diff --git a/config/test-fixtures/create-before-destroy.tf b/config/test-fixtures/create-before-destroy.tf new file mode 100644 index 000000000..7f348aeac --- /dev/null +++ b/config/test-fixtures/create-before-destroy.tf @@ -0,0 +1,10 @@ + +resource "aws_instance" "web" { + ami = "foo" + create_before_destroy = true +} + +resource "aws_instance" "bar" { + ami = "foo" + create_before_destroy = false +} From aef7718778ad79c6d9ececbccb5a6514f991a0a2 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 14:25:54 -0700 Subject: [PATCH 02/14] terraform: support create-before-destroy --- terraform/graph.go | 47 ++++++-- terraform/graph_test.go | 101 +++++++++++++++++- .../graph-diff-create-before/main.tf | 6 ++ 3 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 terraform/test-fixtures/graph-diff-create-before/main.tf diff --git a/terraform/graph.go b/terraform/graph.go index 279c08f26..afc439d25 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -482,6 +482,7 @@ func graphAddConfigResources( // these nodes for you. func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { var nlist []*depgraph.Noun + injected := make(map[*depgraph.Dependency]struct{}) for _, n := range g.Nouns { rn, ok := n.Meta.(*GraphNodeResource) if !ok { @@ -530,13 +531,43 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { newDiff.Destroy = false rd = newDiff - // Add to the new noun to our dependencies so that the destroy - // happens before the apply. - n.Deps = append(n.Deps, &depgraph.Dependency{ - Name: newN.Name, - Source: n, - Target: newN, - }) + // The dependency ordering depends on if the CreateBeforeDestroy + // flag is enabled. If so, we must create the replacement first, + // and then destroy the old instance. + if rn.Config != nil && rn.Config.CreateBeforeDestroy && !rd.Empty() { + dep := &depgraph.Dependency{ + Name: n.Name, + Source: newN, + Target: n, + } + + // Add the old noun to the new noun dependencies so that + // the create happens before the destroy. + newN.Deps = append(newN.Deps, dep) + + // Mark that this dependency has been injected so that + // we do not invert the direction below. + injected[dep] = struct{}{} + + // Add a depedency from the root, since the create node + // does not depend on us + g.Root.Deps = append(g.Root.Deps, &depgraph.Dependency{ + Name: newN.Name, + Source: g.Root, + Target: newN, + }) + + } else { + dep := &depgraph.Dependency{ + Name: newN.Name, + Source: n, + Target: newN, + } + + // Add the new noun to our dependencies so that + // the destroy happens before the apply. + n.Deps = append(n.Deps, dep) + } } rn.Resource.Diff = rd @@ -544,7 +575,6 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { // Go through each noun and make sure we calculate all the dependencies // properly. - injected := make(map[*depgraph.Dependency]struct{}) for _, n := range nlist { deps := n.Deps num := len(deps) @@ -948,6 +978,7 @@ func graphAddRoot(g *depgraph.Graph) { }) } g.Nouns = append(g.Nouns, root) + g.Root = root } // graphAddVariableDeps inspects all the nouns and adds any dependencies diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 858ad1eda..710fb3417 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -652,6 +652,92 @@ func TestGraphAddDiff_module(t *testing.T) { } } +func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { + config := testConfig(t, "graph-diff-create-before") + diff := &Diff{ + Resources: map[string]*InstanceDiff{ + "aws_instance.bar": &InstanceDiff{ + Destroy: true, + Attributes: map[string]*ResourceAttrDiff{ + "ami": &ResourceAttrDiff{ + Old: "abc", + New: "xyz", + RequiresNew: true, + }, + }, + }, + }, + } + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "ami": "abc", + }, + }, + }, + }, + }, + }, + } + + diffHash := checksumStruct(t, diff) + + g, err := Graph(&GraphOpts{ + Config: config, + Diff: diff, + State: state, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphDiffCreateBeforeDestroyStr) + if actual != expected { + t.Fatalf("bad:\n\n%s\n\nexpected:\n\n%s", actual, expected) + } + + // Verify that our original structure has not been modified + diffHash2 := checksumStruct(t, diff) + if diffHash != diffHash2 { + t.Fatal("diff has been modified") + } +} + +func TestGraphInitState(t *testing.T) { + config := testConfig(t, "graph-basic") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*InstanceDiff{ + "aws_instance.foo": &InstanceDiff{ + Destroy: true, + }, + }, + }, + }, + } + + g, err := Graph(&GraphOpts{Module: m, Diff: diff}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphDiffModuleStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestGraphAddDiff_moduleDestroy(t *testing.T) { m := testModule(t, "graph-diff-module") diff := &Diff{ @@ -1044,8 +1130,19 @@ aws_load_balancer.weblb aws_load_balancer.weblb -> provider.aws provider.aws root - root -> aws_load_balancer.weblb -` + root -> aws_load_balancer.weblb` + +const testTerraformGraphDiffCreateBeforeDestroyStr = ` +root: root +aws_instance.bar + aws_instance.bar -> provider.aws +aws_instance.bar (destroy) + aws_instance.bar (destroy) -> aws_instance.bar + aws_instance.bar (destroy) -> provider.aws +provider.aws +root + root -> aws_instance.bar + root -> aws_instance.bar (destroy)` const testTerraformGraphStateStr = ` root: root diff --git a/terraform/test-fixtures/graph-diff-create-before/main.tf b/terraform/test-fixtures/graph-diff-create-before/main.tf new file mode 100644 index 000000000..8d9dccd80 --- /dev/null +++ b/terraform/test-fixtures/graph-diff-create-before/main.tf @@ -0,0 +1,6 @@ +provider "aws" {} + +resource "aws_instance" "bar" { + ami = "abc" + create_before_destroy = true +} From 1aaddafba090083e8a4714dd8c81e5f98270d893 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 15:02:00 -0700 Subject: [PATCH 03/14] terraform: Adding lifecycle config block --- config/config.go | 20 ++++++++++------ config/loader_hcl.go | 24 +++++++++---------- config/loader_test.go | 4 ++-- config/test-fixtures/create-before-destroy.tf | 8 +++++-- terraform/graph.go | 2 +- .../graph-diff-create-before/main.tf | 4 +++- 6 files changed, 37 insertions(+), 25 deletions(-) diff --git a/config/config.go b/config/config.go index b7a1fef6b..0b4350a2a 100644 --- a/config/config.go +++ b/config/config.go @@ -54,13 +54,19 @@ type ProviderConfig struct { // A Terraform resource is something that represents some component that // can be created and managed, and has some properties associated with it. type Resource struct { - Name string - Type string - Count int - RawConfig *RawConfig - Provisioners []*Provisioner - DependsOn []string - CreateBeforeDestroy bool + Name string + Type string + Count int + RawConfig *RawConfig + Provisioners []*Provisioner + DependsOn []string + Lifecycle ResourceLifecycle +} + +// ResourceLifecycle is used to store the lifecycle tuning parameters +// to allow customized behavior +type ResourceLifecycle struct { + CreateBeforeDestroy bool `hcl:"create_before_destroy"` } // Provisioner is a configured provisioner step on a resource. diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 4bf34c376..d777873e7 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -394,7 +394,7 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { delete(config, "count") delete(config, "depends_on") delete(config, "provisioner") - delete(config, "create_before_destroy") + delete(config, "lifecycle") rawConfig, err := NewRawConfig(config) if err != nil { @@ -460,12 +460,12 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { // Check if the resource should be re-created before // destroying the existing instance - var createBeforeDestroy bool - if o := obj.Get("create_before_destroy", false); o != nil { - err = hcl.DecodeObject(&createBeforeDestroy, o) + var lifecycle ResourceLifecycle + if o := obj.Get("lifecycle", false); o != nil { + err = hcl.DecodeObject(&lifecycle, o) if err != nil { return nil, fmt.Errorf( - "Error parsing create_before_destroy for %s[%s]: %s", + "Error parsing lifecycle for %s[%s]: %s", t.Key, k, err) @@ -473,13 +473,13 @@ func loadResourcesHcl(os *hclobj.Object) ([]*Resource, error) { } result = append(result, &Resource{ - Name: k, - Type: t.Key, - Count: count, - RawConfig: rawConfig, - Provisioners: provisioners, - DependsOn: dependsOn, - CreateBeforeDestroy: createBeforeDestroy, + Name: k, + Type: t.Key, + Count: count, + RawConfig: rawConfig, + Provisioners: provisioners, + DependsOn: dependsOn, + Lifecycle: lifecycle, }) } } diff --git a/config/loader_test.go b/config/loader_test.go index d8ab2b6c2..5bb77b931 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -368,7 +368,7 @@ func TestLoad_createBeforeDestroy(t *testing.T) { } // Should enable create before destroy - if !r.CreateBeforeDestroy { + if !r.Lifecycle.CreateBeforeDestroy { t.Fatalf("Bad: %#v", r) } @@ -378,7 +378,7 @@ func TestLoad_createBeforeDestroy(t *testing.T) { } // Should not enable create before destroy - if r.CreateBeforeDestroy { + if r.Lifecycle.CreateBeforeDestroy { t.Fatalf("Bad: %#v", r) } } diff --git a/config/test-fixtures/create-before-destroy.tf b/config/test-fixtures/create-before-destroy.tf index 7f348aeac..a45ff5c19 100644 --- a/config/test-fixtures/create-before-destroy.tf +++ b/config/test-fixtures/create-before-destroy.tf @@ -1,10 +1,14 @@ resource "aws_instance" "web" { ami = "foo" - create_before_destroy = true + lifecycle { + create_before_destroy = true + } } resource "aws_instance" "bar" { ami = "foo" - create_before_destroy = false + lifecycle { + create_before_destroy = false + } } diff --git a/terraform/graph.go b/terraform/graph.go index afc439d25..f375b1f4b 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -534,7 +534,7 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { // The dependency ordering depends on if the CreateBeforeDestroy // flag is enabled. If so, we must create the replacement first, // and then destroy the old instance. - if rn.Config != nil && rn.Config.CreateBeforeDestroy && !rd.Empty() { + if rn.Config != nil && rn.Config.Lifecycle.CreateBeforeDestroy && !rd.Empty() { dep := &depgraph.Dependency{ Name: n.Name, Source: newN, diff --git a/terraform/test-fixtures/graph-diff-create-before/main.tf b/terraform/test-fixtures/graph-diff-create-before/main.tf index 8d9dccd80..2cfe794d1 100644 --- a/terraform/test-fixtures/graph-diff-create-before/main.tf +++ b/terraform/test-fixtures/graph-diff-create-before/main.tf @@ -2,5 +2,7 @@ provider "aws" {} resource "aws_instance" "bar" { ami = "abc" - create_before_destroy = true + lifecycle { + create_before_destroy = true + } } From 8d5d7c32c8ffca2f3b0d6f58622fb505680d3c0d Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 15:10:25 -0700 Subject: [PATCH 04/14] website: Document lifecycle --- .../docs/configuration/resources.html.md | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/website/source/docs/configuration/resources.html.md b/website/source/docs/configuration/resources.html.md index aa0a1bec8..1edad4340 100644 --- a/website/source/docs/configuration/resources.html.md +++ b/website/source/docs/configuration/resources.html.md @@ -49,6 +49,17 @@ There are **meta-parameters** available to all resources: resource. The dependencies are in the format of `TYPE.NAME`, for example `aws_instance.web`. + * `lifecycle` (configuration block) - Customizes the lifecycle + behavior of the resource. The specific options are documented + below. + +The `lifecycle` block allows the following keys to be set: + + * `create_before_destroy` (bool) - This flag is used to ensure + the replacement of a resource is created before the original + instance is destroyed. As an example, this can be used to + create an new DNS record before removing an old record. + ------------- Within a resource, you can optionally have a **connection block**. @@ -87,7 +98,8 @@ The full syntax is: resource TYPE NAME { CONFIG ... [count = COUNT] - [depends_on = [RESOURCE NAME, ...]] + [depends_on = [RESOURCE NAME, ...]] + [LIFECYCLE] [CONNECTION] [PROVISIONER ...] @@ -104,6 +116,14 @@ KEY { } ``` +where `LIFECYCLE` is: + +``` +lifecycle { + [create_before_destroy = true|false] +} +``` + where `CONNECTION` is: ``` From f398708be21ec7423cc2d72f59a05b2f65a6fd59 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 16:15:15 -0700 Subject: [PATCH 05/14] terraform: Adding flag for CreateBeforeDestroy --- terraform/graph.go | 3 +++ terraform/graph_test.go | 6 ++++++ terraform/resource.go | 1 + 3 files changed, 10 insertions(+) diff --git a/terraform/graph.go b/terraform/graph.go index f375b1f4b..9e766d423 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -557,6 +557,9 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { Target: newN, }) + // Set the CreateBeforeDestroy flag on the old noun + rn.Resource.Flags |= FlagCreateBeforeDestroy + } else { dep := &depgraph.Dependency{ Name: newN.Name, diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 710fb3417..d52b3ea18 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -704,6 +704,12 @@ func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { t.Fatalf("bad:\n\n%s\n\nexpected:\n\n%s", actual, expected) } + // Verify the flag is set + r := g.Noun("aws_instance.bar") + if r.Meta.(*GraphNodeResource).Resource.Flags&FlagCreateBeforeDestroy == 0 { + t.Fatalf("missing FlagCreateBeforeDestroy") + } + // Verify that our original structure has not been modified diffHash2 := checksumStruct(t, diff) if diffHash != diffHash2 { diff --git a/terraform/resource.go b/terraform/resource.go index 0df27524a..cd1e86bb1 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -47,6 +47,7 @@ const ( FlagTainted FlagOrphan FlagHasTainted + FlagCreateBeforeDestroy ) // InstanceInfo is used to hold information about the instance and/or From 4fe05428b34e9dc8c8ded88b160a8f3db464780d Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 16:56:41 -0700 Subject: [PATCH 06/14] terraform: Avoid having multiple primaries --- terraform/context.go | 33 +++++++++++++++++++++++++++++++-- terraform/graph.go | 7 +++++-- terraform/graph_test.go | 11 ++++++++--- terraform/resource.go | 3 ++- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 6c3b0f0a3..d5d865e77 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -1264,17 +1264,46 @@ func (c *walkContext) persistState(r *Resource) { rs.Dependencies = r.Dependencies // Assign the instance state to the proper location - if r.Flags&FlagTainted != 0 { + if r.Flags&FlagDeposed != 0 { + // We were previously the primary and have been deposed, so + // now we are the final tainted resource + r.TaintedIndex = len(rs.Tainted) - 1 + rs.Tainted[r.TaintedIndex] = r.State + + } else if r.Flags&FlagTainted != 0 { if r.TaintedIndex >= 0 { // Tainted with a pre-existing index, just update that spot rs.Tainted[r.TaintedIndex] = r.State + + } else if r.Flags&FlagReplacePrimary != 0 { + // We just replaced the primary, so restore the primary + rs.Primary = rs.Tainted[len(rs.Tainted)-1] + + // Set ourselves as tainted + rs.Tainted[len(rs.Tainted)-1] = r.State + } else { // Newly tainted, so append it to the list, update the // index, and remove the primary. rs.Tainted = append(rs.Tainted, r.State) - rs.Primary = nil r.TaintedIndex = len(rs.Tainted) - 1 + rs.Primary = nil } + + } else if r.Flags&FlagReplacePrimary != 0 { + // If the ID is blank (there was an error), then we leave + // the primary that exists, and do not store this as a tainted + // instance + if r.State.ID == "" { + return + } + + // Push the old primary into the tainted state + rs.Tainted = append(rs.Tainted, rs.Primary) + + // Set this as the new primary + rs.Primary = r.State + } else { // The primary instance, so just set it directly rs.Primary = r.State diff --git a/terraform/graph.go b/terraform/graph.go index 9e766d423..4fbb0e13c 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -557,8 +557,11 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { Target: newN, }) - // Set the CreateBeforeDestroy flag on the old noun - rn.Resource.Flags |= FlagCreateBeforeDestroy + // Set the ReplacePrimary flag on the new instance so that + // it will become the new primary, and Diposed flag on the + // existing instance so that it will step down + rn.Resource.Flags |= FlagReplacePrimary + newNode.Resource.Flags |= FlagDeposed } else { dep := &depgraph.Dependency{ diff --git a/terraform/graph_test.go b/terraform/graph_test.go index d52b3ea18..94a357376 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -704,10 +704,15 @@ func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { t.Fatalf("bad:\n\n%s\n\nexpected:\n\n%s", actual, expected) } - // Verify the flag is set + // Verify the flags are set r := g.Noun("aws_instance.bar") - if r.Meta.(*GraphNodeResource).Resource.Flags&FlagCreateBeforeDestroy == 0 { - t.Fatalf("missing FlagCreateBeforeDestroy") + if r.Meta.(*GraphNodeResource).Resource.Flags&FlagReplacePrimary == 0 { + t.Fatalf("missing FlagReplacePrimary") + } + + r = g.Noun("aws_instance.bar (destroy)") + if r.Meta.(*GraphNodeResource).Resource.Flags&FlagDeposed == 0 { + t.Fatalf("missing FlagDeposed") } // Verify that our original structure has not been modified diff --git a/terraform/resource.go b/terraform/resource.go index cd1e86bb1..f76cfcba9 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -47,7 +47,8 @@ const ( FlagTainted FlagOrphan FlagHasTainted - FlagCreateBeforeDestroy + FlagReplacePrimary + FlagDeposed ) // InstanceInfo is used to hold information about the instance and/or From f248ae3aee73654914ca83c4a00bf1c8385090ee Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 22 Sep 2014 17:18:51 -0700 Subject: [PATCH 07/14] terraform: test provising fail create-before-destroy --- terraform/context_test.go | 58 +++++++++++++++++++ terraform/terraform_test.go | 7 +++ .../main.tf | 7 +++ 3 files changed, 72 insertions(+) create mode 100644 terraform/test-fixtures/apply-provisioner-fail-create-before/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 222b54ec4..a9e702b51 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -880,6 +880,61 @@ func TestContextApply_provisionerFail(t *testing.T) { } } +func TestContextApply_provisionerFail_createBeforeDestroy(t *testing.T) { + c := testConfig(t, "apply-provisioner-fail-create-before") + p := testProvider("aws") + pr := testProvisioner() + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + pr.ApplyFn = func(*InstanceState, *ResourceConfig) error { + return fmt.Errorf("EXPLOSION") + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "require_new": "abc", + }, + }, + }, + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Provisioners: map[string]ResourceProvisionerFactory{ + "shell": testProvisionerFuncFixed(pr), + }, + State: state, + }) + + if _, err := ctx.Plan(nil); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err == nil { + t.Fatal("should error") + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyProvisionerFailCreateBeforeDestroyStr) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } +} + func TestContextApply_provisionerResourceRef(t *testing.T) { m := testModule(t, "apply-provisioner-resource-ref") p := testProvider("aws") @@ -3121,6 +3176,9 @@ func testDiffFn( New: v.(string), } + if k == "require_new" { + attrDiff.RequiresNew = true + } diff.Attributes[k] = attrDiff } diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index d01e98acb..52e8700bd 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -218,6 +218,13 @@ aws_instance.foo: type = aws_instance ` +const testTerraformApplyProvisionerFailCreateBeforeDestroyStr = ` +aws_instance.bar: (1 tainted) + ID = bar + require_new = abc + Tainted ID 1 = foo +` + const testTerraformApplyProvisionerResourceRefStr = ` aws_instance.bar: ID = foo diff --git a/terraform/test-fixtures/apply-provisioner-fail-create-before/main.tf b/terraform/test-fixtures/apply-provisioner-fail-create-before/main.tf new file mode 100644 index 000000000..00d32cbc2 --- /dev/null +++ b/terraform/test-fixtures/apply-provisioner-fail-create-before/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "bar" { + require_new = "xyz" + provisioner "shell" {} + lifecycle { + create_before_destroy = true + } +} From 59b7cb171a38e97c9a1efa3fa30a98ab5a89cbf0 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Tue, 23 Sep 2014 10:53:29 -0700 Subject: [PATCH 08/14] terraform: Testing failed apply with create_before_destroy --- terraform/context_test.go | 49 +++++++++++++++++++ terraform/terraform_test.go | 6 +++ .../apply-error-create-before/main.tf | 6 +++ 3 files changed, 61 insertions(+) create mode 100644 terraform/test-fixtures/apply-error-create-before/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index a9e702b51..1aa682395 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -935,6 +935,55 @@ func TestContextApply_provisionerFail_createBeforeDestroy(t *testing.T) { } } +func TestContextApply_error_createBeforeDestroy(t *testing.T) { + c := testConfig(t, "apply-error-create-before") + p := testProvider("aws") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "require_new": "abc", + }, + }, + }, + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + }) + p.ApplyFn = func(info *InstanceInfo, is *InstanceState, id *InstanceDiff) (*InstanceState, error) { + return nil, fmt.Errorf("error") + } + p.DiffFn = testDiffFn + + if _, err := ctx.Plan(nil); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err == nil { + t.Fatal("should have error") + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyErrorCreateBeforeDestroyStr) + if actual != expected { + t.Fatalf("bad: \n%s\n\n\n%s", actual, expected) + } +} + func TestContextApply_provisionerResourceRef(t *testing.T) { m := testModule(t, "apply-provisioner-resource-ref") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 52e8700bd..abc103393 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -254,6 +254,12 @@ aws_instance.foo: num = 2 ` +const testTerraformApplyErrorCreateBeforeDestroyStr = ` +aws_instance.bar: + ID = bar + require_new = abc +` + const testTerraformApplyErrorPartialStr = ` aws_instance.bar: ID = bar diff --git a/terraform/test-fixtures/apply-error-create-before/main.tf b/terraform/test-fixtures/apply-error-create-before/main.tf new file mode 100644 index 000000000..c7c2776eb --- /dev/null +++ b/terraform/test-fixtures/apply-error-create-before/main.tf @@ -0,0 +1,6 @@ +resource "aws_instance" "bar" { + require_new = "xyz" + lifecycle { + create_before_destroy = true + } +} From 465f3f2676e75e2575aa85397828c670149daf8e Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Tue, 23 Sep 2014 11:10:23 -0700 Subject: [PATCH 09/14] terraform: test create-before-destroy with failed destroy --- terraform/context_test.go | 58 +++++++++++++++++++++++++++++++++++++ terraform/terraform_test.go | 6 ++++ 2 files changed, 64 insertions(+) diff --git a/terraform/context_test.go b/terraform/context_test.go index 1aa682395..f6de07a6a 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -984,6 +984,64 @@ func TestContextApply_error_createBeforeDestroy(t *testing.T) { } } +func TestContextApply_errorDestroy_createBeforeDestroy(t *testing.T) { + c := testConfig(t, "apply-error-create-before") + p := testProvider("aws") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "require_new": "abc", + }, + }, + }, + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + }) + p.ApplyFn = func(info *InstanceInfo, is *InstanceState, id *InstanceDiff) (*InstanceState, error) { + // Fail the destroy! + if id.Destroy { + return is, fmt.Errorf("error") + } + + // Create should work + is = &InstanceState{ + ID: "foo", + } + return is, nil + } + p.DiffFn = testDiffFn + + if _, err := ctx.Plan(nil); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err == nil { + t.Fatal("should have error") + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyErrorDestroyCreateBeforeDestroyStr) + if actual != expected { + t.Fatalf("bad: actual:\n%s\n\nexpected:\n%s", actual, expected) + } +} + func TestContextApply_provisionerResourceRef(t *testing.T) { m := testModule(t, "apply-provisioner-resource-ref") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index abc103393..f3cebdcc6 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -260,6 +260,12 @@ aws_instance.bar: require_new = abc ` +const testTerraformApplyErrorDestroyCreateBeforeDestroyStr = ` +aws_instance.bar: (1 tainted) + ID = foo + Tainted ID 1 = bar +` + const testTerraformApplyErrorPartialStr = ` aws_instance.bar: ID = bar From bce9b664d8a499708592b15b4711212435e7ad2f Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Tue, 23 Sep 2014 11:13:50 -0700 Subject: [PATCH 10/14] terraform: test happy path create-before-destroy --- terraform/context_test.go | 52 +++++++++++++++++++ terraform/terraform_test.go | 7 +++ .../apply-good-create-before/main.tf | 6 +++ 3 files changed, 65 insertions(+) create mode 100644 terraform/test-fixtures/apply-good-create-before/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index f6de07a6a..7129bde58 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -582,6 +582,58 @@ func TestContextApply(t *testing.T) { } } +func TestContextApply_createBeforeDestroy(t *testing.T) { + c := testConfig(t, "apply-good-create-before") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "require_new": "abc", + }, + }, + }, + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + }) + + if _, err := ctx.Plan(nil); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + mod := state.RootModule() + if len(mod.Resources) != 1 { + t.Fatalf("bad: %#v", mod.Resources) + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyCreateBeforeStr) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } +} + func TestContextApply_Minimal(t *testing.T) { m := testModule(t, "apply-minimal") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index f3cebdcc6..3df7013e5 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -150,6 +150,13 @@ aws_instance.foo: type = aws_instance ` +const testTerraformApplyCreateBeforeStr = ` +aws_instance.bar: + ID = foo + require_new = xyz + type = aws_instance +` + const testTerraformApplyCancelStr = ` aws_instance.foo: ID = foo diff --git a/terraform/test-fixtures/apply-good-create-before/main.tf b/terraform/test-fixtures/apply-good-create-before/main.tf new file mode 100644 index 000000000..c7c2776eb --- /dev/null +++ b/terraform/test-fixtures/apply-good-create-before/main.tf @@ -0,0 +1,6 @@ +resource "aws_instance" "bar" { + require_new = "xyz" + lifecycle { + create_before_destroy = true + } +} From 9a6c8490a0033f16daee60a770164a87b76d4890 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 29 Sep 2014 15:27:56 -0700 Subject: [PATCH 11/14] terraform: Updating tests for modules --- terraform/context_test.go | 16 ++++++------ terraform/graph_test.go | 52 +++++++++++---------------------------- 2 files changed, 23 insertions(+), 45 deletions(-) diff --git a/terraform/context_test.go b/terraform/context_test.go index 7129bde58..c92e30b40 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -583,7 +583,7 @@ func TestContextApply(t *testing.T) { } func TestContextApply_createBeforeDestroy(t *testing.T) { - c := testConfig(t, "apply-good-create-before") + m := testModule(t, "apply-good-create-before") p := testProvider("aws") p.ApplyFn = testApplyFn p.DiffFn = testDiffFn @@ -606,7 +606,7 @@ func TestContextApply_createBeforeDestroy(t *testing.T) { }, } ctx := testContext(t, &ContextOpts{ - Config: c, + Module: m, Providers: map[string]ResourceProviderFactory{ "aws": testProviderFuncFixed(p), }, @@ -933,7 +933,7 @@ func TestContextApply_provisionerFail(t *testing.T) { } func TestContextApply_provisionerFail_createBeforeDestroy(t *testing.T) { - c := testConfig(t, "apply-provisioner-fail-create-before") + m := testModule(t, "apply-provisioner-fail-create-before") p := testProvider("aws") pr := testProvisioner() p.ApplyFn = testApplyFn @@ -961,7 +961,7 @@ func TestContextApply_provisionerFail_createBeforeDestroy(t *testing.T) { }, } ctx := testContext(t, &ContextOpts{ - Config: c, + Module: m, Providers: map[string]ResourceProviderFactory{ "aws": testProviderFuncFixed(p), }, @@ -988,7 +988,7 @@ func TestContextApply_provisionerFail_createBeforeDestroy(t *testing.T) { } func TestContextApply_error_createBeforeDestroy(t *testing.T) { - c := testConfig(t, "apply-error-create-before") + m := testModule(t, "apply-error-create-before") p := testProvider("aws") state := &State{ Modules: []*ModuleState{ @@ -1009,7 +1009,7 @@ func TestContextApply_error_createBeforeDestroy(t *testing.T) { }, } ctx := testContext(t, &ContextOpts{ - Config: c, + Module: m, Providers: map[string]ResourceProviderFactory{ "aws": testProviderFuncFixed(p), }, @@ -1037,7 +1037,7 @@ func TestContextApply_error_createBeforeDestroy(t *testing.T) { } func TestContextApply_errorDestroy_createBeforeDestroy(t *testing.T) { - c := testConfig(t, "apply-error-create-before") + m := testModule(t, "apply-error-create-before") p := testProvider("aws") state := &State{ Modules: []*ModuleState{ @@ -1058,7 +1058,7 @@ func TestContextApply_errorDestroy_createBeforeDestroy(t *testing.T) { }, } ctx := testContext(t, &ContextOpts{ - Config: c, + Module: m, Providers: map[string]ResourceProviderFactory{ "aws": testProviderFuncFixed(p), }, diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 94a357376..461b49d01 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -653,16 +653,21 @@ func TestGraphAddDiff_module(t *testing.T) { } func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { - config := testConfig(t, "graph-diff-create-before") + m := testModule(t, "graph-diff-create-before") diff := &Diff{ - Resources: map[string]*InstanceDiff{ - "aws_instance.bar": &InstanceDiff{ - Destroy: true, - Attributes: map[string]*ResourceAttrDiff{ - "ami": &ResourceAttrDiff{ - Old: "abc", - New: "xyz", - RequiresNew: true, + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: rootModulePath, + Resources: map[string]*InstanceDiff{ + "aws_instance.bar": &InstanceDiff{ + Destroy: true, + Attributes: map[string]*ResourceAttrDiff{ + "ami": &ResourceAttrDiff{ + Old: "abc", + New: "xyz", + RequiresNew: true, + }, + }, }, }, }, @@ -690,7 +695,7 @@ func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { diffHash := checksumStruct(t, diff) g, err := Graph(&GraphOpts{ - Config: config, + Module: m, Diff: diff, State: state, }) @@ -722,33 +727,6 @@ func TestGraphAddDiff_createBeforeDestroy(t *testing.T) { } } -func TestGraphInitState(t *testing.T) { - config := testConfig(t, "graph-basic") - state := &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: rootModulePath, - Resources: map[string]*InstanceDiff{ - "aws_instance.foo": &InstanceDiff{ - Destroy: true, - }, - }, - }, - }, - } - - g, err := Graph(&GraphOpts{Module: m, Diff: diff}) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTerraformGraphDiffModuleStr) - if actual != expected { - t.Fatalf("bad:\n\n%s", actual) - } -} - func TestGraphAddDiff_moduleDestroy(t *testing.T) { m := testModule(t, "graph-diff-module") diff := &Diff{ From 5207e1d268deb773c64e9195344a3e40a21083df Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Mon, 29 Sep 2014 17:00:45 -0700 Subject: [PATCH 12/14] terraform: test ordering when using create before with depedencies --- terraform/context_test.go | 85 +++++++++++++++++++ terraform/terraform_test.go | 14 +++ .../apply-depends-create-before/main.tf | 11 +++ 3 files changed, 110 insertions(+) create mode 100644 terraform/test-fixtures/apply-depends-create-before/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index c92e30b40..4270b7f57 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1912,6 +1912,91 @@ func TestContextApply_vars(t *testing.T) { } } +func TestContextApply_createBefore_depends(t *testing.T) { + m := testModule(t, "apply-depends-create-before") + h := new(HookRecordApplyOrder) + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.web": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "require_new": "ami-old", + }, + }, + }, + "aws_instance.lb": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "baz", + Attributes: map[string]string{ + "instance": "bar", + }, + }, + }, + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Module: m, + Hooks: []Hook{h}, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + }) + + if _, err := ctx.Plan(nil); err != nil { + t.Fatalf("err: %s", err) + } + + h.Active = true + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + mod := state.RootModule() + if len(mod.Resources) < 2 { + t.Fatalf("bad: %#v", mod.Resources) + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyDependsCreateBeforeStr) + if actual != expected { + t.Fatalf("bad: \n%s\n%s", actual, expected) + } + + // Test that things were managed _in the right order_ + order := h.States + diffs := h.Diffs + if order[0].ID != "bar" || diffs[0].Destroy { + t.Fatalf("should create new instance first: %#v", order) + } + + // Order of the next two is not deterministric, just + // depends on the scheduling. + for i := 1; i <= 2; i++ { + switch order[i].ID { + case "bar": + if !diffs[1].Destroy { + t.Fatalf("missing destroy") + } + case "baz": + default: + t.Fatalf("bad resource") + } + } +} + func TestContextPlan(t *testing.T) { m := testModule(t, "plan-good") p := testProvider("aws") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 3df7013e5..cdcb6ea20 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -150,6 +150,20 @@ aws_instance.foo: type = aws_instance ` +const testTerraformApplyDependsCreateBeforeStr = ` +aws_instance.lb: + ID = foo + instance = foo + type = aws_instance + + Dependencies: + aws_instance.web +aws_instance.web: + ID = foo + require_new = ami-new + type = aws_instance +` + const testTerraformApplyCreateBeforeStr = ` aws_instance.bar: ID = foo diff --git a/terraform/test-fixtures/apply-depends-create-before/main.tf b/terraform/test-fixtures/apply-depends-create-before/main.tf new file mode 100644 index 000000000..b4666f5bc --- /dev/null +++ b/terraform/test-fixtures/apply-depends-create-before/main.tf @@ -0,0 +1,11 @@ + +resource "aws_instance" "web" { + require_new = "ami-new" + lifecycle { + create_before_destroy = true + } +} + +resource "aws_instance" "lb" { + instance = "${aws_instance.web.id}" +} From 61b893b8db39a743adde2ac7eb603fbe179c21d1 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Tue, 30 Sep 2014 11:20:15 -0700 Subject: [PATCH 13/14] depgraph: Adding method to get incoming edges --- depgraph/graph.go | 20 ++++++++++++++++++++ depgraph/graph_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/depgraph/graph.go b/depgraph/graph.go index 44c57d7d6..c190dcfa1 100644 --- a/depgraph/graph.go +++ b/depgraph/graph.go @@ -357,3 +357,23 @@ func (g *Graph) Walk(fn WalkFunc) error { return err } } + +// DependsOn returns the set of nouns that have a +// dependency on a given noun. This can be used to find +// the incoming edges to a noun. +func (g *Graph) DependsOn(n *Noun) []*Noun { + var incoming []*Noun +OUTER: + for _, other := range g.Nouns { + if other == n { + continue + } + for _, d := range other.Deps { + if d.Target == n { + incoming = append(incoming, other) + continue OUTER + } + } + } + return incoming +} diff --git a/depgraph/graph_test.go b/depgraph/graph_test.go index e00f6c543..c883b1417 100644 --- a/depgraph/graph_test.go +++ b/depgraph/graph_test.go @@ -429,3 +429,39 @@ g -> h`) } } } + +func TestGraph_DependsOn(t *testing.T) { + nodes := ParseNouns(`a -> b +a -> c +b -> d +b -> e +c -> d +c -> e`) + + g := &Graph{ + Name: "Test", + Nouns: NounMapToList(nodes), + } + + dNoun := g.Noun("d") + incoming := g.DependsOn(dNoun) + + if len(incoming) != 2 { + t.Fatalf("bad: %#v", incoming) + } + + var hasB, hasC bool + for _, in := range incoming { + switch in.Name { + case "b": + hasB = true + case "c": + hasC = true + default: + t.Fatalf("Bad: %#v", in) + } + } + if !hasB || !hasC { + t.Fatalf("missing incoming edge") + } +} From 1977a5357421fd9d8229518fc4fc1b6bacea2a3a Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Tue, 30 Sep 2014 11:37:49 -0700 Subject: [PATCH 14/14] terraform: Deterministric and correct ordering for deposed nodes --- terraform/context_test.go | 18 ++++++------------ terraform/graph.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/terraform/context_test.go b/terraform/context_test.go index 4270b7f57..6ebd9ac0b 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1982,18 +1982,12 @@ func TestContextApply_createBefore_depends(t *testing.T) { t.Fatalf("should create new instance first: %#v", order) } - // Order of the next two is not deterministric, just - // depends on the scheduling. - for i := 1; i <= 2; i++ { - switch order[i].ID { - case "bar": - if !diffs[1].Destroy { - t.Fatalf("missing destroy") - } - case "baz": - default: - t.Fatalf("bad resource") - } + if order[1].ID != "baz" { + t.Fatalf("update must happen after create: %#v", order) + } + + if order[2].ID != "bar" || !diffs[2].Destroy { + t.Fatalf("destroy must happen after update: %#v", order) } } diff --git a/terraform/graph.go b/terraform/graph.go index 4fbb0e13c..76c51ad11 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -299,6 +299,15 @@ func graphEncodeDependencies(g *depgraph.Graph) { } r := rn.Resource + // If we are using create-before-destroy, there + // are some special depedencies injected on the + // deposed node that would cause a circular depedency + // chain if persisted. We must only handle the new node, + // node the deposed node. + if r.Flags&FlagDeposed != 0 { + continue + } + // Update the dependencies var inject []string for _, dep := range n.Deps { @@ -563,6 +572,27 @@ func graphAddDiff(g *depgraph.Graph, d *ModuleDiff) error { rn.Resource.Flags |= FlagReplacePrimary newNode.Resource.Flags |= FlagDeposed + // This logic is not intuitive, but we need to make the + // destroy depend upon any resources that depend on the + // create. The reason is suppose you have a LB depend on + // a web server. You need the order to be create, update LB, + // destroy. Without this, the update LB and destroy can + // be executed in an arbitrary order (likely in parallel). + incoming := g.DependsOn(n) + for _, inc := range incoming { + // Ignore the root... + if inc == g.Root { + continue + } + dep := &depgraph.Dependency{ + Name: inc.Name, + Source: newN, + Target: inc, + } + injected[dep] = struct{}{} + newN.Deps = append(newN.Deps, dep) + } + } else { dep := &depgraph.Dependency{ Name: newN.Name,