* [PATCH v2 1/3] gitlab: always build container images
2021-02-16 13:29 [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs Daniel P. Berrangé
@ 2021-02-16 13:29 ` Daniel P. Berrangé
2021-02-16 13:29 ` [PATCH v2 2/3] gitlab: add fine grained job deps for all build jobs Daniel P. Berrangé
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-02-16 13:29 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Thomas Huth, Alex Bennée,
Daniel P. Berrangé, Wainer dos Santos Moschetta
Currently we attempt to skip building container images if the commits do
not involve changes to the dockerfiles or gitlab CI definitions.
Conceptually this makes sense, but there is a challenge in the real
world implementation of this in gitlab.
In the case of a CI pipeline triggered from a merge request, GitLab
knows the common ancestor of the merge request and the main git repo,
so it can trivially determine if any of the commits associated with
the MR change the dockerfiles.
In the case of a CI pipeline triggered from a push to a branch, it is
much more difficult. There is no concept of a common ancestor in this
case. Instead GitLab looks at the set of commits in the git push event.
On the surface this may sound reasonable, but it doesn't take into
account that a push event does not always contain the full set of
patches from a branch.
For example, consider pushing 5 commits, one of which contains a
dockerfile change. This will trigger a CI pipeline for the
containers. Now consider you do some more work on the branch and push 3
further commits, so you now have a branch of 8 commits. For the second
push GitLab will only look at the 3 most recent commits, the other 5
were already present. Thus GitLab will not realize that the branch has
dockerfile changes that need to trigger the container build.
This can cause real world problems:
- Push 5 commits to branch "foo", including a dockerfile change
=> rebuilds the container images with content from "foo"
=> build jobs runs against containers from "foo"
- Refresh your master branch with latest upstream master
=> rebuilds the container images with content from "master"
=> build jobs runs against containers from "master"
- Push 3 more commits to branch "foo", with no dockerfile change
=> no container rebuild triggers
=> build jobs runs against containers from "master"
The "changes" conditional in gitlab is OK, *provided* your build
jobs are not relying on any external state from previous builds.
This is NOT the case in QEMU, because we are building container
images and these are cached. This is a scenario in which the
"changes" conditional is not usuable.
The only other way to avoid this problem would be to use the git
branch name as the container image tag, instead of always using
"latest". The downside of this approach is that the user's gitlab
registry will grow significantly until it starts to trigger
GitLab's automatic deletion policy. Every time the user starts
a new branch they will have to trigger a rebuild of the container
images. Given this, we might as well just drop the conditional
and always build the container images. Most of the time docker
will be able to use the layer cache to avoid the most expensive
part of the rebuild process (installing all the RPMs/debs/etc)
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
.gitlab-ci.d/containers.yml | 7 -------
1 file changed, 7 deletions(-)
diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index 90fac85ce4..33e4046e23 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -20,13 +20,6 @@
- docker push "$TAG"
after_script:
- docker logout
- rules:
- - changes:
- - .gitlab-ci.d/containers.yml
- - tests/docker/*
- - tests/docker/dockerfiles/*
- - if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'
- - if: '$CI_COMMIT_REF_NAME == "testing/next"'
amd64-alpine-container:
<<: *container_job_definition
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/3] gitlab: add fine grained job deps for all build jobs
2021-02-16 13:29 [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs Daniel P. Berrangé
2021-02-16 13:29 ` [PATCH v2 1/3] gitlab: always build container images Daniel P. Berrangé
@ 2021-02-16 13:29 ` Daniel P. Berrangé
2021-02-16 13:33 ` Philippe Mathieu-Daudé
2021-02-16 13:29 ` [PATCH v2 3/3] gitlab: fix inconsistent indentation Daniel P. Berrangé
2021-02-24 11:57 ` [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs Daniel P. Berrangé
3 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-02-16 13:29 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Thomas Huth, Alex Bennée,
Daniel P. Berrangé, Wainer dos Santos Moschetta
This allows the build jobs to start running as soon as their respective
container image is ready, instead of waiting for all container builds
to finish.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
.gitlab-ci.d/crossbuilds.yml | 46 ++++++++++++++++++++++++++++
.gitlab-ci.yml | 58 ++++++++++++++++++++++++++++++++++++
2 files changed, 104 insertions(+)
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 66547b6683..d5098c986b 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -41,114 +41,158 @@
cross-armel-system:
extends: .cross_system_build_job
+ needs:
+ job: armel-debian-cross-container
variables:
IMAGE: debian-armel-cross
cross-armel-user:
extends: .cross_user_build_job
+ needs:
+ job: armel-debian-cross-container
variables:
IMAGE: debian-armel-cross
cross-armhf-system:
extends: .cross_system_build_job
+ needs:
+ job: armhf-debian-cross-container
variables:
IMAGE: debian-armhf-cross
cross-armhf-user:
extends: .cross_user_build_job
+ needs:
+ job: armhf-debian-cross-container
variables:
IMAGE: debian-armhf-cross
cross-arm64-system:
extends: .cross_system_build_job
+ needs:
+ job: arm64-debian-cross-container
variables:
IMAGE: debian-arm64-cross
cross-arm64-user:
extends: .cross_user_build_job
+ needs:
+ job: arm64-debian-cross-container
variables:
IMAGE: debian-arm64-cross
cross-i386-system:
extends: .cross_system_build_job
+ needs:
+ job: i386-fedora-cross-container
variables:
IMAGE: fedora-i386-cross
MAKE_CHECK_ARGS: check-qtest
cross-i386-user:
extends: .cross_user_build_job
+ needs:
+ job: i386-fedora-cross-container
variables:
IMAGE: fedora-i386-cross
MAKE_CHECK_ARGS: check
cross-mips-system:
extends: .cross_system_build_job
+ needs:
+ job: mips-debian-cross-container
variables:
IMAGE: debian-mips-cross
cross-mips-user:
extends: .cross_user_build_job
+ needs:
+ job: mips-debian-cross-container
variables:
IMAGE: debian-mips-cross
cross-mipsel-system:
extends: .cross_system_build_job
+ needs:
+ job: mipsel-debian-cross-container
variables:
IMAGE: debian-mipsel-cross
cross-mipsel-user:
extends: .cross_user_build_job
+ needs:
+ job: mipsel-debian-cross-container
variables:
IMAGE: debian-mipsel-cross
cross-mips64el-system:
extends: .cross_system_build_job
+ needs:
+ job: mips64el-debian-cross-container
variables:
IMAGE: debian-mips64el-cross
cross-mips64el-user:
extends: .cross_user_build_job
+ needs:
+ job: mips64el-debian-cross-container
variables:
IMAGE: debian-mips64el-cross
cross-ppc64el-system:
extends: .cross_system_build_job
+ needs:
+ job: ppc64el-debian-cross-container
variables:
IMAGE: debian-ppc64el-cross
cross-ppc64el-user:
extends: .cross_user_build_job
+ needs:
+ job: ppc64el-debian-cross-container
variables:
IMAGE: debian-ppc64el-cross
cross-s390x-system:
extends: .cross_system_build_job
+ needs:
+ job: s390x-debian-cross-container
variables:
IMAGE: debian-s390x-cross
cross-s390x-user:
extends: .cross_user_build_job
+ needs:
+ job: s390x-debian-cross-container
variables:
IMAGE: debian-s390x-cross
cross-s390x-kvm-only:
extends: .cross_accel_build_job
+ needs:
+ job: s390x-debian-cross-container
variables:
IMAGE: debian-s390x-cross
ACCEL_CONFIGURE_OPTS: --disable-tcg
cross-win32-system:
extends: .cross_system_build_job
+ needs:
+ job: win32-fedora-cross-container
variables:
IMAGE: fedora-win32-cross
cross-win64-system:
extends: .cross_system_build_job
+ needs:
+ job: win64-fedora-cross-container
variables:
IMAGE: fedora-win64-cross
cross-amd64-xen-only:
extends: .cross_accel_build_job
+ needs:
+ job: amd64-debian-cross-container
variables:
IMAGE: debian-amd64-cross
ACCEL: xen
@@ -156,6 +200,8 @@ cross-amd64-xen-only:
cross-arm64-xen-only:
extends: .cross_accel_build_job
+ needs:
+ job: arm64-debian-cross-container
variables:
IMAGE: debian-arm64-cross
ACCEL: xen
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 7adb9a4cef..32cc6bd4a2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -109,6 +109,8 @@ acceptance-system-alpine:
build-system-ubuntu:
<<: *native_build_job_definition
+ needs:
+ job: amd64-ubuntu2004-container
variables:
IMAGE: ubuntu2004
CONFIGURE_ARGS: --enable-fdt=system --enable-slirp=system
@@ -141,6 +143,8 @@ acceptance-system-ubuntu:
build-system-debian:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-container
variables:
IMAGE: debian-amd64
CONFIGURE_ARGS: --enable-fdt=system
@@ -186,6 +190,8 @@ acceptance-system-debian:
build-system-fedora:
<<: *native_build_job_definition
+ needs:
+ job: amd64-fedora-container
variables:
IMAGE: fedora
CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
@@ -219,6 +225,8 @@ acceptance-system-fedora:
build-system-centos:
<<: *native_build_job_definition
+ needs:
+ job: amd64-centos8-container
variables:
IMAGE: centos8
CONFIGURE_ARGS: --disable-nettle --enable-gcrypt --enable-fdt=system
@@ -252,6 +260,8 @@ acceptance-system-centos:
build-system-opensuse:
<<: *native_build_job_definition
+ needs:
+ job: amd64-opensuse-leap-container
variables:
IMAGE: opensuse-leap
CONFIGURE_ARGS: --enable-fdt=system
@@ -284,6 +294,8 @@ acceptance-system-opensuse:
build-disabled:
<<: *native_build_job_definition
+ needs:
+ job: amd64-fedora-container
variables:
IMAGE: fedora
CONFIGURE_ARGS:
@@ -366,6 +378,8 @@ build-disabled:
# available.
build-tcg-disabled:
<<: *native_build_job_definition
+ needs:
+ job: amd64-centos8-container
variables:
IMAGE: centos8
script:
@@ -386,6 +400,8 @@ build-tcg-disabled:
build-user:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-user-cross-container
variables:
IMAGE: debian-all-test-cross
CONFIGURE_ARGS: --disable-tools --disable-system
@@ -393,6 +409,8 @@ build-user:
build-user-static:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-user-cross-container
variables:
IMAGE: debian-all-test-cross
CONFIGURE_ARGS: --disable-tools --disable-system --static
@@ -401,6 +419,8 @@ build-user-static:
# Only build the softmmu targets we have check-tcg tests for
build-some-softmmu:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-user-cross-container
variables:
IMAGE: debian-all-test-cross
CONFIGURE_ARGS: --disable-tools --enable-debug
@@ -412,6 +432,8 @@ build-some-softmmu:
# we skip cris-linux-user as it doesn't use the common run loop
build-user-plugins:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-user-cross-container
variables:
IMAGE: debian-all-test-cross
CONFIGURE_ARGS: --disable-tools --disable-system --enable-plugins --enable-debug-tcg --target-list-exclude=sparc64-linux-user,cris-linux-user
@@ -427,6 +449,8 @@ build-user-centos7:
build-some-softmmu-plugins:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-user-cross-container
variables:
IMAGE: debian-all-test-cross
CONFIGURE_ARGS: --disable-tools --disable-user --enable-plugins --enable-debug-tcg
@@ -435,6 +459,8 @@ build-some-softmmu-plugins:
clang-system:
<<: *native_build_job_definition
+ needs:
+ job: amd64-fedora-container
variables:
IMAGE: fedora
CONFIGURE_ARGS: --cc=clang --cxx=clang++
@@ -464,6 +490,8 @@ tsan-build:
# These targets are on the way out
build-deprecated:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-user-cross-container
variables:
IMAGE: debian-all-test-cross
CONFIGURE_ARGS: --disable-docs --disable-tools
@@ -501,6 +529,8 @@ gprof-gcov:
build-oss-fuzz:
<<: *native_build_job_definition
+ needs:
+ job: amd64-fedora-container
variables:
IMAGE: fedora
script:
@@ -519,6 +549,8 @@ build-oss-fuzz:
build-tci:
<<: *native_build_job_definition
+ needs:
+ job: amd64-fedora-container
variables:
IMAGE: fedora
script:
@@ -542,6 +574,8 @@ build-tci:
# However we can't test against KVM on Gitlab-CI so we can only run unit tests
build-coroutine-ucontext:
<<: *native_build_job_definition
+ needs:
+ job: amd64-ubuntu2004-container
variables:
IMAGE: ubuntu2004
CONFIGURE_ARGS: --with-coroutine=ucontext --disable-tcg
@@ -549,6 +583,8 @@ build-coroutine-ucontext:
build-coroutine-sigaltstack:
<<: *native_build_job_definition
+ needs:
+ job: amd64-ubuntu2004-container
variables:
IMAGE: ubuntu2004
CONFIGURE_ARGS: --with-coroutine=sigaltstack --disable-tcg
@@ -560,6 +596,8 @@ build-coroutine-sigaltstack:
# which had some API differences.
build-crypto-old-nettle:
<<: *native_build_job_definition
+ needs:
+ job: amd64-centos7-container
variables:
IMAGE: centos7
TARGETS: x86_64-softmmu x86_64-linux-user
@@ -581,6 +619,8 @@ check-crypto-old-nettle:
build-crypto-old-gcrypt:
<<: *native_build_job_definition
+ needs:
+ job: amd64-centos7-container
variables:
IMAGE: centos7
TARGETS: x86_64-softmmu x86_64-linux-user
@@ -602,6 +642,8 @@ check-crypto-old-gcrypt:
build-crypto-only-gnutls:
<<: *native_build_job_definition
+ needs:
+ job: amd64-centos7-container
variables:
IMAGE: centos7
TARGETS: x86_64-softmmu x86_64-linux-user
@@ -623,18 +665,24 @@ check-crypto-only-gnutls:
# We don't need to exercise every backend with every front-end
build-trace-multi-user:
<<: *native_build_job_definition
+ needs:
+ job: amd64-ubuntu2004-container
variables:
IMAGE: ubuntu2004
CONFIGURE_ARGS: --enable-trace-backends=log,simple,syslog --disable-system
build-trace-ftrace-system:
<<: *native_build_job_definition
+ needs:
+ job: amd64-ubuntu2004-container
variables:
IMAGE: ubuntu2004
CONFIGURE_ARGS: --enable-trace-backends=ftrace --target-list=x86_64-softmmu
build-trace-ust-system:
<<: *native_build_job_definition
+ needs:
+ job: amd64-ubuntu2004-container
variables:
IMAGE: ubuntu2004
CONFIGURE_ARGS: --enable-trace-backends=ust --target-list=x86_64-softmmu
@@ -642,12 +690,16 @@ build-trace-ust-system:
# Check our reduced build configurations
build-without-default-devices:
<<: *native_build_job_definition
+ needs:
+ job: amd64-centos8-container
variables:
IMAGE: centos8
CONFIGURE_ARGS: --without-default-devices --disable-user
build-without-default-features:
<<: *native_build_job_definition
+ needs:
+ job: amd64-debian-container
variables:
IMAGE: debian-amd64
CONFIGURE_ARGS: --without-default-features --disable-user
@@ -657,6 +709,8 @@ build-without-default-features:
check-patch:
stage: build
image: $CI_REGISTRY_IMAGE/qemu/centos8:latest
+ needs:
+ job: amd64-centos8-container
script: .gitlab-ci.d/check-patch.py
except:
variables:
@@ -668,6 +722,8 @@ check-patch:
check-dco:
stage: build
image: $CI_REGISTRY_IMAGE/qemu/centos8:latest
+ needs:
+ job: amd64-centos8-container
script: .gitlab-ci.d/check-dco.py
except:
variables:
@@ -678,6 +734,8 @@ check-dco:
build-libvhost-user:
stage: build
image: $CI_REGISTRY_IMAGE/qemu/fedora:latest
+ needs:
+ job: amd64-fedora-container
before_script:
- dnf install -y meson ninja-build
script:
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs
2021-02-16 13:29 [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs Daniel P. Berrangé
` (2 preceding siblings ...)
2021-02-16 13:29 ` [PATCH v2 3/3] gitlab: fix inconsistent indentation Daniel P. Berrangé
@ 2021-02-24 11:57 ` Daniel P. Berrangé
2021-02-24 12:45 ` Thomas Huth
3 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-02-24 11:57 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Thomas Huth, Alex Bennée,
Wainer dos Santos Moschetta
ping - can we get this series merged so we reduce the chances of
stale container images for people working across multiple branches
On Tue, Feb 16, 2021 at 01:29:51PM +0000, Daniel P. Berrangé wrote:
> This series fixes a problem with our gitlab CI rules that cause
> container builds to be skipped. See the commit description in the
> first patch for the details on this problem.
>
> The overall result of this series though is a small increase in overall
> pipeline time.
>
> Previously
>
> - When container jobs are skipped: approx 1hr 5 mins
> - When container jobs are run, cached by docker: approx 1hr 15 mins
> - When container jobs are run, not cached by docker: approx 1hr 30 mins
>
> With this series applied the first scenario no longer exists, so
> all piplines are either 1hr 15 or 1hr 30 depending on whether the
> container phase is skipped.
>
> On the plus side the builds are more reliable as we're actually
> building container images at correct times.
>
> There is still a race condition though where build jobs can run
> with the wrong containers. This happens if you push two different
> branches to gitlab with different docker file content. If the
> container jobs for the 2nd branch finish before the 1st
> branch runs its build jobs, the 1st branch can end up using
> containers fro the second branch. The only fix to truely fix
> that would be to stop using "latest" docker tag and always
> use a tag based on the branch name. This would mean we build
> up a growing set of docker images in the gitlab registry.
>
> At least this series is much more correct that what exists in
> git currently. I'm intending to continue to investigate a solution
> for the remaining race condition, but don't want to block this
> series from merging, since this already solves the problem for the
> majority of contributors' usage.
>
> Changed in v2:
>
> - Set needs rules for cross jobs too
>
> Daniel P. Berrangé (3):
> gitlab: always build container images
> gitlab: add fine grained job deps for all build jobs
> gitlab: fix inconsistent indentation
>
> .gitlab-ci.d/containers.yml | 7 ----
> .gitlab-ci.d/crossbuilds.yml | 46 ++++++++++++++++++++++
> .gitlab-ci.yml | 74 ++++++++++++++++++++++++++++++++----
> 3 files changed, 112 insertions(+), 15 deletions(-)
>
> --
> 2.29.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 8+ messages in thread