-
Notifications
You must be signed in to change notification settings - Fork 0
Adding support for multistage-build #1
base: main
Are you sure you want to change the base?
Conversation
bbc5f2a to
e43ec6b
Compare
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
e43ec6b to
4f0065e
Compare
alecbcs
left a comment
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.
This looks great! I just added a couple of comments, most of which are just questions for my own interest.
| dockerfile := "FROM ghcr.io/autamus/" + currentContainer + ":" + currentVersion + " as base\n" + | ||
| "FROM " + base + "\n" + | ||
| "COPY --from=base " + fromPath + " " + toPath + "\n" + | ||
| "ENV PATH=/opt/spack/bin:$PATH\n" + |
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.
If the toPath is different than /opt/spack/bin I think we should load that here, right? So that no matter where you dump the directory of build binaries they should be accessible in the container's path.
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.
Oh I see. Hmm. So the toPath I think would actually be /opt/software, here is the original Dockerfile I tested (that worked)
FROM autamus/clingo as clingo
FROM autamus/dyninst as dyninst
FROM spack/ubuntu-bionic
COPY --from=clingo /opt/software /opt/spack/opt/spack
COPY --from=dyninst /opt/software /opt/spack/opt/spack
ENV PATH=/opt/spack/bin:$PATH
WORKDIR /opt/spack
RUN rm -rf opt/spack/.spack-db/
ENTRYPOINT ["/bin/bash"]
There is a dependency on spack/ubuntu-bionic because without it the autamus containers don't have a spack install. So when we add /opt/spack/bin to the path that is always going to be adding the executable spack to the path (not a binary produced by a library, which we aren't copying from /opt/spack in the autamus containers at all! We are mostly interested in the installs of software (not in the view) at /opt/software:
# ls /opt/software/linux-ubuntu18.04-x86_64/gcc-7.5.0/
bzip2-1.0.8-l7cablykbwihc3dodfslpdapfecyp6ap libiconv-1.16-jearpk4xci4zc7dkrza4fufaqfkq7rfl readline-8.1-cp2spc5nyc7yp7ix4vcv67z3c5egnpvq
clingo-5.5.0-lrqchvxh37jbt74wsoagrnhqo232j5bs libmd-1.0.3-jbh4lbggrfzy4p6mfaxeiemlgdtjw2gt sqlite-3.35.5-6icmek2ierjksptwfaz6lcynpbaqfi4l
expat-2.3.0-5bxa3uilnexzrdl2wjezkhbusqofoukj libxml2-2.9.10-yn2r3wfhiilelyulh5toteicdtxjhw7d tar-1.34-hoxca47aiicpqdwnpqxsdcjw5nl7zkf6
gdbm-1.19-xewrv7uqabfsq4gjhizdnhmexgi77zma ncurses-6.2-uba77xwfqaytmx3v5eye43deqq5rqsun util-linux-uuid-2.36.2-dod5oncbwvw2oz3f6pqtszkrzdc6fkpl
gettext-0.21-buy5sq7jaxwny4eyujzu7r2jpwik6d6b openssl-1.1.1k-l6unsppgaas64422cyporf7cfzvn4fow xz-5.2.5-komekkmyciga3kl24edjmredhj3uyt7v
libbsd-0.11.3-6zivdaml3pxyvkf7a5npqdhj7jr5gvll patchelf-0.12-7bwvapv7y32znnhd2wrappqdyyad6czq zlib-1.2.11-smoyzzo2qhzpn6mg6rd3l2p7b23enshg
libffi-3.3-hyhbnrmaxc5veijr2iyveifkj4azv5dv python-3.9.6-hxkg444hitp57yqtpr3wavzasvm5l2yj
So TLDR the /opt/spack/bin is from the spack ubuntu bionic base, which is not represented here, but would be a layer we add. The reason I added the ENV command here is because I'm not sure where else I could put it. But you are right if the user doesn't eventually add the ubuntu bionic image it wouldn't do anything.
| "ENV PATH=/opt/spack/bin:$PATH\n" + | ||
| "WORKDIR /opt/spack\n" + | ||
| "RUN rm -rf /opt/spack/.spack-db/\n" + | ||
| "ENTRYPOINT [\"/bin/bash\"]\n" |
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.
Does ENTRYPOINT ["/bin/bash", "-l", "-c"] not work here? Or is there a reason we'd prefer not to use -l and -c?
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.
I usually just do /bin/bash.
| // Add support for build cache | ||
| // The lines replaced here are the same line, one with and one without monitor | ||
| buildOld := "RUN cd /opt/spack-environment && spack env activate . && spack install --fail-fast && spack gc -y" | ||
| buildOldMonitor := "RUN cd /opt/spack-environment && spack env activate . && export SPACKMON_USER=$(cat /run/secrets/su) && export SPACKMON_TOKEN=$(cat /run/secrets/st) && spack install --fail-fast && spack gc -y" |
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.
Is this because the containerize PR finally got merged and it's now creating Spack Monitor Commands within the Dockerfile?
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.
Yes, the command to replace is different if the Dockerfile has monitor vs. not.
This pull request will add support for doing a multistage build - meaning a second Dockerfile (Dockerfile.layer) that can use the original container, copy over some source and destination, use some base container, and then generate an output variable for the suffix. I'm not sure how to test this locally, but I was thinking I could PR to autamus registry to try testing it out. What do you think @alecbcs ?
Signed-off-by: vsoch vsoch@users.noreply.github.com