ci(framework:skip) Add e2e test to test Docker images #4218
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| name: "format-docker-build-args" | ||
| description: "Format the build args to work with the wretry.action" | ||
| inputs: | ||
| build-args: | ||
| description: "List of build-time variables." | ||
| outputs: | ||
| build-args: | ||
| description: "Build args formatted to work with the wretry.action" | ||
| value: ${{ steps.build-args.outputs.build-args }} | ||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - id: build-args | ||
| shell: python | ||
| run: | | ||
| import os | ||
|
|
||
| # Adds two spaces to the line breaks to ensure proper indentation | ||
| # when passing the multi-line string to the wretry.action. | ||
| # Without it, the multi-line string is passed like this: | ||
| # | ||
| # build-args: | | ||
| # ARG1= | ||
| # ARG2= | ||
| # ARG3= | ||
| # | ||
| # This causes the Docker action to interpret ARG2 and ARG3 as keys instead | ||
| # of values of the multi-line string. | ||
| build_args = '''${{ inputs.build-args }}'''.replace("\n", "\n ") | ||
|
|
||
| with open(os.environ['GITHUB_OUTPUT'], 'a') as fh: | ||
| print("build-args<<EOF", file=fh) | ||
| print(build_args, file=fh) | ||
| print("EOF", file=fh) |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,104 @@ | ||||||||
| name: Reusable docker image test workflow | ||||||||
|
|
||||||||
| on: | ||||||||
| workflow_call: | ||||||||
| inputs: | ||||||||
| build-args: | ||||||||
| description: "List of build-time variables." | ||||||||
| required: false | ||||||||
| type: string | ||||||||
| ref: | ||||||||
| required: false | ||||||||
| type: string | ||||||||
| secrets: | ||||||||
| dockerhub-user: | ||||||||
| required: true | ||||||||
| dockerhub-token: | ||||||||
| required: true | ||||||||
|
|
||||||||
| permissions: | ||||||||
| contents: read | ||||||||
|
|
||||||||
| jobs: | ||||||||
| test: | ||||||||
| name: Test image | ||||||||
| runs-on: "ubuntu-22.04" | ||||||||
| timeout-minutes: 10 | ||||||||
| steps: | ||||||||
| - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 | ||||||||
|
There was a problem hiding this comment. What's the benefit of using a hash instead of the version (as we've previously used in other CI steps)? Is it so that we can use a minor+patched version? There was a problem hiding this comment. I also see this repeated in other steps like for |
||||||||
| with: | ||||||||
| ref: ${{ inputs.ref }} | ||||||||
|
|
||||||||
| - name: Format build args | ||||||||
| uses: ./.github/actions/format-docker-build-args | ||||||||
| id: build-args | ||||||||
| with: | ||||||||
| build-args: ${{ inputs.build-args }} | ||||||||
|
|
||||||||
| - id: matrix | ||||||||
|
There was a problem hiding this comment. Let's describe this step:
Suggested change
There was a problem hiding this comment. It looks like we actually don't use a matrix strategy here. Is this planned for another PR? There was a problem hiding this comment. You're right, we're not directly using the matrix strategy, but instead we generate the matrix using the |
||||||||
| run: | | ||||||||
| python dev/build-docker-image-matrix.py --flwr-version e2e-test --simple > matrix.json | ||||||||
| echo "matrix=$(cat matrix.json)" >> $GITHUB_OUTPUT | ||||||||
| echo "base-file-dir=$(jq -r .base.images[0].file_dir matrix.json)" >> $GITHUB_OUTPUT | ||||||||
| echo "base-namespace-repository=$(jq -r .base.images[0].namespace_repository matrix.json)" >> $GITHUB_OUTPUT | ||||||||
|
|
||||||||
| - name: Set up Docker Buildx | ||||||||
| uses: docker/setup-buildx-action@988b5a0280414f521da01fcc63a27aeeb4b104db # v3.6.1 | ||||||||
| with: | ||||||||
| # https://github.com/docker/build-push-action/issues/1116#issuecomment-2132899905 | ||||||||
| driver: docker | ||||||||
|
|
||||||||
| - name: Build base image | ||||||||
| uses: Wandalen/wretry.action@6feedb7dedadeb826de0f45ff482b53b379a7844 # v3.5.0 | ||||||||
| with: | ||||||||
| action: docker/build-push-action@5cd11c3a4ced054e52742c5fd54dca954e0edd85 # v6.7.0 | ||||||||
| attempt_limit: 60 # 60 attempts * (9 secs delay + 1 sec retry) = ~10 mins | ||||||||
| attempt_delay: 9000 # 9 secs | ||||||||
| with: | | ||||||||
| pull: true | ||||||||
| context: ${{ steps.matrix.outputs.base-file-dir }} | ||||||||
| load: true | ||||||||
| tags: ${{ steps.matrix.outputs.base-namespace-repository }}:e2e-test | ||||||||
| build-args: | | ||||||||
| ${{ steps.build-args.outputs.build-args }} | ||||||||
|
|
||||||||
| - name: Build binary images | ||||||||
chongshenng marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| run: | | ||||||||
| set -e -o pipefail | ||||||||
|
|
||||||||
| jq -c '.binary.images[]' <<< '${{ steps.matrix.outputs.matrix }}' | while read -r i; do | ||||||||
| FILE_DIR=$(echo "$i" | jq -r .file_dir) | ||||||||
| NAMESPACE_REPOSITORY=$(echo "$i" | jq -r .namespace_repository) | ||||||||
| docker build --build-arg BASE_IMAGE="e2e-test" -t "${NAMESPACE_REPOSITORY}:e2e-test" "${FILE_DIR}" | ||||||||
| done | ||||||||
|
|
||||||||
| - name: Bootstrap | ||||||||
| uses: ./.github/actions/bootstrap | ||||||||
| with: | ||||||||
| python-version: 3.9 # Explicitly set this so that we can add a matrix later | ||||||||
| poetry-skip: 'true' | ||||||||
| - name: Install Flower | ||||||||
| run: python -m pip install . | ||||||||
chongshenng marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
|
||||||||
| - name: Test images | ||||||||
| env: | ||||||||
| FLWR_VERSION: e2e-test | ||||||||
chongshenng marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| PROJECT_DIR: e2e-test | ||||||||
| working-directory: src/docker/complete | ||||||||
| run: | | ||||||||
| flwr new ${PROJECT_DIR} --framework NumPy --username flwrlabs | ||||||||
|
|
||||||||
| docker compose -f certs.yml up --build | ||||||||
| sudo chown -R 49999:49999 superexec-certificates/* superlink-certificates/* | ||||||||
|
|
||||||||
| cat <<- EOT >> "${PROJECT_DIR}/pyproject.toml" | ||||||||
| [tool.flwr.federations.local-deployment-tls] | ||||||||
| address = "127.0.0.1:9093" | ||||||||
| root-certificates = "../superexec-certificates/ca.crt" | ||||||||
| EOT | ||||||||
|
Comment on lines
+91
to
+98
There was a problem hiding this comment. Will we include a matrix of tests for secure/insecure connections and client authentication later? There was a problem hiding this comment. Yes, we can include those tests when we in the |
||||||||
|
|
||||||||
| docker compose -f compose.yml -f with-tls.yml up --build -d | ||||||||
| docker compose logs -f & | ||||||||
|
|
||||||||
| timeout 30 flwr run ${PROJECT_DIR} local-deployment-tls | ||||||||
| timeout 120 docker compose logs -f superexec | grep -q 'Run finished' | ||||||||
|
Comment on lines
+103
to
+104
There was a problem hiding this comment. I like that this test is very straightforward. Is it still worth for us to run it with There was a problem hiding this comment. That's a good question. With There was a problem hiding this comment. Ok, does it makes sense to do this in 2 steps?
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||||||||||||||||
| name: Test Docker | ||||||||||||||||||||
|
|
||||||||||||||||||||
| on: | ||||||||||||||||||||
| push: | ||||||||||||||||||||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. I think this is quite resource intensive. Maybe we can limit the workflow to
Suggested change
There was a problem hiding this comment. It will be removed. This workflow is just used to run the Docker e2e test workflow There was a problem hiding this comment. Maybe I'm missing something, can the workflow be triggered if we don't specify |
||||||||||||||||||||
|
|
||||||||||||||||||||
| jobs: | ||||||||||||||||||||
| # parameters: | ||||||||||||||||||||
| # if: github.repository == 'adap/flower' | ||||||||||||||||||||
| # name: Collect docker build parameters | ||||||||||||||||||||
| # runs-on: ubuntu-22.04 | ||||||||||||||||||||
| # timeout-minutes: 10 | ||||||||||||||||||||
| # outputs: | ||||||||||||||||||||
| # pip-version: ${{ steps.versions.outputs.pip-version }} | ||||||||||||||||||||
| # setuptools-version: ${{ steps.versions.outputs.setuptools-version }} | ||||||||||||||||||||
| # flwr-version-ref: ${{ steps.versions.outputs.flwr-version-ref }} | ||||||||||||||||||||
| # matrix: ${{ steps.versions.outputs.matrix }} | ||||||||||||||||||||
| # steps: | ||||||||||||||||||||
| # - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # - uses: ./.github/actions/bootstrap | ||||||||||||||||||||
| # id: bootstrap | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # - id: versions | ||||||||||||||||||||
| # run: | | ||||||||||||||||||||
| # echo "pip-version=${{ steps.bootstrap.outputs.pip-version }}" >> "$GITHUB_OUTPUT" | ||||||||||||||||||||
| # echo "setuptools-version=${{ steps.bootstrap.outputs.setuptools-version }}" >> "$GITHUB_OUTPUT" | ||||||||||||||||||||
| # echo "flwr-version-ref=git+${{ github.server_url }}/${{ github.repository }}.git@${{ github.sha }}" >> "$GITHUB_OUTPUT" | ||||||||||||||||||||
| # python dev/build-docker-image-matrix.py --flwr-version unstable --simple > matrix.json | ||||||||||||||||||||
| # echo "matrix=$(cat matrix.json)" >> $GITHUB_OUTPUT | ||||||||||||||||||||
|
Comment on lines
+7
to
+29
There was a problem hiding this comment. Should these lines be uncommented once the parameters are fully defined? There was a problem hiding this comment. It will be removed. This workflow is just used to run the Docker e2e test workflow There was a problem hiding this comment. I think I've the same response as above. |
||||||||||||||||||||
|
|
||||||||||||||||||||
| build-docker-base-images: | ||||||||||||||||||||
| name: Build base images | ||||||||||||||||||||
| if: github.repository == 'adap/flower' | ||||||||||||||||||||
| uses: ./.github/workflows/_docker-test.yml | ||||||||||||||||||||
| # needs: parameters | ||||||||||||||||||||
| with: | ||||||||||||||||||||
| build-args: | | ||||||||||||||||||||
| PIP_VERSION=24.1 | ||||||||||||||||||||
| SETUPTOOLS_VERSION=70 | ||||||||||||||||||||
| FLWR_VERSION=1.11.0 | ||||||||||||||||||||
| secrets: | ||||||||||||||||||||
| dockerhub-user: ${{ secrets.DOCKERHUB_USERNAME }} | ||||||||||||||||||||
| dockerhub-token: ${{ secrets.DOCKERHUB_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.