fix(ami-housekeeper): don't delete referenced AMIs in default config #4623
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 472cc5f the default config was migrated to use SSM for AMI lookup. A parameter is created which stores a reference to the AMI. By default, this parameter is called
${var.ssm_paths.root}/${var.ssm_paths.config}/ami_id
.The housekeeper is a process that looks for AMIs which can be deleted because they're no longer used. It does this in a couple of ways:
The problem is that we were looking for SSM parameters like this:
i.e. we were looking for parameters which contain the hardcoded string
ami-id
. This is different to the new default ofami_id
. So we weren't considering the right AMIs to be in use.What would be a better approach would be to reference the values dynamically. This means resolving from the template, and handling the passed-in options, if there are any. We're documenting that we support wildcards, so also support that here too.
The default value in the launch template became
resolve:ssm:<id or AMI>
, so we need to make sure to ask EC2 to resolve for us when looking up the template. In that way we get the actual AMI ID rather than the alias.This can be a bit challenging to understand, so the comments are improved.
Comprehensive tests are added to try to ensure this all works as expected.
Closes: #4571