hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.32k stars 9.49k forks source link

Support use cases with conditional logic #1604

Closed phinze closed 7 years ago

phinze commented 9 years ago

It's been important from the beginning that Terraform's configuration language is declarative, which has meant that the core team has intentionally avoided adding flow-control statements like conditionals and loops to the language.

But in the real world, there are still plenty of perfectly reasonable scenarios that are difficult to express in the current version of Terraform without copious amounts of duplication because of the lack of conditionals. We'd like Terraform to support these use cases one way or another.

I'm opening this issue to collect some real-world example where, as a config author, it seems like an if statement would really make things easier.

Using these examples, we'll play around with different ideas to improve the tools Terraform provides to the config author in these scenarios.

So please feel free to chime in with some specific examples - ideally with with blocks of Terraform configuration included. If you've got ideas for syntax or config language features that could form a solution, those are welcome here too.

(No need to respond with just "+1" / :+1: on this thread, since it's an issue we're already aware is important.)

pll commented 8 years ago

@cihatgenc - Ah, okay. Very cool. Thanks for explaining this to me!

nathanielks commented 8 years ago

I really like the when syntax as well. There are a few domains that are used only in production, so when I spin up intermediate environments they're not needed at all. My current solution is to have a script generate the json for each environment, so having a when property on a resource and only create when that condition is satisfied is muy bueno.

marekrogala commented 8 years ago

this would be very useful for me. Is there any chance for this to be implemented soon?

pll commented 8 years ago

@marekrogala - Fwiw - I've been able work around most of my requirements for conditionals by using boolean logic and splats.

ketzacoatl commented 8 years ago

While "neat hacks" are fun, or helpful, or whatever, it's sucks that we fill up our clean Terraform with crazy and unreadable hacks to get "if, then, else" logic to fit in place.

pll commented 8 years ago

I wouldn't call boolean logic a "neat hack". And I wouldn't say that my Terraform code is any less clean than it was or any less readable. In fact, I'd actually it's quite an elegant solution that prevents polluting your code with a lot of 'if, then, else' logic. I go out of my way to avoid using that construct and case statements in any language to begin with.

ketzacoatl commented 8 years ago

Sorry if I am not being clear @pll, I am referring to solutions like: ${replace(replace(var.baz, replace(var.foo, "/^$/", "/^.*$/"), ""), "/^$/", var.bar)}. This is an unreadable hack, in my book - it's doing one thing, but takes a lot of thought to understand the original intention or purpose.

pll commented 8 years ago

@ketzacoatl - Oh, agreed. That is a complete hack! What I did with my boolean logic hack was to set a variable to 0 or 1, then use count to determine whether or not to actually create the object. I found that pretty elegant.

ketzacoatl commented 8 years ago

In a way, what you describe is "clean" (again, in my book), but not when it comes to the implementation - eg, if you have to start thinking about list indexes, simply because you want a conditional/boolean on/off for an object... we have a problem

mengesb commented 8 years ago

I think the 'when' syntax is the cleanest most declarative approach that we can take in solving this problem.

As it stands, with my development, I'd have to create several variants of my modules in order to handle the several variations of the implementation. While I embrace and totally agree with being declarative I think some basic logic structures don't complicate the reading and give extremely valuable control to both the module author and consumer.

jasonf20 commented 8 years ago

a for would be very useful. For example, we want to create a DNS record for each server we start up, and an associated health check for it. The current available method will create one DNS record with all the IPs of the servers. This does not allow for health checks (since they are for the entire record, and creating the health checks is also per a singular IP).

I would like to do something like the following:

for (servers.ips) resource "dns" "record" {
   ip = "${current.public_ip}"
   ...
} 

Or something similar.

This is just one example, having a for statement will also simplify lots of our terraform scripts which create lots of very similar instances (we use a module to help with this, but we still need to configure the module over and over again just to change a few params)

BSick7 commented 8 years ago

The notion of a for is partially supported. You can do the following. I oversimplified depending on what data type servers.ips is.

resource "dns" "record" {
  ip = "${lookup(servers.ips, count.index)}"
  count = "${length(servers.ips)}"
}
jasonf20 commented 8 years ago

@BSick7 That is actually a cool trick. Thanks, I will give it a try.

If the "thing" created is more complex though then you would need to create a module, but as-long as count can be supplied for modules as well it should work.

jasonf20 commented 8 years ago

@BSick7 Unfortunately

count.index: count.index is only valid within resources

This makes it more difficult (perhaps still possible though), will just have to access dependencies by index also.

jasonf20 commented 8 years ago

@BSick7 I gave it a try. However I have error's due to Cycle dependency (thought I don't think there is one)

Here is the record and hc definition:

resource "aws_route53_health_check" "my-server_hc" {
    port = "18083"
    type = "HTTP"
    resource_path = "/"
    failure_threshold = "5"
    request_interval = "10"

    count = "${length(split(var.instance_public_ips))}"
    ip_address = "${element(split(var.instance_public_ips), count.index)}"

    tags = {
        Name = "my-server-health-check"
    }
}

resource "aws_route53_record" "my-server" {
    zone_id = "${var.zone_id}"

    name = "my-server${var.delimiter}${var.domain_suffix}"

    set_identifier = "${var.set_identifier}"

    type = "A"
    ttl = "${var.ttl}"
    weight = 1

    health_check_id = "${element(aws_route53_health_check.my-server_hc.*.id, count.index)}"

    count = "${length(split(var.instance_public_ips))}"

    records = [ "${lookup(split(var.instance_public_ips), count.index)}"]
}

And I get the following error:

* Cycle: module.dns.aws_route53_health_check.my-server_hc (destroy), module.dsp.module.my-server.output.instance_public_ips, module.dsp.output.haproxy-public-ips, module.dns.var.instance_public_ips, module.dns.aws_route53_record.my-server (destroy), module.dsp.module.my-server.aws_instance.cloudinit (destroy), module.dsp.module.my-server.aws_instance.cloudinit
steve-jansen commented 8 years ago

Sharing a hack @BSick7 and I use to enable/disable resources.

We use a string variable to set count = 0 when the string is empty, or count = 1 when the string is non-empty.

An overly simplified example:

variable "foo" {
  type = "string"
  default = ""
}

resource "aws_route53_record" "foo" {
   zone_id = "${aws_route53_zone.primary.zone_id}"
   name = "${var.foo}"
   type = "A"
   ttl = "300"
   records = ["${aws_eip.lb.public_ip}"]

  # HACK to return 0 if var.foo is empty, or 1 if non-empty.
  # Disables this resource when var.foo is not set.
  # We're assuming that the # char would be an invalid char in this string (e.g., FQDN)
  count = "${length(compact(split("#", var.foo)))}"
}

Credit: @thegedge ^^^

pll commented 8 years ago

@steve-jansen : You can just set the variable to either 1 or 0 and not worry about using length().

variable "createN" { default = 0 }

resource "aws_instance" "foo" {
  count = "${var.createN}"
  ...
}

I use this to create VGW resources. Some VPCs have a directConnect, and therefore need a VGW created, others don't. Set 'create_vgw = 1' and you get a vgw, set it to 0, you don't.

steve-jansen commented 8 years ago

@pll yep, we use that simple 0/1 too. The string method is just another tool in the toolbox if you don't want to remember to sync the state of two variables in Atlas. Our use case is a module that should or should not run based on a value existing. Both ways work.

ocxo commented 8 years ago

We tried something like this at one point:

variable "flag" {
  default = {
    stage = 1
    prod = 0
  }
}
variable "env" { default = "stage" }
...
count = "${lookup(var.flag, var.env)}"

But it made Terraform really sad and error barfy. I'll have to try again to see if this has been fixed in a recent release.

pll commented 8 years ago

@fromonesrc - My experience has been that using lookup() and length() tend to cause the most sadness and barfiness. Which is why I mentioned my simple 0/1 hack. It seems less prone to sadness :)

lubars commented 8 years ago

I want the choice of having a /data directory that's either a mount point for an ebs_block_device, or a simple directory off the root_block_device. The technique I was hoping to use was to skip creation of the ebs_block_device when the volume_size was 0, but I don't see any way to make creation of the ebs_block_device conditional (which is how I landed in this thread).

sheerun commented 8 years ago

Maybe instead of inventing brand new declarative language, embed Lua interpreter and allow for specifying pure functions in it? I think it's better option for everyone. Haproxy and Nginx already embed Lua interpreters to give few examples.

ketzacoatl commented 8 years ago

Saltstack has the concept of "renderers", they are responsible for "rendering" the text that becomes the code for salt to parse. It seems like an over-abstraction, except it provides a completely pluggable system, so some user-developers use the default (jinja), while others write in python directly or another templating engine (like mako). In practice, it has been very enjoyable to work with saltstack because of this type of flexibility. A lua interpreter would be interesting, if provided along with an analog to salt's renderers. Either way, I would also agree that this greatly simplifies the question of how to provide conditionals and things like that.

sheerun commented 8 years ago

In #5278 I proposed embedding lua interpreter with sample syntax. Most importantly it allows for discovery of used variables, so terraform knows when to re-execute them. For example:

instance_type = "#{
  if ${count.index} > 3 then
    return 'c4.xlarge'
  else
    return 'm3.large'
  end
}"

#{} defines code to evaluate, and ${} interpolates used variables. In this case terraform knows to re-evaluate this function each time count.index change, otherwise use cached result from tfstate.

BSick7 commented 8 years ago

I am not a huge fan of adding lua interpreter to terraform. One of the great things about terraform is that it unifies many technologies and providers into one language. It seems like a detractor from simplification.

apparentlymart commented 8 years ago

@sheerun thanks for the Lua suggestion!

For others who want to discuss the merits and details of that, let's do that in the comments of #5278 rather than here, since that's a pretty big architectural shift with lots of implications and I it's better to let this issue keep its original purpose of just collecting use-cases.

@BSick7, please feel free to repeat your feedback on the other issue. :grinning:

franklinwise commented 8 years ago

There are two separate issues here.

  1. Conditional Creation of Resources. Do or don't create them. (Is Declarative, Is simple)
  2. Random conditional logic like jinja2 is to ansible. (Is procedural, is complex)

I'm proposing that we implement No. 1 above and move No. 2 to another thread because No 1 is valuable, simple and has a clean obvious way of implementing it. Which is by using "when" declaratively, which many others liked (see my comment on Oct 7, 2015). I'll repost it here for convenience:

resource "aws_instance" "machine" {
  when = "${var.atlas_enabled}"
  ami = "${atlas_artifact.machine.id}"
}

resource "aws_instance" "machine" {
  when = "not ${var.atlas_enabled}"
  ami = "${var.ami_string}"
}

If you want to do No. 2, we should look at how ansible does it with jinja2 templates in the string.

tehmaspc commented 8 years ago

@franklinwise - i like this. I agree there are two things here.

ketzacoatl commented 8 years ago

I vote to support your proposal.

mengesb commented 8 years ago

+1

There are real world needs to address both at some time, however I think maintaining the original purpose (declarative conditionals) should be the focus of this thread. I'd hope this is on the roadmap by now.

~Brian

On Thu, Mar 3, 2016 at 12:14 PM, ketzacoatl notifications@github.com wrote:

I vote to support your proposal.

— Reply to this email directly or view it on GitHub https://github.com/hashicorp/terraform/issues/1604#issuecomment-191942876 .

lubars commented 8 years ago

+1 / support

aavileli commented 8 years ago

Dont forget people use the tool because of its simplicity. Keep the tool simple with conditional creation of resources which seems the best solution. Having an option to remove an attribute from the resource instead of removing the complete resource is an idea to keep in mind as it is less typing and less error prone.

sheerun commented 8 years ago

I agree simple conditional logic would be better than full-blown interpreter at least for now.

It would be nice if terraform introduced boolean logic ad comparisons into interpolation syntax we can get little more advanced expressions as well (&& || == !=)

pll commented 8 years ago

@franklinwise - Regarding your example:

resource "aws_instance" "machine" {
  when = "${var.atlas_enabled}"
  ami = "${atlas_artifact.machine.id}"
}

One can already accomplish this using count = "${var.true_or_false}" and setting that value to 0 or 1. Unfortunately, there's no enforced data integrity in the terraform DSL. One could set var.true_or_false to 8 since there's no way to declare variable type or enforce a var is essentially a boolean.

Using your method has some obvious aesthetic advantages. In short, it's clear, concise, and obvious that there's a condition to be met. Whereas using count = the intent is not obvious at all unless the reader of the code has become quite familiar with the terraform DSL. Even then, it can be tough to decipher.

Implicit in your example though, is enforced data integrity. The value of such a variable can be interpreted to be anything more or less than 0/1, or true/false. Either there has to be type enforcement, or terraform must ignore the actual value and use only that there is or is not value.

As long as that is built in to the implementation, I would wholeheartedly support this solution!

Great idea, thanks for sharing it!

+1

davekonopka commented 8 years ago

re: @franklinwise's point #1:

  1. Conditional Creation of Resources. Do or don't create them. (Is Declarative, Is simple)

The when parameter seems like it would be declarative friendly. I ran into a use case along these lines with a module last week and posted about it in more detail.

rodlogic commented 8 years ago

For my existing problem, I just need to ignore resources based on an input variable. So count = 0 seems like a work around for now.

What about conditional outputs? I.e. if I don't create resource A, I don't want to output A's IP address.

ErikEvenson commented 8 years ago

@rodlogic a good work around is to wrap your terraform apply call in a bash script and then use terraform output in some bash conditional.

pll commented 8 years ago

@rodlogic - If you create resources based on count, create your outputs based on splats.

output "public_ids" { value = "${join(",",aws_subnet.public.*.id) }" }

nathanielks commented 8 years ago

@pll @rodlogic thankfully 0.7 will have better support for splats too! https://github.com/hashicorp/terraform/issues/2821#issuecomment-195051359

pll commented 8 years ago

@nathanielks any idea when .7 is due out ?

nathanielks commented 8 years ago

I don't, unfortunately. That's something the core team would be able to answer!

pll commented 8 years ago

Okay, thanks :)

thegranddesign commented 8 years ago

There were far too many comments for me to read through here, but I'll throw my two cents in.

I don't think a declarative syntax is going to cut it enough to be useful. If we're going to go down this route, my feeling is that we may as well do it "right".

I agree that Salt's approach is very nice. The JSON is put through a renderer before being ingested. This seems like the most flexible approach.

I know some are floating the idea of adding a declarative when on the resource. As others have said, this works for the resource, but, let's say for example placement_group on an aws_autoscaling_group. The placement_group is only valid if the instance type is > t2.micro. So what we need is something like (using ERB syntax, but Jinja or anything else would work as well).

resource "aws_autoscaling_group" "my_asg" {
<% unless ['t2.nano', 't2.micro'].include? var.servers.size -%>
  placement_group "my_placement_group"
  # Other stuff
<% end -%>
}

Or whatever.

nathanielks commented 8 years ago

@thegranddesign I'd encourage you to take the time :smile:. There's some good stuff.

thegranddesign commented 8 years ago

@nathanielks I read through about half. I've got work that needs done. :wink:

nathanielks commented 8 years ago

@thegranddesign :+1:

thegranddesign commented 8 years ago

Ok, I finished reading the rest of the comments. when will definitely handle some but not all use cases as I described in my original comment.

I'll move further discussion to #5278

nathanielks commented 8 years ago

@thegranddesign I secretly just wanted to find out how fast you can read

glenjamin commented 8 years ago

One of my favourite parts of terraform is that it's really easy to do a bit of scripting externally and generate terraform JSON.

For this reason, I don't think adding a pre-processor into the core is particularly beneficial.

IMO it makes sense to focus on the logic features which cannot be achieved by a pre-processor.

serialseb commented 8 years ago

A when is already achievable very easily with count, be it as a 1 or 0 from true/false, or as a count * 0/1.

when is nice but only cleans up the existing syntax. I'd find a not() operator bloody useful, and an isemtpy(), as we don't have conditionals and those require regex tricks to work, which is just not nice.

passing empty arrays is hard too, as "" is not an empty, and you ahve to use the convulated compact(split("",var.myvalue)) to turn an empty string into an empty array.

So if it was a vote, i'd ask for more expressive ways of using the existing vars to do conditionals, rather than additional conditional arguments on resources, although if we could get both i'd be jolly happy.