Implement delete old AMIs feature #4

s2504s · 2017-08-22T18:19:06Z

What

Implement delete old AMIs

Why

Old AMIs should be deleted after retention period

osterman · 2017-08-23T16:05:52Z

ami_cleanup.py

-    ).get(
-        'Reservations', []
-    )
+images = ec2.images.filter(Owners=[os.environ['ami_owner']],  Filters=[{'Name': 'tag-key', 'Values': ['AMIDeleteOn']}])


This does not filter on instance id. Won't there be multiple lambdas deployed at the same time? How do we keep them from stepping on each other? @comeanother

…ction

osterman · 2017-08-24T18:45:43Z

main.tf

@@ -112,7 +120,7 @@ resource "aws_lambda_function" "ami_backup" {
 resource "aws_lambda_function" "ami_cleanup" {
  filename         = "${path.module}/ami_cleanup.zip"
  function_name    = "${module.label_cleanup.id}"
-  description      = "Cleanup old AMI backups"
+  description      = "Automatically removal AMI images of EC2 instance (remove AMI)"


Automatically remove AMIs that have expired (delete AMI)

const-bon · 2017-08-25T12:48:28Z

README.md

@@ -17,8 +17,7 @@ Notes:

 ```
 module "lambda_ami_backup" {
-  source = "git::https://github.com/cloudposse/tf_lambda_ami_backup.git?ref=master"
-
+  source            = "git::https://github.com/cloudposse/tf_lambda_ami_backup.git?ref=master"


Replace tf_lambda_ami_backup with tf_ami_backup

const-bon · 2017-08-25T12:50:25Z

ami_cleanup.py

@@ -2,13 +2,22 @@
 #
 # @author Robert Kozora <[email protected]>
 #
-# This script will search for all instances having a tag with "Backup" or "backup"
+# This script will search for all instances having a tag with "AMIDeleteOn"


Replace instances with AMIs.

const-bon · 2017-08-25T12:50:42Z

ami_cleanup.py

 # on it. As soon as we have the instances list, we loop through each instance
 # and reference the AMIs of that instance. We check that the latest daily backup
 # succeeded then we store every image that's reached its DeleteOn tag's date for
 # deletion. We then loop through the AMIs, deregister them and remove all the
-# snapshots associated with that AMI.
-
+# snapshots associated withg that AMI.


Replace withg with with.

const-bon · 2017-08-25T12:51:08Z

ami_cleanup.py

+from __future__ import print_function
+from __future__ import print_function
+from __future__ import print_function
+from __future__ import print_function


Remove line duplicates.

const-bon · 2017-08-25T13:20:42Z

ami_cleanup.py

+        if image.name.startswith(label_id + '-' + instance_id):
+
+            # Count this image's occurance
+            imagecount = imagecount + 1


Delete unused variable.

const-bon · 2017-08-25T13:20:45Z

ami_cleanup.py


-    print "About to process the following AMIs:"
-    print imagesList
+    imagecount = 0


Delete unused variable.

const-bon · 2017-08-25T13:28:19Z

ami_cleanup.py

+                delete_date = False
+
+            today_time = datetime.datetime.now().strftime('%m-%d-%Y')
+            # today_fmt = today_time.strftime('%m-%d-%Y')


Delete unused code.

osterman

LGTM... I think the AMI prefix on AMIDeleteOn is redundant since the tag is applied to an AMI it does not disambiguate in any useful way.

const-bon · 2017-08-28T13:21:21Z

@s2504s was it tested?

const-bon · 2017-08-28T14:16:11Z

Backup and Cleanup has been successfully tested manually:

Manually trigger backup.
Change AMIDeleteOn tag of newly created AMI.
Manually trigger cleanup.

Implement delete old AMIs

984ef9f

s2504s requested review from osterman and const-bon August 22, 2017 18:19

s2504s self-assigned this Aug 22, 2017

Search special tag on AMI

07c9e9f

osterman reviewed Aug 23, 2017

View reviewed changes

s2504s added 2 commits August 23, 2017 21:01

Terraform fmt. Modified the variable name

e42a4e5

Added ability to remove AMI per-instance. Modified name of Lambda fun…

d6a68cd

…ction

osterman reviewed Aug 24, 2017

View reviewed changes

s2504s added 2 commits August 25, 2017 12:58

Modified comment

22784ca

Modified README

db9ffa0

const-bon suggested changes Aug 25, 2017

View reviewed changes

const-bon mentioned this pull request Aug 25, 2017

Change approach to choose instance in order to backup #3

Closed

s2504s added 2 commits August 25, 2017 19:10

Changed README, ami_cleanup.py

997b686

Removed default value in variables.tf

16acc3b

osterman approved these changes Aug 28, 2017

View reviewed changes

const-bon approved these changes Aug 28, 2017

View reviewed changes

const-bon changed the title ~~Implement delete old AMIs~~ Implement delete old AMIs feature Aug 28, 2017

const-bon merged commit 5866299 into master Aug 28, 2017

const-bon deleted the fix_cleanup branch August 28, 2017 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement delete old AMIs feature #4

Implement delete old AMIs feature #4

s2504s commented Aug 22, 2017

osterman Aug 23, 2017

osterman Aug 24, 2017 •

edited

Loading

s2504s Aug 25, 2017

const-bon Aug 25, 2017

const-bon Aug 25, 2017

s2504s Aug 25, 2017

const-bon Aug 25, 2017

s2504s Aug 25, 2017

const-bon Aug 25, 2017

s2504s Aug 25, 2017

const-bon Aug 25, 2017

s2504s Aug 25, 2017

const-bon Aug 25, 2017

s2504s Aug 25, 2017

const-bon Aug 25, 2017

s2504s Aug 25, 2017

osterman left a comment

const-bon commented Aug 28, 2017

const-bon commented Aug 28, 2017

Implement delete old AMIs feature #4

Implement delete old AMIs feature #4

Conversation

s2504s commented Aug 22, 2017

What

Why

Choose a reason for hiding this comment

osterman Aug 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osterman left a comment

Choose a reason for hiding this comment

const-bon commented Aug 28, 2017

const-bon commented Aug 28, 2017

osterman Aug 24, 2017 •

edited

Loading