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

remove DRY_RUN flag from AMI cleaning script #1416

Merged
merged 1 commit into from
Dec 18, 2024
Merged

remove DRY_RUN flag from AMI cleaning script #1416

merged 1 commit into from
Dec 18, 2024

Conversation

yob
Copy link
Contributor

@yob yob commented Dec 18, 2024

I've run some test builds in dry mode, and spot checked the job logs for multiple regions. Everything I checked looked as expected:

  • all regions show elastic stack AMIs tagged with a version are left untouched
  • all regions show only elastic stack AMIs in the list
  • all regions cap the deletions at only 10 images (for now)

This will require expanding the IAM role to permit deregistering images and deleting snapshots.

I propose leaving the 10-image cap for now. In the unlikely event of a mistake, this will limit the blast radius of deregistered images. We can increase the number once we build confidence.

I've run some test builds in dry mode, and spot checked the job logs for
multiple regions. Everything I checked looked as expected:

* all regions show elastic stack AMIs tagged with a version are left
  untouched
* all regions show only elastic stack AMIs in the list
* all regions cap the deletions at only 10 images (for now)

This will require expanding the IAM role to permit deregistering
images and deleting snapshots.

I propose leaving the 10-image cap for now. In the unlikely event of a
mistake, this will limit the blast radius of deregistered images. We can
increase the number once we build confidence.
@@ -4,7 +4,7 @@ steps:
agents:
queue: "oss-deploy"
env:
DRY_RUN: true
# DRY_RUN: true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left this in but commented out, for discoverability

Copy link
Contributor

@DrJosh9000 DrJosh9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌴 👍😎🍹

@yob yob merged commit e19bc74 into main Dec 18, 2024
1 check passed
@yob yob deleted the clean-amis-for-real branch December 18, 2024 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants