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.

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

Implement a cleanup of old Docker images and untagged images #2302

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Implement a cleanup of old Docker images and untagged images #2302

merged 1 commit into from
Apr 26, 2023

Conversation

stumpylog
Copy link
Contributor

@stumpylog stumpylog commented Apr 20, 2023

This PR adds a new workflow which will runs when a PR closes to remove the associated Docker image which was built from the PR and images which are untagged. In an ideal world, this would be something built into github, but until that day... This helps keep the packages with tags down to just the released versions and those PRs which are still in some sort of active state.

Stale Images

I've tested this with a read PAT in a sample repository here: https://github.com/stumpylog/immich-sample-repo/actions/runs/4791378901

Taking immich-server as the example, the list of images which would be removed.

The logic is pretty simple (and encapsulated into an action). It filters to packages with 1 tag (extra security), with match the regex ^pr-(\d+)$|^(\d+)$. The capturing groups are used to get a PR number. If the PR is closed (which also means merged), the package is considered done with and can be removed

Untagged Images

Requires a little more knowledge of Docker and registries to understand, but not terrible.

Basically, when a tag is updated to point to a newly built image (say the PR or branch was updated), the old image doesn't get removed. In fact, it could still be accessed via a tag of the form @sha256:<digest>. But most people are using :main or :pr-1234, not a SHA.

So the action looks for images with no tags applied and not pointed to by a multi-arch manifest and removes those.

TBD

  • The repository would need a PAT with the OAuth scope of package deletion in the secrets
  • The action requires the delete option to be set to actually delete anything. For now, it's more like a dry run

@vercel
Copy link

vercel bot commented Apr 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Apr 25, 2023 2:26pm

@alextran1502
Copy link
Contributor

Have you had a chance to test this yet? Do you want me to merge in and test on a few PR?

@stumpylog
Copy link
Contributor Author

Sort of, is the best answer. It's one of those difficult to test things. I linked a couple places in a run above, where I pointed the script to read from immich-app/immich instead of the fork. The output there all made sense.

Merging it (with the PAT setup) and without the --delete option would help gather the real data from the real repo better. I've tried as much as possible to ensure nothing happens without the flag.

@brighteyed
Copy link
Contributor

Maybe it is better to use GitHub provided action

Pros:

  • GitHub provided
  • It seems it doesn't require PAT
  • No need to debug and maintain

@stumpylog
Copy link
Contributor Author

That would be great if it worked but I can see a few problems there. It doesn't look really geared towards containers.

  • No support for multi-arch images (Multi-arch containers actions/delete-package-versions#90), meaning removing "untagged" images would break all image pulls
  • No way to tell it only delete pr-1234, it would either remove them all or remove none through the ignore-versions parameter
  • Still requires Admin access for the action, slightly different than a PAT

It'd be great if an action was provided for this, or a retention setting or something, but at this point I haven't found anything for it

@stumpylog
Copy link
Contributor Author

I'm going to convert this back to a draft to do a little rework on it. All this logic and code is 99% the same as what I used in paperless-ngx for cleaning up our old images. And if it's going to be doubled, I might as well make it reusable in a action (or two).

My apologies this won't unsubscribe people from the thread. I'll keep any testing to a different branch until I'm ready with this one again.

@stumpylog stumpylog marked this pull request as draft April 22, 2023 15:52
@stumpylog
Copy link
Contributor Author

  • I've packaged all the logic up into two actions
  • tested them against immich here and well as against live data here.
  • You can see the list of versions which will be untagged here for immich-server for example.
    • I've checked a number of the PRs manually and the images to be removed do correspond to closed PRs.
  • At this moment, there's so many to remove it will almost hit rate limits with all the packages
  • Currently an ongoing incident causing some failures pulling the action and with the API requests. I suspect with packages as well. I'll look again once that's resolved

Overall, I think this would be a good clean up step to take, not only helping surface released versions, but also just being good stewards and not storing images forever.

This adds a workflow to clean containers when the pull request closes
and remove untagged images generated as tags are updated
@stumpylog stumpylog marked this pull request as ready for review April 25, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants