aries-cloudagent-python
aries-cloudagent-python copied to clipboard
feat: add dockerfiles for building images from repo
Putting this out there to hopefully spark some discussion.
This PR adds Dockerfiles for publishing images for ACA-Py directly from this repo (i.e. no dependencies on bcgovimages). I made some opinionated decisions that I hope will catch some people's attention (so you can tell me why I'm wrong :slightly_smiling_face:):
- Rather than building aries-askar, indy-shared-rs, and indy-vdr directly in the image, I opted to instead let it use the binaries bundled in the python packages. Good chance I'm wrong, but I'm about 70% sure these libraries might have been taking precedence over those included in von-image anyways.
- I am thinking of "indy-less" ACA-Py as the default.
ghcr.io/hyperledger/aries-cloudagent-python:py3.6-0.7.4
image would not have any Indy libraries installed. To get an image with Indy included, I'm thinking the image name+tag might look like:ghcr.io/hyperledger/aries-cloudagent-python:py3.6-indy-1.16.0-0.7.4
. - The ACA-Py+Indy image does NOT include Indy CLI (von-image did).
- The default ACA-Py image's user is now
aries
instead ofindy
. The ACA-Py+Indy image's user will still beindy
(this inconsistency may be undesirable...). - I introduced an
indy-python
image that can be reused as a base for things needing an image with just python3-indy and built Indy libraries. This is the base image used by the ACA-Py+Indy image. I would like to see this published asghcr.io/hyperledger/indy-python
. - Rather than using pyenv to get a specific python version, I opted to just use the relevant python docker image for the requested version, i.e.
python:3.6.13-slim-buster
. Python packages are installed into the system site packages (rather than the user site packages or a virtual environment). - ~For now, I've followed the example of the aries-cloudagent-container repo and am installing ACA-Py through the published PyPI package. It might make more sense to be able to publish directly from the repo contents?~ With the GHA additions, the images are now built directly from the repo.
I get the feeling that we can further prune a lot of the installed packages but (for now) I've left in most of those originally installed in von-image.
I've included a simple Makefile purely to demonstrate building the image and I do not anticipate that we'd leave this Makefile around. I'll plan on removing it before this PR moves on to potentially getting merged.
This is relevant to #1854, #1856, and #1871.
cc @WadeBarnes @andrewwhitehead @swcurran @burdettadam
Codecov Report
Merging #1888 (38252c0) into main (1fb0bca) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## main #1888 +/- ##
=======================================
Coverage 93.53% 93.53%
=======================================
Files 539 539
Lines 34559 34559
=======================================
Hits 32326 32326
Misses 2233 2233
Okay, some updates. I've added Github Actions for various operations:
- Unit testing
- Tests for ACA-Py without Indy
- These tests run directly on the runner.
- Tests for ACA-Py with Indy
- These tests run inside of a container (necessary for Indy library to be available). The image for the container is built and pushed, if needed, to the ghcr.io for the current repository. The images are
acapy-test:py{python-version}-{indy-version}-{hash of dependent files}
andindy-python-test:py{python-version}-{indy-version}-{hash of dependent files}
.
- These tests run inside of a container (necessary for Indy library to be available). The image for the container is built and pushed, if needed, to the ghcr.io for the current repository. The images are
- These are currently configured to run on each PR but it is likely more appropriate to schedule runs of most versions and just select a preferred python version and run (minimally) one with indy or one with indy and one without.
- Tests for ACA-Py without Indy
- Nightly Builds
- If a nightly build at the current hash of the repo doesn't yet exist, these will build an ACA-Py without Indy and an ACA-Py with Indy images at midnight each day. I included these more as a proof of concept than anything. I'm not strongly opinionated about whether we should or should not do nightly builds but we certainly have that ability.
- Publish images
- Publish an image of ACA-Py without Indy. This will produce an image with the name+tag
ghcr.io/{repo-owner}/aries-cloudagent-python:py{python-version}-{acapy-version}
. For exampleghcr.io/hyperledger/aries-cloudagent-python:py3.7-0.7.4
. - Publish an image of ACA-Py with Indy. This will produce an image with the name+tag
ghcr.io/{repo-owner}/aries-cloudagent-python:py{python-version}-indy-{indy-version}-{acapy-version}
. For exampleghcr.io/hyperledger/aries-cloudagent-python:py3.7-indy-1.16.0-0.7.4
. - Publish an image of
Indy-Python
, the base image for ACA-Py with Indy. This will produce an image with the name+tagghcr.io/{repo-owner}/indy-python:py{python-version}-{indy-version}
. For example:ghcr.io/hyperledger/indy-python:py3.7-1.16.0
. - These are currently only set to be triggered manually but it seems appropriate to have them run automatically on release.
- Publish an image of ACA-Py without Indy. This will produce an image with the name+tag
The Tests (Indy)
workflows are currently failing (I believe) because of permissions of this repo to publish packages. If we want to pursue this approach, we'd probably need to reach out to Ry to help set things up.
The
Tests (Indy)
workflows are currently failing (I believe) because of permissions of this repo to publish packages. If we want to pursue this approach, we'd probably need to reach out to Ry to help set things up.
It's the permissions around PRs causing the issue. PRs don't have access to GitHub secrets including GITHUB_TOKEN
. The images can only be published after the PR gets merged.
It's the permissions around PRs causing the issue. PRs don't have access to GitHub secrets including
GITHUB_TOKEN
. The images can only be published after the PR gets merged.
Ah, well that makes perfect sense :slightly_smiling_face:
To see some completed workflows, check here for the runs from my testing on Indicio's fork: https://github.com/Indicio-tech/aries-cloudagent-python/pull/66/checks (I think those are publicly accessible??)
I don't have the background to assess this work. Can someone help out and work with @dbluhm on this?
@PaulWen, @dbluhm does this related to the PR that Paul submitted earlier on Dockerfiles and the like -- #1856
I tagged v1.0.0-rc0, and would love to see that published using the new mechanism. I did the publish an image the old way.
@swcurran as @dbluhm also added the pipelines for building and publishing the Docker images I would consider #1856 to be obsolete
SonarCloud Quality Gate failed.
0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells
No Coverage information
8.1% Duplication
I think it would make sense to move the indy build into indy-sdk instead of having it in aries-cloudagent-python.
I'm going to take a final pass at this this week, update documentation, make a few updates to the actions that I discovered would be helpful while testing similar setups on other repositories, and then push for this to get merged. I'll make some noise when I have those updates in and seek review from maintainers.
I found that there were a number of things I could simplify after looking at this with fresh eyes. I'm working through testing those simplifications. Progress is a little slower than I'd hoped due to competing priorities but I've tapped in some of the rest of my team to help move this along.
Hopefully my final update for this PR:
I've significantly simplified the github actions and published images. The Indy variant is now handled as a multistage build rather than requiring a separate published image. If we pushed for an "indy-python" image to be published from the Indy SDK repo, we could simplify this further but I feel the multistage build is a welcome simplification for now.
Tests are no longer dependent on published images; this means that a docker image is built to run the Indy tests. I've optimized this as much as possible without depending on external images; the action uses the cache action to cache docker image layers. This dramatically speeds up build times. This can likewise be improved significantly if we pushed for an "indy-python" image but, again, in the absence of such an image, I believe this solution to be adequate.
I've implemented the suggestions previously discussed for the PRs and nightly tests. Tests triggered by PRs will only run the "default" python version. Tests are also run on a cron job (currently set to every night at midnight) that will run tests on all currently supported python versions. The workflows for these tests are now neatly reused between the PR tests and nightly tests workflows (more a moral victory than anything :smile:).
The image publish workflows can be triggered either by creating a release or by manually triggering them. Manually triggering is a good option for ad hoc dev builds. As such, I chose to just remove the nightly build and publish actions.
I added docs discussing the two image variants and brief descriptions of the github actions.
The current "default" python version is Python 3.6 (run on PRs). The current set of "supported python versions" is 3.6, 3.7, 3.8, 3.9, and 3.10 (run on nightly tests). In actuality, I expect the nightly tests to fail on some or most of these other versions until we finish up #1854.
I'm sure further improvements can be made (further image slimming, minor optimizations to test workflows, etc.) but I think this is a good starting point. I'm eager to see this merged :slightly_smiling_face:
cc @swcurran @ianco @WadeBarnes
@swcurran @ianco @WadeBarnes Anything preventing us from merging in this PR? Thanks!
@dbluhm, I'd like to look over the changes in detail first. I'm almost at a point were I can do that.
Thanks for the feedback @WadeBarnes! Working through them now.
Should a similar approach be taken with Dockerfile to replace Dockerfile.test?
Fair question; I think we could add the test image as a stage in the main Dockerfile but I don't think I would recommend this. The test image being a stage for the indy setup was essentially required for us to have the indy-base available without needing to separately commit it to an image. We don't have the same requirements for the standard, non-indy setup. There's no shared base (besides just a python image) between the main docker image and the test docker image.
@WadeBarnes @swcurran in the interest of keeping this moving forward, here's a quick update: I've attempted to correct the issues and pushed what I hope will fix at least the tag name collision issues. I have some currently broken work on running tests before publishing a release which can be found here: https://github.com/dbluhm/aries-cloudagent-python/tree/fix/release-version-issues
I propose that, unless we have major issues with these as currently structured, we go ahead and merge at least the PR related actions so we can overcome CircleCI issues and then commit to fixing the release workflow ASAP. I'd be happy to prepare a separate PR with just the actions for running tests on nightly and PRs if that would help us feel more comfortable.
@dbluhm, I'll have a look and get back to you with a decision. Separating the PRs is a good option, but I want to quickly see if the issues are fixed.
@dbluhm, It looks like there are still some bugs to workout with the images published with the workflows that are triggered by a release. The images are still getting overwritten, as they are not getting tagged with the python version or -indy
tag.
References:
- https://github.com/WadeBarnes/aries-cloudagent-python/actions/runs/3743057801
- https://github.com/WadeBarnes/aries-cloudagent-python/actions/runs/3743058781
- https://github.com/WadeBarnes/aries-cloudagent-python/actions/runs/3743062864
- https://github.com/WadeBarnes/aries-cloudagent-python/actions/runs/3743062872
- https://github.com/WadeBarnes/aries-cloudagent-python/pkgs/container/aries-cloudagent-python/versions
The pr-tests
workflow is working; https://github.com/WadeBarnes/aries-cloudagent-python/actions/runs/3743130229
I think the best way forward is to separate the PRs. Please proceed with preparing a separate PR with just the actions for running tests on nightly and PRs.
This PR is being broken up into more bite-sized pieces. See:
- #2058
- #2059
- Another PR will be opened for docker images and publishing those images.