Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow AMI change to be ignored #325

Closed
jensenbox opened this issue Apr 10, 2023 · 9 comments · Fixed by #331
Closed

Allow AMI change to be ignored #325

jensenbox opened this issue Apr 10, 2023 · 9 comments · Fixed by #331

Comments

@jensenbox
Copy link

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
Copy link

lallish commented Apr 21, 2023

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
Copy link
Member

antonbabenko commented Apr 28, 2023

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
Copy link
Author

jensenbox commented Apr 28, 2023 via email

@antonbabenko
Copy link
Member

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

@jensenbox
Copy link
Author

Thank you very much!

@lallish
Copy link

lallish commented Apr 28, 2023

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 prevent_destroy 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
Copy link
Member

This issue has been resolved in version 5.0.0 🎉

@antonbabenko
Copy link
Member

@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
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants