[Feature request] Keep only n tagged images (and ignore specific tag) #1
Comments
ManiMatter
changed the title
|
Thanks! Something like? steps:
- uses: dataaxiom/[email protected]
with:
keep-n-tagged: 10
exclude-tags: dev
token: ${{secrets.GITHUB_TOKEN}} |
|
I think your proposal is perfect |
|
I've implemented the feature. Please test it out on the v1.0.1 or v1 tag/release. I added a dry-run option also if you want to test with that first to simulate everything |
|
Hey, super cool that you built it so quickly, much appreciated. I let it run with these settings, and got the following error Settings: Error:
Job: |
|
Btw - "cherry on top" - does the script print out at the very end how many containers it removed? |
|
Yes, it outputs the following kind of log currently: I've just found some further optimizations which I'll push and release right now, it includes a wildcard matching feature for the tag/exclude tags It doesn't yet print out total number of images deleted. |
|
cheers. will test it when the next version is ready, let me know |
|
I've push the latest changes, v1 or v1.0.2. |
|
Can you trigger your pipeline again and see if the id error still exists? |
|
I think I can harden then platform image deletion.I think you might have some manifests that don't link to actual images. I'll make the change right now. |
|
re-ran the job right now, ended up in the same place: https://github.com/ManiMatter/decluttarr/actions/runs/9083315844/job/24964895577 Not sure I understand this one?
|
|
I'm testing a fix. I've added some extra guards to handled corrupt manifests. On the image order I have to sort the remaining "images" once all the processing is done, what I found was the sort function had to be reversed between keep-n-tagged and keep-n-untagged logic. Which I wouldn't expect, but my test showed it needed it. (Still need to verify it more) |
|
Thanks for the PR!!!!! uses: dataaxiom/ghcr-cleanup-action@main |
|
pleasure. Now it passes, but I see these warnings:
https://github.com/ManiMatter/decluttarr/actions/runs/9085371392/job/24968536043 |
|
thats great. yes I added those warnings, you have some corrupt image manifests, but all good. It just skips over them. Any way to tell if the action keep the most recent tags? |
nice one! 🥇 From what I see, the following are not removed, which looks good. I see that the next version v1.35.0 is removed, so everything looks right.
Job: On the warnings regarding the corrupt image manifests, would these remain there after the job has run?
|
|
That's great to hear it's all calculating the cleanup correctly. I would expect those warnings to be on only that job. The image manifests which have corruptions are deleted on that run so it won't loop through that manifest file again in a future job. When new tags come up for deletions on new jobs it would asses those manifests then for processing, which would be new errors if there are any. I'm making a few minor changes (for stats output) and then I'll release 1.0.3 later today and retag v1 Thanks so much for the feature request and testing. It made me fix a bunch of things with the keep-n scenarios. |
I think I can answer my own question. Here is what's happening. This github action doesn't properly work with multi-arch images, and kills the untagged images belonging to multi-arch images. Here's an example: This image has 3 architectures, but they were deleted by the github action.
Your script correctly says that the 3 latter ones can't be found (no wonder, they were deleted). I would have a suggestion though here: Why not add another setting "remove-broken-multi-arch-images" (or something along these lines, or just do the following additional cleanup without even having an explicit setting for it)? What do you think? Update: Just seen that you posted this while I wrote:
Does this mean that you have already baked the removal in (without user having to specifically put a setting), or you are removing them "by chance" because they may fall into scope (e.g. because they are older than 10 versions)? |
|
Yeah, that's exactly what is likely happening. It's the main reason I wrote this version, none of the existing cleanup actions (even dedicated container ones supports multiarch images) which corrupted some of my images so I disabled the actions. That's a interesting idea to proactively validate the image manifest. I almost think that should be a different standalone mode producing a report. As multi arch image may be corrupt but still work for a given platform, if that underlying image still exists the pull would likely work on a given host. In your case it looked like all the underlying images were gone. This issue did make me think of a scenario where maybe multiple multi-architecture manifests may point to the same image digest. I need to walk through the code on that potential. I was thinking this morning another standalone mode could be a "recovery" mode whereby you could specify the package id's to recover any packages that were accidentally deleted from the action directly (unrelated to the issue above). Otherwise you have to jig some web api call manually to recover them.
Yes, that's correct for the images that fall into scope on each run. But your idea would proactively valid the 10 other versions in your scenario, but it probably shouldn't touch them. |
|
you're right, there could be multi-arch containers that miss some manifests, but not others. Would this be a valid approach?
In the part where the images are deleted, I would then not show the warning that the respective manifest is missing (simply skip that) Thoughts? |
|
Good suggestions. It agree removing the warning for in scope deletions is probably a good idea. I'll make that change.
That's a good idea for fully broken images in scope. Out of scope is a bit more problematic for the other cleanup modes (not keep-n-tagged) Let me review the code.
For out of scope processing that might be more challenging as the current different modes: delete by untagged, delete by tag and keep-n-untagged operate a bit different to the keep-n-tagged. In my case I'm currently using the delete untagged (default), and expect the total number of images to grow without ever deleting released images. Maybe a "verify" option could be used to enable out of scope scanning/reporting, which then could be turned off when the images 'healthy' You are probably right there are a lot of broken manifests out there in ghcr.io considering the lack of multi-arch support, so this kind of functionality would be useful for projects that switch to this action. |
|
Let me think about your latest suggestion more, I'll get back to you. As long as it can support all the cleanup modes I agree some better validation should be used. |
|
just wanted to again thank you - fab work. really enjoing the exchange with you here on this :) |
|
thanks for being my first external user/tester. it's validated that it's currently needed. |
|
I've seen this new option:
with "fix them", does this mean it will remove those partially broken platform packages as we discussed above with the "partial-manifest-removal: true" flag? Or does this simply mean that the logs spit out which packages are broken, so that I can then go in and fix them myself? Many thanks for clarifying :) |
|
Hey I did add a few extra features:
I planning to build some automated tests next, it's quite a manual process to test all these scenarios. Once that is done then there is other options which could be added: deleting by date, by download count, package restoration etc |
|
I'm impressed by how many features you have already added to this action in so little time! Based on your this statement, my understanding is that packages that are fully broken (no working underlying manifests) are removed already.
However, I am not sure I understand how the partial-manifest-removal mode may conflict with the different action modes.
I understand there are essentially 4 modes.
In all options, I would expect that if partial-manifest-removal is on, packages that are partially broken would be removed in any case. Meaning: Even if they are listed as "excluded" or if there are 10 untagged items of which one is partially broken and we say keep 100, it would still reduce the number to 9. E.g., the "fixing" should supersede everything. I was thinking first whether something that is "excluded" should also be excluded from partial-manifest-removal. However, if we clearly document that partial-manifest-removal takes priority over exclusion, then I think it's fine that partial-manifest-removal supersedes. If we were to give priority to exclusion over partial-manifest-removal, it would start getting tricky because of wildcard support. If somebody is not OK with exclusion being ignored for partial-manifest-removal, then I guess the user should not use partial-manifest-removal, but instead use the "validate" option and remove these items manually. What do you think? I am probably missing an important aspect that you have in mind, when cautioning that the partial-manifest-removal may conflict with the other modes. Could you elaborate please? Maybe we can brainstorm it out together :) Cheers |
|
I currently have coded that exclusion takes priority over everything, and it currently supports wildcards. I have a pre processing step that process all the tags for exclusion. I think that should stay that way, while ironically it's a cleanup action it should take a super cautious approach to deletion and cleanup. Your point about partial-manifest-removal in all cases for the other modes is where it gets tricky. As from a user perspective it kind of implies including into scope all images as you point out. But the untagged and tags modes currently leave all out of scope images alone. So it might lead to more user error unless there is more general approach about deleting. I would propose changing the partial-manifest-removal to a delete-partial-manifests. I was thinking yesterday that there may be a number of delete- options which are additive. Like delete-older-then, delete-downloads-less-then and delete-partial-manifests, which get processed before the keep and tags modes. In this setup strangely mode 1 is really an explicit delete-untagged option, might might be default option. So the core logic would be to process excludes, process delete options, then process the keep options. Which then got me thinking the project needs automated testing asap. So that these other functions can be added easily and without breaking all the other modes. haha |
|
makes me think that maybe the tags option should really be delete-tags instead to be clearer and in line with the other delete- options |
|
agree to all you said :) typescript / github actions etc are not my forte at all, and i'm actually studying how you are coding it, and am learning a lot from it. thank you |
|
Hi ManiMatter |
|
Great news! I‘m excited to test it. Hope to find time tmrw to do so! thanks so much for your work and for considering this feature suggestion! |
|
I've also removed the delete untagged mode from the keep-n-tagged, so you'll need to add --delete-untagged to your setup if you want to delete untagged images. I've changed the setup so no you can have all the modes --keep-n-tagged --keep-n-untagged --delete-untagged --delete-tags now at the same time. |
|
thanks for the info! Maybe stupid question: how can I test this action before you will have released it? |
|
You can reference the main branch like this in your action setup, which will allow you test before release.
|
|
I've let it run on "dry-run", looks promising
I'm glad I won't have to delete those manually. You mentioned that you want to add a few more checks and features before releasing v1.0.8.
|
|
I've released v1.0.8/v1 ff99a6e, so closing this issue. BIG thanks for your feedback/help. |
|
.. thanks so much for your help, I let it run productively... so happy I didn't have to do it manually. One observation: The Dry-run number and the actual deletes differed. Could it be that dry-run double counts certain things? Dry-run; |
|
That's very weird. I can t see how looking at the code, so I'll need to run some tests and analyze more |
|
I changed from the @main to @v1.0.8, remove dry-run and pushed the change, which would (based on my git flow) create 4 new containers (multi arch + 3 versions) and then clean up the packages. so if anything, I‘d have expected the nr of packages go up (by 4) rather than down. the only other 2 explanations I can think of is that what I tested a few days back (@main) and what i ran now (v1.0.8) are different, or that the dry run has some sort of double counts… not a big issue though: the result looks correct after the cleanup thanks again for your great work on this |
|
I think the reduced count is likely as you commented out the keep-n-tagged, which then switched to cleaning up only untagged images. So that didn't delete any tagged images (excepted 2 it looks like which were ghost images). |
|
arghhhhhhhhhhh |
|
All good. It was good to verify. Thanks for all your help from the beginning. I think the 1.0.8 version is pretty solid, we'll see, haha. |
|
Yes, it seems to work like a charm. did you raise a feature request on their end by any chance? |
|
Ah, downloads, that's true. I noticed in your repo you have some images in the 5k mark, so it will become and issue. |
|
Yeah if we can avoid scrapping we should at all cost; would make the solution brittle and if scrapping incorrectly (and there are no safety measure to detect that) may lead to involuntary deletion. Threshold-wise I would even set it lower in my case, 500 or 1000.. guess that should be parametrizable. That‘s a detail though, and irrelevant if we can‘t get hold of the download stats in the first place If we were able to solve it, I was thinking of using it in combination with keep-n-tagged. Exclude anything > download threshold and from the rest keep the latest 10 versions. |
Hi,
Fantastic work, thank you so much for solving this headache.
I have a feature request to keep only n-tagged images, and to always keep a specific tag as well.
Background:
I have a setup where I have two types of containers:
Whenever I push to "dev" branch, it will create a new container tagged "dev", and untag the old "dev" container; this helps me to test the image before releasing it.
When I then merge to the "main" branch, a new versioned (e.g. 4.2.1) container is created.
Proposal
What I would love to do is the following:
The text was updated successfully, but these errors were encountered: