azure: build Docker images as part of the pipeline#5198
Conversation
pks-t
force-pushed
the
pks/azure-docker-builds
branch
15 times, most recently
from
381564f to
b8fde3f
Compare
|
Sure enough, upgrading to Ubuntu Xenial uncovered multiple bugs in Valgrind, which will simply fail all the time due to a bug in Vex: https://bugs.launchpad.net/ubuntu/+source/valgrind/+bug/1574437 and a bug with libgcrypt. Other than that this works as expected now |
pks-t
force-pushed
the
pks/azure-docker-builds
branch
5 times, most recently
from
b8128e3 to
8dd4efa
Compare
|
After a whole lot of tries this finally builds. It was more problematic than expected, but this is mostly caused by upgrading our CI from Trusty (which has been EOL'd in April 2019) to Xenial. With Xenial came the usual set of bugs in dependencies we're already used to with Trusty, but I wanted to solve these without requiring Bintray. In fact, this development process already demonstrates why I'm apt to replace the current setup which uses an external Docker registry: It's so much easier to test changes to how the images are set up, to iterate and maintain. So here's the catch: this PR adds about 1:50m to build the Docker images. BUT: It also shaves off nearly two minutes of build time by converting to Ninja. So we are actually a tiny bit faster than before. While we could decrease build time by two minutes if we were to continue using the registry and switch to Ninja regardless, I think that the benefits of building ad-hoc images greatly overweigh saving two minutes of build time. Especially considering that the slow path is still Windows or rather MinGW. |
|
Oh, by the way: this PR possibly also fixes Coverity. Coverity builds have been referencing libgit2/trusty-openssl images, and I don't think we generate these anymore. So I've switched it to use the new infrastructure as well, which could possibly fix it. |
|
Neat, nice to see the reduction in time from using Ninja. Curious, though: what if instead of regenerating the docker image every time as part of libgit2's CI, we most that CI bit out into its own stage that only runs on changes to the Dockerfiles. IOW, we have a CI process for the docker images that builds and deploys to dockerhub. That way we only regenerate them when they've changed and we keep that 2 minutes of savings that you shaved off by using Ninja? That's sort of why I had dockerfiles in their own repo that we might do that someday. (Though, to be honest, that's not necessary, we could use path scopes to avoid rebuilding things, but I think it simplifies things.) |
|
I thought about that too, but rather as a nice optimization after this is merged (if we agree on it, that is). I'd really prefer to keep the Dockerfiles in here, though. Having a second repository adds just too much hassle, if you ask me:
I'd love to have a cache here, though. I think I saw that one can enable Docker registries with Azure, and I'd imagine that we could just push images in there. If possible, that would be a very nice improvement. We needn't care about tags in there, though, all builds need to be cached based on Dockerfile contents only, otherwise we'd have problems if e.g. a PR makes changes to a Dockerfile. |
The Docker images used for our continuous integration builds currently live in the libgit2/libgit2-docker repository. To make any changes in them, one has to make a PR there, get it reviewed, re-build the images and publish them to Docker Hub. This process is slow and tedious, making it harder than necessary to perform any updates to our Docker-based build pipeline. To fix this, we include all Dockerfiles used by Azure from the mentioned repository and inline them into our own repo. Instead of having to manually push them to the CI, it will now build the required containers on each pull request, allowing much greater flexibility.
Support for the LTS release Ubuntu 14.04 Trusty has been dropped in April 2019, but Azure is still using Trusty as its primary platform to build and test against. Let's deprecate it in favor of Xenial.
The Coverity build is still referencing an old "trusty-openssl" container that is not provided by either our own now-inlined images nor by the libgit2/libgit2-docker repository. Convert it to build and use Xenial images instead.
We're about to phase out support for Trusty, but neither Bionic nor Xenial images provide the mbedTLS library that's available in Trusty. Build them for both to pull them in line with Trusty.
While Xenial provides libssh2 in its repositories, it only has version 1.5.0 available. This version will unfortunately not be able to connect to GitHub due to their removal of weak cryptographic standards [1]. To still enable our CI to execute tests against GitHub, we thus have to update the provided libssh2 version to a newer one. Manually install libssh2 1.8.2 on Xenial. There's no need to do the same for Bionic, as it already provides libssh2 1.8.0. [1]: https://github.blog/2018-02-01-crypto-removal-notice/
Reformat both Xenial and Bionic's Dockerfiles to use best practices. Most importantly, we now run `apt-get update` and `apt-get install` in one step followed up by removing the package lists to speed up installation and keep down the image size.
Pass the flag "--no-install-recommends" to apt-get in order to trim down the number of packages installed, both reducing build time and image size. As this also causes some required packages to not be installed anymore, add these explicitly to the set of packages installed.
The Valgrind version shipped with Xenial has some bugs that keep our tests from working due to bad interactions with openssl [1]. Fix this by using the "hola-launchpad/valgrind" PPA that provides a newer version for which the bug has been fixed. [1]: https://bugs.launchpad.net/ubuntu/+source/valgrind/+bug/1574437
While we were still supporting Trusty, using Ninja as a build tool would have required us to first setup pip and then use it to install Ninja. As a result, the speedups from using Ninja were drowned out by the time required to install Ninja. But as we have deprecated Trusty now, both Xenial and Bionic have recent versions of Ninja in their repositories and thus we can now use Ninja.
When using mbedTLS as backend, then the user may specify the location of where system certificates are installed. If no such location is provided by the user, CMake will try to autodetect the location by using the openssl executable, if installed. If no location could be detected, then the mbedTLS is essentially worthless as it is completely unable to verify any certificates. To avoid use of such misconfigured mbedTLS configurations, let's error out if we were unable to find out the location.
The MESSAGE() function expects as first argument the message type, e.g. STATUS or FATAL_ERROR. In some places, we were misusing this to either not provide any type, which would then erroneously print the message to standard error, or to use FATAL instead of FATAL_ERROR. Fix all of these instances. Also, remove some MESSAGE invocations that are obvious leftovers from debugging the build system.
pks-t
force-pushed
the
pks/azure-docker-builds
branch
from
8dd4efa to
212b6f6
Compare
|
Rebased. I'd still be in favor of merging this :) Another usecase is the PCRE/PCRE2 conversion, where I'd love to simply install libpcre2 to have it tested as part of our CI |
|
So, I would say that broadly speaking I'm 👍 on this. In a perfect world, we wouldn't re-build the containers each time and would cache them. But that's not something that we need to block this on. |
|
🎉 🎉 🎉 Thanks a lot, @ethomson! :) |
2 participants
The Docker images used for our continuous integration builds currently
live in the libgit2/libgit2-docker repository. To make any changes in
them, one has to make a PR there, get it reviewed, re-build the images
and publish them to Docker Hub. This process is slow and tedious, making
it harder than necessary to perform any updates to our Docker-based
build pipeline.
To fix this, we include all Dockerfiles used by Azure from the mentioned
repository and inline them into our own repo. Instead of having to
manually push them to the CI, it will now build the required containers
on each pull request, allowing much greater flexibility.
This is more of a test whether this is feasible at all with regards to build times. It'd be rad if it was possible as that'd reduce another maintenance burden.