search

Tecnativa / doodba

Base image for making the creation of customized Odoo environments a piece of cake
Apache License 2.0
423 stars 302 forks source link

Add support for arm64 build (tested on Mac M1) #573

Closed PCatinean closed 5 months ago

PCatinean commented 10 months ago

This PR introduces support for ARM builds and has been tested on Mac M1, a basic rundown on the changes:

1) Download resources from the same release location but use the targeted architecture 2) Update GEOIP_UPDATER_VERSION to the latest version as arm64 builds are also available 3) Force wkhtmltopdf version to 0.12.6 since it also has an arm64 build (fail build otherwise) 4) Automatically upgrade odoo requirements.txt for arm64 build and force gevent==21.12.0 and greenlet==1.1.0 (minimum versions required for a successful build otherwise cython will thrown an error (i.e rc/gevent/_gevent_cgreenlet.pxd:181:6: Variables cannot be declared with 'cpdef'. Use 'cdef' instead.)

ap-wtioit commented 10 months ago

this should fix / implements #567 and #342 for version 16.0

bosd commented 8 months ago

Thanks for this PR, Could you also make one for V14?

pedrobaeza commented 8 months ago

Please rebase and solve conflicts, and do the pre-commit fixes in the same commit that introduces the error.

PCatinean commented 8 months ago

PR rebased and I also added arm builds for Odoo version 14, 15 and 17. I have build tested all of them locally on Mac M1

pedrobaeza commented 8 months ago

Thanks for it. Seems OK. The only point I don't know right now is where to indicate that the pushed build is for both platforms. Do you know it?

PCatinean commented 8 months ago

I think we need to adjust the ./hooks/build script or use the available github actions but either way it seems docker buildx is required for multi-platform builds: https://docs.docker.com/build/ci/github-actions/multi-platform.

I will make another branch on my fork and link it to my docker hub then do some tests there and later propose those changes to this PR as well.

In the meantime people can use this PR to build the *-onbuild image locally.

bosd commented 8 months ago

Would like to functional test this, I have an arm server available. But I'm light on experience.

How can this be tested locally? :blush: :angel: I've tried copying the contents of the 14.0-dockerfile into my odoo/Dockerfile of the doodba-copier-template but produces an error.

``` > [base 8/10] COPY entrypoint.d common/entrypoint.d: ------ Dockerfile:90 -------------------- 88 | COPY build.d common/build.d 89 | COPY conf.d common/conf.d 90 | >>> COPY entrypoint.d common/entrypoint.d 91 | RUN mkdir -p auto/addons auto/geoip custom/src/private \ 92 | && ln /usr/local/bin/direxec common/entrypoint \ -------------------- ERROR: failed to solve: failed to compute cache key: failed to calculate checksum of ref c65c21a4-8130-4ed7-9f2c-00eafd782e20::tw9or30qgl9vfkb72ljrcqszc: "/entrypoint.d": not found ```
PCatinean commented 8 months ago

@bosd I am not sure what is your setup but here are a few steps on how to do this, if you need more details I think it's better to open a github discussion rather than continuing on this PR.

1) First you need to clone my branch locally git clone https://github.com/pledra/doodba.git -b arm-builds doodba-arm 2) Here you use either an ARM computer when building and just run something like docker build -f 14.0.Dockerfile . -t doodba:14.0-arm-onbuild or if you are not on ARM then you need to use docker buildx https://docs.docker.com/engine/reference/commandline/buildx/ and specify arm64 platform when building 3) Follow the steps at https://github.com/Tecnativa/doodba-copier-template to create a new template 4) After you have finished all the steps go into scaffolding/odoo/Dockerfile and replace FROM ghcr.io/tecnativa/doodba:${ODOO_VERSION}-onbuild with the above built image doodba:14.0-arm-onbuild 5) Now when you build the image locally from the scaffolding it should use the arm base image.

One last thing to note and this will be probably a PR on other repositories, for the development environment some support images are missing arm builds as well.

bosd commented 8 months ago

@PCatinean Thanks!! I managed to build it :tada: Had to mod the 14.0 and 15.0 dockerfiles to WKHTMLTOPDF_VERSION=0.12.6.1

One last thing to note and this will be probably a PR on other repositories, for the development environment some support images are missing arm builds as well.

Indeed on the dev env I got an error This error is because there is no image build for linux/arm64/v8 platform. on pgweb and the proxy. For the proxys whitelist there is a pr in: https://github.com/Tecnativa/docker-whitelist/pull/15 I'm a bit puzzled on the pgweb one, as pgweb supports arm,

PCatinean commented 8 months ago

@bosd happy to hear it worked. Yes I will have to take a look at the other images as well, for the wkhtmltopdf version it is a build argument so you can just do --build-arg WKTHTMLTOPDF_VERSION=version and it will use that no need to change the original file

bosd commented 8 months ago

Yeah, We're getting there to arm support.. But there is still some todo on devel env.: after inv start The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested But that isunrelated to this pr :wink:

``` root@debian:~/doodarm# inv start WARN[0000] mount of type `volume` should not define `bind` option WARN[0000] mount of type `volume` should not define `bind` option [+] Running 4/0 ✔ Volume "doodarm_filestore" Created0.0s ✔ Volume "doodarm_db" Created0.0s [+] Running 5/5odarm-db-1 Crea ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea[+] Running 5/5odarm-db-1 Crea ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea[+] Running 5/5odarm-db-1 Crea ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea[+] Running 5/5odarm-db-1 Crea ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea[+] Running 5/5odarm-db-1 Star ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea[+] Running 5/5odarm-db-1 Star ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea[+] Running 5/5odarm-db-1 Star ✔ Volume "doodarm_filestore" Created0.0s e requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Volume "doodarm_db" Created0.0s iner doodarm-odoo-1 Crea ✔ Container doodarm-db-1 Started0.0s ! db The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested 0.0s ✔ Container doodarm-odoo-1 Started0.1s ```

so you can just do --build-arg WKTHTMLTOPDF_VERSION=version and it will use that no need to change the original file

Thanks, that will likely work. Well from usability point of view, personally I would prefer to set it by default to the arm supported version instead of throwing an error.

pedrobaeza commented 8 months ago

Any news about how to publish this for merging it?

bosd commented 8 months ago

Any news about how to publish this for merging it?

Not sure what you mean by this. My usability comment is non blocker. I'd say it is ready for merge.

pedrobaeza commented 8 months ago

It's not for you, @bosd. I mean this comment: https://github.com/Tecnativa/doodba/pull/573#issuecomment-1868580100

bosd commented 7 months ago

ping @PCatinean Can you attend to pedro's question?

PCatinean commented 7 months ago

@pedrobaeza here are the latest changes that add for support of multi-arch builds. The following adjustments have been made:

  1. In order to not complicate the CI process by passing explicit arguments and also make the images work out of the box, version 14.0 and 15.0 get a "Forcing wkhtmltopdf version to 0.12.6.1" message for ARM64 builds (otherwise they wouldn't work anyway).
  2. The hooks dir has been removed since the scripts inside (push & build) are deprecated. There are github actions that achieve the same result, have more support and also handle the docker buildx (required for building for other architectures).
  3. The platforms variable is configurable per odoo version and can be extended with more architectures in the future.

The arm64 builds are a bit slow around this step Building wheel for libsass (setup.py):. I am sure it can be optimized but for the time being they are increasing the build time a bit.

Tests have been made locally and pushed to: https://hub.docker.com/repository/docker/pledra/doodba/tags?page=1&ordering=last_updated. The architectures available are shown there and when doing a regular pull it should reconcile with the architecture of the system automatically.

If there are any other tests I can make or info to share please let me know.

PCatinean commented 7 months ago

Hi @pedrobaeza so the PR is finally ready.

After a few tests I discovered that the execution time of both test and build (now with 2 platforms) in one single job hit a timeout or resource limit of the github runner. I have now separated the test and build-push into two separate jobs. I think it's better/cleaner this way anyhow since tests alone take around 40 minutes. Given the test job also builds the image to run the tests (i think) we can even set the "build & push" job to run only when merged in master and thus reducing the pipeline time and avoid building on PR's alone.

I tested locally on my fork with the same github variable names, the only difference is the github repo name "pledra/doodba" instead of "tecnativa/doodba".

PR: https://github.com/pledra/doodba/pull/1 Docker repo: https://hub.docker.com/repository/docker/pledra/doodba/tags?page=1&ordering=last_updated

On the PR the test and build ran but only when merged to master did the push also get activated.

Hope this work for you :)

bosd commented 6 months ago

@pedrobaeza Is this one ok now? Ready for merge :pray:

PCatinean commented 6 months ago

@pedrobaeza since this is a bigger change to the workflow if you feel uncomfortable with it we can configure a different branch or a separate repo to push to inside this PR and test there before pushing to master.

bosd commented 5 months ago

Any update? 👀

PCatinean commented 5 months ago

Until there is some time to review this PR I will maintain my fork and rebase every now and then to make the ARM build available for anyone who needs them:

https://github.com/pledra/doodba https://hub.docker.com/repository/docker/pledra/doodba

pedrobaeza commented 5 months ago

Please check when built if everything is OK and pushed, or tell me back if not.