hashicorp / terraform-aws-consul

A Terraform Module for how to run Consul on AWS using Terraform and Packer
Apache License 2.0
399 stars 482 forks source link

Enable setting up lifecycle hooks on the ASG #223

Closed mr-miles closed 3 years ago

mr-miles commented 3 years ago

Without this change, it is not possible to create lifecycle hooks that apply when the ASG is first created. If you use an "aws_autoscaling_lifecycle_hook" resource, the instances are created before the hook is attached and so they don't get it.

The specific use-case is for the ASG to only consider the instances InService when the cluster has established itself and stabilised - this can be some minutes after the instance has completed its boot sequence.

hashicorp-cla commented 3 years ago

CLA assistant check
All committers have signed the CLA.

mr-miles commented 3 years ago

Will do.

I also just discovered that alltrue / anytrue were only added in tf 0.14 - are you happy that I just bump the minimum version in the docs to reflect it (rather than re-complicate the validation logic)

On Wed, May 26, 2021 at 1:04 PM Yevgeniy Brikman @.***> wrote:

@.**** commented on this pull request.

OK, code looks great now. One final request: could you pull in the latest from master? Looks like we must've merged something while this PR was open and there is now a merge conflict.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hashicorp/terraform-aws-consul/pull/223#pullrequestreview-668931055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQD4G56YHW7EP4UHAJV2DTPTPUFANCNFSM44LECJZA .

brikis98 commented 3 years ago

Will do. I also just discovered that alltrue / anytrue were only added in tf 0.14 - are you happy that I just bump the minimum version in the docs to reflect it (rather than re-complicate the validation logic) On Wed, May 26, 2021 at 1:04 PM Yevgeniy Brikman @.> wrote: @*.** commented on this pull request. OK, code looks great now. One final request: could you pull in the latest from master? Looks like we must've merged something while this PR was open and there is now a merge conflict. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#223 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQD4G56YHW7EP4UHAJV2DTPTPUFANCNFSM44LECJZA .

Ah, good point. Yea, let's bump the min version. Thanks!

mr-miles commented 3 years ago

Merge conflict sorted

brikis98 commented 3 years ago

Tests passed! Merging now.

brikis98 commented 3 years ago

https://github.com/hashicorp/terraform-aws-consul/releases/tag/v0.10.1

mr-miles commented 3 years ago

All good and using it here

On Fri, 28 May 2021 at 14:27, Yevgeniy Brikman @.***> wrote:

https://github.com/hashicorp/terraform-aws-consul/releases/tag/v0.10.1

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hashicorp/terraform-aws-consul/pull/223#issuecomment-850418809, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQD4DMSMXIE7RBJURDT23TP6K5XANCNFSM44LECJZA .