Permit raspberry pi (and other architectures) builds #985

abondis · 2019-04-01T04:10:25Z

What type of PR?

Enhancement

What does this PR do?

Add an option to select base images and permit building for different CPU architectures.

Related issue(s)

N/A

Prerequistes

documentation updated accordingly
Unless it's docs or a minor change: add changelog entry file.

abondis · 2019-04-11T19:04:23Z

it's strange, bors says 'build succeeded' but it seems like it doesn't report it's status ...

kaiyou · 2019-06-24T06:11:32Z

It does if you click the link in the comment. Bors merely creates branches and merges them, Travis is the one to run the checks.

kaiyou · 2019-06-24T06:17:52Z

This is pretty cool, I have a few comments and questions at this point.

Really sorry about it, but we are migrating to alpine 3.10 and if merged first it might have some (minor) impact on your PR, starting with not using edge anymore.
Why are you copying qemu in the php images? I am probably missing something there.
You are changing quite a lot of things in the admin Dockerfile, some of them which do not seem to be related to arm. Could you detail why?

abondis · 2019-06-24T14:32:49Z

Hi, thank you for the review, here are the answers :

Changing to 3.10 should be ok, I'll take a look when I have time
the qemu file is needed when you build for arm from a non arm plateform. Most of my changes are just permitting a variable to override the image to use so the build script can use an arm image. The one I used in the build script already has the qemu except for the php image and I could not find any other way to do that.
In admin I think I only changed the image name and added the variable. Maybe you meant the nginx one ? I don't remember exactly, I recall having issues but can't recall why.
Another thing I don't know, is how to make the CI actually run the arm build as for now it still only calls the normal x86 build ..

muhlemmer · 2019-08-21T15:17:13Z

In admin I think I only changed the image name and added the variable. Maybe you meant the nginx one ? I don't remember exactly, I recall having issues but can't recall why.
Another thing I don't know, is how to make the CI actually run the arm build as for now it still only calls the normal x86 build ..

I don't believe travis-CI has anything else then X86_64. They've closed an issue without fix regarding this travis-ci/travis-ci#3376

abondis · 2019-08-21T15:46:53Z

@muhlemmer actually the problem is not TravisCI: the build for Raspberry Pi can be done from an x86_64 machine, that is why there is some qemu related files. The problem is just that I don't know what to configure where to make it run the command :)

muhlemmer

This is an awesome idea! Unfortunately I don't have Pi for testing. However, I've reviewed the code and have left some comments. Basically some house-keeping details.

Besides the comments, the conflicts with master will need to be resolved. Notably the upgrade to alpine:3.10.

muhlemmer · 2019-08-21T15:24:41Z

core/nginx/Dockerfile

- && pip3 install idna requests watchdog
+RUN apk add --no-cache certbot nginx nginx-mod-mail openssl curl
+RUN apk del --no-cache py3-openssl
+RUN pip3 install idna requests watchdog



Most changes in this file have nothing to do ARM support. Please revert non-relevant changes. Also, using multiple RUNstatements will create new layers and additional image size. Where possibleRUNoperations need to remain grouped using&& ` syntax.

muhlemmer · 2019-08-21T15:31:42Z

docs/rpi_build.rst

+
+- ``DISTRO``: is the main distro used (ie: alpine:3.8)
+- ``PHP_DISTRO``: is used for the ``rainloop`` and ``roundcube`` images
+- ``EDGE_DISTRO``: is used for ``radicale`` as edge has dulwich and radicale as packages


Edge is no longer used:

Mailu/optional/radicale/Dockerfile

Line 1 in 13d6cf2

FROM alpine:3.10

optional/radicale/Dockerfile

muhlemmer · 2019-08-21T15:34:31Z

optional/radicale/Dockerfile


 RUN echo "@testing http://nl.alpinelinux.org/alpine/edge/testing" >> /etc/apk/repositories \
- && apk add --no-cache radicale@testing py-dulwich@testing curl bash
+  && apk add --no-cache py3-dulwich@testing curl bash
+RUN pip3 install radicale



Why drop official distro package for pip one? This change should not be related to ARM. If it is an improvement by itself, please open a specific pull request.

muhlemmer · 2019-08-21T15:35:44Z

setup/Dockerfile

-RUN apk add --no-cache curl \
- && pip install -r requirements.txt
+RUN apk add --no-cache curl python3 py3-pip
+RUN pip3 install -r requirements.txt



Keep these actions in a single RUN statement, in order to avoid additional layers.

tests/build_arm.sh

webmails/rainloop/Dockerfile

webmails/roundcube/Dockerfile

muhlemmer

I've reviewed the changes all seems good. However, the test build is failing:

Step 9/25 : ARG DISTRO=alpine:3.10
 ---> Running in 76172496030d
Removing intermediate container 76172496030d
 ---> 38fbda409b3a
Step 10/25 : FROM $DISTRO
Service 'admin' failed to build: base name ($DISTRO) should not be blank
The command "docker-compose -f tests/build.yml build" failed and exited with 1 during .

It seems that docker cleans up just before running a second FROM, clearing the environment from one layer to another. Have you tried a local build on a x86_64?

abondis · 2019-08-27T12:31:43Z

Ah yes I'm going to take a look at this this week. Thanks.

…

On Tue, Aug 27, 2019, 7:07 AM Tim Möhlmann, ***@***.***> wrote: ***@***.**** commented on this pull request. I've reviewed the changes all seems good. However, the test build is failing <https://travis-ci.org/Mailu/Mailu/builds/575974626#L1206-L1212>: Step 9/25 : ARG DISTRO=alpine:3.10 ---> Running in 76172496030d Removing intermediate container 76172496030d ---> 38fbda409b3a Step 10/25 : FROM $DISTRO Service 'admin' failed to build: base name ($DISTRO) should not be blank The command "docker-compose -f tests/build.yml build" failed and exited with 1 during . It seems that docker cleans up just before running a second FROM, clearing the environment from one layer to another. Have you tried a local build on a x86_64? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#985?email_source=notifications&email_token=AACAD23SQJULS4UZVQT2XNTQGUDHLA5CNFSM4HCTH7C2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCCZEG6A#pullrequestreview-280118136>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACAD2Y3MDMQUWT6XPNZPFTQGUDHLANCNFSM4HCTH7CQ> .

fergalmoran · 2019-08-27T15:58:14Z

Does... does... does this mean what I think it means?

abondis · 2019-08-27T16:10:58Z

I don't know, what do you think it means ? It build the docker images to run mailu on a raspberry pi ( and probably other ARM boards). I still have to make the build pass.

…

On Tue, Aug 27, 2019, 11:58 AM Fergal Moran, ***@***.***> wrote: Does... does... does this mean what I think it means? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#985?email_source=notifications&email_token=AACAD264HTKI76Q2AY6RWE3QGVFJVA5CNFSM4HCTH7C2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5IHRUI#issuecomment-525367505>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACAD23PAB3S7DYXCVYOQ2LQGVFJVANCNFSM4HCTH7CQ> .

fergalmoran · 2019-08-27T18:42:48Z

@abondis Sorry - I've been following this PR with interest for a while...
I was just getting excited that the build succeeded....

mergify · 2019-08-30T14:23:25Z

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

bors · 2019-08-30T14:53:23Z

try

Build succeeded

continuous-integration/travis-ci/push

kaiyou · 2019-08-30T16:07:21Z

Oh dear. It passed. 👏

muhlemmer

Hey! all looks good to go 1 more small remark.

I will do functionality testing tonight.

CHANGELOG.md.20190330

Pull request has been modified.

muhlemmer

🎉

kaiyou

One very tiny hint from my side, looks good otherwise

kaiyou · 2019-09-18T06:09:15Z

setup/Dockerfile

-RUN apk add --no-cache curl \
- && pip install -r requirements.txt
+RUN apk add --no-cache curl python3 py3-pip
+RUN pip3 install -r requirements.txt


Small notice, this should not be split in two layera.

abondis · 2019-09-24T03:31:31Z

Good catch, I thought I didn't modify it, but I guess it's some extra from the rebase ... I'll try to take fix it soon. Thanks.

…

On Wed, Sep 18, 2019, 2:09 AM kaiyou, ***@***.***> wrote: ***@***.**** requested changes on this pull request. One very tiny hint from my side, looks good otherwise ------------------------------ In setup/Dockerfile <#985 (comment)>: > RUN mkdir -p /app WORKDIR /app COPY requirements.txt requirements.txt -RUN apk add --no-cache curl \ - && pip install -r requirements.txt +RUN apk add --no-cache curl python3 py3-pip +RUN pip3 install -r requirements.txt Small notice, this should not be split in two layera. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#985?email_source=notifications&email_token=AACAD2452PVJDF3LAICTZI3QKHAY7A5CNFSM4HCTH7C2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFCELKQ#pullrequestreview-289686954>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACAD2ZH3SUDY2Y4PDU3NO3QKHAY7ANCNFSM4HCTH7CQ> .

Pull request has been modified.

kaiyou

Looks good, thank you for fixing :)

mergify · 2019-10-20T20:40:48Z

bors r+

985: Permit raspberry pi (and other architectures) builds r=mergify[bot] a=abondis ## What type of PR? Enhancement ## What does this PR do? Add an option to select base images and permit building for different CPU architectures. ### Related issue(s) N/A ## Prerequistes - [X] documentation updated accordingly - [x] Unless it's docs or a minor change: add [changelog](https://mailu.io/master/contributors/guide.html#changelog) entry file. Co-authored-by: Aurélien Bondis <aurelien.bondis@gmail.com> Co-authored-by: Aurelien <aurelien.bondis@gmail.com>

bors · 2019-10-20T21:09:42Z

Build succeeded

continuous-integration/travis-ci/push