singularityhub / sregistry

server for storage and management of singularity images
https://singularityhub.github.io/sregistry
Mozilla Public License 2.0
103 stars 42 forks source link

Adding support for Minio cleanup when image no longer referenced #319

Closed vsoch closed 4 years ago

vsoch commented 4 years ago

This pull request will to a check to see if a previously referenced image file has changed, and delete it if this is the case. This will close #318.

@RonaldEnsing I anticipated an issue of a user uploading the same container to multiple collections (with different name/tag) and so I'd like to get your feedback on this. What I do before the delete is a query to make sure there indeed is only one of the container:

# Case 2: Exists and not frozen (replace)
storage = existing.get_storage()
if storage != container.get_storage() and not DISABLE_MINIO_CLEANUP:

    # Ensure that we don't have the container referenced by another collection
    # The verison would be the same, regardless of the collection/container name
    count = Container.objects.filter(version=container.version).count()

    # only delete from Minio not same filename, and if there is only one count
    if count == 1:
        print("Deleting no longer referenced container %s from Minio" % storage)
        minioClient.remove_object(MINIO_BUCKET, storage)

Also note there is a settings variable DISABLE_MINIO_CLEANUP with default false that the user can control. When you get a chance, please take a look and test it out! I've also done some pyflakes linting to cleanup unused imports.

Signed-off-by: vsoch vsochat@stanford.edu

RonaldEnsing commented 4 years ago

Thanks @vsoch! Overwriting the container works, but I noticed that if I click the 'delete container' button in the sregistry web interface, the container is not removed in minio. I'm not sure if I should file a separate issue for this, just let me know.

vsoch commented 4 years ago

Oup that's a bug! Let me fix it and add to this current PR.

vsoch commented 4 years ago

okay @RonaldEnsing I've made a shared function to handle the delete! I tested locally and it seems to work - one testing issue I ran into is forgetting to refresh the collection view after adding / re-writing a bunch of containers with continued pushes, so when I clicked the delete button it returned 404 (because the id had changed) so just make sure to refresh before you do that :)

RonaldEnsing commented 4 years ago

The container is now removed successfully. Thanks @vsoch!

vsoch commented 4 years ago

Definitely! Let's squash and merge and get this show on the road! Thanks again for your patience!