-
Notifications
You must be signed in to change notification settings - Fork 16
[ENH] new docker container to build apptainer containers. #150
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
base: master
Are you sure you want to change the base?
Conversation
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.
Summary of Changes
Hello @bpinsard, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new Docker container environment, built on Alpine Linux, specifically designed to simplify the process of building Apptainer containers. It comes pre-configured with all the essential tools and libraries, including Apptainer itself, Datalad, and Datalad-container, providing a ready-to-use setup for containerization workflows.
Highlights
- New Dockerfile for Apptainer Builds: An Alpine-based Dockerfile has been added to facilitate the creation of Apptainer containers.
- Essential Dependencies Included: The Dockerfile pre-installs key tools such as Apptainer, Datalad, and Datalad-container, along with other necessary utilities like git and openssh-client.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces a new Dockerfile for building Apptainer containers. The implementation is a good start, but there are opportunities to optimize the Docker image by improving layer management, cleanup, and removing unnecessary packages. My review includes suggestions to combine RUN layers for better efficiency and to reconsider the inclusion of a testing package to keep the image lean.
RUN apk add --no-cache apptainer py3-pytest ca-certificates libseccomp squashfs-tools tzdata fuse2fs fuse-overlayfs squashfuse \ | ||
python3 py3-pip git openssh-client git-annex curl bzip2 bash\ | ||
&& mkdir -p $APPTAINER_TMPDIR \ | ||
&& cp /usr/share/zoneinfo/UTC /etc/localtime \ | ||
&& apk del tzdata \ | ||
&& rm -rf /tmp/* /var/cache/apk/* | ||
|
||
RUN pip install --break-system-packages --no-cache-dir datalad datalad-container |
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.
The apk
and pip
installations are in separate RUN
layers, and the cleanup logic is suboptimal. This can lead to a larger final image size because temporary files from the pip
installation are not cleaned up. It's a Docker best practice to chain related commands into a single RUN
layer and perform cleanup at the end. This creates a more compact and efficient image layer.
Additionally, rm -rf /var/cache/apk/*
is redundant when using the --no-cache
flag with apk add
.
RUN apk add --no-cache \
apptainer \
py3-pytest \
ca-certificates \
libseccomp \
squashfs-tools \
fuse2fs \
fuse-overlayfs \
squashfuse \
python3 \
py3-pip \
git \
openssh-client \
git-annex \
curl \
bzip2 \
bash \
tzdata \
&& pip install --break-system-packages --no-cache-dir datalad datalad-container \
&& mkdir -p $APPTAINER_TMPDIR \
&& cp /usr/share/zoneinfo/UTC /etc/localtime \
&& apk del tzdata \
&& rm -rf /tmp/*
FROM alpine:3.21.3 | ||
ENV PATH="/usr/local/apptainer/bin:$PATH" \ | ||
APPTAINER_TMPDIR="/tmp-apptainer" | ||
RUN apk add --no-cache apptainer py3-pytest ca-certificates libseccomp squashfs-tools tzdata fuse2fs fuse-overlayfs squashfuse \ |
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.
The py3-pytest
package is being installed. pytest
is a testing framework and is generally not required in a production or builder image unless its explicit purpose is to execute tests. Including it adds unnecessary bloat to the image, increasing its size and potential attack surface. If this image is not intended for running tests, please consider removing this package.
- name: Log in to the Container registry | ||
uses: docker/login-action@v3.3.0 | ||
with: | ||
registry: ${{ env.REGISTRY }} |
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.
do you know value to use if for docker hub? https://github.com/docker/login-action?tab=readme-ov-file#docker-hub then does not use any then
username: repro-bot | ||
password: ${{ secrets.REPROBOT_REGISTRY_TOKEN }} |
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.
username: repro-bot | |
password: ${{ secrets.REPROBOT_REGISTRY_TOKEN }} | |
username: ${{ secrets.DOCKER_LOGIN }} | |
password: ${{ secrets.DOCKER_TOKEN }} |
to be more inline with setup we have in https://github.com/ReproNim/reprostim/blob/master/.github/workflows/docker.yml
This PR adds:
Registry auth needs to be polished (generate token, store as secret... )