Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build and push multiple (ARM/ARM64) architecture images on Docker Hub #73

Merged
merged 5 commits into from
Dec 21, 2024

Conversation

CpuID
Copy link
Contributor

@CpuID CpuID commented Dec 21, 2024

This will make running Open Dynamic Export on smaller/embedded hardware easier, eg. Home Assistant Green/Blue/Yellow/Raspberry Pi + Apple Silicon

Originally I was going with linux/arm64 only as an addition, but after some reading you also need specific arm/arm64 variants for some of the more powerful ARM devices out there. This may need extras added over time, but this change should cover the bulk of the required options.

https://en.wikipedia.org/wiki/Raspberry_Pi#Specifications shows most from a Pi2B upwards will work. Fairly sure the ODROID devices are arm64/v8, I'll confirm next week :)

Confirmed on MacOS it wants to use linux/arm64/v8 with an Apple M1 Pro

@longzheng
Copy link
Owner

I don't have any experience with this but looks good.

It's a bit hard fo rme to manually trigger the workflow on a remote branch so I'll just copy your branch first

@longzheng
Copy link
Owner

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

The test pipeline takes a fair bit longer than the original one, there's no parallelisation in there right now.

If it's a problem there are ways to do it (separate steps/jobs per arch), but considering you only build/push for new tags, is it an issue currently...? If you're happy to just let it take the extra minutes in serial for now, that's fine :)

@longzheng
Copy link
Owner

I don't mind it taking a while, it's not a rush, however it seems to be stuck/frozen on this

#18 [linux/arm/v7 build 3/5] RUN --mount=type=bind,source=package.json,target=package.json     --mount=type=bind,source=package-lock.json,target=package-lock.json     --mount=type=cache,target=/root/.npm     npm ci

I cancelled the previous run after 15min https://github.com/longzheng/open-dynamic-export/actions/runs/12442729443/job/34741150224 just in case it was a once-off problem, but it seems to be stuck on the same platform in the new run https://github.com/longzheng/open-dynamic-export/actions/runs/12442729443/job/34741409521#step:6:402

@longzheng
Copy link
Owner

Looks like it may be this issue docker/build-push-action#1071

However instead of downgrading the Node image I wonder if changing the GitHub Action runner image will also work

@@ -43,6 +43,7 @@ jobs:
with:
context: .
push: true
platforms: linux/amd64,linux/arm64,linux/arm/v7,linux/arm64/v8
Copy link
Owner

@longzheng longzheng Dec 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw here https://github.com/docker/build-push-action/blob/master/.github/build-push-summary.png that this also support a list format, which might be easier to understand

Suggested change
platforms: linux/amd64,linux/arm64,linux/arm/v7,linux/arm64/v8
platforms:
- linux/amd64
- linux/arm64
- linux/arm/v7
- linux/arm64/v8

@longzheng
Copy link
Owner

Ah damn the macOS runner doesn't contain Docker https://github.com/actions/runner-images/blob/main/images/macos/macos-14-Readme.md

@longzheng longzheng force-pushed the CpuID-arm64-docker-images branch from 770a39e to f080190 Compare December 21, 2024 06:42
@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Interesting... I wonder if we should skip arm/v7 temporarily and get the other 2 in there at least? Then try figure out a solution for arm/v7 as a separate PR?

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

I agree downgrading Node is less than ideal...

@longzheng
Copy link
Owner

Yeah maybe we revisit linux/arm/v7, it sounds like there's some efforts/attempts to get this problem fixed upstream but I'm having a hard time following what exactly is the root cause and how far away the fix is.

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Yep seems reasonable 👍 did you want to kick off another test with 6694b8d ?

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Looking at https://en.wikipedia.org/wiki/Raspberry_Pi#Specifications it's only the Pi2B that's impacted by not having arm/v7

@longzheng
Copy link
Owner

Yep seems reasonable 👍 did you want to kick off another test with 6694b8d ?

Hmmm now it's complaining about the new syntax https://github.com/longzheng/open-dynamic-export/actions/runs/12443121813/workflow

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Ah annoying lol, I'll adjust :)

@longzheng
Copy link
Owner

Oh wait the list syntax is not an array https://github.com/docker/build-push-action/tree/master?tab=readme-ov-file#inputs

List type is a newline-delimited string

cache-from: |
  user/app:cache
  type=local,src=path/to/dir

So I think it should be

        with:
          context: .
          push: true
          # TODO: find a workaround for https://github.com/docker/build-push-action/issues/1071 without downgrading NodeJS
          # linux/arm/v7
          platforms: |
            linux/amd64
            linux/arm64
            linux/arm64/v8

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Most other projects out there tend to use the CSV syntax by the looks of it... https://github.com/search?q=%22platforms%3A%22+path%3A.github%2Fworkflows+language%3AYAML&type=code&l=YAML shall we just stick with it for consistency?

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Give a1c0dff a try

@longzheng
Copy link
Owner

Sure sounds good, agree the list syntax is kind of unexpected

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

I double checked, looks like Home Assistant Green and Blue are both arm64/v8 hardware, and Apple Silicon is arm64/v8 also. Home Assistant Yellow is a Pi4 which is the same again.

@longzheng
Copy link
Owner

It completed successfully but had a warning https://github.com/longzheng/open-dynamic-export/actions/runs/12443199747/job/34742129818#step:6:443

 1 warning found (use docker --debug to expand):
 - Duplicate platform result requested "linux/arm64"

@longzheng
Copy link
Owner

According to this arm64/v8 is the same as arm64 https://stackoverflow.com/a/70889505

@longzheng longzheng merged commit f202280 into longzheng:main Dec 21, 2024
2 checks passed
@CpuID CpuID deleted the CpuID-arm64-docker-images branch December 21, 2024 08:02
@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

aha interesting, thx

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

@CpuID
Copy link
Contributor Author

CpuID commented Dec 21, 2024

Can confirm it runs fine using Orbstack on MacOS/Apple Silicon now 👍 handy for tuning configs locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants