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 image for optimade on ghcr.io #1171

Merged
merged 19 commits into from May 16, 2022

Conversation

CasperWA
Copy link
Member

@CasperWA CasperWA commented May 10, 2022

Closes #1111

Remove docker-compose.yml, which wasn't necessary, in favor of updating the Dockerfile, making it more "production ready" and fixing up the associated entrypoint bash script in .docker/run.sh.

The main point of this PR is to add the CD job of building and publishing the Dockerfile as a container image on the GitHub Container Registry (ghcr.io).
This will be done in the following way: Every time a push to master happens, the job will run. Building an image and tagging it as optimade with the versions latest as well as the current optimade package version (0.17.0 as of writing this).

This image will be available for everyone under the full name: ghcr.io/materials-consortia/optimade.

Due to the nature of tagging a newly built image with the version, a natural succession will happen with tagged images. The latest tag will always point be built from the head commit of master, while the versioned tags will correspond to the latest commit to bear the specific version. E.g., for the previous version (0.16.12 this would be 03ef91b, i.e., the commit immediately prior to the 0.17.0 release commit).


Still to do:

  • Test the CD job with a test run on GitHub (this will release an image, but I will delete it again afterwards).
  • Extend and update the documentation regarding using the Dockerfile.

Updated tasks:

  • Only publish version-tagged images during releases.
  • Test the updated CD jobs.

This job runs on every push to `master` and builds and tags the
`Dockerfile` with `latest` and the current version of the `optimade`
package.
Update Dockerfile significantly, making it more "production ready".
Extend the `.docker/run.sh` file to have more relevant options.
Update CI job to use `docker` instead of `docker compose`.
@codecov
Copy link

codecov bot commented May 10, 2022

Codecov Report

Merging #1171 (4f3b36f) into master (89caf64) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1171   +/-   ##
=======================================
  Coverage   92.42%   92.42%           
=======================================
  Files          68       68           
  Lines        3909     3909           
=======================================
  Hits         3613     3613           
  Misses        296      296           
Flag Coverage Δ
project 92.42% <100.00%> (ø)
validator 92.42% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
optimade/adapters/structures/utils.py 81.30% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89caf64...4f3b36f. Read the comment docs.

@ml-evs
Copy link
Member

ml-evs commented May 10, 2022

Can't immediately check, but the only thing that comes to mind are the limits on the number of packages we can have (i.e. whether we can keep the builds per version). We could consider getting this organisation upgraded to an academic one (equivalent to GitHub pro) to increase the limits.

@CasperWA
Copy link
Member Author

Indeed - if we want this to be truly useful there are some costs associated. But mainly I would use this in CI/CD in the context of GH Actions, where all data transfers are free. If at some point a limit is reached, one may always be able to clone the package and build anew at least.

@ml-evs
Copy link
Member

ml-evs commented May 11, 2022

Closes #1111

Remove docker-compose.yml, which wasn't necessary, in favor of updating the Dockerfile, making it more "production ready" and fixing up the associated entrypoint bash script in .docker/run.sh.

The main point of this PR is to add the CD job of building and publishing the Dockerfile as a container image on the GitHub Container Registry (ghcr.io). This will be done in the following way: Every time a push to master happens, the job will run. Building an image and tagging it as optimade with the versions latest as well as the current optimade package version (0.17.0 as of writing this).

Are such bleeding edge images necessary, do you think? I wouldn't be against latest just pointing to the last release version (in any case, stable should probably point to the last released version). master isn't guaranteed to be a working snapshot (although of course we try).

Due to the nature of tagging a newly built image with the version, a natural succession will happen with tagged images. The latest tag will always point be built from the head commit of master, while the versioned tags will correspond to the latest commit to bear the specific version. E.g., for the previous version (0.16.12 this would be 03ef91b, i.e., the commit immediately prior to the 0.17.0 release commit).

Can't we switch the build for released versions to trigger on release only?

@CasperWA
Copy link
Member Author

CasperWA commented May 11, 2022

Are such bleeding edge images necessary, do you think? I wouldn't be against latest just pointing to the last release version (in any case, stable should probably point to the last released version). master isn't guaranteed to be a working snapshot (although of course we try).

I'd like these latest images for GH Actions CI/CD workflows in other projects, to be honest. Although I could always clone and build. But the whole motivation for this was to make it easier for CI/CD to start a test OPTIMADE server. And in that context I'd like to have the possibility of testing against the latest and upcoming changes.

Can't we switch the build for released versions to trigger on release only?

This would make it more true to the version matching up with the code that can be retrieved from the releases on PyPI as well - so not against this.

@ml-evs
Copy link
Member

ml-evs commented May 11, 2022

I'd like these latest images for GH Actions CI/CD workflows in other projects, to be honest. Although I could always clone and build. But the whole motivation for this was to make it easier for CI/CD to start a test OPTIMADE server. And in that context I'd like to have the possibility of testing against the latest and upcoming changes.

Fair enough, a personal use case is the best argument :)

Can't we switch the build for released versions to trigger on release only?

This would make it more true to the version matching up with the code that can be retrieved from the releases on PyPI as well - so not against this.

I would strongly prefer this; I guess this can be achieved by separating the image build workflow into its own file that is triggered by both releases/master pushes and some detection of which one was the trigger?

@CasperWA
Copy link
Member Author

I would strongly prefer this; I guess this can be achieved by separating the image build workflow into its own file that is triggered by both releases/master pushes and some detection of which one was the trigger?

I'll just create 2 separate jobs I think - hmm, I'll think about it. We're already doing something similar concerning the documentation release.

A re-usable workflow will be used to build and publish the container
image.
It will be called from the release workflow as well as the workflow
that's running when an update to 'master' takes place.
@CasperWA
Copy link
Member Author

CasperWA commented May 12, 2022

As a comment - the many commits above (from db9eda0 to fd90ff4) show in the CI/CD that calling the re-usable workflow for publishing the container image seems to work as intended.
It is set up so that for any push to master that isn't due to a new release will result in a new container image being published with the latest tag.
If a release is created the workflow will be run such that it will build and publish a container image with both the latest tag as well as the given version as a tag (e.g., 0.17.0). In this case, the "push to master" should not run. This is set up to coincide and reuse the check for when to release docs, which has been running successfully for a while, and has the same logical workflow concerning versioning, releases and updates to master.

The only thing missing now is some documentation, essentially.

Add `typing.Optional` to type annotations in an attempt to fix docs
building locally.
@CasperWA CasperWA marked this pull request as ready for review May 13, 2022 14:56
@CasperWA
Copy link
Member Author

Note, after this PR has been merged, a new container image should be built and published with only the latest tag. Then I will remove the current image, and thereby also the 0.17.0 tag, and finally I'll make the image public (it is currently private).

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Looks great @CasperWA - few comments below about the docs.

INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
@JPBergsma
Copy link
Contributor

I am a bit late to the party, but I am wondering whether it would not be more logical to name the release that corresponds to the current master devellop or nightly and let latest point to the last release. For me, that seems more inline with how other software packages do it.

INSTALL.md Show resolved Hide resolved
.docker/run.sh Outdated Show resolved Hide resolved
Co-authored-by: Johan Bergsma <29785380+JPBergsma@users.noreply.github.com>
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
INSTALL.md Show resolved Hide resolved
Create new top-level subjects "Deployment" and "Concepts".
Move most of the new container information under the "Deployment"
section.
@CasperWA
Copy link
Member Author

I have updated the ToC for the documentation, moving stuff around and creating a couple more top-level subjects: "Deployment" and "Concepts".
The major part of the text concerning the container docs have been moved under "Deployment".

I have furthermore updated the .pages files to use nav instead of the out-dated arrange key. This makes it possible to add custom titles in the ToC and more.

@CasperWA CasperWA requested review from ml-evs and JPBergsma May 16, 2022 10:44
@CasperWA
Copy link
Member Author

CasperWA commented May 16, 2022

I am a bit late to the party, but I am wondering whether it would not be more logical to name the release that corresponds to the current master devellop or nightly and let latest point to the last release. For me, that seems more inline with how other software packages do it.

Good point!

I can quickly change this, having the releases update the latest tag as well as creating a version-specific tag and then change the current latest image generation to develop.
nightly I'd consider more an image generation that's created as a cronjob. At night... ;) That's will not be the case here.

Add a sentence concerning the `develop` tag in the installation
documentation.
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

This looks great to me now @CasperWA, thanks. I was going to suggest breaking up the "getting started" guides a bit at some point anyway, so this is perfect.

Merge as you wish!

@CasperWA
Copy link
Member Author

Merge as you wish!

This all right for you as well @JPBergsma?

@JPBergsma
Copy link
Contributor

Looking through the files, I did not see anything strange other than what I have already mentioned.
I was still planning to walk through the instructions on my machine. I do not know if you want to wait for this. If not you can merge it.

@CasperWA
Copy link
Member Author

CasperWA commented May 16, 2022

Looking through the files, I did not see anything strange other than what I have already mentioned. I was still planning to walk through the instructions on my machine. I do not know if you want to wait for this. If not you can merge it.

Hmm - I'd rather just merge and then have a separate issue for docs updates - if necessary :)
I already ran through the instructions locally :)

@CasperWA CasperWA merged commit 4d154f7 into master May 16, 2022
@CasperWA CasperWA deleted the cwa/close-1111-docker-image-ghcr.io branch May 16, 2022 12:13
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.

Release a container (Docker) image for developers
3 participants