[PATCH] gitlab-ci: Remove unused container images

Thomas Huth posted 1 patch 3 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210219110950.2308025-1-thuth@redhat.com
.gitlab-ci.d/containers.yml | 92 -------------------------------------
1 file changed, 92 deletions(-)
[PATCH] gitlab-ci: Remove unused container images
Posted by Thomas Huth 3 years, 2 months ago
We're building a lot of containers in the gitlab-CI that we never use.
This takes away network bandwidth and CPU time from other jobs for no
use, so let's remove them for now. The individual containers could be
re-added later when we really need them.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/containers.yml | 92 -------------------------------------
 1 file changed, 92 deletions(-)

diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index 90fac85ce4..233d34c59b 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -53,13 +53,6 @@ amd64-debian11-container:
   variables:
     NAME: debian11
 
-alpha-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-alpha-cross
-
 amd64-debian-cross-container:
   <<: *container_job_definition
   stage: containers-layer2
@@ -88,13 +81,6 @@ arm64-debian-cross-container:
   variables:
     NAME: debian-arm64-cross
 
-arm64-test-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian11-container']
-  variables:
-    NAME: debian-arm64-test-cross
-
 armel-debian-cross-container:
   <<: *container_job_definition
   stage: containers-layer2
@@ -109,27 +95,6 @@ armhf-debian-cross-container:
   variables:
     NAME: debian-armhf-cross
 
-hppa-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-hppa-cross
-
-m68k-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-m68k-cross
-
-mips64-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-mips64-cross
-
 mips64el-debian-cross-container:
   <<: *container_job_definition
   stage: containers-layer2
@@ -151,20 +116,6 @@ mipsel-debian-cross-container:
   variables:
     NAME: debian-mipsel-cross
 
-powerpc-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-powerpc-cross
-
-ppc64-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-ppc64-cross
-
 ppc64el-debian-cross-container:
   <<: *container_job_definition
   stage: containers-layer2
@@ -172,13 +123,6 @@ ppc64el-debian-cross-container:
   variables:
     NAME: debian-ppc64el-cross
 
-riscv64-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-riscv64-cross
-
 s390x-debian-cross-container:
   <<: *container_job_definition
   stage: containers-layer2
@@ -186,37 +130,6 @@ s390x-debian-cross-container:
   variables:
     NAME: debian-s390x-cross
 
-sh4-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-sh4-cross
-
-sparc64-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-sparc64-cross
-
-tricore-debian-cross-container:
-  <<: *container_job_definition
-  stage: containers-layer2
-  needs: ['amd64-debian10-container']
-  variables:
-    NAME: debian-tricore-cross
-
-xtensa-debian-cross-container:
-  <<: *container_job_definition
-  variables:
-    NAME: debian-xtensa-cross
-
-cris-fedora-cross-container:
-  <<: *container_job_definition
-  variables:
-    NAME: fedora-cris-cross
-
 amd64-fedora-container:
   <<: *container_job_definition
   variables:
@@ -237,11 +150,6 @@ win64-fedora-cross-container:
   variables:
     NAME: fedora-win64-cross
 
-amd64-ubuntu1804-container:
-  <<: *container_job_definition
-  variables:
-    NAME: ubuntu1804
-
 amd64-ubuntu2004-container:
   <<: *container_job_definition
   variables:
-- 
2.27.0


Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Philippe Mathieu-Daudé 3 years, 2 months ago
On 2/19/21 12:09 PM, Thomas Huth wrote:
> We're building a lot of containers in the gitlab-CI that we never use.
> This takes away network bandwidth and CPU time from other jobs for no
> use, so let's remove them for now. The individual containers could be
> re-added later when we really need them.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .gitlab-ci.d/containers.yml | 92 -------------------------------------
>  1 file changed, 92 deletions(-)

I'm not enthusiast with this patch because I use various in this list
from time to time for testing or cross build/disas binaries. Not having
these containers used mainstream probably show the failure of the
project to add good testing coverage on these targets. Most of them are
for hobbyist with little time. Removing them will make it even harder
to add tests. Can't we keep them disabled? Or put them in manual mode?

Why is the CI rebuilding them, shouldn't them be cached or pulled from
the registry?
Maybe this show having all them in the same containers.yml file is not
good enough? Any suggestion for splitting it, so lowly used containers
don't get rebuild every time another often used one change the YAML
file?

Thanks,

Phil.

Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Thomas Huth 3 years, 2 months ago
On 19/02/2021 13.00, Philippe Mathieu-Daudé wrote:
> On 2/19/21 12:09 PM, Thomas Huth wrote:
>> We're building a lot of containers in the gitlab-CI that we never use.
>> This takes away network bandwidth and CPU time from other jobs for no
>> use, so let's remove them for now. The individual containers could be
>> re-added later when we really need them.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   .gitlab-ci.d/containers.yml | 92 -------------------------------------
>>   1 file changed, 92 deletions(-)
> 
> I'm not enthusiast with this patch because I use various in this list
> from time to time for testing or cross build/disas binaries.

When I look at our current huge list of containers, I wonder how do we know 
which containers still get used (in the sense of not only build), and which 
ones are likely already bit-rotten? And why do we need that many containers? 
Why both, debian-arm64-test-cross.docker and debian-arm64-cross.docker and 
not combine them? And why do we need that many individual cross-compiler 
docker files if we already have debian-all-test-cross.docker that can be 
used to test most of them? ... for me, as a docker ignorant, this is all 
very opaque and some clean up IMHO could really help here.

> Not having
> these containers used mainstream probably show the failure of the
> project to add good testing coverage on these targets. Most of them are
> for hobbyist with little time. Removing them will make it even harder
> to add tests.

Do you really use the docker files from the gitlab registry? I'd rather 
expected that people build those locally in case they need them...?

> Can't we keep them disabled? Or put them in manual mode?

Well, I guess manual mode is fine, too, as long as they don't waste CPU 
cycles and network bandwidth anymore for most people who don't need them.

  Thomas


Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Alex Bennée 3 years, 2 months ago
Thomas Huth <thuth@redhat.com> writes:

> On 19/02/2021 13.00, Philippe Mathieu-Daudé wrote:
>> On 2/19/21 12:09 PM, Thomas Huth wrote:
>>> We're building a lot of containers in the gitlab-CI that we never use.
>>> This takes away network bandwidth and CPU time from other jobs for no
>>> use, so let's remove them for now. The individual containers could be
>>> re-added later when we really need them.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   .gitlab-ci.d/containers.yml | 92 -------------------------------------
>>>   1 file changed, 92 deletions(-)
>> 
>> I'm not enthusiast with this patch because I use various in this list
>> from time to time for testing or cross build/disas binaries.
>
> When I look at our current huge list of containers, I wonder how do we know 
> which containers still get used (in the sense of not only build), and which 
> ones are likely already bit-rotten? And why do we need that many containers? 
> Why both, debian-arm64-test-cross.docker and debian-arm64-cross.docker and 
> not combine them?

debian64-arm64-test-cross is based of testing because we need a newer
GCC than the QEMU cross compiler to build the SVE and MTE tests for
aarch64 check-tcg.

> And why do we need that many individual cross-compiler 
> docker files if we already have debian-all-test-cross.docker that can be 
> used to test most of them? ... for me, as a docker ignorant, this is all 
> very opaque and some clean up IMHO could really help here.

Because it's quite heavyweight for users who may only be building one or
two arches to pull down all the compilers in one image.

>
>> Not having
>> these containers used mainstream probably show the failure of the
>> project to add good testing coverage on these targets. Most of them are
>> for hobbyist with little time. Removing them will make it even harder
>> to add tests.
>
> Do you really use the docker files from the gitlab registry? I'd rather 
> expected that people build those locally in case they need them...?
>
>> Can't we keep them disabled? Or put them in manual mode?
>
> Well, I guess manual mode is fine, too, as long as they don't waste CPU 
> cycles and network bandwidth anymore for most people who don't need them.
>
>   Thomas


-- 
Alex Bennée

Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Philippe Mathieu-Daudé 3 years, 2 months ago
On 2/19/21 2:10 PM, Thomas Huth wrote:
> On 19/02/2021 13.00, Philippe Mathieu-Daudé wrote:
>> On 2/19/21 12:09 PM, Thomas Huth wrote:
>>> We're building a lot of containers in the gitlab-CI that we never use.
>>> This takes away network bandwidth and CPU time from other jobs for no
>>> use, so let's remove them for now. The individual containers could be
>>> re-added later when we really need them.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   .gitlab-ci.d/containers.yml | 92 -------------------------------------
>>>   1 file changed, 92 deletions(-)
>>
>> I'm not enthusiast with this patch because I use various in this list
>> from time to time for testing or cross build/disas binaries.
> 
> When I look at our current huge list of containers, I wonder how do we
> know which containers still get used (in the sense of not only build),
> and which ones are likely already bit-rotten? And why do we need that
> many containers? Why both, debian-arm64-test-cross.docker and
> debian-arm64-cross.docker and not combine them? And why do we need that
> many individual cross-compiler docker files if we already have
> debian-all-test-cross.docker that can be used to test most of them? ...
> for me, as a docker ignorant, this is all very opaque and some clean up
> IMHO could really help here.

debian-arm64-cross.docker is to cross-build QEMU, while
debian-arm64-test-cross.docker to cross-build the TCG tests.

debian-arm64-test-cross.docker could probably now be replaced
by debian-all-test-cross.docker.

IIRC the problem we had was on aarch64 hosts many cross-build
packages were broken, so we had to restrict them to the bare
minimal to be able to cross-build the TCG tests there.

>> Not having
>> these containers used mainstream probably show the failure of the
>> project to add good testing coverage on these targets. Most of them are
>> for hobbyist with little time. Removing them will make it even harder
>> to add tests.
> 
> Do you really use the docker files from the gitlab registry? I'd rather
> expected that people build those locally in case they need them...?

TBH I pull from registry 99% of the time. I only build locally if I the
mainstream image is missing something, and I want to add what is missing
in a patch. But even there once finished I pull from my namespace
registry and test with that image, as this is what other will use too.
I want to use the same images from our registry, not my local ones.

>> Can't we keep them disabled? Or put them in manual mode?
> 
> Well, I guess manual mode is fine, too, as long as they don't waste CPU
> cycles and network bandwidth anymore for most people who don't need them.
> 
>  Thomas
> 
> 

Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Alex Bennée 3 years, 2 months ago
Thomas Huth <thuth@redhat.com> writes:

> We're building a lot of containers in the gitlab-CI that we never use.
> This takes away network bandwidth and CPU time from other jobs for no
> use, so let's remove them for now. The individual containers could be
> re-added later when we really need them.

They are used - when people run check-tcg their builds are cached via
gitlab.

>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .gitlab-ci.d/containers.yml | 92 -------------------------------------
>  1 file changed, 92 deletions(-)
>
> diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
> index 90fac85ce4..233d34c59b 100644
> --- a/.gitlab-ci.d/containers.yml
> +++ b/.gitlab-ci.d/containers.yml
> @@ -53,13 +53,6 @@ amd64-debian11-container:
>    variables:
>      NAME: debian11
>  
> -alpha-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-alpha-cross
> -
>  amd64-debian-cross-container:
>    <<: *container_job_definition
>    stage: containers-layer2
> @@ -88,13 +81,6 @@ arm64-debian-cross-container:
>    variables:
>      NAME: debian-arm64-cross
>  
> -arm64-test-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian11-container']
> -  variables:
> -    NAME: debian-arm64-test-cross
> -
>  armel-debian-cross-container:
>    <<: *container_job_definition
>    stage: containers-layer2
> @@ -109,27 +95,6 @@ armhf-debian-cross-container:
>    variables:
>      NAME: debian-armhf-cross
>  
> -hppa-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-hppa-cross
> -
> -m68k-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-m68k-cross
> -
> -mips64-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-mips64-cross
> -
>  mips64el-debian-cross-container:
>    <<: *container_job_definition
>    stage: containers-layer2
> @@ -151,20 +116,6 @@ mipsel-debian-cross-container:
>    variables:
>      NAME: debian-mipsel-cross
>  
> -powerpc-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-powerpc-cross
> -
> -ppc64-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-ppc64-cross
> -
>  ppc64el-debian-cross-container:
>    <<: *container_job_definition
>    stage: containers-layer2
> @@ -172,13 +123,6 @@ ppc64el-debian-cross-container:
>    variables:
>      NAME: debian-ppc64el-cross
>  
> -riscv64-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-riscv64-cross
> -
>  s390x-debian-cross-container:
>    <<: *container_job_definition
>    stage: containers-layer2
> @@ -186,37 +130,6 @@ s390x-debian-cross-container:
>    variables:
>      NAME: debian-s390x-cross
>  
> -sh4-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-sh4-cross
> -
> -sparc64-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-sparc64-cross
> -
> -tricore-debian-cross-container:
> -  <<: *container_job_definition
> -  stage: containers-layer2
> -  needs: ['amd64-debian10-container']
> -  variables:
> -    NAME: debian-tricore-cross
> -
> -xtensa-debian-cross-container:
> -  <<: *container_job_definition
> -  variables:
> -    NAME: debian-xtensa-cross
> -
> -cris-fedora-cross-container:
> -  <<: *container_job_definition
> -  variables:
> -    NAME: fedora-cris-cross
> -
>  amd64-fedora-container:
>    <<: *container_job_definition
>    variables:
> @@ -237,11 +150,6 @@ win64-fedora-cross-container:
>    variables:
>      NAME: fedora-win64-cross
>  
> -amd64-ubuntu1804-container:
> -  <<: *container_job_definition
> -  variables:
> -    NAME: ubuntu1804
> -
>  amd64-ubuntu2004-container:
>    <<: *container_job_definition
>    variables:


-- 
Alex Bennée

Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Philippe Mathieu-Daudé 3 years, 2 months ago
On 2/20/21 10:10 PM, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> We're building a lot of containers in the gitlab-CI that we never use.

Maybe we should run the check-tcg tests for each target on Gitlab?

>> This takes away network bandwidth and CPU time from other jobs for no
>> use, so let's remove them for now. The individual containers could be
>> re-added later when we really need them.
> 
> They are used - when people run check-tcg their builds are cached via
> gitlab.

Indeed:

$ git grep container_image
tests/tcg/configure.sh:107:  container_image=
tests/tcg/configure.sh:111:      container_image=debian-arm64-test-cross
tests/tcg/configure.sh:115:      container_image=debian-alpha-cross
tests/tcg/configure.sh:120:      container_image=debian-armhf-cross
tests/tcg/configure.sh:124:      container_image=fedora-cris-cross
tests/tcg/configure.sh:128:      container_image=debian-hppa-cross
tests/tcg/configure.sh:132:      container_image=fedora-i386-cross
tests/tcg/configure.sh:136:      container_image=debian-m68k-cross
tests/tcg/configure.sh:140:      container_image=debian-mips64el-cross
tests/tcg/configure.sh:144:      container_image=debian-mips64-cross
tests/tcg/configure.sh:148:      container_image=debian-mipsel-cross
tests/tcg/configure.sh:152:      container_image=debian-mips-cross
tests/tcg/configure.sh:156:      container_image=debian-powerpc-cross
tests/tcg/configure.sh:160:      container_image=debian-ppc64-cross
tests/tcg/configure.sh:164:      container_image=debian-ppc64el-cross
tests/tcg/configure.sh:168:      container_image=debian-riscv64-cross
tests/tcg/configure.sh:172:      container_image=debian-s390x-cross
tests/tcg/configure.sh:176:      container_image=debian-sh4-cross
tests/tcg/configure.sh:180:      container_image=debian-sparc64-cross
tests/tcg/configure.sh:184:      container_image=debian-xtensa-cross
tests/tcg/configure.sh:261:  if test $got_cross_cc = no && test
"$container" != no && test -n "$container_image"; then
tests/tcg/configure.sh:262:    echo "DOCKER_IMAGE=$container_image" >>
$config_target_mak

> 
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  .gitlab-ci.d/containers.yml | 92 -------------------------------------
>>  1 file changed, 92 deletions(-)

Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Thomas Huth 3 years, 2 months ago
On 20/02/2021 22.10, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> We're building a lot of containers in the gitlab-CI that we never use.
>> This takes away network bandwidth and CPU time from other jobs for no
>> use, so let's remove them for now. The individual containers could be
>> re-added later when we really need them.
> 
> They are used - when people run check-tcg their builds are cached via
> gitlab.

Oh, I didn't know that. But where's the magic that tells the tcg tests to 
use the registry from gitlab?
"grep -ri gitlab tests/tcg" does not show any results?
Also some documentation would be helpful, docs/devel/testing.rst does not 
talk about gitlab at all...?

  Thomas


Re: [PATCH] gitlab-ci: Remove unused container images
Posted by Alex Bennée 3 years, 2 months ago
Thomas Huth <thuth@redhat.com> writes:

> On 20/02/2021 22.10, Alex Bennée wrote:
>> 
>> Thomas Huth <thuth@redhat.com> writes:
>> 
>>> We're building a lot of containers in the gitlab-CI that we never use.
>>> This takes away network bandwidth and CPU time from other jobs for no
>>> use, so let's remove them for now. The individual containers could be
>>> re-added later when we really need them.
>> 
>> They are used - when people run check-tcg their builds are cached via
>> gitlab.
>
> Oh, I didn't know that. But where's the magic that tells the tcg tests to 
> use the registry from gitlab?
> "grep -ri gitlab tests/tcg" does not show any results?

It's in the docker makefiles:

  DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),registry.gitlab.com/qemu-project/qemu)

which are dependencies for the appropriate tests.  

> Also some documentation would be helpful, docs/devel/testing.rst does not 
> talk about gitlab at all...?

I'll see if I can improve that.

>
>   Thomas


-- 
Alex Bennée