All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs
@ 2021-02-16 13:29 Daniel P. Berrangé
  2021-02-16 13:29 ` [PATCH v2 1/3] gitlab: always build container images Daniel P. Berrangé
                   ` (3 more replies)
  0 siblings, 4 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

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




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [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

* [PATCH v2 3/3] gitlab: fix inconsistent indentation
  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 ` [PATCH v2 2/3] gitlab: add fine grained job deps for all build jobs Daniel P. Berrangé
@ 2021-02-16 13:29 ` 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, 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

The standard is to use 2 space indent, not 3.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .gitlab-ci.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 32cc6bd4a2..870d5f83f5 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -282,14 +282,14 @@ check-system-opensuse:
     MAKE_CHECK_ARGS: check
 
 acceptance-system-opensuse:
-   <<: *native_test_job_definition
-   needs:
-     - job: build-system-opensuse
-       artifacts: true
-   variables:
-     IMAGE: opensuse-leap
-     MAKE_CHECK_ARGS: check-acceptance
-   <<: *acceptance_definition
+  <<: *native_test_job_definition
+  needs:
+    - job: build-system-opensuse
+      artifacts: true
+  variables:
+    IMAGE: opensuse-leap
+    MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
 
 
 build-disabled:
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] gitlab: add fine grained job deps for all build jobs
  2021-02-16 13:29 ` [PATCH v2 2/3] gitlab: add fine grained job deps for all build jobs Daniel P. Berrangé
@ 2021-02-16 13:33   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-16 13:33 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel
  Cc: Thomas Huth, Alex Bennée, Wainer dos Santos Moschetta

On 2/16/21 2:29 PM, Daniel P. Berrangé wrote:
> 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(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[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

* Re: [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs
  2021-02-24 11:57 ` [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs Daniel P. Berrangé
@ 2021-02-24 12:45   ` Thomas Huth
  2021-02-24 12:47     ` Daniel P. Berrangé
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2021-02-24 12:45 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Alex Bennée, Wainer dos Santos Moschetta

On 24/02/2021 12.57, Daniel P. Berrangé wrote:
> ping - can we get this series merged so we reduce the chances of
> stale container images for people working across multiple branches

It's in already:

  https://gitlab.com/qemu-project/qemu/-/commit/c31fa24e9690ef62b

You should also have gotten the PULL mail on CC: ? Did it get lost?

  Thomas



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] fix build failures from incorrectly skipped container build jobs
  2021-02-24 12:45   ` Thomas Huth
@ 2021-02-24 12:47     ` Daniel P. Berrangé
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-02-24 12:47 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Philippe Mathieu-Daudé,
	Alex Bennée, qemu-devel, Wainer dos Santos Moschetta

On Wed, Feb 24, 2021 at 01:45:10PM +0100, Thomas Huth wrote:
> On 24/02/2021 12.57, Daniel P. Berrangé wrote:
> > ping - can we get this series merged so we reduce the chances of
> > stale container images for people working across multiple branches
> 
> It's in already:
> 
>  https://gitlab.com/qemu-project/qemu/-/commit/c31fa24e9690ef62b
> 
> You should also have gotten the PULL mail on CC: ? Did it get lost?

Doh, sorry for the noise. I am indeed behind on my branch.

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

end of thread, other threads:[~2021-02-24 12:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH v2 2/3] gitlab: add fine grained job deps for all build jobs 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é
2021-02-24 12:45   ` Thomas Huth
2021-02-24 12:47     ` Daniel P. Berrangé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.