ci(docker): build each platform in a different worker #32

michaelfig · 2023-11-29T16:39:07Z

This is an optimisation to reduce the amount of disk space and build time needed on each worker. It currently uses a self-hosted runner for creating ARM images, falling back to standard ubuntu-latest GitHub runners.

michaelfig · 2023-11-30T02:07:28Z

Testing and publishing just for linux/amd64:

michaelfig · 2023-12-03T15:02:26Z

... and we're publishing again! A self-hosted ARM64 worker takes less time than the ubuntu-latest worker:

michaelfig · 2023-12-03T16:34:25Z

need to change jobs required by branch protection from test-proposals to test-proposals (linux/amd64).

michaelfig · 2023-12-05T17:58:54Z

This is R4R, having split out the misc cleanups I made into another PR, #39.

mhofman

I think the failure notification got dropped, probably mistakenly?

Can we add a manual workflow trigger, and test it through that to make sure generating and publishing all platform images works as expected? (I expect that we'll need to add the manual workflow trigger to the main branch first)

Edit: sorry for the delayed review. For some reason I'm not getting notifications for PR reviews in this repo

mhofman · 2023-12-11T14:50:04Z

.github/workflows/ci.yml

+          DEFAULT_PLATFORM=linux/amd64
+          echo "default=$DEFAULT_PLATFORM" >> $GITHUB_OUTPUT
+          if ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }}; then
+            platforms='["'"$DEFAULT_PLATFORM"'"]'


It took me a minute to parse the quoting here.

Added a comment.

mhofman · 2023-12-11T14:53:01Z

.github/workflows/ci.yml

+      - name: Set up QEMU for cross-platform builds
+        uses: docker/setup-qemu-action@v3
+        if: ${{ matrix.platform != env.DEFAULT_PLATFORM }}


QEMU is still needed when running arm on macos-latest-xlarge ?

I made the setup-qemu unconditional so that we can easily modify the runner configuration without needing to change things in multiple places.

mhofman · 2024-01-05T16:26:51Z

.github/workflows/ci.yml

+          sep=
+          SUFFIXED=
+          uarch=$(echo "${{ matrix.platform }}" | tr / _)
+          for TAG in ${{ steps.meta.outputs.tags }}; do
+            SUFFIXED="$SUFFIXED$sep$TAG-$uarch"
+            if test -z "$sep"; then
+              sep=,
+              echo "tag=$TAG-$uarch" >> $GITHUB_OUTPUT
+            fi
+          done
+          echo "tags=$SUFFIXED" >> $GITHUB_OUTPUT


This is pretty opaque too.

mhofman · 2024-01-05T16:28:33Z

.github/workflows/ci.yml

+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      packages: write


Why is the explicit permission required here?

The permissions are a restriction, not a grant.

mhofman · 2024-01-05T16:29:51Z

.github/workflows/ci.yml

-      - name: notify on failure
-        if: failure() && github.event_name != 'pull_request'
-        uses: ./.github/actions/notify-status


This seems to be completely gone, any reason to remove master failure notifications ?

The notification will fail due to missing GitHub secrets, but I'll leave it here just so we can correct the secrets to make it work again.

mhofman · 2024-01-10T17:18:36Z

.github/workflows/ci.yml

-      # but it's re-building the last stage. This is deemed good enough for now.
-      # see https://github.com/moby/moby/issues/34715
-      - name: Build and push complete image
+      - name: Compute tags


Nit, can we move "compute tags" earlier in the job / before the build step so we can easily check the tags before the job completes?

.github/workflows/ci.yml

michaelfig · 2024-01-10T20:54:07Z

Can we add a manual workflow trigger, and test it through that to make sure generating and publishing all platform images works as expected? (I expect that we'll need to add the manual workflow trigger to the main branch first)

https://github.com/Agoric/agoric-3-proposals/actions shows both the manual and PR triggers working correctly.

michaelfig · 2024-01-10T20:57:54Z

both the manual and PR triggers

That is, the manual and PR runs.

turadg · 2024-01-10T23:21:24Z

.github/workflows/ci.yml

+    strategy:
+      matrix:
+        platform: ${{ fromJSON(needs.platforms.outputs.platforms) }}
+    # Run on our own self-hosted ARM64 machine if the platform is ARMish.


where is operation of this machine documented?

mhofman

Seeing the manual run results, I'm confident this does what it claims it does!

michaelfig self-assigned this Nov 29, 2023

michaelfig force-pushed the mfig-split-docker branch 6 times, most recently from ee20006 to cbb273a Compare November 30, 2023 00:34

michaelfig force-pushed the mfig-split-docker branch 4 times, most recently from 84ea843 to 79968d1 Compare November 30, 2023 18:39

michaelfig changed the title ~~ci(docker): run each platform on a different worker~~ ci(docker): build each platform in a different worker Nov 30, 2023

michaelfig force-pushed the mfig-split-docker branch 2 times, most recently from 3b95574 to e819776 Compare November 30, 2023 21:05

turadg mentioned this pull request Dec 1, 2023

Revert "ci(docker): build multiplatform images on push to main or workflow dispatch" #37

Merged

michaelfig force-pushed the mfig-split-docker branch 13 times, most recently from 8bf18ca to f359b44 Compare December 3, 2023 04:33

michaelfig requested a review from mhofman December 3, 2023 16:32

michaelfig mentioned this pull request Dec 3, 2023

ci: add new platform matrix to be required #38

Merged

michaelfig changed the base branch from main to mfig-platform-matrix December 3, 2023 17:38

michaelfig force-pushed the mfig-split-docker branch from d265dab to d66da47 Compare December 3, 2023 17:40

Base automatically changed from mfig-platform-matrix to main December 3, 2023 21:54

michaelfig force-pushed the mfig-split-docker branch 3 times, most recently from 4c1cb64 to 2c2eeac Compare December 5, 2023 17:48

mhofman requested changes Jan 5, 2024

View reviewed changes

michaelfig added 3 commits January 10, 2024 10:30

ci(docker): run each platform on a different worker

299d6dc

ci(docker): use self-hosted arm64 builds

025a29e

fixup! ci(docker): run each platform on a different worker

453fd99

michaelfig force-pushed the mfig-split-docker branch from 2c2eeac to 453fd99 Compare January 10, 2024 17:00

mhofman reviewed Jan 10, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

michaelfig force-pushed the mfig-split-docker branch 2 times, most recently from 96c5f3e to f1a9c5f Compare January 10, 2024 17:43

fixup! ci(docker): run each platform on a different worker

cf9dfcb

michaelfig force-pushed the mfig-split-docker branch 3 times, most recently from 82fb01b to cf9dfcb Compare January 10, 2024 19:30

turadg reviewed Jan 10, 2024

View reviewed changes

turadg requested a review from mhofman January 11, 2024 18:54

mhofman approved these changes Jan 11, 2024

View reviewed changes

turadg merged commit c6caffa into main Jan 11, 2024
10 of 14 checks passed

turadg deleted the mfig-split-docker branch January 11, 2024 23:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(docker): build each platform in a different worker #32

ci(docker): build each platform in a different worker #32

michaelfig commented Nov 29, 2023 •

edited

Loading

michaelfig commented Nov 30, 2023

michaelfig commented Dec 3, 2023

michaelfig commented Dec 3, 2023 •

edited

Loading

michaelfig commented Dec 5, 2023

mhofman left a comment •

edited

Loading

mhofman Dec 11, 2023

michaelfig Jan 10, 2024

mhofman Dec 11, 2023

michaelfig Jan 10, 2024

mhofman Jan 5, 2024

mhofman Jan 5, 2024

michaelfig Jan 10, 2024

mhofman Jan 5, 2024

michaelfig Jan 10, 2024

mhofman Jan 10, 2024

michaelfig Jan 10, 2024

michaelfig commented Jan 10, 2024

michaelfig commented Jan 10, 2024

turadg Jan 10, 2024

mhofman left a comment

ci(docker): build each platform in a different worker #32

ci(docker): build each platform in a different worker #32

Conversation

michaelfig commented Nov 29, 2023 • edited Loading

michaelfig commented Nov 30, 2023

michaelfig commented Dec 3, 2023

michaelfig commented Dec 3, 2023 • edited Loading

michaelfig commented Dec 5, 2023

mhofman left a comment • edited Loading

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

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

michaelfig commented Jan 10, 2024

michaelfig commented Jan 10, 2024

Choose a reason for hiding this comment

mhofman left a comment

Choose a reason for hiding this comment

michaelfig commented Nov 29, 2023 •

edited

Loading

michaelfig commented Dec 3, 2023 •

edited

Loading

mhofman left a comment •

edited

Loading