ci: Cleanup AWS resources better - #2405
Draft
mmlb wants to merge 3 commits into
Draft
Conversation
AMI_ID is obviously always required but would fail later in the test, better to do so early IMO. This test isn't really being run locally at the moment if/when it does, I want to make it easier by using the nix package instead which will set this before running.
Our clean up has been pretty loose. We've had left over key pair, security groups and instances too. This new script fixes all of those. AMIs are only deleted if the --delete-ami is passed in which is not done for Release AMI Nix workflow.
The previous clean up was pretty lightweight this new version is more complete. First it looks for any old resources to get their executionId and then passes those down to the actual clean up script which is reusing the one that runs per-ci job so that we don't have duplication that may drift. AMIs are never deleted by this script so are safe.
PostgreSQL Extension Dependency Analysis: PR #2405
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
PostgreSQL Package Dependency Analysis: PR #2405
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
Contributor
There was a problem hiding this comment.
Pull request overview
Centralizes AWS AMI build cleanup and expands stale-resource detection across CI workflows.
Changes:
- Adds shared scripts for execution-specific and stale-resource cleanup.
- Tags Packer resources for discovery and cleanup.
- Updates CI workflows and testinfra execution identifiers.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ci/cleanup-ami-build-resources.sh |
Adds shared AWS resource cleanup. |
ci/cleanup-stale-ami-build-resources.sh |
Discovers stale build executions. |
amazon-amd64-nix.pkr.hcl |
Adds cleanup tags to amd64 resources. |
amazon-arm64-nix.pkr.hcl |
Adds cleanup tags to arm64 resources. |
stage2-nix-psql.pkr.hcl |
Adds cleanup tags to stage-two resources. |
testinfra/test_ami_nix.py |
Requires explicit build and AMI identifiers. |
.github/workflows/ami-release-nix.yml |
Uses shared cleanup for releases. |
.github/workflows/testinfra-ami-build.yml |
Uses shared cleanup after testing. |
.github/workflows/cleanup-stale-ec2.yml |
Runs the stale-resource cleaner. |
.github/workflows/qemu-image-build.yml |
Simplifies, but does not centralize, cleanup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+72
to
+74
| for id in "${ids[@]}"; do | ||
| echo "$cleaner" "$id" | ||
| done |
| --filters 'Name=tag-key,Values=packerExecutionId' 'Name=tag:appType,Values=postgres' | ||
| --output json | ||
| ) | ||
| readarray -t ids < <( |
Comment on lines
150
to
151
| run: | | ||
| aws ec2 describe-instances --filters "Name=tag:packerExecutionId,Values=${EXECUTION_ID}" --query "Reservations[].Instances[].InstanceId" --output text | xargs -r aws ec2 terminate-instances --instance-ids |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What kind of change does this PR introduce?
Maintenance
What is the current behavior?
We do a pretty bad job cleaning up packer and testinfra created resources in general, especially when a job timesout/is cancelled. We have a cron job that is supposed to catch cases that are missed but it only does instances that are running.
What is the new behavior?
Moved the logic from each GHA workflow into an actual script that is used by all of them, including the stale clean up recurring job. The stale clean up job just goes through and finds executionIds or testinfra-run-ids and passes them to the execution clean up script. AMIs are not cleaned up.
Additional context
Having these in scripts is much nicer to develop than in yaml plus we can also easily run them locally for testing/one-offs too.