Build arm containers with new github actions arm runner (PP-2139) #2280

jonathangreen · 2025-02-10T20:14:50Z

Description

Build our images using the github ARM runners.

This makes several changes:

Actually run out unit tests on the ARM image (I think this is a bit win, it was too slow with emulation, but since native libraries can cause issues, its nice to verify that our tests actually run in the built arm image).
Technically our images get pushed every commit now, but they are not tagged until the tests pass.
- This lets us use the images in other workflow jobs easily, while making it unlikely anyone will come across a broken image.
- It has the advantage that if desired, you could pull the image via its hash for debugging.
The build takes place in a (pretty ugly IMO) two step process, where native images are built for each platform, then they are combined together into a manifest and tagged.
- This is the process that that docker documentation recommends for a multipart build like this, so despite being kind of ugly, it is the officially blessed way to do things.

Motivation and Context

Github now has native arm runners:
community/community#148648

This allows us to build our ARM images faster, and without relying on emulation which was causing issues (see: actions/runner-images#11471).

This work is mainly based on this documentation from docker:
https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners

But adapted to our environment / workflow.

How Has This Been Tested?

I did some testing of these workflows on my fork
Workflows running in CI on this PR

Checklist

I have updated the documentation accordingly.
All new and existing tests passed.

codecov · 2025-02-10T20:31:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.12%. Comparing base (c88b13b) to head (b76b858).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2280   +/-   ##
=======================================
  Coverage   91.12%   91.12%           
=======================================
  Files         363      363           
  Lines       41327    41327           
  Branches     8846     8846           
=======================================
  Hits        37660    37660           
  Misses       2405     2405           
  Partials     1262     1262

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jonathangreen · 2025-02-10T20:16:57Z

.github/actions/poetry/action.yml

@@ -51,7 +51,7 @@ runs:
      id: cache
      with:
        path: ${{ steps.poetry-dir.outputs.home }}
-        key: ${{ runner.os }}-poetry${{ inputs.version }}-install-py${{ steps.python-version.outputs.version }}
+        key: ${{ runner.os }}-${{ runner.arch }}-poetry${{ inputs.version }}-install-py${{ steps.python-version.outputs.version }}


This could be broken out to a separate PR if we want. Our action to install poetry didn't take into account the runners architecture before, so it tried to use the same cache for both intel and arm, causing poetry installs to fail on whichever platform wasn't in the cache.

This adds the platform to the cache key, so it will work with new arm based runners.

jonathangreen · 2025-02-10T20:18:15Z

.github/workflows/build-base-image.yml

+      - main
+    paths:
+      - .github/workflows/build-base-image.yml
+      - docker/Dockerfile.baseimage


We now update the base image on pushes to main that modify the base-image workflow or dockerfile. Previously this was done as part of build.yml, but based on the new structure, pushing this within its own workflow made more sense to me.

jonathangreen · 2025-02-10T20:25:01Z

.github/workflows/build.yml

    needs: [build]
    permissions:
      contents: read
    strategy:
      fail-fast: false
      matrix:
-        platform: ["linux/amd64", "linux/arm64"]
-        image: ["scripts", "webapp"]


The majority of the time in this workflow is spent pulling the image, rather then running the tests, so it made sense to combine the job for scripts and webapp, since running the tests themselves is quick.

jonathangreen · 2025-02-10T20:35:24Z

docker/Dockerfile.ci

@@ -1,4 +1,4 @@
-FROM opensearchproject/opensearch:1 as opensearch
+FROM opensearchproject/opensearch:1 AS opensearch


This isn't strictly necessary, but resolves a build warning about mixed cases in statements

Yeah, it's funny how persnickety Docker is about this. 😂

jonathangreen · 2025-02-10T20:36:24Z

docker/ci/test_migrations.yml

@@ -1,5 +1,3 @@
-version: "3.9"
-


Again not strictly necessary, just resolves a build warning about version being deprecated

jonathangreen · 2025-02-10T20:37:37Z

docker/ci/test_scripts.sh

 # Wait for container to start
 wait_for_runit "$container"

 # Make sure database initialization completed successfully
-timeout 240s grep -q 'Initialization complete' <(docker compose logs "$container" -f 2>&1)
+timeout 240s grep -q -e 'Initialization complete' -e "Migrations complete" <(docker compose logs "$container" -f 2>&1)


I needed to change this, because there is a race condition now that we test both webapp and scripts in the same container, previously we would have always hit initialization, now the first container to start does it.

jonathangreen · 2025-02-10T20:41:46Z

tests/manager/api/util/test_xray.py

-from unittest.mock import MagicMock, call
+from unittest.mock import MagicMock, call, patch
+
+import pytest


These test changes could be broken out to separate PR. They are necessary because we now run the tests against a container that has a version set. The tests previously assumed this was not the cause, causing some of these tests to fail.

The changes just mock the __version__ variable so it doesn't matter if its set or not, the tests will always behave correctly.

tdilauro

This looks good! 🎸

Try running with new arm based runner

9173a3c

jonathangreen requested a review from a team February 10, 2025 20:14

jonathangreen added 2 commits February 10, 2025 16:23

Fix up some comments

5457906

Cleanup more comments

de23853

jonathangreen added 2 commits February 10, 2025 16:34

roll back docker-compose changes

a7b47bf

Update another comment

a110c6c

jonathangreen commented Feb 10, 2025

View reviewed changes

Use ubuntu 24.04 image everywhere

b76b858

jonathangreen mentioned this pull request Feb 11, 2025

Build arm containers with new github actions arm runner (PP-2139) ThePalaceProject/library-registry#765

Merged

2 tasks

tdilauro approved these changes Feb 11, 2025

View reviewed changes

jonathangreen merged commit b8ba6cc into main Feb 11, 2025
19 checks passed

jonathangreen deleted the feature/workflow-arm-runner branch February 11, 2025 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build arm containers with new github actions arm runner (PP-2139) #2280

Build arm containers with new github actions arm runner (PP-2139) #2280

jonathangreen commented Feb 10, 2025

codecov bot commented Feb 10, 2025 •

edited

Loading

jonathangreen Feb 10, 2025

jonathangreen Feb 10, 2025

jonathangreen Feb 10, 2025

jonathangreen Feb 10, 2025

tdilauro Feb 11, 2025

jonathangreen Feb 10, 2025

jonathangreen Feb 10, 2025

jonathangreen Feb 10, 2025

tdilauro left a comment

		@@ -1,4 +1,4 @@
		FROM opensearchproject/opensearch:1 as opensearch
		FROM opensearchproject/opensearch:1 AS opensearch

Build arm containers with new github actions arm runner (PP-2139) #2280

Build arm containers with new github actions arm runner (PP-2139) #2280

Conversation

jonathangreen commented Feb 10, 2025

Description

Motivation and Context

How Has This Been Tested?

Checklist

codecov bot commented Feb 10, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tdilauro left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 10, 2025 •

edited

Loading