From 64adb277f982ef78079648c27b063d7b6fcc15a6 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 29 Jan 2019 11:23:06 -0500 Subject: [PATCH 1/5] PEP-8 list membership checks --- main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main.py b/main.py index f6cf3a1..2a57a95 100644 --- a/main.py +++ b/main.py @@ -152,12 +152,12 @@ def discover_delete_images(regionname): def append_to_list(list, id): - if not {'imageDigest': id} in list: - list.append({'imageDigest': id}) + if {"imageDigest": id} not in list: + list.append({"imageDigest": id}) def append_to_tag_list(list, id): - if not id in list: + if id not in list: list.append(id) From 407b6b23a26f09d04e09f45de3c2acce0e4edac0 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 29 Jan 2019 11:30:37 -0500 Subject: [PATCH 2/5] PEP-8 --- main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main.py b/main.py index 2a57a95..1749d5a 100644 --- a/main.py +++ b/main.py @@ -17,6 +17,7 @@ import re import boto3 + try: import requests except ImportError: @@ -43,6 +44,7 @@ def initialize(): IMAGES_TO_KEEP = int(os.environ.get('IMAGES_TO_KEEP', 100)) IGNORE_TAGS_REGEX = os.environ.get('IGNORE_TAGS_REGEX', "^$") + def handler(event, context): initialize() if REGION == "None": From feaaf773aa7fffb8ec72accb1302db5e244c93f9 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 29 Jan 2019 11:32:21 -0500 Subject: [PATCH 3/5] Use defined value for REGION rather than overloading None This avoids needing to do type-conversions which look like errors --- main.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/main.py b/main.py index 1749d5a..b0643a6 100644 --- a/main.py +++ b/main.py @@ -23,7 +23,7 @@ except ImportError: from botocore.vendored import requests -REGION = None +REGION = "ALL" DRYRUN = None IMAGES_TO_KEEP = None IGNORE_TAGS_REGEX = None @@ -35,8 +35,8 @@ def initialize(): global IMAGES_TO_KEEP global IGNORE_TAGS_REGEX - REGION = os.environ.get('REGION', "None") DRYRUN = os.environ.get('DRYRUN', "false").lower() + REGION = os.environ.get("REGION", "ALL") if DRYRUN == "false": DRYRUN = False else: @@ -47,9 +47,10 @@ def initialize(): def handler(event, context): initialize() - if REGION == "None": - partitions = requests.get("https://raw.githubusercontent.com/boto/botocore/develop/botocore/data/endpoints.json").json()[ - 'partitions'] + partitions = requests.get( + "https://raw.githubusercontent.com/boto/botocore/develop/botocore/data/endpoints.json" + ).json()["partitions"] + if REGION == "ALL": for partition in partitions: if partition['partition'] == "aws": for endpoint in partition['services']['ecs']['endpoints']: @@ -210,7 +211,7 @@ def delete_images(ecr_client, deletesha, deletetag, id, name): if args.region: os.environ["REGION"] = args.region else: - os.environ["REGION"] = "None" + os.environ["REGION"] = "ALL" os.environ["DRYRUN"] = args.dryrun.lower() os.environ["IMAGES_TO_KEEP"] = args.imagestokeep os.environ["IGNORE_TAGS_REGEX"] = args.ignoretagsregex From 8d02f813dfdeae1cf22aa45a174027cb9709d1e8 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 29 Jan 2019 11:39:42 -0500 Subject: [PATCH 4/5] Update dry-run handling * Use easier to read variable names following PEP-8 * Default to a dry-run unless deletion is confirmed * Use a mutually-exclusive argument group to require command-line runs to specify either --dry-run or --delete-images. * Have --region default to $AWS_DEFAULT_REGION if set --- main.py | 80 ++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/main.py b/main.py index b0643a6..66c18c8 100644 --- a/main.py +++ b/main.py @@ -24,25 +24,26 @@ from botocore.vendored import requests REGION = "ALL" -DRYRUN = None +DRY_RUN = True IMAGES_TO_KEEP = None IGNORE_TAGS_REGEX = None def initialize(): global REGION - global DRYRUN + global DRY_RUN global IMAGES_TO_KEEP global IGNORE_TAGS_REGEX - DRYRUN = os.environ.get('DRYRUN', "false").lower() REGION = os.environ.get("REGION", "ALL") - if DRYRUN == "false": - DRYRUN = False + + if os.environ.get("DRY_RUN", "").lower() == "false": + DRY_RUN = False else: - DRYRUN = True - IMAGES_TO_KEEP = int(os.environ.get('IMAGES_TO_KEEP', 100)) - IGNORE_TAGS_REGEX = os.environ.get('IGNORE_TAGS_REGEX', "^$") + DRY_RUN = True + + IMAGES_TO_KEEP = int(os.environ.get("IMAGES_TO_KEEP", 100)) + IGNORE_TAGS_REGEX = os.environ.get("IGNORE_TAGS_REGEX", "^$") def handler(event, context): @@ -177,7 +178,7 @@ def delete_images(ecr_client, deletesha, deletetag, id, name): i = 0 for deletesha_chunk in chunks(deletesha, 100): i += 1 - if not DRYRUN: + if not DRY_RUN: delete_response = ecr_client.batch_delete_image( registryId=id, repositoryName=name, @@ -197,22 +198,49 @@ def delete_images(ecr_client, deletesha, deletetag, id, name): # Below is the test harness -if __name__ == '__main__': - request = {"None": "None"} - parser = argparse.ArgumentParser(description='Deletes stale ECR images') - parser.add_argument('-dryrun', help='Prints the repository to be deleted without deleting them', default='true', - action='store', dest='dryrun') - parser.add_argument('-imagestokeep', help='Number of image tags to keep', default='100', action='store', - dest='imagestokeep') - parser.add_argument('-region', help='ECR/ECS region', default=None, action='store', dest='region') - parser.add_argument('-ignoretagsregex', help='Regex of tag names to ignore', default="^$", action='store', dest='ignoretagsregex') +if __name__ == "__main__": + parser = argparse.ArgumentParser( + description="Deletes stale ECR images", + formatter_class=argparse.ArgumentDefaultsHelpFormatter, + ) + + # We want the user to explicitly opt-in to deleting images so we'll have a + # mutually-exclusive option group which requires running with either + # --dry-run or --delete-images + delete_mode = parser.add_mutually_exclusive_group(required=True) + + delete_mode.add_argument( + "--dry-run", + help="Prints the images to be deleted without deleting them", + action="store_true", + dest="dry_run", + ) + delete_mode.add_argument( + "--delete-images", + help="Delete the images (cancels --dry-run)", + action="store_false", + dest="dry_run", + ) + + parser.add_argument( + "--images-to-keep", help="Number of image tags to keep", default=100 + ) + + parser.add_argument( + "--region", + help="AWS region", + default=os.environ.get("AWS_DEFAULT_REGION", "ALL"), + ) + + parser.add_argument( + "--ignore-tags-regex", help="Regex of tag names to ignore", default="^$" + ) args = parser.parse_args() - if args.region: - os.environ["REGION"] = args.region - else: - os.environ["REGION"] = "ALL" - os.environ["DRYRUN"] = args.dryrun.lower() - os.environ["IMAGES_TO_KEEP"] = args.imagestokeep - os.environ["IGNORE_TAGS_REGEX"] = args.ignoretagsregex - handler(request, None) + + os.environ["REGION"] = args.region + os.environ["DRY_RUN"] = str(args.dry_run).lower() + os.environ["IMAGES_TO_KEEP"] = str(args.images_to_keep) + os.environ["IGNORE_TAGS_REGEX"] = args.ignore_tags_regex + + handler({"None": "None"}, None) From 21fe8208f36f0d0fb390fc229f2a0215f19ccf51 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 29 Jan 2019 12:06:36 -0500 Subject: [PATCH 5/5] Tidy delete_images * Easier to understand variable names * Print output has consistent use of colons * Fix typo * Use enumerate rather than reinventing it * Nicer display of the digests which would be deleted --- main.py | 45 ++++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/main.py b/main.py index 66c18c8..8f265b7 100644 --- a/main.py +++ b/main.py @@ -102,7 +102,7 @@ def discover_delete_images(regionname): for repository in repositories: print("------------------------") - print("Starting with repository :" + repository['repositoryUri']) + print("Starting with repository:", repository["repositoryUri"]) deletesha = [] deletetag = [] tagged_images = [] @@ -118,7 +118,7 @@ def discover_delete_images(regionname): append_to_list(deletesha, image['imageDigest']) print("Total number of images found: {}".format(len(tagged_images) + len(deletesha))) - print("Number of untagged images found {}".format(len(deletesha))) + print("Number of untagged images found: {}".format(len(deletesha))) tagged_images.sort(key=lambda k: k['imagePushedAt'], reverse=True) @@ -132,7 +132,7 @@ def discover_delete_images(regionname): if imageurl not in running_sha: running_sha.append(image['imageDigest']) - print("Number of running images found {}".format(len(running_sha))) + print("Number of running images found: {}".format(len(running_sha))) for image in tagged_images: if tagged_images.index(image) >= IMAGES_TO_KEEP: @@ -152,7 +152,7 @@ def discover_delete_images(regionname): repository['repositoryName'] ) else: - print("Nothing to delete in repository : " + repository['repositoryName']) + print("Nothing to delete in repository: " + repository["repositoryName"]) def append_to_list(list, id): @@ -171,29 +171,32 @@ def chunks(l, n): yield l[i:i + n] -def delete_images(ecr_client, deletesha, deletetag, id, name): - if len(deletesha) >= 1: - ## spliting list of images to delete on chunks with 100 images each - ## http://docs.aws.amazon.com/AmazonECR/latest/APIReference/API_BatchDeleteImage.html#API_BatchDeleteImage_RequestSyntax - i = 0 - for deletesha_chunk in chunks(deletesha, 100): - i += 1 +def delete_images( + ecr_client, image_ids_to_delete, tagged_for_deletion, repository_id, repository_name +): + if image_ids_to_delete: + # spliting list of images to delete on chunks with 100 images each + # http://docs.aws.amazon.com/AmazonECR/latest/APIReference/API_BatchDeleteImage.html#API_BatchDeleteImage_RequestSyntax + for chunk_number, image_id_chunk in enumerate( + chunks(image_ids_to_delete, 100), start=1 + ): if not DRY_RUN: delete_response = ecr_client.batch_delete_image( - registryId=id, - repositoryName=name, - imageIds=deletesha_chunk + registryId=repository_id, + repositoryName=repository_name, + imageIds=image_id_chunk, ) print(delete_response) else: - print("registryId:" + id) - print("repositoryName:" + name) - print("Deleting {} chank of images".format(i)) - print("imageIds:", end='') - print(deletesha_chunk) - if deletetag: + print(f"Repository: {repository_name} ({repository_id})") + print(f"Deleting chunk #{chunk_number}: {len(image_id_chunk)} images") + print("Image digests:") + for image_id in image_id_chunk: + print("\t", image_id["imageDigest"]) + + if tagged_for_deletion: print("Image URLs that are marked for deletion:") - for ids in deletetag: + for ids in tagged_for_deletion: print("- {} - {}".format(ids["imageUrl"], ids["pushedAt"]))