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

Issues when pushing and pulling images #255

Closed Aneoshun closed 4 years ago

Aneoshun commented 4 years ago

Hey,

I managed to have SRegistry working on my Synology NAS, coupled with LDAP without major issues. However, I observe two very weird behaviours when pushing and pulling images:

First problem: pushing twice the same image removes the image from disk (not from db).

I start with a new collection named mycollection, and I can push first image without issue:

$ singularity push -U airl_env_base_ci.sif library://antoine/mycollection/airl_env:base
WARNING: Skipping container verifying
 343.32 MiB / 343.32 MiB [=======================================================================================================================================================================================================================] 100.00% 26.11 MiB/s 13s

and I can see the image in the file system:

$ ls images/mycollection/
airl_env-sha256.0e0f94ed05b5a8828ea96240c13d285682e084877026c734a7e3786b58525aec.sif

So far so good.

Now, if I want to reupload the same image, with the same name and same tag, I assume that the image will just be replaced. The push command gives me exactly the same results:

$ singularity push -U airl_env_base_ci.sif library://antoine/mycollection/airl_env:base
WARNING: Skipping container verifying
 343.32 MiB / 343.32 MiB [=======================================================================================================================================================================================================================] 100.00% 25.22 MiB/s 13s

The web interface still shows the single image of my collection. Howver, the image is deleted from the filesystem:

$  ls -a images/mycollection/
.  ..

Of course, a pull command will fail, as the file does not exist anymore. Is this an intended behaviour?

Second problem: Pulling an existing image fails.

If I have an image (that exists, and that has not been delete like above), the pull command fails:

$ singularity push -U airl_env_base_ci.sif library://antoine/mycollection/airl_env:base_2
WARNING: Skipping container verifying
 343.32 MiB / 343.32 MiB [=======================================================================================================================================================================================================================] 100.00% 25.94 MiB/s 13s
$ cd - # change folder to not overwrite airl_env_base_ci.sif
$ singularity pull library://antoine/mycollection/airl_env:base_2 
INFO:    Downloading library image
 343.32 MiB / 343.32 MiB [========================================================================================================================================================================================================================] 100.00% 64.38 MiB/s 5s
FATAL:   While pulling library image: could not pull image: file hash(sha256.0e0f94ed05b5a8828ea96240c13d285682e084877026c734a7e3786b58525aec) and expected hash(base_2) does not match

I have the same behaviour if I add the output file name in the pull command.


I am using singularity 3.4.1 (recompiled to accept http remote), and my SRegistry is based on the master branch downloaded yesterday. Please, let me know if you want more information, or some logs. Thanks in advance for your help!

vsoch commented 4 years ago

okay working to reproduce this! I also just deployed a fresh sregistry from master, installed singularity from master (with the change of https -> http):

singularity version 3.5.1+122-ge4d837e58+dirty

And now I pulled a busybox image to test:

singularity pull docker://busybox

Created a collection in the interface called "mycollection" And push a first time

singularity push -U busybox_latest.sif library://vsoch/mycollection/busybox:base

Confirmed that it's there:

$ ls images/mycollection/
busybox-sha256.2ff1334a800cca8bb6edcfd495d397168863720905405e9f928fb30f410662b4.sif

Now I'll push again, exactly the same command, reproduced the issue! Now I'll take a look at the code to see what might be up.


Okay about 30 minutes later, I think I figured out what's going on. The image is saved fine, but since we are using overwrite storage:

class OverwriteStorage(FileSystemStorage):

    def get_available_name(self, name, max_length=None):
        # If the filename already exists, remove it as if it was a true file system
        if self.exists(name):
            os.remove(os.path.join(settings.MEDIA_ROOT, name))
        return name

it seems to be deleting it on the data file save. I put in a print statement to verify this, and there it is in the logs!

Deleting /var/www/images/mycollection/busybox-sha256.2ff1334a800cca8bb6edcfd495d397168863720905405e9f928fb30f410662b4.sif

I fixed this.

For the second error, I'm wondering if there is a change for how Singularity is calculating the hash. The API from Singularity Registry returns the actual hash (noted as what is found) and Singularity seems to be returning that the hash is "base" (which is the tag).

vsoch commented 4 years ago

@RonaldEnsing have you run into this issue with your install? It smells like some change to Singularity - the behavior is different than before because it's using the tag instead of the hash returned.

RonaldEnsing commented 4 years ago

I haven't run into this (yet).

vsoch commented 4 years ago

hey @Aneoshun I really tried here - spent 7 hours on it today, not really well reflected in the actual changes because it's such an impossible issue. There seem to be different behaviors depending on the version of Singularity, and depending on the version of the scs-library-client, so this is unfortunately the best that I can do. https://github.com/singularityhub/sregistry/pull/256

Sylabs of course can't make their server open source because that's how they make profit (to pay for the "on prem deployment"). It feels very anti-open source, and anti-community, but that's life I guess. I wonder if that's even working out for them?

Anyway, you can review #256 and see if it is of any improvement. I don't think I'll be able to work much more on this, it's like trying to dig a hole while someone else keeps filling it up with dirt.

Aneoshun commented 4 years ago

Hey @vsoch! Thanks a lot. I will try #256 tomorrow (it already quite late here) and let you know. I really appreciate your efforts (and everything you created around Singularity, like the Gitlab-ci builder!).

I am sure that finding the right balance between open/proprietary-source is very challenging, but if it enables the continuous development of projects like singularity, why not.

More soon!

vsoch commented 4 years ago

Sounds good! And if you have any python knowledge, or can help to essentially reverse engineer the Sylabs API, I think we could better address these issues. For example, for the previously working first implementation, there is a GET for a container uri that uses a hash that was only for pulling. Now it seems that the same endpoint is also being used for GET. The other challenge is that the Singularity / scs library client don't seem to create tags for existing containers until after the fact. This is why if you look at the code you'll see the creation of DUMMY- tags - we have to create dummy tags that are assured to not exist (otherwise integrity error) to create the container, period. And then the interesting thing I noticed is that when a tag isn't provided, it doesn't POST to GetCollectionTagsView (which for sregistry prints GET ContainerTagView) and that's important because it's the only time when the actual tag (e.g., your "base") is associated with the just created container. This also introduces a bug if the user doesn't provide a tag at all because that second POST doesn't happen, the DUMMY container remains silently and is cleaned up later.

Anyway, if you could help by any means, it would be greatly appreciated! I've been running this as a one man show (woman, dinosaur, whatever) and having others that care about it would lift some of the burden off of me. I'd like sregistry to be a reliable open source registry for Singularity users, so whatever is necessary for that, we do that :)

Aneoshun commented 4 years ago

I do have some python knowledge, and I would be happy to help! I am currently running all my lab on singularity, so I would be glad if I can contribute to the open-source tools and services around!

vsoch commented 4 years ago

Woot! Super awesome :)

For some quick feedback on what turned out to be the core issue of pushing a second, the image file saves okay, but when the singularity client calls the endpoint to update tags, it would find another container that existed and delete it here. That's fine and dandy, except the container has a pre_delete to delete associated image files here. Since the image files have the same name for the same container and tag, this means that we would immediately delete the imagefile associated with the container, although the object for it would remain (hence seeing it in the interface). If you were to look up the imagefile for the newly saved container, the path would be defined but not actually exist. So the fix was to run a post_delete and only delete the imagefile fully given that the count of associated containers was 0. And since we don't want redundant imagefile objects, we also check if one already exists for the collection with the name in question (with the container version).

The second issue (for the pull) comes down to the namespace for the urls, and this is where it gets trickier because the behavior seems different between what it was before (when developed) and now. I was able to resolve the weird error about the hash (a tag) not matching by switching the order of the urls in the url-resolver, but then that can break the other endpoint (GET). It would be really nice if there was some documentation along with their client, but it's likely that it will just need to be reverse engineered (again!)

Anyhoo I know it's late for you, no need to respond, just wanted to write this down while it's on my mind.