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

Feat/add container cd #142

Merged
merged 5 commits into from
Dec 12, 2023
Merged

Feat/add container cd #142

merged 5 commits into from
Dec 12, 2023

Conversation

RobertRosca
Copy link
Member

This PR adds a Dockerfile for DAMNIT and a CI workflow to build a container and push it to GHCR. Intended usecase is adding a new desktop shortcut to the Visa VMs which will start up DAMNIT via singularity.

A follow up PR could (should?) bundle a small example DB with the image which can be selected from the GUI at startup so that we can say "Click the DAMNIT button/copy-paste this docker or singularity command to see a demo of the GUI", since without that it's just the DAMNIT GUI with an empty table and nothing to see.

To try and keep the size of the container as small as possible it's a multi-stage Dockerfile where the build stage sets up the build environment and a venv in /app with the required python dependencies, then the final unnamed stage copies over the /app directory, installs runtime requirements, then installs DAMNIT into the venv.

The resulting image is still relatively huge (~1GB) but a bit smaller than it would otherwise be with build tools/layers included.

Options if size is an issue
  • After an (unrelated) meeting w/ DESY-people about VISA/containerisation we decided that, if download speeds are too slow for the image, we can publish the container to the DESY GitLab Registry as well as GHCR. That way the downloads will be done over LAN and should be much faster than from GitHub's registry.
  • Alternatively there are tools to make containers slimmer and only store the files required to run a given application, this is done by triggering all the functionality of an application (e.g. by running tests) and only retaining files which were opened during this period. An approach like this can hugely reduce container size, but can also introduce issues if all files are not found during the probing phase.

Starting the GUI from the container can be done with:

# docker, excluded mounts:
docker run --rm -it --net=host -e DISPLAY=:0 ghcr.io/european-xfel/damnit:$TAG

# singularity:
apptainer run docker://ghcr.io/european-xfel/damnit:$TAG

As for the actual tags, the workflow will build the image and push it to the container registry with different tags based on the workflow trigger:

event tag
tag release damnit:{x.y.z}, damnit:latest - 'latest' and version no.
pull request damnit:pr-{N} - pull request number
merge/push to master damnit:edge - 'edge' for most recent commit to master

Which covers the main use cases I think we'd have: using a tagged release, a current PR, and using current head.

Tested this with some local PRs to a fork of the repo here: RobertRosca#3

  • On PR trigger image tag was ghcr.io/robertrosca/damnit:pr-3, created from the PR branch
  • On PR merge ghcr.io/robertrosca/damnit:edge, created from master branch
  • On creating tag ghcr.io/robertrosca/damnit:0.1.0 and latest, created from master branch

Few additional comments:

  1. I can modify the CD/tag generation a bit so that you can get per-branch tags, which would let you run damnit:mid_4656 to start up a specific branch. Seems like this would fit the feature-branch development style better than the current approach.

    But the problem with this is that any push to any branch would trigger the build, and branch names may not be unique. So if you merge a branch and create a new one w/ the same name then the image would be modified

    I would rather leave it as is, then if you want a feature-branch image build you can make a PR and it'll tag the image with the pr number.

  2. Image build happens in parallel to tests. Could be set to only trigger after tests pass, but it's not a big deal.

  3. There was some other kinda important point that I forgot... will leave this here in case I remember

version on release/tag, 'edge' on merge to master, 'pr-{pr-no}' on PR
enables experimental GHCR caching, huge build speedups

see https://docs.docker.com/build/ci/github-actions/cache/#github-cache
@RobertRosca RobertRosca force-pushed the feat/add-container-cd branch from 2dfec5f to efb83ab Compare November 27, 2023 14:34
@RobertRosca
Copy link
Member Author

Worked as expected and pushed an image with tag pr-142: https://github.com/European-XFEL/DAMNIT/pkgs/container/damnit

You can test it out with:

docker run --rm -it --net=host -e DISPLAY=:0 ghcr.io/european-xfel/damnit:pr-142

Which should open the GUI directly.

@RobertRosca RobertRosca marked this pull request as ready for review November 27, 2023 14:40
@RobertRosca RobertRosca self-assigned this Nov 27, 2023
set PYTHONDONTWRITEBYTECODE=1
Copy link
Member

@JamesWrigley JamesWrigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@RobertRosca RobertRosca merged commit 7ebb2a4 into master Dec 12, 2023
2 checks passed
@RobertRosca RobertRosca deleted the feat/add-container-cd branch December 12, 2023 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants