search

snok / container-retention-policy

GitHub action for pruning old GHCR container image versions.
MIT License
178 stars 29 forks source link

Using skip-tags together with keep-at-least #66

Closed fernandolucchesi closed 2 months ago

fernandolucchesi commented 12 months ago

Hi!

When I use the skip-tags option together with keep-at-least, the keep-at-least is ignored. How can I ensure at least 5 images are kept, and that I also skip certain tags from being cleaned up? (meaning that more than 5 images could be kept for example...)

sondrelg commented 12 months ago

Hi Fernando!

To me, it seems that any matching tag listed in skip-tags will be marked to not be deleted here.

Then, if keep-at-least is specified, tasks to delete images marked for deletion will be dropped to ensure at least some amount of images are kept, here.

A task to delete an image is only added if delete_image is set to True, which doesn't happen for skipped images. Tasks are always pruned when keep-at-least is > 0.

So from reading the code I don't understand how these are in conflict. Could you elaborate on what you're seeing?

fernandolucchesi commented 11 months ago

Hi Fernando!

To me, it seems that any matching tag listed in skip-tags will be marked to not be deleted here.

Then, if keep-at-least is specified, tasks to delete images marked for deletion will be dropped to ensure at least some amount of images are kept, here.

A task to delete an image is only added if delete_image is set to True, which doesn't happen for skipped images. Tasks are always pruned when keep-at-least is > 0.

So from reading the code I don't understand how these are in conflict. Could you elaborate on what you're seeing?

Hmmm... I just looked at the code and believe that the confusion is due to a bug in the dry_run report option. The dry_run output is done before the skip-tags logic is taken into consideration. So my logs are reporting that all images (except the ones with skip-tag) would be deleted, and only afterward the images are removed from the "to delete task array".

sondrelg commented 11 months ago

So there is not an issue with over-deletion of images, but rather, the dry-run output is falsely reporting that images would have been deleted - when in fact, some of those tasks would be dropped without the dry-run flag?

If so, then I guess we could push a tuple of (image_name, version.id, task) to tasks and do the dry-run logic after we've cancelled tasks instead? Would you be interesting in creating a PR with a fix?

fernandolucchesi commented 11 months ago

That is correct. Sure, I will create a PR when I get some spare time

sondrelg commented 2 months ago

This issue has been fixed in the v3 release 👍 Sorry for the delay!

The migration guide for v3 is included in the release post 👍 If you run into any issues, please share them in the issue opened for tracking the v3 release ☺️