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.

Sign up for GitHub

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

cmd-coreos-prune: fix skipping of images action #4021

Merged
merged 2 commits into from
Feb 10, 2025
Merged

cmd-coreos-prune: fix skipping of images action #4021

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51358ca
cmd-coreos-prune: fix skipping of images action
dustymabe Feb 6, 2025
bb83fcc
cmd-coreos-prune: update error handling for s3.delete_object
dustymabe Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/cmd-coreos-prune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,15 @@ def main():

# Skip if the action has been handled previously for the build
if action in previous_cleanup:
# If we are in here then there has been some previous cleanup of
# this type run for this build. For all types except `images` we
# can just continue.
if action != "images":
print(f"\t\tBuild {build_id} has already had {action} pruning completed")
continue
print(f"\t\tBuild {build_id} has already had {action} pruning completed")
continue
# We have to handle the "images" action separately because the key in the previous
# cleanup is "images-kept" and not "images"
if action == 'images' and 'images-kept' in previous_cleanup:
# OK `images` has been pruned before, but we need to check
# that all the images were pruned that match the current policy.
# i.e. there may be additional images we need prune
elif set(images_to_keep) == set(previous_cleanup.get("images-kept", [])):
if set(images_to_keep) == set(previous_cleanup.get("images-kept", [])):
print(f"\t\tBuild {build_id} has already had {action} pruning completed")
continue

Expand Down Expand Up @@ -411,10 +410,11 @@ def prune_images(s3, build, images_to_keep, dry_run, bucket, prefix):
s3.delete_object(Bucket=bucket, Key=image_prefix)
print(f"\t\t\tPruned {name} image for {build.id} for {build.arch}")
except botocore.exceptions.ClientError as e:
if e.response['Error']['Code'] == 'NoSuchKey':
print(f"\t\t\t{bucket}/{image_prefix} already pruned.")
else:
errors.append(e)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, feels like it's still worth a comment somewhere in this area that the API returns 204 in the ENOENT case (and/or link to the docs page). As someone reviewing this code, I wouldn't have guessed that so my first thought would've been that it's a bug.

Copy link
Member Author

Choose a reason for hiding this comment

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

added a comment

# Note that even if the object doesn't exist the delete_object()
# will still return Success (`204 Success (No Content)`) so we
# don't need to handle that error case here.
# https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObject.html#API_DeleteObject_RequestSyntax
errors.append(e)
if errors:
print(f"\t\t\tFound errors when pruning images for {build.id}:")
for e in errors:
Expand Down
Loading