aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

feat: add dockerfiles for building images from repo

Open dbluhm opened this issue 1 year ago • 13 comments

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 of indy. The ACA-Py+Indy image's user will still be indy (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 as ghcr.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

dbluhm avatar Aug 06 '22 21:08 dbluhm

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           

codecov-commenter avatar Aug 06 '22 22:08 codecov-commenter

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} and indy-python-test:py{python-version}-{indy-version}-{hash of dependent files}.
    • 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.
  • 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 example ghcr.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 example ghcr.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+tag ghcr.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.

dbluhm avatar Aug 16 '22 17:08 dbluhm

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.

dbluhm avatar Aug 16 '22 17:08 dbluhm

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.

WadeBarnes avatar Aug 16 '22 17:08 WadeBarnes

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:

dbluhm avatar Aug 16 '22 17:08 dbluhm

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??)

dbluhm avatar Aug 16 '22 17:08 dbluhm

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 avatar Aug 19 '22 21:08 swcurran

@swcurran as @dbluhm also added the pipelines for building and publishing the Docker images I would consider #1856 to be obsolete

PaulWen avatar Aug 21 '22 07:08 PaulWen

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
8.1% 8.1% Duplication

sonarcloud[bot] avatar Aug 23 '22 15:08 sonarcloud[bot]

I think it would make sense to move the indy build into indy-sdk instead of having it in aries-cloudagent-python.

reflectivedevelopment avatar Aug 23 '22 16:08 reflectivedevelopment

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.

dbluhm avatar Sep 14 '22 21:09 dbluhm

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Sep 17 '22 16:09 sonarcloud[bot]

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.

dbluhm avatar Sep 29 '22 17:09 dbluhm

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

dbluhm avatar Oct 19 '22 03:10 dbluhm

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Oct 25 '22 02:10 sonarcloud[bot]

@swcurran @ianco @WadeBarnes Anything preventing us from merging in this PR? Thanks!

dbluhm avatar Oct 25 '22 02:10 dbluhm

@dbluhm, I'd like to look over the changes in detail first. I'm almost at a point were I can do that.

WadeBarnes avatar Oct 25 '22 13:10 WadeBarnes

Thanks for the feedback @WadeBarnes! Working through them now.

dbluhm avatar Dec 02 '22 16:12 dbluhm

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.

dbluhm avatar Dec 02 '22 16:12 dbluhm

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Dec 02 '22 20:12 sonarcloud[bot]

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Dec 20 '22 02:12 sonarcloud[bot]

@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 avatar Dec 20 '22 02:12 dbluhm

@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.

WadeBarnes avatar Dec 20 '22 17:12 WadeBarnes

@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.

WadeBarnes avatar Dec 20 '22 18:12 WadeBarnes

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.

dbluhm avatar Dec 22 '22 21:12 dbluhm