Skip to content

Conversation

@georgiastuart
Copy link

This is intended to fix issue #522 in SHPC.

The Problem

Since Apptainer is forked from Singularity 3+, it needs to be included in the check for Singularity 3+ in the pull function:

if pull_folder:
        final_image = os.path.join(pull_folder, os.path.basename(name))

        # Regression Singularity 3.* onward, PULLFOLDER not honored
        # https://github.com/sylabs/singularity/issues/2788
        if "version 3" in self.version(): <---- HERE!!
            name = final_image
            pull_folder = None  # Don't use pull_folder
    else:
        final_image = name

Otherwise, containers are pulled to $PWD instead of the intended pull_folder.

The Fix

Just add a check for apptainer as well:

if pull_folder:
        final_image = os.path.join(pull_folder, os.path.basename(name))

        # Regression Singularity 3.* onward, PULLFOLDER not honored
        # https://github.com/sylabs/singularity/issues/2788
        if ("version 3" in self.version()) or ("apptainer" in self.version()):
            name = final_image
            pull_folder = None  # Don't use pull_folder
    else:
        final_image = name

@vsoch
Copy link
Member

vsoch commented Mar 27, 2022

You beat me to it! So I'd rather not have "apptainer" hard coded in the files here - my suggestion is to allow some kind of flag (either from the environment or given to the client to start) that simply skips the check if it's set. And likely we can start to remove the if/else between the two because nobody should be using 2.x anymore.

I'll have time to put in a PR later today unless you beat me to it! Sorry for delay we had fires yesterday, things are looking better today but I was so exhausted I slept a toooon!

@vsoch
Copy link
Member

vsoch commented Mar 27, 2022

okay will have PR soon! Im gutting out a little more of the 2.x functionality so it's a bit more extensive.

vsoch added a commit that referenced this pull request Mar 27, 2022
…import, export

the reason being so we can easily support apptainer and singularityce installs,
which both provide an executable singularity but with different versions! Also,
people should not be using the 2.x command groups at this point because there are too
many security issues. We can support this in some small way by not making it easy/possible
to do here, unless they want to install an older version. This will fix #190

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@georgiastuart
Copy link
Author

Awesome! Closing since your solution is more robust.

@georgiastuart georgiastuart deleted the apptainer-pull-compatibility branch March 27, 2022 18:46
vsoch added a commit that referenced this pull request Mar 27, 2022
#191)

* removing checks for versions across all files and 2.x image commands import, export

the reason being so we can easily support apptainer and singularityce installs,
which both provide an executable singularity but with different versions! Also,
people should not be using the 2.x command groups at this point because there are too
many security issues. We can support this in some small way by not making it easy/possible
to do here, unless they want to install an older version. This will fix #190

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
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.

Container pulled to cwd not container_base

2 participants