-
Notifications
You must be signed in to change notification settings - Fork 10
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
OCI container creation at the LPC #18
Conversation
…an OCI style image containing the current CMSSW release.
@kpedro88 I believe I have addressed all of your review comments. Please let me know if you're happy with the results. |
containerize/Dockerfile
Outdated
ARG CMSSW_VERSION | ||
ARG TAR | ||
|
||
COPY ${TAR} /home/cmsusr/${CMSSW_VERSION}.tar.gz |
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.
Similar to previous comment, could this be:
COPY ${TAR} ${WORKDIR}/${CMSSW_VERSION}.tar.gz
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 changed it to /home/${USER}
as the ${WORKDIR}
and ${HOME}
variables are not available and misleading, respectively.
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.
Why is ${WORKDIR}
not available? In fact, it's defined in this file but then never used anywhere...
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.
That's not an argument or environment variable. The line:
WORKDIR /home/${USER}
Is the location the user will be dropped into when they open the container, as opposed to '/'.
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.
Ah okay, it's a special Docker directive. In that case, I propose:
ARG USERDIR /home/${USER}
WORKDIR ${USERDIR}
then ${USERDIR}
can be referenced where useful.
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.
Okay, take a look. I think I've implemented what you asked for.
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, this is better.
…option to override default username in container and some changes to the Dockerfile for sustainability.
containerize/Dockerfile
Outdated
ARG NONPRIVILEGED_USER=cmsusr | ||
ARG USERDIR=/home/${NONPRIVILEGED_USER} | ||
WORKDIR ${USERDIR} | ||
COPY --from=builder --chown=cmsusr:cmsusr /home/cmsusr/${CMSSW_VERSION} ${USERDIR}/${CMSSW_VERSION} |
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 ${USERDIR}
works here, can't you define ARG USER_BUILD cmsusr
and then USER ${USER_BUILD}
, etc. to work around the #19 issue?
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 can certainly do what you're suggesting, and it does partially get around the problem. However, it won't get around #19 because I still won't be able to evaluate arguments in the parameters (chown
) of the COPY
command. Let me know what you think now.
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 think this is an improvement. Can you update #19 to clarify the point about using arguments specifically in the parameters of the COPY
command?
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.
Okay, #19 is updated.
2be2c3e
to
9431c84
Compare
+1 |
Given the presence of buildah, the containerize.sh script will create an OCI style image containing the current CMSSW release. The only prerequisites are (which are checked before running):
/etc/subuid
and/etc/subgid
The basic workflow would be: