search

hashicorp / terraform-aws-nomad

A Terraform Module for how to run Nomad on AWS using Terraform and Packer
Apache License 2.0
254 stars 187 forks source link

Limiting value for EC2 instance tag #8

Open dgrstl opened 7 years ago

dgrstl commented 7 years ago

https://github.com/hashicorp/terraform-aws-nomad/blob/223be6dfee91b7d94464609c65ae5a43ae6a1adb/modules/nomad-cluster/main.tf#L32

${var.cluster_tag_value} makes more sense and would make it consistent with the Consul module (https://github.com/hashicorp/terraform-aws-consul/blob/master/modules/consul-cluster/main.tf#L39). In any case, feels like each module should work in roughly the same way.

brikis98 commented 7 years ago

Sure, consistency seems like a good idea. Should be an easy PR fix if anyone has the time :)

MatthiasScholz commented 7 years ago

After investigating into the issue I would rather consider changing this line: key = "${var.cluster_tag_key}" ( nomad-cluster/main.tf#L31 ) to key = "Name" to be consistent with consul-cluster/main.tf#L32.

The value ${var.cluster_tag_key} will be filled by default to "Name" (nomad-cluster/variables.tf#L74) and this module does not needs to make use of the functionality intended for cluster_tag_value.

brikis98 commented 7 years ago

Thinking about it some more, we may want to change how we do tags a little bit similar to how it's described here: https://github.com/hashicorp/terraform-aws-vault/pull/12#pullrequestreview-67844307. We set some default tags that should work reasonably, but also take in a map of tags that allows the user to add/override tags as they wish.