From 0699cde1d47ac4ba0ae33c3d9f026c5af9faf2c3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 17:10:17 -0700 Subject: [PATCH 1/3] config: depends_on meta-parameter --- config/config.go | 1 + config/loader_libucl.go | 28 +++++++++++++++++++--------- config/loader_test.go | 9 +++++++++ config/test-fixtures/basic.tf | 2 ++ config/test-fixtures/basic.tf.json | 3 ++- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/config/config.go b/config/config.go index cab5f728a..f2c8b41b5 100644 --- a/config/config.go +++ b/config/config.go @@ -42,6 +42,7 @@ type Resource struct { Count int RawConfig *RawConfig Provisioners []*Provisioner + DependsOn []string } // Provisioner is a configured provisioner step on a resource. diff --git a/config/loader_libucl.go b/config/loader_libucl.go index cd14774fd..2599a6d16 100644 --- a/config/loader_libucl.go +++ b/config/loader_libucl.go @@ -352,16 +352,11 @@ func loadResourcesLibucl(o *libucl.Object) ([]*Resource, error) { err) } - // Remove the "count" from the config, since we treat that special - delete(config, "count") - - // Delete the "provisioner" section from the config since - // that is treated specially. - delete(config, "provisioner") - - // Delete the "connection" section since we handle that - // seperately + // Remove the fields we handle specially delete(config, "connection") + delete(config, "count") + delete(config, "depends_on") + delete(config, "provisioner") rawConfig, err := NewRawConfig(config) if err != nil { @@ -401,6 +396,20 @@ func loadResourcesLibucl(o *libucl.Object) ([]*Resource, error) { } } + // If we have depends fields, then add those in + var dependsOn []string + if deps := r.Get("depends_on"); deps != nil { + err := deps.Decode(&dependsOn) + deps.Close() + if err != nil { + return nil, fmt.Errorf( + "Error reading depends_on for %s[%s]: %s", + t.Key(), + r.Key(), + err) + } + } + // If we have provisioners, then parse those out var provisioners []*Provisioner if po := r.Get("provisioner"); po != nil { @@ -422,6 +431,7 @@ func loadResourcesLibucl(o *libucl.Object) ([]*Resource, error) { Count: count, RawConfig: rawConfig, Provisioners: provisioners, + DependsOn: dependsOn, }) } } diff --git a/config/loader_test.go b/config/loader_test.go index 0cb8183cf..168f4e931 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -393,6 +393,13 @@ func resourcesStr(rs []*Resource) string { } } + if len(r.DependsOn) > 0 { + result += fmt.Sprintf(" dependsOn\n") + for _, d := range r.DependsOn { + result += fmt.Sprintf(" %s\n", d) + } + } + if len(r.RawConfig.Variables) > 0 { result += fmt.Sprintf(" vars\n") @@ -479,6 +486,8 @@ do const basicResourcesStr = ` aws_instance[db] (x1) security_groups + dependsOn + aws_instance.web vars resource: aws_security_group.firewall.*.id aws_instance[web] (x1) diff --git a/config/test-fixtures/basic.tf b/config/test-fixtures/basic.tf index 84cbd8b31..a8938c018 100644 --- a/config/test-fixtures/basic.tf +++ b/config/test-fixtures/basic.tf @@ -31,6 +31,8 @@ resource aws_instance "web" { resource "aws_instance" "db" { security_groups = "${aws_security_group.firewall.*.id}" + + depends_on = ["aws_instance.web"] } output "web_ip" { diff --git a/config/test-fixtures/basic.tf.json b/config/test-fixtures/basic.tf.json index a7d3b308c..7c43b5b30 100644 --- a/config/test-fixtures/basic.tf.json +++ b/config/test-fixtures/basic.tf.json @@ -20,7 +20,8 @@ "resource": { "aws_instance": { "db": { - "security_groups": ["${aws_security_group.firewall.*.id}"] + "security_groups": ["${aws_security_group.firewall.*.id}"], + "depends_on": ["aws_instance.web"] }, "web": { From 20da842bcfb1b0a903d76536795918f1dfd4126f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 17:16:48 -0700 Subject: [PATCH 2/3] config: validate dependsOn --- config/config.go | 11 +++++++++++ config/config_test.go | 7 +++++++ config/test-fixtures/validate-bad-depends-on/main.tf | 3 +++ config/test-fixtures/validate-good/main.tf | 2 ++ 4 files changed, 23 insertions(+) create mode 100644 config/test-fixtures/validate-bad-depends-on/main.tf diff --git a/config/config.go b/config/config.go index f2c8b41b5..012fcee6f 100644 --- a/config/config.go +++ b/config/config.go @@ -155,6 +155,17 @@ func (c *Config) Validate() error { } dupped = nil + // Make sure all dependsOn are valid in resources + for n, r := range resources { + for _, d := range r.DependsOn { + if _, ok := resources[d]; !ok { + errs = append(errs, fmt.Errorf( + "%s: resource depends on non-existent resource '%s'", + n, d)) + } + } + } + for source, vs := range vars { for _, v := range vs { rv, ok := v.(*ResourceVariable) diff --git a/config/config_test.go b/config/config_test.go index 9b141fa8f..508a281ab 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -16,6 +16,13 @@ func TestConfigValidate(t *testing.T) { } } +func TestConfigValidate_badDependsOn(t *testing.T) { + c := testConfig(t, "validate-bad-depends-on") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_badMultiResource(t *testing.T) { c := testConfig(t, "validate-bad-multi-resource") if err := c.Validate(); err == nil { diff --git a/config/test-fixtures/validate-bad-depends-on/main.tf b/config/test-fixtures/validate-bad-depends-on/main.tf new file mode 100644 index 000000000..60ca18c7e --- /dev/null +++ b/config/test-fixtures/validate-bad-depends-on/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "web" { + depends_on = ["aws_instance.db"] +} diff --git a/config/test-fixtures/validate-good/main.tf b/config/test-fixtures/validate-good/main.tf index 965877a10..5a4f87159 100644 --- a/config/test-fixtures/validate-good/main.tf +++ b/config/test-fixtures/validate-good/main.tf @@ -32,4 +32,6 @@ resource aws_instance "web" { device_index = 0 description = "Main network interface" } + + depends_on = ["aws_security_group.firewall"] } From f47956d62f01e2c630cc1a0b76d578f3737f1262 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 18:20:03 -0700 Subject: [PATCH 3/3] terraform: dependsOn builds into the graph --- terraform/graph.go | 45 +++++++++++++++++++ terraform/graph_test.go | 25 +++++++++++ .../test-fixtures/graph-depends-on/main.tf | 5 +++ 3 files changed, 75 insertions(+) create mode 100644 terraform/test-fixtures/graph-depends-on/main.tf diff --git a/terraform/graph.go b/terraform/graph.go index 2d8bb92d5..db3889a2e 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -106,6 +106,9 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { // and no dependencies. graphAddConfigResources(g, opts.Config, opts.State) + // Add explicit dependsOn dependencies to the graph + graphAddExplicitDeps(g) + // Next, add the state orphans if we have any if opts.State != nil { graphAddOrphans(g, opts.Config, opts.State) @@ -377,6 +380,48 @@ func graphAddDiff(g *depgraph.Graph, d *Diff) error { return nil } +// graphAddExplicitDeps adds the dependencies to the graph for the explicit +// dependsOn configurations. +func graphAddExplicitDeps(g *depgraph.Graph) { + depends := false + + rs := make(map[string]*depgraph.Noun) + for _, n := range g.Nouns { + rn, ok := n.Meta.(*GraphNodeResource) + if !ok { + continue + } + + rs[rn.Config.Id()] = n + if len(rn.Config.DependsOn) > 0 { + depends = true + } + } + + // If we didn't have any dependsOn, just return + if !depends { + return + } + + for _, n1 := range rs { + rn1 := n1.Meta.(*GraphNodeResource) + for _, d := range rn1.Config.DependsOn { + for _, n2 := range rs { + rn2 := n2.Meta.(*GraphNodeResource) + if rn2.Config.Id() != d { + continue + } + + n1.Deps = append(n1.Deps, &depgraph.Dependency{ + Name: d, + Source: n1, + Target: n2, + }) + } + } + } +} + // graphAddMissingResourceProviders adds GraphNodeResourceProvider nodes for // the resources that do not have an explicit resource provider specified // because no provider configuration was given. diff --git a/terraform/graph_test.go b/terraform/graph_test.go index 4d1795878..dcbeb7ce1 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -51,6 +51,21 @@ func TestGraph_cycle(t *testing.T) { } } +func TestGraph_dependsOn(t *testing.T) { + config := testConfig(t, "graph-depends-on") + + g, err := Graph(&GraphOpts{Config: config}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphDependsStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestGraph_state(t *testing.T) { config := testConfig(t, "graph-basic") state := &State{ @@ -347,6 +362,16 @@ root root -> aws_load_balancer.weblb ` +const testTerraformGraphDependsStr = ` +root: root +aws_instance.db + aws_instance.db -> aws_instance.web +aws_instance.web +root + root -> aws_instance.db + root -> aws_instance.web +` + const testTerraformGraphDiffStr = ` root: root aws_instance.foo diff --git a/terraform/test-fixtures/graph-depends-on/main.tf b/terraform/test-fixtures/graph-depends-on/main.tf new file mode 100644 index 000000000..2a2d904dc --- /dev/null +++ b/terraform/test-fixtures/graph-depends-on/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "web" {} + +resource "aws_instance" "db" { + depends_on = ["aws_instance.web"] +}