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

Add docker action #478

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

StefanSchoof
Copy link
Contributor

Add a github docker build action.
This is my first github action, so please give much feedback as possible ;)
This requires to set the DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets.
This is building a multi arch image for the linux/amd64, linux/arm64, linux/arm/v7, linux/arm/v6 platforms.
It extract the version from the CMAKELists.txt. The image will get a list of tags: latest, the major version, the major and minor version, the major, minor and sub version and also the commit id. So we have vzlogger:latest, vzlogger:0, vzlogger:0.8, vzlogger:0.8.0 and vzlogger:commit-<shortcommitid>. I add the commit version, because currently no incrementing version number exists and this allows in a case of a problem to go back to a previous version.
I pushed this to test the process to https://hub.docker.com/r/stefanschoof/vzlogger/tags
This action is trigger on every push and pr, but only if a commit to the master branch the images will pushed to docker hub. I think with this approach it is possible to find build breaks on other platforms quicker. Building all platform images will take up to 30 mins.
A sample pr run https://github.com/StefanSchoof/vzlogger/runs/2243563277?check_suite_focus=true
A sample master branch run https://github.com/StefanSchoof/vzlogger/runs/2244080338?check_suite_focus=true
I tried a cache, but since this would cache the libsml and libmbus builds, I deiced against it.

@r00t-
Copy link
Contributor

r00t- commented Apr 1, 2021

neat!

for one, it seems this means, we need to create/manage an "official" vzlogger docker channel now...?

for another,
i am personally opposed to running so many different builds for any MR build.
as discussed in #457
we get this for free, but still should be considerate with resource usage - the more we use, the sooner the service will go away again.
(i actually manage ci for a large project for my day job.)
my approach would be to

  • replace the regular build by the dockerfile build (as suggested in make Dockerfile two-staged #472 already)
  • run at most one additional build on a different os/arch by default
  • build all arches only on a final build for approval to merge
  • build/upload docker images only on merge to master, or if explicitly requested

(the latter two don't seem to be possible on github in any sane way, as there's no way to specify inputs. (at work we trigger PR builds on explicit request only, due to their size, and those requests take parameters)

it already happens that i push a commit, the pipeline starts, but then i notice a typo in a commit message or comment, push a fix, and the next pipeline starts before the previous one even finishes.
(the same thing had us switch to explicit requests at work)

@StefanSchoof
Copy link
Contributor Author

There is already a docker account for mbmd and volkszaheler.org https://hub.docker.com/u/volkszaehler. I think having vzlogger also there would be nice.

Currently the dockerbuild does not run tests, see

RUN cmake -DBUILD_TEST=off \
. Without changing this, we should not use the docker build as only build.

I will try to setup the build as you suggested.

@r00t-
Copy link
Contributor

r00t- commented Apr 2, 2021

@andig:
mbmd and the associated dockerhub account appear to be your doing.
are you ok with expanding it's use as in here,
and can/will you provide the required secrets?

@r00t-
Copy link
Contributor

r00t- commented Apr 2, 2021

@StefanSchoof:
what's the reasoning to NOT run the tests in the docker build, if any?
(i've been meaning to ask that for the original Dockerfile PR already.)
specifcially when building different architectures, it would be good to ensure the tests are green, to have some level of confirmation that the compiled code will actually work on the target.
and the tests can be run in the builder image, need not be installed in the final image.
(or is there some cross-compilation magic in there that does not provide a an environment to actually run the compiled code it? from the code in this MR it doesn't seem so.)

@StefanSchoof
Copy link
Contributor Author

I just took the make command from the install.sh. I tested a docker build with tests. It works, but the runtime grow to 54 mins (https://github.com/StefanSchoof/vzlogger/runs/2251902640?check_suite_focus=true). Should we run the test on every platform?

There is not cross-compilation used. It used a qemu code behind many levels (docker buildx builder, linux bmf, qemu libs). This is also the reason, why this is so slow.

@andig
Copy link
Contributor

andig commented Apr 2, 2021

Moin Zusammen, das ist eine wahnsinnige Ressourcenverschwendung. Vorschlag:

@andig
Copy link
Contributor

andig commented Apr 2, 2021

and can/will you provide the required secrets?

@r00t- it's a Docker org. I would suggest to add you to the org (docker user?) and setup committer access for libsml and this repo?

@andig
Copy link
Contributor

andig commented Apr 2, 2021

and setup committer access for libsml and this repo?

vzlogger solltest Du schon haben!

@StefanSchoof
Copy link
Contributor Author

Nur noch armd64 und armv6 ohne Test reduziert die build time auf 15 min: https://github.com/StefanSchoof/vzlogger/runs/2252885724?check_suite_focus=true

Welche builds für vzlogger sinnvoll und was zu viel ist, kann ich nicht gut bewerten. Das hängt ja davon, ab wie wahrscheinlich ein Fehler durch die Build gefunden wird, den später zu finden mehr Arbeit macht. Ich möchte nur am Ende ein armv6 image im docker hub haben und ein bisschen Erfahrung mit github actions ;)

@r00t-
Copy link
Contributor

r00t- commented Apr 2, 2021

@andig:
while you're at it, can you give me admin access to libsml, or enable fast-forward merges yourself?

@andig
Copy link
Contributor

andig commented Apr 2, 2021

while you're at it, can you give me admin access to libsml, or enable fast-forward merges yourself?

both done

@r00t-
Copy link
Contributor

r00t- commented Apr 2, 2021

judging from those build speeds, that running of docker builds inside qemu appears to be a horrible stopgap solution i wouldn't want to use in production. (would only use it to supply arm images for releases or such.)
IF we do arm builds in an (emulated or not) native environment, but not test them, that seems a little silly.
if we don't test, cross-compilation would be so much more efficient.
(and then if there's no other way, spin up docker on emulated arm just to copy the finished binary inside an image.)

@StefanSchoof
Copy link
Contributor Author

Okay, so a GitHub action that on runs on a push on the master branch and then builds without tests and publish to docker hub. Which platforms should be build in the image?

@r00t-
Copy link
Contributor

r00t- commented Apr 2, 2021

@StefanSchoof:
it's up to you really,
i personally can't tell what those docker images might be useful for.
vzlogger is so small and simple to build from source,
that the use-case for downloading 100mb image just to run it is beyond my imagination.
having native arm testing would seem useful, as we have lots of arm users, but as we found, this solution is too inefficient to provide that.

personally i would only see some use in providing static arm binaries,
it might be handy to have those for MRs, to make it easy for users to test new code on their pies. (but then again that applies only to users who can't handle the few simple steps required to compile themselves.)

maybe @andig has more of an opinion.

@r00t-
Copy link
Contributor

r00t- commented Apr 2, 2021

This is my first github action, so please give much feedback as possible ;)

i'm also new to github actioms specifically,
i skimmed over your code, and it seemed ok,
might have a deeper look later.

@StefanSchoof
Copy link
Contributor Author

StefanSchoof commented Apr 2, 2021

The I will go with armv6 and amd64. Adding a platform if someone has a use case is easy.

I had serval pi installation where I installed so many things, I was no longer know which package I installed for what and on a is upgrade to next raspbian version some things always broke. So I started to use docker images. Also I can put all my config in some files and push it to GitHub as backup, like in https://github.com/StefanSchoof/volkszaehler.org-config

I think I will get to the adjustments in the next week.

@andig
Copy link
Contributor

andig commented Apr 2, 2021

Cross compile statt native compile in qemu würde das bochmal deutlich beschleunigen. Mergeable wäre das nur bei Buildzeiten <10min

@StefanSchoof
Copy link
Contributor Author

So here the github action, that only runs on master

@andig
Copy link
Contributor

andig commented Apr 6, 2021

Moin Stefan, wie lange läuft der Build jetzt?

@J-A-U
Copy link
Collaborator

J-A-U commented Apr 6, 2021

I guess we can use the time this PR did run: 2m44s

@StefanSchoof
Copy link
Contributor Author

Yes, it runs, see https://github.com/StefanSchoof/vzlogger/actions/runs/721635854
The pr did build not build the docker images. This now runs only on master branch

@andig
Copy link
Contributor

andig commented Apr 6, 2021

Ich würde vorschlagen das nicht zu mergen. 15min Buildzeit für ein Dockerimage dass sich relativ leicht selbst bauen lässt erscheint mir eine ziemliche Ressourcenverschwendung.

@StefanSchoof
Copy link
Contributor Author

I did test the docker build and it took 90m on my raspberry pi zero w. Without the build of the libs, the cmake and make took 21 m. So the build on the build server with qemu is faster then on the real hardware.

@r00t-
Copy link
Contributor

r00t- commented Apr 7, 2021

my personal opinion, using docker on a underpowered hardware like a raspberry pi zero is a horrible idea.
but i wouldn't want to stop anybody from doing so if they think it's a good idea.

also my opinion, and as stated before, i think we should be somewhat sensible in using resources, even, or especially if they are free,
and running a build in an emulator for 15 minutes that would take a few seconds natively on the same hardware is a horrible idea.
@andig seems to agree here.

i wonder how we could improve on this...

does anybody have any experience cross-compiling vzlogger?
and could one get a cross-compiled binary into the emulator for test-execution and/or docker-building?

another solution would be to run the build natively on more powerful arm hardware.
we could connect some hardware like a raspberry pi 4 as an executor to run the builds on.
but somebody would have to do that and pay for the hardware and resources.
i'd even probably have a spare system,
but then, for now it seems @StefanSchoof would be the only user, and could do this himself.

is there some other way we can get a native arm system?
amazon ec2 and such offer arm instances by now,
buy i guess that won't arrive on gitlab ci anytime soon.

@r00t-
Copy link
Contributor

r00t- commented Apr 7, 2021

just cross-compiled vzlogger using buildroot...
went pretty smooth.
just that it built libproto.so libvz-api.so libvz.so as shared libs and installed them 😆
and then doesn't run on my raspberry pi due to libc version mismatch.

@StefanSchoof
Copy link
Contributor Author

If you a are assuming, that I am the only user that is using docker on a raspberry I agree. I can setup my own docker hub repro and use it. But I think, that there are more users that want to use this image, but I have no data to prove it.

Getting a real arm build agent is not a easy task. GitHub has none, amazon will cost (nothing that really big if created only on build time, but there needs to setup a billing) and you get much complexity in creating this, maintain a arm build agent, that runs 24/7 only for this purpose sound for me like a greater waste of resources, cross completion will only reduce the time for the build, installing the dependency will still need qemu and time.

I suggest I remove the arm platform from this PR and we see if in the future we see requests in the mailing list or on other places where people want to run the vzlogger in arm docker.

@StefanSchoof
Copy link
Contributor Author

@r00t-
Copy link
Contributor

r00t- commented Apr 10, 2021

i am 90% done with a Dockerfile for cross-compiling, based on buildroot.
the build takes less than 15 minutes, including building the cross-compiler itself and all libs from source. (on a powerful machine, i might add. have not tested on github ci yet.)
subsequent vzlogger builds are <1min (if we can cache the cross-compiler image on dockerhub or such. it should be possible to get it down to ~300mb.)

there's two pieces missing:

  • configure vzlogger for static compilation
    (as pointed out above, the cross-compile currently builds the internal libproto.so libvz-api.so libvz.so as shared libs)
    also we should link all other libs statically, to make the binary portable,
    specifically libc, as the cross-build does not use glibc, simply for efficiency, see also [MeterW1therm] avoid GLOB_BRACE, don't require glibc #470 .
    anybody trying to do this can simply test on x86_64

  • if you want to build your arm docker images based on this:
    work out how to copy the vzlogger binary from the x86_64 cross-compiler image to the arm image.
    basically a builder-pattern where the builder image is x86_64 but the target image that files are copied onto is arm.
    i have no idea if this is possible with that qemu setup.
    in worst-case, stash the vzlogger binary on an external service.

@StefanSchoof
Copy link
Contributor Author

Great. The docker from has a platform flag (https://docs.docker.com/engine/reference/builder/#from) that allows us to use the x86 first Stage and a arm second stage. If only copy are in the second stage no qemu is needed. So if all needed libs are able to copy from the builder, static linking is not a must for an arm docker images without qemu.

@r00t-
Copy link
Contributor

r00t- commented Apr 11, 2021

If only copy are in the second stage no qemu is needed.

even more efficient for your case, if no qemu needed at all.
you want to copy the libs from the builder so that you don't need to run any package installation?
it should also be possible to upload a base image with any required packages preinstalled. (i was going to suggest that before, as your qemu build also spends significant amounts of time installing packages.
(btw, debian also provides the option to "cross"-debootstrap a system, and only run the necessary parts on the target, have you considered that for use with the image-building?

i would still like to use qemu (with or without docker) to run the unit-tests after a build, can you look into that?

So if all needed libs are able to copy from the builder, static linking is not a must for an arm docker images without qemu.

that is only half of the truth.
re-using the libs requires either that they are compatible with the ld.so (and possibly libc) on the target, or that you run the application in a chroot.
also, i would prefer if we could offer those binaries for use without docker, where statically linked ones achieve the best compatibility.

@r00t-
Copy link
Contributor

r00t- commented Apr 11, 2021

@StefanSchoof:
why did i miss this earlier:
with a static binary, building a docker image becomes trivial:

FROM scratch
COPY binary /binary
ENTRYPOINT /binary

no OS image needed at all.
(just the glorified chroot that docker mostly is.)

(with buildroot, one would probably COPY output/target / to grab the binary and libraries, but i'd rather have a static binary anyway.)

@StefanSchoof
Copy link
Contributor Author

StefanSchoof commented Apr 20, 2021

i would still like to use qemu (with or without docker) to run the unit-tests after a build, can you look into that?

Since the build time in qemu rises with tests from 15min to 54min, I think the main time will be needed for the tests. Has this big resource need enough value for the project to be justified?

@r00t-
Copy link
Contributor

r00t- commented Apr 20, 2021

Since the build time in qemu rises with tests from 15min to 54min, I think the main time will be needed for the tests.

the slow part is building the tests.
executing them should be much faster (but feel free to try and report).

the idea is to cross-compile the tests and then execute in the emulator.

sadly i'm kinda stuck with #486 on how to get a static build. (and time/motivation)
anybody interested could try to get a static native build working, that could then be ported to the cross-compile setup.

@r00t- r00t- mentioned this pull request Jan 17, 2023
@maxberger
Copy link
Contributor

my personal opinion, using docker on a underpowered hardware like a raspberry pi zero is a horrible idea. but i wouldn't want to stop anybody from doing so if they think it's a good idea.

Rationale why I use docker:
a) I use that on all my home servers, so I only have 1 management system
b) easy upgrade / downgrade
c) no need to mess with libs

The overhead is small enough to be usable even on the pizero.

@maxberger
Copy link
Contributor

For architecture selection: Please build: linux/arm/v6, linux/arm64, linux/amd64. Those are the most-used out there.

@guidoffm
Copy link

This action uses a deprecated checkout action (v2) instead of v4.

Also, it uses Docker Hub, that has more an more restrictions.

My version is here: #663

Please have a look at it. It also signs the created image. I don't know if this is needed. But my action comes from the GitHub blueprint.

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