-
Notifications
You must be signed in to change notification settings - Fork 3
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
Adding Apptainer/Singularity support #134
Conversation
…r apptainer/singularity
…paration for Apptainer/Singularity container support
… to bootrstrap from docker image
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## v0.3 #134 +/- ##
==========================================
- Coverage 97.26% 95.50% -1.77%
==========================================
Files 10 11 +1
Lines 988 1157 +169
==========================================
+ Hits 961 1105 +144
- Misses 27 52 +25
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot again! These comments are some suggestions, mostly stylistic. I still need to conduct some smoke tests; and will report to you again.
(If you can, please also set the indentation to 4 spaces. Unfortunately, RStudio doesn't support the .editorconfig
file provided. If you are using RStudio, you might add your .Rproj
.)
…nerate_container_readme() to work with both types of containers; (3) apptainerize() inherits duplicate params from dockerize()
Smoke
|
@e-kotov Did you try to apptainerize with somewhere <- ""
x <- resolve(pkgs = "quanteda", snapshot_date = "2018-10-06")
apptainerize(x, somewhere, image = "rstudio") It can be built fine. cd somewhere
apptainer build container.sif container.def But can't be launched correctly. Specifically ## wouldn't run, with error
apptainer instance start container.sif
## can access via 0.0.0.0:8787, but error
apptainer exec container.sif /usr/lib/rstudio-server/bin/rserver --auth-none=1 --auth-pam-helper-path=pam-helper
## password won't pass
PASSWORD='abc123' singularity exec container.sif /usr/lib/rstudio-server/bin/rserver --auth-none=0 --auth-pam-helper-path=pam-helper But I must admit I am a pretty lousy apptainer user (just learned to use it 72 hours ago). Maybe you would have a better idea how it works. |
@chainsawriot I have tested, but in a slightly different environment. I am woking on (a) fixes and (b) readme for apptainer. Some more testing needs to be done, as it does not work in every environment reliably yet. I have addressed most of your comments already, but I have not pushed my commits to GitHub yet. I will let you know once I re-check everything so that you don't have to spend your time troubleshooting. Thanks for your feedback and patience. |
…ngly incompatible with Apptainer/Singularity
…Studio IDE in Apptainer/Singularity
…t be started using `root` user
…uires binding .rstudio to the container (2) add default port parameter so that it is obvious how to change it
…hat are needed for start of the instance, but not the install process
… instructions in running containers
I did some more testing and here are the details on how to run those. I am also adding the info below to package readme, Apptainer readme template and FAQ vignette.
|
…iroment of a running container
@chainsawriot I have addressed all of your review suggestions and questions and have done some more testing on a VPS with both admin and user privileges. I hope my README additions will easily allow you to check the updates and successfully run an Apptainer container. |
For quicker tests, I use:
It is not 'foolproof', but for quick iteration it is ideal, as data.table has no dependencies and builds very quickly during container builds. |
…pteinerize() and are carbon copies of tests in tests_dockerize.R
@e-kotov Thanks a lot! I can get all smoke tests running. I will merge it now. I think there are still some minor issues related to the documentation. But those can be fixed later. Again, thanks a lot for your tremendous contribution to this package! |
@chainsawriot Thank you too! I will look out for issues on your repo related to Apptainer/Singularity. |
I have added support for Apptainer (formerly Singularity) containers.
To do this I:
dockerize.R
toinstallarion.R
, specifically:.insert_materials_dir()
and.write_dockerfile()
. Also.write_dockerfile()
is now more generic.write_container_file()
, because it serves bothdockerize()
andapptainerize()
..insert_materials_dir()
now also serves both functions and distinguishes between docker and apptainer/singularity containers. It seems safe to do so, all existing tests pass and I also added a separate test for it intest_apptainerize.R
.apptainerize()
function (with name variations). I am not sure you want to "pollute" the package withsingularize()
synonyms. Kindly let me know what you think.apptainerize()
with formal tests that mirror what you have for `dockerize()' + I did a lot of testing using the actual singularity module on a linux server. All singularity definitions seem to work just fine.debootstrap
installed https://apptainer.org/docs/user/latest/appendix.html#id17 therefore, at least for now, I think it is much more user friendly to skip the caching of Debian image and just use the Docker image of debian:eol in the Apptainer/Singularity definition.