Add dash at the end of name prefixes

[miguelaferreira]

to align them between all resources.

[Etiene]

Actually we just realised that this would be a backwards incompatible change, so that means that people currently using this module would have their auto scaling groups destroyed. One solution to that could be having a new var with the name?

[eedwards-sk]

@Etiene people using this module should be pulling in an explicit version, so why not just make a major version bump for it? Nobody should be pulling in new module versions and pushing them out to prod without testing them first, I hope :)

(Otherwise what's the point of modules and versioning in the first place, if we can never make breaking changes...)

[brikis98]

Because there's no upgrade path; it will end up deleting the ASG and other resources.

[eedwards-sk]

Okay, that's fair. I'm still new to public modules, I guess I'm more accustomed to breaking changes with most template changes. Thanks for the response.

[miguelaferreira]

I'm happy to make a new PR that introduces a boolean variable to change the names of the resources or keep them as they were. I do need to know what should the default value for that variable be?

Let me know.

[josh-padnick]

Perhaps it's enough if users just manually specify cluster- (with a trailing dash) for their var.cluster_name? Although it's not as elegant as the user merely specifying cluster, it's hard to justify a significant breaking change when users could achieve this change on their own with no breaking change.

[eedwards-sk]

@josh-padnick for me thats not a solution because some resources already append a dash which means I'd end up with cluster--FOO

consistency is what is needed

[josh-padnick]

@brikis98 Can you suggest a next step here? Do we just accept the breaking change, but provide detailed upgrade instructions?

[brikis98]

The problem is that there is no viable upgrade path with the PR as submitted. If you change the name of certain AWS resources (e.g., an ASG), it does a delete + recreate, which would cause downtime in between. No set of terraform state mv commands will fix that.

So the only viable option is to add one or more variables that let users customize the name of these resources, including whether a dash is appended or not. By default, the variables will be set to empty string, and the code will append the dash; but users who need to migrate can set those variables to the old names with no dash, so there is no diff, and therefore, no outage.

Example:

variable "asg_name" {
  default = ""
}

# In the ASG code...
name_prefix = "${var.asg_name == "" ? "${var.cluster_name}-" : var.asg_name}"

[eedwards-sk]

I noticed this got reverted.

Is anyone able to provide a new PR similar to @brikis98 suggestions? I'm close to launching a consul cluster for live use and I'd love to satisfy my OCD desire to unify the -'s first 😛