[RFC PATCH v2 0/3] ci: Speed up container stage

[RFC PATCH v2 0/3] ci: Speed up container stage

[Fabiano Rosas]

Turns out we were already pulling the image, but for forks we were
pulling from a different registry and for the main project we had
caching disabled due to a bug with docker/gitlab.

I've now replaced the usage of docker.py with a plain docker command
and reintroduced the caching.

I also removed the COMMON_TAG which was unused.

v1:
https://lore.kernel.org/r/20230223142154.31975-1-farosas@suse.de

I'm not sure if this was discussed previously, but I noticed we're not
pulling the images we push to the registry at every pipeline run.

I would expect we don't actually need to rebuild container images at
_every_ pipeline run, so I propose we add a "docker pull" to the
container templates. We already have that for the docker-edk2|opensbi
images.

Some containers can take a long time to build (14 mins) and pulling
the image first without building can cut the time to about 3
mins. With this we can save almost 2h of cumulative CI time per
pipeline run:

| master   | pull-only |  diff    | container
| 0:02:34  | 0:02:09   | 00:00:25 |  alpha-debian-cross-container
| 0:04:45  | 0:02:40   | 00:02:05 |  amd64-alpine-container
| 0:09:51  | 0:02:56   | 00:06:55 |  amd64-centos8-container
| 0:07:21  | 0:02:49   | 00:04:32 |  amd64-debian-container
| 0:06:00  | 0:02:37   | 00:03:23 |  amd64-debian-cross-container
| 0:14:22  | 0:03:41   | 00:10:41 |  amd64-debian-user-cross-container
| 0:10:14  | 0:03:24   | 00:06:50 |  amd64-fedora-container
| 0:12:09  | 0:02:49   | 00:09:20 |  amd64-opensuse-leap-container
| 0:07:33  | 0:02:45   | 00:04:48 |  amd64-ubuntu2004-container
| 0:08:28  | 0:03:07   | 00:05:21 |  arm64-debian-cross-container
| 0:04:27  | 0:02:58   | 00:01:29 |  armel-debian-cross-container
| 0:08:01  | 0:02:55   | 00:05:06 |  armhf-debian-cross-container
| 0:03:33  | 0:02:18   | 00:01:15 |  cris-fedora-cross-container
| 0:00:28  | 0:00:28   | 00:00:00 |  docker-edk2
| 0:00:25  | 0:00:28   |-00:00:03 |  docker-opensbi
| 0:08:34  | 0:03:10   | 00:05:24 |  hexagon-cross-container
| 0:02:34  | 0:02:08   | 00:00:26 |  hppa-debian-cross-container
| 0:04:50  | 0:02:28   | 00:02:22 |  i386-fedora-cross-container
| 0:02:36  | 0:02:12   | 00:00:24 |  m68k-debian-cross-container
| 0:02:40  | 0:02:09   | 00:00:31 |  mips-debian-cross-container
| 0:02:38  | 0:02:09   | 00:00:29 |  mips64-debian-cross-container
| 0:04:28  | 0:02:48   | 00:01:40 |  mips64el-debian-cross-container
| 0:07:07  | 0:02:46   | 00:04:21 |  mipsel-debian-cross-container
| 0:03:51  | 0:02:21   | 00:01:30 |  powerpc-test-cross-container
| 0:08:52  | 0:03:00   | 00:05:52 |  ppc64el-debian-cross-container
| 0:06:07  | 0:02:49   | 00:03:18 |  python-container
| 0:04:37  | 0:02:26   | 00:02:11 |  riscv64-debian-cross-container
| 0:02:39  | 0:02:08   | 00:00:31 |  riscv64-debian-test-cross-container
| 0:08:03  | 0:03:00   | 00:05:03 |  s390x-debian-cross-container
| 0:02:34  | 0:02:08   | 00:00:26 |  sh4-debian-cross-container
| 0:02:37  | 0:02:09   | 00:00:28 |  sparc64-debian-cross-container
| 0:04:25  | 0:02:17   | 00:02:08 |  tricore-debian-cross-container
| 0:12:51  | 0:03:27   | 00:09:24 |  win32-fedora-cross-container
| 0:11:16  | 0:03:29   | 00:07:47 |  win64-fedora-cross-container
| 0:03:28  | 0:02:20   | 00:01:08 |  xtensa-debian-cross-container
                       | 01:57:30 |

We would need to devise a mechanism (not included here) to force the
re-build of the container images when needed, perhaps an environment
variable or even a whole new "container build" stage before the
"container" stage.

What do you think?

Fabiano Rosas (3):
  gitlab: Use plain docker in container-template.yml
  gitlab: Cache container images
  gitlab: Remove COMMON_TAG from container-template.yml

 .gitlab-ci.d/container-template.yml | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.35.3

[RFC PATCH v2 1/3] gitlab: Use plain docker in container-template.yml

[Fabiano Rosas]

Our dockerfiles no longer reference layers from other qemu images so
we can now use 'docker build' on them.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 .gitlab-ci.d/container-template.yml | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
index c434b9c8f3..f417452212 100644
--- a/.gitlab-ci.d/container-template.yml
+++ b/.gitlab-ci.d/container-template.yml
@@ -13,10 +13,7 @@
   script:
     - echo "TAG:$TAG"
     - echo "COMMON_TAG:$COMMON_TAG"
-    - ./tests/docker/docker.py --engine docker build
-          -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker"
-          -r $CI_REGISTRY/qemu-project/qemu
-    - docker tag "qemu/$NAME" "$TAG"
+    - docker build --tag $TAG -f "tests/docker/dockerfiles/$NAME.docker" "."
     - docker push "$TAG"
   after_script:
     - docker logout
-- 
2.35.3

[RFC PATCH v2 2/3] gitlab: Cache container images

[Fabiano Rosas]

Make use of the --cache-from option from 'docker build' by including
build layers when building and then pulling a previously built image
before the next build.

This was previously done by the docker.py script, but got disabled due
to bad interactions with certain runners. See commit 6ddc3dc7a8
("tests/docker: don't use BUILDKIT in GitLab either").

We now believe those issues to be fixed, so restore the caching
functionality as it brings a significant reduction in container build
times.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 .gitlab-ci.d/container-template.yml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
index f417452212..31e4e36a7d 100644
--- a/.gitlab-ci.d/container-template.yml
+++ b/.gitlab-ci.d/container-template.yml
@@ -13,7 +13,10 @@
   script:
     - echo "TAG:$TAG"
     - echo "COMMON_TAG:$COMMON_TAG"
-    - docker build --tag $TAG -f "tests/docker/dockerfiles/$NAME.docker" "."
+    - docker pull "$TAG" || true
+    - docker build --tag "$TAG" --cache-from "$TAG"
+      --build-arg BUILDKIT_INLINE_CACHE=1
+      -f "tests/docker/dockerfiles/$NAME.docker" "."
     - docker push "$TAG"
   after_script:
     - docker logout
-- 
2.35.3

[RFC PATCH v2 3/3] gitlab: Remove COMMON_TAG from container-template.yml

[Fabiano Rosas]

We're not using the COMMON_TAG and we cannot have a push to a
different registry than we pull from because that would get in the way
of forks testing their changes to a dockerfile.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 .gitlab-ci.d/container-template.yml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
index 31e4e36a7d..e7c1fa1522 100644
--- a/.gitlab-ci.d/container-template.yml
+++ b/.gitlab-ci.d/container-template.yml
@@ -6,13 +6,11 @@
     - docker:dind
   before_script:
     - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
-    - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/$NAME:latest"
     - apk add python3
     - docker info
     - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"
   script:
     - echo "TAG:$TAG"
-    - echo "COMMON_TAG:$COMMON_TAG"
     - docker pull "$TAG" || true
     - docker build --tag "$TAG" --cache-from "$TAG"
       --build-arg BUILDKIT_INLINE_CACHE=1
-- 
2.35.3

Re: [RFC PATCH v2 2/3] gitlab: Cache container images

[Daniel P. Berrangé]

On Fri, Feb 24, 2023 at 09:52:06AM -0300, Fabiano Rosas wrote:
> Make use of the --cache-from option from 'docker build' by including
> build layers when building and then pulling a previously built image
> before the next build.
> 
> This was previously done by the docker.py script, but got disabled due
> to bad interactions with certain runners. See commit 6ddc3dc7a8
> ("tests/docker: don't use BUILDKIT in GitLab either").
> 
> We now believe those issues to be fixed, so restore the caching
> functionality as it brings a significant reduction in container build
> times.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  .gitlab-ci.d/container-template.yml | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
> index f417452212..31e4e36a7d 100644
> --- a/.gitlab-ci.d/container-template.yml
> +++ b/.gitlab-ci.d/container-template.yml
> @@ -13,7 +13,10 @@
>    script:
>      - echo "TAG:$TAG"
>      - echo "COMMON_TAG:$COMMON_TAG"
> -    - docker build --tag $TAG -f "tests/docker/dockerfiles/$NAME.docker" "."
> +    - docker pull "$TAG" || true
> +    - docker build --tag "$TAG" --cache-from "$TAG"

We should have

   --cache-from "$TAG" --cache-from "$COMMON_TAG"

The reason is that users may not keep their branches up2date on a regular
basis. As such the registry associated with the fork may not be useful
as a caching source. Thus if we include cache from the upstream repo, this
will benefit forks.  This was the rational behind having TAG + COMMON_TAG
originally, but we lost it somewhere along the way.

> +      --build-arg BUILDKIT_INLINE_CACHE=1
> +      -f "tests/docker/dockerfiles/$NAME.docker" "."
>      - docker push "$TAG"
>    after_script:
>      - docker logout

With 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 :|

Re: [RFC PATCH v2 1/3] gitlab: Use plain docker in container-template.yml

[Daniel P. Berrangé]

On Fri, Feb 24, 2023 at 09:52:05AM -0300, Fabiano Rosas wrote:
> Our dockerfiles no longer reference layers from other qemu images so
> we can now use 'docker build' on them.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  .gitlab-ci.d/container-template.yml | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml
> index c434b9c8f3..f417452212 100644
> --- a/.gitlab-ci.d/container-template.yml
> +++ b/.gitlab-ci.d/container-template.yml
> @@ -13,10 +13,7 @@
>    script:
>      - echo "TAG:$TAG"
>      - echo "COMMON_TAG:$COMMON_TAG"
> -    - ./tests/docker/docker.py --engine docker build
> -          -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker"
> -          -r $CI_REGISTRY/qemu-project/qemu
> -    - docker tag "qemu/$NAME" "$TAG"
> +    - docker build --tag $TAG -f "tests/docker/dockerfiles/$NAME.docker" "."
>      - docker push "$TAG"

I think I wouldn't separate this from patch 2, because in effect
we're throwing away caching here, and then re-adding it in the
next patch. IOW, the removal of usage of docker.py requires both
patches combined.


With 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 :|