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

Server: Add arm64 support for Joplin Server #10882

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from

Conversation

maresmar
Copy link
Sponsor

Summary

This pull request introduces ARM64 container pipeline for Joplin Server and adds the necessary dependencies for ARM64. I have successfully tested and used the image created in my repository on my Raspberry Pi 5.

I have changed ServerDockerImage to build the container using Docker buildx (based on the buildServerDocker.js script) and I have changed Main to not been called for Joplin Server tags. I did all changes trying to maximize mimicking of the existing workflow.

Execution scenarios

Possible limitations

  • I didn't test the AMD64 container locally, but the existing tests are passing.
  • I was unable to test Docker Hub push and impact of skip-duplicate-actions
  • The buildServerDocker.js could be probably removed and run_ci.sh could be simplified, but I wasn't sure if you want to keep an local push script.
  • Docker tag is created from git tag, so to create 1.2.3-beta the git tag has to be server-v1.2.3-beta.
  • Building both ARM64 and AMD64 containers took 1 hour and 40 minutes in my repository.

Copy link
Contributor

github-actions bot commented Aug 16, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@maresmar
Copy link
Sponsor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Aug 16, 2024
@maresmar
Copy link
Sponsor Author

recheck

@laurent22
Copy link
Owner

  • The buildServerDocker.js could be probably removed and run_ci.sh could be simplified, but I wasn't sure if you want to keep an local push script.

Actually, we generally try to put as much as possible in the TypeScript files because it's easier to work with. It's difficult to run a GitHub Action locally while it's trivial to run a TypeScript file.

I'm struggling to review this PR because you changed so much code in the action. Is it no possible to update the TypeScript file to do what you want?

@maresmar
Copy link
Sponsor Author

I was afraid you would ask me for something like this 😢. To be honest, I only know that these GitHub actions are large pieces of JavaScript that set up QEMU and Docker BuildX to work together in the default GitHub runner. I'm not very confident that I could do it better without missing many edge cases.

I've been trying to change only the Docker-related parts, and most of the changes I've made are related to container image tag generation. What if I revert all the changes and just introduce a new job called ServerDockerImageArm64 that would only upload the Docker arm64 image to the GitHub package registry?

I could also try modifying buildServerDocker.js to use docker buildx, but setting up a local environment for multi-platform builds seems much more complex (see https://docs.docker.com/build/building/multi-platform/), and I have never tried it. I usually use a branch to experiment with CI/CD, so I've never tried it on localhost. I could probably experiment with some mixed solution, like using docker buildx build Dockerfile.server --platform linux/amd64,linux/arm64 in buildServerDocker.js and connecting it with these docker/setup-qemu-action@v3 actions, but I'm not currently sure what changes I need to make to get it running on localhost or GitHub.

@laurent22
Copy link
Owner

Thanks for clarifying. I have a preference for using TypeScript scripts for all this because it's generally easier and safer to write in that language, but if your GitHub Actions implementation works well, then why not.

Could you please confirm what we will get once that action is executed? Currently the script pushes four tags:

  • beta
  • 3-beta
  • 3.0-beta
  • 3.0.1-beta

(see here: https://hub.docker.com/r/joplin/server/tags )

Will we get the same once we run your script?

Also looking at the existing buildServerDocker.ts file, can you think of anything that may be missing in your new implementation?

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