Skip to content

Conversation

@martadams89
Copy link
Contributor

This PR adds arm64 builds back in.

I forked the repo and tested it against my own and appeared to run as expected.

@martadams89
Copy link
Contributor Author

@acasajus @cquintana92 @nguyenkims can someone review this? It resolves #1566 (comment)

@acasajus
Copy link
Collaborator

acasajus commented Nov 8, 2024

We disabled the arm builds because there were some dependencies that were making the tests really flaky and that failed the production deployments. As soon as we fix those dependencies we will enable arm back.

@ctrl-i
Copy link

ctrl-i commented Nov 23, 2024

We disabled the arm builds because there were some dependencies that were making the tests really flaky and that failed the production deployments. As soon as we fix those dependencies we will enable arm back.

Any progress on this? If you told us which dependencies were making which test flaky we might be able to help out. I haven't noticed any flakiness in my testing.

@martadams89
Copy link
Contributor Author

I've just sync'd my fork to the latest code, updated the workflow to mirror the latest in main and everything seemed fine - https://github.com/martadams89/sl-app/actions/runs/12846003655

Here's the latest arm64 image - https://registry.hub.docker.com/r/martadams89/sl-app

@ctrl-i
Copy link

ctrl-i commented Jan 19, 2025

I've just sync'd my fork to the latest code, updated the workflow to mirror the latest in main and everything seemed fine - https://github.com/martadams89/sl-app/actions/runs/12846003655

Here's the latest arm64 image - https://registry.hub.docker.com/r/martadams89/sl-app

@martadams89 Unfortunately this latest build does not seem to work for me. The database migration service fails with errors in the log of:

exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error

If I disable the migration service, simplelogin-init fails with a single error in the log of:

exec /code/.venv/bin/python: exec format error

I notice that the file sizes of both the AMD and ARM builds are the same on your docker hub which might mean they have not built correctly?

@martadams89
Copy link
Contributor Author

I've just sync'd my fork to the latest code, updated the workflow to mirror the latest in main and everything seemed fine - https://github.com/martadams89/sl-app/actions/runs/12846003655
Here's the latest arm64 image - https://registry.hub.docker.com/r/martadams89/sl-app

@martadams89 Unfortunately this latest build does not seem to work for me. The database migration service fails with errors in the log of:

exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error
exec /code/.venv/bin/alembic: exec format error

If I disable the migration service, simplelogin-init fails with a single error in the log of:

exec /code/.venv/bin/python: exec format error

I notice that the file sizes of both the AMD and ARM builds are the same on your docker hub which might mean they have not built correctly?

Looks like something in the move to rye causes the arm64 images to fall over.

I've created a branch/tag of 4.62.0 where arm64 still seems compatible. https://hub.docker.com/repository/docker/martadams89/sl-app/tags/v4-62-0/sha256:74286061a6eaca8163a6c5de2ae49686971ee52f5d05514989bea9d2357f4d99

@ctrl-i
Copy link

ctrl-i commented Jan 19, 2025

Looks like something in the move to rye causes the arm64 images to fall over.

I've created a branch/tag of 4.62.0 where arm64 still seems compatible.

@martadams89 I have just pulled your 4.62.0 branch and can confirm everything works as normal again on that branch. Hopefully you can resolve the rye dependency.

@martadams89
Copy link
Contributor Author

@ctrl-i I found that amd64 was hardcoded in the Dockerfile and the build workflow was using the rye action with a hardcoded checksum.

I've adjusted the Dockerfile to be image agnostic, and to download the correct version of rye based upon the TARGETARCH. The rye checksum is now validated dependant on the architecture of the build server. The build workflow removes the static checksum entry on the rye action, as I don't think this can be called dynamically based upon the architecture of the build server.

my master image https://hub.docker.com/repository/docker/martadams89/sl-app/tags has been updated to this version.

I'd appreciate If @acasajus or another maintainer could review and merge these changes back into the simple-login:master branch.

@ctrl-i
Copy link

ctrl-i commented Jan 19, 2025

@martadams89 I tried the new build but I am still getting the 'exec format error' and had to revert again to your 4.62.0 build.

The new builds are 300Mb smaller than the old ones... are all the dependencies getting included?

@martadams89
Copy link
Contributor Author

@ctrl-i I've just double checked here and I'm using my master arm64 image - https://hub.docker.com/layers/martadams89/sl-app/master/images/sha256-bc87c0bbd02c5d915b04af1877a695f73bf5a10b499bcc4f4047c6fc7668024a

you may need to pin to the sha or delete your containers and prune the image.

It looks like the upstream images are smaller, I assume that the switch to rye resulted in smaller image sizes.

@ctrl-i
Copy link

ctrl-i commented Jan 20, 2025

you may need to pin to the sha or delete your containers and prune the image.

@martadams89 You were right, a quick prune this morning got things moving :)

@acasajus @cquintana92 @nguyenkims Can we get this merged now? It has been several months now and @martadams89 has shown there is no real reason to not add the arm64 builds back in.

@martadams89
Copy link
Contributor Author

martadams89 commented Jan 24, 2025

@ctrl-i looks like upstream moved from rye to uv - I've made the necessary changes in the Dockerfile and main workflow to incorporate these changes for arm64 builds.

@acasajus @cquintana92 @nguyenkims can this be merged back?

Edit: still working on making these builds complete, will update this comment once confimed.

@acasajus
Copy link
Collaborator

Hey, thanks for creating the PR. can you fix the conflict? I'll merge afterwards.

…e will use simplelogin/app-ci-arm64


Image name is dicated by variable at top of workflow
@martadams89
Copy link
Contributor Author

I've split the workflows out so that there is a seperate one for arm64, which is allowed to fail.

This will then create a seperate build job, and create an image called simplelogin/app-ci-arm64

This means there's a seperate image that gets created for arm64 builds. I think this is the only way to run it from a seperate workflow as, as far as I know, the tags will fight and overwrite each other if they both have the name simplelogin/app-ci

Would this be valid for merging?

This is my arm64 image built by the same workflow https://registry.hub.docker.com/r/martadams89/sl-app-arm64

This is a failed build - https://github.com/martadams89/sl-app/actions/runs/13070392809 and a sucessful build https://github.com/martadams89/sl-app/actions/runs/13070436242

@quietsy
Copy link

quietsy commented Feb 3, 2025

After bashing my head on the arm build inconsistency issue, I finally found the cause: actions/runner-images#11471
You can fix it by changing runs-on: ubuntu-latest to runs-on: ubuntu-22.04 in the workflows.

@martadams89
Copy link
Contributor Author

thanks @quietsy for figuring it out.

@acasajus I've simplifed the workflow so that it does the same steps as present, just with including linux/arm64

I tested it here and it built fine.

@martadams89
Copy link
Contributor Author

@acasajus any movement here? I'm now repeatedly successfully building arm64 images

@martadams89
Copy link
Contributor Author

@acasajus @cquintana92 @nguyenkims

Any updates here, can this get merged now?

@ctrl-i
Copy link

ctrl-i commented Feb 26, 2025

@martadams89 Are the arm64 builds still being successfully built? Looking at your docker hub the last amd64 build was 3 days ago (at time of writing this) and the last arm64 24 days ago. The last build with both arm64 and amd64 builds available was over a month ago (v4.62.0).

@martadams89
Copy link
Contributor Author

martadams89 commented Feb 27, 2025

@ctrl-i I had to update my GitHub actions file to match the recent changes upstream, along with adjusting the Dockerfile (it would't build for some reason).

I took inspiration from @quietsy's Dockerfile on the LSIO repo - https://github.com/linuxserver-labs/docker-simplelogin - but it looks like it's building correctly now.

I've merged them changes back into this branch so it should be good to go.

@ctrl-i
Copy link

ctrl-i commented Feb 27, 2025

I've merged them changes back into this branch so it should be good to go.

I can confirm it is building properly and have installed it successfully.

@quietsy
Copy link

quietsy commented Mar 2, 2025

Great job @martadams89 , hope it'll get merged soon.

@martadams89
Copy link
Contributor Author

Any further updates here? @acasajus @cquintana92 @nguyenkims

@BJReplay
Copy link

Great job @martadams89 - I've just found my builds on arm are failing, and found your PR and docker images, and I'm back up to date.

@luisfalvarez-dev
Copy link

Hello SimpleLogin team, and thank you for maintaining such a vital open-source project!

I'm writing to express my strong interest in the return of official ARM/ARM64 Docker builds. My self-hosted instance runs on an energy-efficient ARM server, and the previous multi-arch support was perfect for this kind of setup.

I noticed in the community discussions that user @martadams89 has successfully managed to build their own updated ARM64 images, which is amazing. This shows that it's technically feasible to support this architecture with the latest versions.

Having this support in the official image would be a game-changer for a large part of the self-hosting community that relies on ARM devices.

Is there anything we, the community, can do to help get this officially re-implemented? Would the team be open to reviewing the work from @martadams89 or providing guidance on what would be needed to merge this capability? I'd be more than happy to help test any experimental builds.

Thank you!

@martadams89
Copy link
Contributor Author

@luisfalvarez-dev I've tried to get this merged a few times but it seems there's little interest.

FWIW I build arm64 images on my fork as soon as new updates are released here. I'd love for it to be officially supported, but until then feel free to use my auto-updating fork.

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.

6 participants