terraform-aws-modules / terraform-aws-ec2-instance

Terraform module to create AWS EC2 instance(s) resources πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/ec2-instance/aws
Apache License 2.0
754 stars 1.87k forks source link

Allow AMI change to be ignored #325

Closed jensenbox closed 1 year ago

jensenbox commented 1 year ago

Many times we will construct an EC2 instance with the current AMI at the time but would prefer it to not be replaced. We source the AMI from a filtered data "aws_ami" declaration. If the ID for the AMI changes we would like to have the EC2 stay as it is and not be replaced.

Describe the solution you'd like.

I think having some sort of lifecycle flag for changed parameters would make this possible.

Describe alternatives you've considered.

Hardcoding the AMI directly into the source.

lallish commented 1 year ago

This is the biggest issue with this module. It's the lack of lifecycle and ignore_changes like:

lifecycle {
    prevent_destroy = true
}

ignore_changes = [
    ami
]

Because you will never again want to run your terraform state again if you use the ec2_instance module, in the risk of the entire instance getting replaced. But that makes terraform flawed, because its entire point is to run the whole main state and make sure that configuration matches.

antonbabenko commented 1 year ago

There is no way in Terraform to parametrize ignore_changes values to allow users who like the default experience and users who want to ignore changes in some arguments (like ami) in this module.

@lallish This is not the flaw of this module, but the intended behavior. Users specify ami and get an instance provisioned for them. If ami changes - the instance is recreated.

Usually, you want to use the data source aws_ami to get the AMI ID dynamically. If you can find filter arguments in a very detailed way, you should not have the rotation very often. It may not be what you want. Just saying.

There is nothing else we can do in this module for this issue.

331 fixes it. :)

jensenbox commented 1 year ago

Thanks for the response on this.

I do use the AMI filter technique but it still rotates the AMI ID too often for my taste - once a month or so.

I was just thinking that maybe a work around would be to store the AMI ID in parameter store and have it ignore the value changes after initial creation.

I can certainly do that outside of the module but it would be cool to have it integrated.

Thoughts?

On Fri, Apr 28, 2023, 7:17 a.m. Anton Babenko @.***> wrote:

There is no way in Terraform to parametrize ignore_changes values to allow users who like the default experience and users who want to ignore changes in some arguments (like ami) in this module.

@lallish https://github.com/lallish This is not the flaw of this module, but the intended behavior. Users specify ami and get an instance provisioned for them. If ami changes - the instance is recreated.

Usually, you want to use the data source aws_ami to get the AMI ID dynamically. If you can find filter arguments in a very detailed way, you should not have the rotation very often. It may be not what you want. Just saying.

There is nothing else we can do in this module for this issue.

β€” Reply to this email directly, view it on GitHub https://github.com/terraform-aws-modules/terraform-aws-ec2-instance/issues/325#issuecomment-1527640607, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABOGUPMBO2NC7ODQ6TKH3LXDPGNVANCNFSM6AAAAAAWZLFDFY . You are receiving this because you authored the thread.Message ID: <terraform-aws-modules/terraform-aws-ec2-instance/issues/325/1527640607@ github.com>

antonbabenko commented 1 year ago

@jensenbox It has just been added in #331, which will be merged shortly.

jensenbox commented 1 year ago

Thank you very much!

lallish commented 1 year ago

Nice! Since you want to get the latest AMI through filtering for new instances but still keep the old AMI for old instances so they aren't replced. Makes it a lot more reliable now! The _preventdestroy would also be a good thing but is another topic. I don't see much else that would frequently replace the instance other than new AMIs.

antonbabenko commented 1 year ago

This issue has been resolved in version 5.0.0 :tada:

antonbabenko commented 1 year ago

@lallish The value of prevent_destroy can't be parametrized, so users won't be able to delete instance if we have prevent_destroy = true in the module. This is the behavior of Terraform.

github-actions[bot] commented 1 year ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.