terraform-aws-modules / terraform-aws-autoscaling

Terraform module to create AWS Auto Scaling resources πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/autoscaling/aws
Apache License 2.0
289 stars 553 forks source link

Possibility to prevent terraform from modifying launchtemplate image_id #214

Closed p24-max closed 1 year ago

p24-max commented 1 year ago

Is your request related to a problem? Please describe.

I deployed an autoscaling setup with this terraform template, the image_id property is required and I did set it to an amazon-linux-2 id. I also make use of the imagebuilder which is able to update the autoscaling launchtemplate configuration (distribution configuration) and update the launch template image_id property after successful image creation. The issue is now when I apply terraform template again, terraform changes the image_id of the launchtemplate (which was modified by the imagebuilder) back to the amazon-linux-2-id which is defined in the terraform template.

Describe the solution you'd like.

Possibility (eg. a config flag) to not recover the launch template image_id when it has been changed outside of terraform. This config flag should set lifecycle-ignore-changes: https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes

Describe alternatives you've considered.

Always start image builder after applying terraform, but autoscaling tries to deploy a wrong machine to the loadbalancer until the image is built successfully.

Additional context

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

p24-max commented 1 year ago

Still relevant for me. @bryantbiggs / @schniber what do you think?

schniber commented 1 year ago

Hey @p24-max What about you update your Terraform config with the new image id after you run your image builder process outside of Terraform ?

It's normal that Terraform tries to revert what happened outside, because the state becomes in drift with the reality.

Bests

p24-max commented 1 year ago

@schniber thanks for your response.

This is a possibility. But would rise the need for manual effort each time a new ami is built (and the build is started with automatic trigger). AWS image builder distribution is able to automatically set the new ami-ID to the launch template when image building is complete.

Currently, I'm working around with a git patch which works fine:

diff --git a/cloud/production/autoscaling/.terragrunt-cache/40Po78N9luxchDIngWUvs21Y8-8/7ii-Ht_IxPnCZFWq71fjiqG-f_c/main.tf b/cloud/production/autoscaling/.terragrunt-cache/40Po78N9luxchDIngWUvs21Y8-8/7ii-Ht_IxPnCZFWq71fjiqG-f_c/main.tf
--- a/cloud/production/autoscaling/.terragrunt-cache/40Po78N9luxchDIngWUvs21Y8-8/7ii-Ht_IxPnCZFWq71fjiqG-f_c/main.tf    
+++ b/cloud/production/autoscaling/.terragrunt-cache/40Po78N9luxchDIngWUvs21Y8-8/7ii-Ht_IxPnCZFWq71fjiqG-f_c/main.tf    (date 1669827840207)
@@ -334,6 +334,9 @@

   lifecycle {
     create_before_destroy = true
+    ignore_changes = [
+      image_id
+    ]
   }

   tags = var.tags

Making the ignore_changes configurable would help me a lot. I can also create a pullrequest but first I'd like to know if it is worth it or you dont want such a feature.

bryantbiggs commented 1 year ago

This is not something that can be supported I am afraid. I am not familiar with image builder but if it is modifying launch templates and triggering the autoscaling group to roll out changes, that directly conflicts with Terraform which wants to be the sole owner of the infra it manages.

The change you are proposing is not valid because launch templates are immutable. For any change to a launch template, this is made in a new version of the launch template - so in theory you would be telling the autoscaling group to ignore the launch template entirely which is not a great idea at all

p24-max commented 1 year ago

Sorry, maybe I explained a bit wrong: It is not modifying the launch template, it is creating a new version and just changing the image_id to the new built ami-ID.

Terraform is still the owner of the infra, except the image_id property of the launch template in this case.

I got your point and I totally agree with that. But it would also not be a good idea to build a lambda which is getting triggered when an image build is complete which then triggers the terraform infra deploy pipeline injecting the new ami-id as variable when AWS has a native feature to rollout the new ami to the launch template? It would introduce a lot of additional complexity.

bryantbiggs commented 1 year ago

But it would also not be a good idea to build a lambda which is getting triggered when an image build is complete which then triggers the terraform infra deploy pipeline injecting the new ami-id as variable when AWS has a native feature to rollout the new ami to the launch template?

I'm not following this?

Again, I don't know how Image Builder works but if its just creating a new launch template version, you can specify in this module what version of the launch template you wish to use. If it is updating the autoscaling group to use the new version it creates, thats a problem and won't jive with Terraform

p24-max commented 1 year ago

Again, I don't know how Image Builder works

ok. Maybe @schniber has knowledge about the same?

If it is updating the autoscaling group to use the new version it creates

No. The autoscaling group uses launchtemplate version = $Default which is managed by terraform. Image builder just sets the newly created launchtemplate version to default. This is why it is working, except the necessary patch which I shared above.

bryantbiggs commented 1 year ago

Have you tried setting update_default_version = false? or specifying the default_version?

p24-max commented 1 year ago

Currently it is set update_default_version = true to be able to rollout changes to the launchtemplate from within terraform (eg. when I'd like to change instance_type).

bryantbiggs commented 1 year ago

right - but thats the source of your issue, no?

I think you have all the info you need - this isn't a module related issue but a fundamental issue related to understanding how Terraform works and the expected outcome when interacting with outside influences that are making changes to things Terraform manages

p24-max commented 1 year ago

right - but thats the source of your issue, no?

No, because setting it to false will result in the situation that changes are not getting activated in AWS cloud. On infra change, terraform would create a new launchtemplate version which will not be used (as its not set to default). Setting this config option to true will introduce the issue with the image_id (which I described in previous posts).

schniber commented 1 year ago

Hello, I agree with @bryantbiggs on this one. You need to choose who is the single source of truth for the AMI id to be used by the launch template. It looks like it's image builder in your case.

Ideally you could use Image builder to generate the AMI only (and not changing the Launch template) and in a second step update the inputs of the module and let it run. This way Terraform would still be your single source of truth for managing your ASG while Image Builder would be for the AMI.

I don't see any technical solution allowing both use cases, unless If I missed something.

Happy new year by the way !

p24-max commented 1 year ago

Happy new year by the way !

:) Thank you @schniber , same to you!

You need to choose who is the single source of truth [...]. It looks like it's image builder in your case.

Yes its the image builder.

Ideally you could use Image builder to generate the AMI only [...] and in a second step update the inputs of the module and let it run

This is more effort / complexity on my end, as AWS offers the automatic distribution feature which is really simple to configure, also with terraform: https://docs.aws.amazon.com/imagebuilder/latest/userguide/dist-using-launch-template.html and this works great.


I think I will go with the patch https://github.com/terraform-aws-modules/terraform-aws-autoscaling/issues/214#issuecomment-1368096712 . Maybe in the future someone else will also faces this problem so that it might get improved in this module in the future.

Anyway, thanks for your help and have a great start in 2023

stewartcampbell commented 1 year ago

What about using an SSM parameter instead of an AMI ID?

https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/create-launch-template.html#use-an-ssm-parameter-instead-of-an-ami-id

Not even sure if that works with this module yet, as it's only just been released. Just about to try it and thought I'd check open issues first :)

Edit: works as long as your AMI ID is the only string there. Does not work if you have nested JSON as you can't pick an attribute, e.g., resolve:ssm:/some/param/name:image_id.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

schniber commented 1 year ago

Hello All,

Thanks @stewartcampbell πŸ‘πŸ½

@p24-max : the new feature for SSM parameter use in a launch template could probably help in your case provided that Image Builder maintains an SSM Parameter store entry. Here's an AWS sample to achieve such : https://github.com/aws-samples/amazon-ec2-image-builder-samples/tree/master/lambda/latest_image_tracker

In this case, there's no need to update the module, since you just need to refer the ssm parameter store in the image_id attribute as @stewartcampbell explained above.

Hope this helps !

Cheers

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 1 year ago

This issue was automatically closed because of stale in 10 days

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.