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

Docker container for CIVET-2.1.1 #2

Merged
merged 4 commits into from Jan 13, 2020
Merged

Docker container for CIVET-2.1.1 #2

merged 4 commits into from Jan 13, 2020

Conversation

@jennydaman
Copy link
Contributor

jennydaman commented Jan 3, 2020

I wrote a Dockerfile to compile CIVET-2.1.1 in an Ubuntu 18.04 container.
Docker enables the application to run on any host operating system. Moreover, some issues regarding "best practices" are handled by Docker such as isolation of the CIVET quarantine from the host system and providing scope to the environment variables set by init.sh.

perl and imagemagick are installed for verify_clasp and verify_civet. perl is a required dependency to run CIVET_Processing_Pipeline (this should be noted in README.md)

In the builder stage, git-lfs is installed and git is configured to use HTTPS instead of SSH for pulling repositories listed in Makefile.
netpbm-10.35.94 is decompressed early so that we can sideload a Makefile.config. The provided Makefile.config was previously generated by following the directions from README.md. Its presence before ./install.sh tells make to skip ./configure, making the entire script non-interactive.

CIVET_Full_Project/Makefile

Lines 1591 to 1599 in c934d28

if [ ! -e Makefile.config ] ; then \
echo "**************************************************************" ;\
echo "* You are about to configure netpbm for the CIVET quarantine *" ;\
echo "* Choose all defaults options except libraries=static, *" ;\
echo "* Svgalib=none and X11=none. *" ;\
echo "**************************************************************" ;\
./configure ; \
fi ; \
${MAKE} --keep-going; \

Using Docker multi-stage builds, the binaries are copied (without unnecessary build tools) to the base Ubuntu 18.04 image. The final image size is 3.05 GB (builder size was 9.74GB). Some build files are deleted, though I decided to keep /include for developers.

Finally, the function of init.sh is replicated using the ENV instruction. The shell is guaranteed to have the proper environment variables set, so it's possible to invoke CIVET binaries (e.g. surface_fit, sphere_mesh) directly from docker run.

docker build -t civet:2.1.1 $PWD
jennydaman added 4 commits Jan 2, 2020
@gdevenyi

This comment has been minimized.

Copy link
Contributor

gdevenyi commented Jan 3, 2020

Amazing work!

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 6, 2020

I am going to build this later this week and try it out

@prioux prioux self-requested a review Jan 6, 2020
@prioux prioux added the enhancement label Jan 6, 2020
@sandywang

This comment has been minimized.

Copy link
Contributor

sandywang commented Jan 8, 2020

Hi @jennydaman, great work! I will try to review and merge it and modify the README.md this week.
Thank you very much for the contribution!

Best,
Xindi

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

I am building this right now, and double checking everything.

I must say I really like your Dockerfile, @jennydaman , it's clean and the fact that you are using named stages (base and builder) really keep the resulting container quite clean.

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

What I want to make sure is that we can use this image to launch other tools that are build in it too, like any of the individual minc tools, and the CIVET_QC_Pipeline.

@jennydaman

This comment has been minimized.

Copy link
Contributor Author

jennydaman commented Jan 10, 2020

Thank you @prioux!

Should be able to, e.g.

$ docker run --rm civet:2.1.1 surface_fit
Usage:     surface_fit  help not implemented yet

Copyright Alan C. Evans
Professor of Neurology
McGill University
docker run --rm -v "$PWD/mni_icbm_00100_t1.mnc:/v.mnc" civet:2.1.1 mincinfo /v.mnc
file: /v.mnc
image: signed__ short 0 to 4095
image dimensions: xspace zspace yspace
dimension name         length         step        start
--------------         ------         ----        -----
xspace                    170            1      -82.551
zspace                    256           -1      102.811
yspace                    256           -1      161.156

Of course, I ran the full job_test before this PR.

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

Yes, everything works fine. This is very good work, I approve the PR.

Do you prefer a Squash merge (with a single commit, but it would be detached from your fork and you'll have to delete the branch in force mode) or a normal merge (with 4 commits)?

@prioux
prioux approved these changes Jan 10, 2020
Copy link
Member

prioux left a comment

All is fine.

@jennydaman

This comment has been minimized.

Copy link
Contributor Author

jennydaman commented Jan 10, 2020

You should squash it to shorten the log.

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

I'm going to push the image to Dockerhub as mcin/civet:2.1.1

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

DockerHub now has mcin/civet:2.1.1 . This means it can also be used through Singularity, e.g. on the Compute Canada clusters.

@sandywang

This comment has been minimized.

Copy link
Contributor

sandywang commented Jan 10, 2020

@jennydaman

This comment has been minimized.

Copy link
Contributor Author

jennydaman commented Jan 10, 2020

@sandywang what version of Docker are you on? Multi-stage builds require 17.05 or higher.

https://docs.docker.com/develop/develop-images/multistage-build/

My docker version is Docker version 19.03.5-ce, build 633a0ea838

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

I replied to Xindi by email; our lab's docker development machine also has 19.03.5, and it builds the container just fine.

@prioux prioux requested a review from sandywang Jan 10, 2020
@sandywang

This comment has been minimized.

Copy link
Contributor

sandywang commented Jan 10, 2020

@prioux

This comment has been minimized.

Copy link
Member

prioux commented Jan 10, 2020

@sandywang I don't understand. The Dockerfile is self contained, it has all the necessary code and tools to build itself, including git lfs. I think something else is the problem, I really suggest you let me have a look at how you're trying to build this.

@sandywang sandywang merged commit ae7e0ca into aces:master Jan 13, 2020
@jennydaman jennydaman deleted the jennydaman:docker branch Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.