[PATCH 0/8] Patch series to set up a ppc64le CI

[PATCH 0/8] Patch series to set up a ppc64le CI

[Lucas Mateus Castro(alqotel)]

This patch series aim to make easier to set up a compilation and CI
environment in PPC64 and PPC64LE machines.

The first 2 patches is a fix not related to ppc64.
Patch 3 and 4 also affect some other architectures.
Patches 5 to 7 are adding Power specific additions.

Patch 8 is a RFC for a current way to run the docker tests in PPC64LE.

Lucas Mateus Castro(alqotel) (8):
  tests/docker: Fix alpine dockerfile
  scripts/ci/setup: ninja missing from build-environment
  scripts/ci/setup: Fix libxen requirements
  scripts/ci/setup: spice-server only on x86 aarch64
  scripts/ci/setup: Add ppc64le to vars.yml template
  scripts/ci/setup: Add Fedora to build-environment.yml
  scripts/ci/setup: Added debian to build-environment.yml
  tests/docker: Selective line reading by python script

 scripts/ci/setup/build-environment.yml | 54 +++++++++++++++++++++-----
 scripts/ci/setup/vars.yml.template     |  1 +
 tests/docker/docker.py                 | 15 +++++--
 tests/docker/dockerfiles/alpine.docker |  3 ++
 4 files changed, 61 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH 1/8] tests/docker: Fix alpine dockerfile

[Lucas Mateus Castro(alqotel)]

Currently the run script uses 'readlink -e' but the image only has the
busybox readlink, this commit add the coreutils package which
contains the readlink with the '-e' option.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 tests/docker/dockerfiles/alpine.docker | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
index 3f4c0f95cb..2943a99730 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -21,6 +21,7 @@ RUN apk update && \
         cdrkit \
         ceph-dev \
         clang \
+        coreutils \
         ctags \
         curl-dev \
         cyrus-sasl-dev \
-- 
2.25.1

Re: [PATCH 1/8] tests/docker: Fix alpine dockerfile

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 01:36:25PM -0300, Lucas Mateus Castro(alqotel) wrote:
> Currently the run script uses 'readlink -e' but the image only has the
> busybox readlink, this commit add the coreutils package which
> contains the readlink with the '-e' option.

Use of 'readlink' is discouraged in favour of 'realpath'. AFAICT, we
can just do that change and not need the '-e' flag anyway.

> 
> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
> ---
>  tests/docker/dockerfiles/alpine.docker | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> index 3f4c0f95cb..2943a99730 100644
> --- a/tests/docker/dockerfiles/alpine.docker
> +++ b/tests/docker/dockerfiles/alpine.docker
> @@ -21,6 +21,7 @@ RUN apk update && \
>          cdrkit \
>          ceph-dev \
>          clang \
> +        coreutils \
>          ctags \
>          curl-dev \
>          cyrus-sasl-dev \

This file contents is autogenerated, so editting it manually is
wrong and changes will be lost.


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: [PATCH 1/8] tests/docker: Fix alpine dockerfile

[Lucas Mateus Martins Araujo e Castro]

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]


On 27/07/2022 14:09, Daniel P. Berrangé wrote:

> On Wed, Jul 27, 2022 at 01:36:25PM -0300, Lucas Mateus Castro(alqotel) wrote:
>> Currently the run script uses 'readlink -e' but the image only has the
>> busybox readlink, this commit add the coreutils package which
>> contains the readlink with the '-e' option.
> Use of 'readlink' is discouraged in favour of 'realpath'. AFAICT, we
> can just do that change and not need the '-e' flag anyway.
So a patch just changing
-BASE="$(dirname $(readlink -e $0))"
+BASE="$(dirname $(realpath $0))"

Ok, I'll send it with v2.

>
>> Signed-off-by: Lucas Mateus Castro(alqotel)<lucas.araujo@eldorado.org.br>
>> ---
>>   tests/docker/dockerfiles/alpine.docker | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
>> index 3f4c0f95cb..2943a99730 100644
>> --- a/tests/docker/dockerfiles/alpine.docker
>> +++ b/tests/docker/dockerfiles/alpine.docker
>> @@ -21,6 +21,7 @@ RUN apk update && \
>>           cdrkit \
>>           ceph-dev \
>>           clang \
>> +        coreutils \
>>           ctags \
>>           curl-dev \
>>           cyrus-sasl-dev \
> This file contents is autogenerated, so editting it manually is
> wrong and changes will be lost.

True, that was one of the reasons I had problems with patch 8 (it 
changes the dockerfiles directly) but forgot that it'd apply here as well.

Thanks,
-- 
Lucas Mateus M. Araujo e Castro
Instituto de Pesquisas ELDORADO 
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

[-- Attachment #2: Type: text/html, Size: 2823 bytes --]

Re: [PATCH 1/8] tests/docker: Fix alpine dockerfile

[Thomas Huth]

On 27/07/2022 18.36, Lucas Mateus Castro(alqotel) wrote:
> Currently the run script uses 'readlink -e' but the image only has the
> busybox readlink, this commit add the coreutils package which
> contains the readlink with the '-e' option.
> 
> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
> ---
>   tests/docker/dockerfiles/alpine.docker | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> index 3f4c0f95cb..2943a99730 100644
> --- a/tests/docker/dockerfiles/alpine.docker
> +++ b/tests/docker/dockerfiles/alpine.docker
> @@ -21,6 +21,7 @@ RUN apk update && \
>           cdrkit \
>           ceph-dev \
>           clang \
> +        coreutils \
>           ctags \
>           curl-dev \
>           cyrus-sasl-dev \

Not a good idea. If you look at the top of the file, you can see:

# THIS FILE WAS AUTO-GENERATED

So your modifications will be overwritten the next time someone runs the 
lcitool.

I guess you'd need to modify tests/lcitool/projects/qemu.yml instead? Daniel?

Anyway, it might be better to fix the part that uses "readlink -e" ... Seems 
like busybox' readlink supports -f at least, so maybe it's enough to switch 
to -f instead of -e ?

  Thomas

Re: [PATCH 1/8] tests/docker: Fix alpine dockerfile

[Thomas Huth]

On 28/07/2022 08.52, Thomas Huth wrote:
> On 27/07/2022 18.36, Lucas Mateus Castro(alqotel) wrote:
>> Currently the run script uses 'readlink -e' but the image only has the
>> busybox readlink, this commit add the coreutils package which
>> contains the readlink with the '-e' option.
>>
>> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
>> ---
>>   tests/docker/dockerfiles/alpine.docker | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/docker/dockerfiles/alpine.docker 
>> b/tests/docker/dockerfiles/alpine.docker
>> index 3f4c0f95cb..2943a99730 100644
>> --- a/tests/docker/dockerfiles/alpine.docker
>> +++ b/tests/docker/dockerfiles/alpine.docker
>> @@ -21,6 +21,7 @@ RUN apk update && \
>>           cdrkit \
>>           ceph-dev \
>>           clang \
>> +        coreutils \
>>           ctags \
>>           curl-dev \
>>           cyrus-sasl-dev \
> 
> Not a good idea. If you look at the top of the file, you can see:
> 
> # THIS FILE WAS AUTO-GENERATED
> 
> So your modifications will be overwritten the next time someone runs the 
> lcitool.
> 
> I guess you'd need to modify tests/lcitool/projects/qemu.yml instead? Daniel?

Never mind, my e-mail program messed up the threading again, so I did not 
see that Daniel already replied.

  Thomas

[PATCH 2/8] scripts/ci/setup: ninja missing from build-environment

[Lucas Mateus Castro(alqotel)]

ninja-build is missing from the RHEL environment, so a system prepared
with that script would still fail to compile QEMU.
Tested on a Fedora 36

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 scripts/ci/setup/build-environment.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
index 232525b91d..6df3e61d94 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -150,6 +150,7 @@
           - libepoxy-devel
           - libgcrypt-devel
           - lzo-devel
+          - ninja-build
           - make
           - mesa-libEGL-devel
           - nettle-devel
-- 
2.25.1

[PATCH 3/8] scripts/ci/setup: Fix libxen requirements

[Lucas Mateus Castro(alqotel)]

XEN hypervisor is only available in ARM and x86, but the yaml only
checked if the architecture is different from s390x, changed it to
a more accurate test.
Tested this change on a Ubuntu 20.04 ppc64le.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 scripts/ci/setup/build-environment.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
index 6df3e61d94..7535228685 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -97,7 +97,7 @@
         state: present
       when:
         - ansible_facts['distribution'] == 'Ubuntu'
-        - ansible_facts['architecture'] != 's390x'
+        - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
 
     - name: Install basic packages to build QEMU on Ubuntu 20.04
       package:
-- 
2.25.1

Re: [PATCH 3/8] scripts/ci/setup: Fix libxen requirements

[Alex Bennée]

"Lucas Mateus Castro(alqotel)" <lucas.araujo@eldorado.org.br> writes:

> XEN hypervisor is only available in ARM and x86, but the yaml only
> checked if the architecture is different from s390x, changed it to
> a more accurate test.
> Tested this change on a Ubuntu 20.04 ppc64le.
>
> Signed-off-by: Lucas Mateus Castro(alqotel)
> <lucas.araujo@eldorado.org.br>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH 4/8] scripts/ci/setup: spice-server only on x86 aarch64

[Lucas Mateus Castro(alqotel)]

Changed build-environment.yml to only install spice-server on x86_64 and
aarch64 as this package is only available on those architectures.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 scripts/ci/setup/build-environment.yml | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
index 7535228685..43cf8c759f 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -160,7 +160,6 @@
           - python36
           - rdma-core-devel
           - spice-glib-devel
-          - spice-server
           - systemtap-sdt-devel
           - tar
           - zlib-devel
@@ -168,3 +167,14 @@
       when:
         - ansible_facts['distribution_file_variety'] == 'RedHat'
         - ansible_facts['distribution_version'] == '8'
+
+    - name: Install packages only available on x86 and aarch64
+      dnf:
+        # Spice server not available in ppc64le
+        name:
+          - spice-server
+        state: present
+      when:
+        - ansible_facts['distribution_file_variety'] == 'RedHat'
+        - ansible_facts['distribution_version'] == '8'
+        - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
-- 
2.25.1

[PATCH 5/8] scripts/ci/setup: Add ppc64le to vars.yml template

[Lucas Mateus Castro(alqotel)]

Added ppc64le so that the gitlab-runner.yml could be used to set up
ppc64le runners.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 scripts/ci/setup/vars.yml.template | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/ci/setup/vars.yml.template b/scripts/ci/setup/vars.yml.template
index e48089761f..2c84698b87 100644
--- a/scripts/ci/setup/vars.yml.template
+++ b/scripts/ci/setup/vars.yml.template
@@ -8,5 +8,6 @@ ansible_to_gitlab_arch:
   x86_64: amd64
   aarch64: arm64
   s390x: s390x
+  ppc64le: ppc64le
 # A unique token made available by GitLab to your project for registering runners
 gitlab_runner_registration_token: PLEASE_PROVIDE_A_VALID_TOKEN
-- 
2.25.1

[PATCH 6/8] scripts/ci/setup: Add Fedora to build-environment.yml

[Lucas Mateus Castro(alqotel)]

Minicloud doesn't have a RHEL image, but it does have Fedora 34 and 35
images and both use DNF as package manager, so just change the ansible facts
to check if it's RHEL or Fedora

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 scripts/ci/setup/build-environment.yml | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
index 43cf8c759f..a7d53d0f70 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -165,8 +165,10 @@
           - zlib-devel
         state: present
       when:
-        - ansible_facts['distribution_file_variety'] == 'RedHat'
-        - ansible_facts['distribution_version'] == '8'
+        - |
+           (ansible_facts['distribution'] == 'RedHat' and
+            ansible_facts['distribution_version'] == '8') or
+            ansible_facts['distribution'] == 'Fedora'
 
     - name: Install packages only available on x86 and aarch64
       dnf:
@@ -175,6 +177,8 @@
           - spice-server
         state: present
       when:
-        - ansible_facts['distribution_file_variety'] == 'RedHat'
-        - ansible_facts['distribution_version'] == '8'
+        - |
+           (ansible_facts['distribution'] == 'RedHat' and
+            ansible_facts['distribution_version'] == '8') or
+            ansible_facts['distribution'] == 'Fedora'
         - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
-- 
2.25.1

[PATCH 7/8] scripts/ci/setup: Added debian to build-environment.yml

[Lucas Mateus Castro(alqotel)]

Minicloud has a PPC64 BE Debian11 image which can be used for the CI,
so add Debian to the build-environment.yml so it can be configured with
ansible-playbook.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 scripts/ci/setup/build-environment.yml | 31 +++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
index a7d53d0f70..b5d415496f 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -31,9 +31,11 @@
         update_cache: yes
         upgrade: yes
       when:
-        - ansible_facts['distribution'] == 'Ubuntu'
+        - |
+            ansible_facts['distribution'] == 'Ubuntu' or
+            ansible_facts['distribution'] == 'Debian'
 
-    - name: Install basic packages to build QEMU on Ubuntu 20.04
+    - name: Install basic packages to build QEMU on Ubuntu 20.04 or Debian11
       package:
         name:
           - ccache
@@ -56,7 +58,6 @@
           - libibverbs-dev
           - libiscsi-dev
           - libjemalloc-dev
-          - libjpeg-turbo8-dev
           - liblzo2-dev
           - libncurses5-dev
           - libncursesw5-dev
@@ -86,17 +87,37 @@
           - sparse
           - xfslibs-dev
         state: present
+      when:
+        - |
+            ansible_facts['distribution'] == 'Ubuntu' or
+            ansible_facts['distribution'] == 'Debian'
+
+    - name: Install Ubuntu exclusive packages to build QEMU
+      package:
+        name:
+          - libjpeg-turbo8-dev
+        state: present
       when:
         - ansible_facts['distribution'] == 'Ubuntu'
 
-    - name: Install packages to build QEMU on Ubuntu 20.04 on non-s390x
+    - name: Install Debian exclusive packages to build QEMU
+      package:
+        name:
+          - libjpeg62-turbo-dev
+        state: present
+      when:
+        - ansible_facts['distribution'] == 'Debian'
+
+    - name: Install packages to build QEMU on Ubuntu 20.04 or Debian11 on non-s390x
       package:
         name:
           - libspice-server-dev
           - libxen-dev
         state: present
       when:
-        - ansible_facts['distribution'] == 'Ubuntu'
+        - |
+            ansible_facts['distribution'] == 'Ubuntu' or
+            ansible_facts['distribution'] == 'Debian'
         - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
 
     - name: Install basic packages to build QEMU on Ubuntu 20.04
-- 
2.25.1

[RFC PATCH 8/8] tests/docker: Selective line reading by python script

[Lucas Mateus Castro(alqotel)]

Building some images failed on ppc64le because the dockerfile tried to
install some packages that are only available in x86 and arm64, to solve
this while still having those packages be available in those architectures
a comment was put before the installation command to instruct the python
script into ignoring those lines for some architectures (in this case
ppc64le)

Overall I'm not a big fan of the way I solved this problem, so I'd like
to know if anyone has a better way to make these dockerfilse work in
PPC64LE.

For context the base images used here are available in PPC64LE but some
of the packages installed are not (in alpine's case it's XEN, which is
only available to x86 and ARM), so this patch create a ignore_list which
is set on a per-architecture basis, and any packages in a dockerfile in
this ignore_list will not be copied to the temporary dockerfile used in
the docker command.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 tests/docker/docker.py                 | 15 ++++++++++++---
 tests/docker/dockerfiles/alpine.docker |  2 ++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index d0af2861b8..9b962d1c78 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -14,6 +14,7 @@
 import os
 import sys
 import subprocess
+import platform
 import json
 import hashlib
 import atexit
@@ -207,8 +208,15 @@ def _read_qemu_dockerfile(img_name):
 
 def _dockerfile_preprocess(df):
     out = ""
+    ignore_list = []
     for l in df.splitlines():
-        if len(l.strip()) == 0 or l.startswith("#"):
+        if len(l.strip()) == 0:
+            continue
+        if l.startswith("#"):
+            if len(l.split()) >= 3:
+                if l.split()[1] == "ignore":
+                    if platform.processor() in l.split()[2].split(','):
+                        ignore_list += l.split()[3].split(',')
             continue
         from_pref = "FROM qemu/"
         if l.startswith(from_pref):
@@ -219,7 +227,8 @@ def _dockerfile_preprocess(df):
             inlining = _read_qemu_dockerfile(l[len(from_pref):])
             out += _dockerfile_preprocess(inlining)
             continue
-        out += l + "\n"
+        if not any(x in l.split() for x in ignore_list):
+            out += l + "\n"
     return out
 
 
@@ -330,7 +339,7 @@ def build_image(self, tag, docker_dir, dockerfile,
         tmp_df = tempfile.NamedTemporaryFile(mode="w+t",
                                              encoding='utf-8',
                                              dir=docker_dir, suffix=".docker")
-        tmp_df.write(dockerfile)
+        tmp_df.write(_dockerfile_preprocess(dockerfile))
 
         if user:
             uid = os.getuid()
diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
index 2943a99730..5cec46d8f2 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -6,6 +6,8 @@
 
 FROM docker.io/library/alpine:edge
 
+# Lines to by ignored when this file is read by the python script
+# ignore ppc64le,ppc64 xen-dev
 RUN apk update && \
     apk upgrade && \
     apk add \
-- 
2.25.1

Re: [RFC PATCH 8/8] tests/docker: Selective line reading by python script

[Philippe Mathieu-Daudé via]

+Erik/Daniel

On Wed, Jul 27, 2022 at 6:37 PM Lucas Mateus Castro(alqotel)
<lucas.araujo@eldorado.org.br> wrote:
>
> Building some images failed on ppc64le because the dockerfile tried to
> install some packages that are only available in x86 and arm64, to solve
> this while still having those packages be available in those architectures
> a comment was put before the installation command to instruct the python
> script into ignoring those lines for some architectures (in this case
> ppc64le)
>
> Overall I'm not a big fan of the way I solved this problem, so I'd like
> to know if anyone has a better way to make these dockerfilse work in
> PPC64LE.
>
> For context the base images used here are available in PPC64LE but some
> of the packages installed are not (in alpine's case it's XEN, which is
> only available to x86 and ARM), so this patch create a ignore_list which
> is set on a per-architecture basis, and any packages in a dockerfile in
> this ignore_list will not be copied to the temporary dockerfile used in
> the docker command.

Shouldn't this be done on lcitool side?
(https://gitlab.com/libvirt/libvirt-ci/-/tree/master/lcitool)

> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
> ---
>  tests/docker/docker.py                 | 15 ++++++++++++---
>  tests/docker/dockerfiles/alpine.docker |  2 ++
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index d0af2861b8..9b962d1c78 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -14,6 +14,7 @@
>  import os
>  import sys
>  import subprocess
> +import platform
>  import json
>  import hashlib
>  import atexit
> @@ -207,8 +208,15 @@ def _read_qemu_dockerfile(img_name):
>
>  def _dockerfile_preprocess(df):
>      out = ""
> +    ignore_list = []
>      for l in df.splitlines():
> -        if len(l.strip()) == 0 or l.startswith("#"):
> +        if len(l.strip()) == 0:
> +            continue
> +        if l.startswith("#"):
> +            if len(l.split()) >= 3:
> +                if l.split()[1] == "ignore":
> +                    if platform.processor() in l.split()[2].split(','):
> +                        ignore_list += l.split()[3].split(',')
>              continue
>          from_pref = "FROM qemu/"
>          if l.startswith(from_pref):
> @@ -219,7 +227,8 @@ def _dockerfile_preprocess(df):
>              inlining = _read_qemu_dockerfile(l[len(from_pref):])
>              out += _dockerfile_preprocess(inlining)
>              continue
> -        out += l + "\n"
> +        if not any(x in l.split() for x in ignore_list):
> +            out += l + "\n"
>      return out
>
>
> @@ -330,7 +339,7 @@ def build_image(self, tag, docker_dir, dockerfile,
>          tmp_df = tempfile.NamedTemporaryFile(mode="w+t",
>                                               encoding='utf-8',
>                                               dir=docker_dir, suffix=".docker")
> -        tmp_df.write(dockerfile)
> +        tmp_df.write(_dockerfile_preprocess(dockerfile))
>
>          if user:
>              uid = os.getuid()
> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> index 2943a99730..5cec46d8f2 100644
> --- a/tests/docker/dockerfiles/alpine.docker
> +++ b/tests/docker/dockerfiles/alpine.docker
> @@ -6,6 +6,8 @@
>
>  FROM docker.io/library/alpine:edge
>
> +# Lines to by ignored when this file is read by the python script
> +# ignore ppc64le,ppc64 xen-dev
>  RUN apk update && \
>      apk upgrade && \
>      apk add \
> --
> 2.25.1
>

Re: [RFC PATCH 8/8] tests/docker: Selective line reading by python script

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 11:20:56PM +0200, Philippe Mathieu-Daudé wrote:
> +Erik/Daniel
> 
> On Wed, Jul 27, 2022 at 6:37 PM Lucas Mateus Castro(alqotel)
> <lucas.araujo@eldorado.org.br> wrote:
> >
> > Building some images failed on ppc64le because the dockerfile tried to
> > install some packages that are only available in x86 and arm64, to solve
> > this while still having those packages be available in those architectures
> > a comment was put before the installation command to instruct the python
> > script into ignoring those lines for some architectures (in this case
> > ppc64le)
> >
> > Overall I'm not a big fan of the way I solved this problem, so I'd like
> > to know if anyone has a better way to make these dockerfilse work in
> > PPC64LE.
> >
> > For context the base images used here are available in PPC64LE but some
> > of the packages installed are not (in alpine's case it's XEN, which is
> > only available to x86 and ARM), so this patch create a ignore_list which
> > is set on a per-architecture basis, and any packages in a dockerfile in
> > this ignore_list will not be copied to the temporary dockerfile used in
> > the docker command.
> 
> Shouldn't this be done on lcitool side?
> (https://gitlab.com/libvirt/libvirt-ci/-/tree/master/lcitool)

Yes, to fix this properly we'll need to identify which packages are
architecture specific, and create separate 'RUN' commands that skip
those packages on arches in question. Doable, but not entirely easy.

> 
> > Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
> > ---
> >  tests/docker/docker.py                 | 15 ++++++++++++---
> >  tests/docker/dockerfiles/alpine.docker |  2 ++
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> > index d0af2861b8..9b962d1c78 100755
> > --- a/tests/docker/docker.py
> > +++ b/tests/docker/docker.py
> > @@ -14,6 +14,7 @@
> >  import os
> >  import sys
> >  import subprocess
> > +import platform
> >  import json
> >  import hashlib
> >  import atexit
> > @@ -207,8 +208,15 @@ def _read_qemu_dockerfile(img_name):
> >
> >  def _dockerfile_preprocess(df):
> >      out = ""
> > +    ignore_list = []
> >      for l in df.splitlines():
> > -        if len(l.strip()) == 0 or l.startswith("#"):
> > +        if len(l.strip()) == 0:
> > +            continue
> > +        if l.startswith("#"):
> > +            if len(l.split()) >= 3:
> > +                if l.split()[1] == "ignore":
> > +                    if platform.processor() in l.split()[2].split(','):
> > +                        ignore_list += l.split()[3].split(',')
> >              continue
> >          from_pref = "FROM qemu/"
> >          if l.startswith(from_pref):
> > @@ -219,7 +227,8 @@ def _dockerfile_preprocess(df):
> >              inlining = _read_qemu_dockerfile(l[len(from_pref):])
> >              out += _dockerfile_preprocess(inlining)
> >              continue
> > -        out += l + "\n"
> > +        if not any(x in l.split() for x in ignore_list):
> > +            out += l + "\n"
> >      return out
> >
> >
> > @@ -330,7 +339,7 @@ def build_image(self, tag, docker_dir, dockerfile,
> >          tmp_df = tempfile.NamedTemporaryFile(mode="w+t",
> >                                               encoding='utf-8',
> >                                               dir=docker_dir, suffix=".docker")
> > -        tmp_df.write(dockerfile)
> > +        tmp_df.write(_dockerfile_preprocess(dockerfile))
> >
> >          if user:
> >              uid = os.getuid()
> > diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> > index 2943a99730..5cec46d8f2 100644
> > --- a/tests/docker/dockerfiles/alpine.docker
> > +++ b/tests/docker/dockerfiles/alpine.docker
> > @@ -6,6 +6,8 @@
> >
> >  FROM docker.io/library/alpine:edge
> >
> > +# Lines to by ignored when this file is read by the python script
> > +# ignore ppc64le,ppc64 xen-dev
> >  RUN apk update && \
> >      apk upgrade && \
> >      apk add \
> > --
> > 2.25.1
> >
> 

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: [PATCH 0/8] Patch series to set up a ppc64le CI

[Daniel Henrique Barboza]

Lucas,

If you're still interested, patches 1-4 are worth re-sending in separate.
You'll need to address Daniel's comments on patch 1. The other 3 patches
LGTM.

Patches 5-8 can wait. We're not certain about Unicamp's Minicloud
availability in the future, so there's no point in setting up our CI
based on Minicloud's specifics.


Thanks,

Daniel

On 7/27/22 13:36, Lucas Mateus Castro(alqotel) wrote:
> This patch series aim to make easier to set up a compilation and CI
> environment in PPC64 and PPC64LE machines.
> 
> The first 2 patches is a fix not related to ppc64.
> Patch 3 and 4 also affect some other architectures.
> Patches 5 to 7 are adding Power specific additions.
> 
> Patch 8 is a RFC for a current way to run the docker tests in PPC64LE.
> 
> Lucas Mateus Castro(alqotel) (8):
>    tests/docker: Fix alpine dockerfile
>    scripts/ci/setup: ninja missing from build-environment
>    scripts/ci/setup: Fix libxen requirements
>    scripts/ci/setup: spice-server only on x86 aarch64
>    scripts/ci/setup: Add ppc64le to vars.yml template
>    scripts/ci/setup: Add Fedora to build-environment.yml
>    scripts/ci/setup: Added debian to build-environment.yml
>    tests/docker: Selective line reading by python script
> 
>   scripts/ci/setup/build-environment.yml | 54 +++++++++++++++++++++-----
>   scripts/ci/setup/vars.yml.template     |  1 +
>   tests/docker/docker.py                 | 15 +++++--
>   tests/docker/dockerfiles/alpine.docker |  3 ++
>   4 files changed, 61 insertions(+), 12 deletions(-)
> 

Re: [PATCH 0/8] Patch series to set up a ppc64le CI

[Lucas Mateus Martins Araujo e Castro]

On 06/09/2022 16:52, Daniel Henrique Barboza wrote:
> Lucas,
> 
> If you're still interested, patches 1-4 are worth re-sending in separate.
> You'll need to address Daniel's comments on patch 1. The other 3 patches
> LGTM.
> 
> Patches 5-8 can wait. We're not certain about Unicamp's Minicloud
> availability in the future, so there's no point in setting up our CI
> based on Minicloud's specifics.
Ok, also patch 8 needs a different approach as pointed by Philippe and 
Daniel.
I'll probably resend 1-4 with the fix on patch 1 tomorrow.
> 
> 
> Thanks,
> 
> Daniel
> 
> On 7/27/22 13:36, Lucas Mateus Castro(alqotel) wrote:
>> This patch series aim to make easier to set up a compilation and CI
>> environment in PPC64 and PPC64LE machines.
>>
>> The first 2 patches is a fix not related to ppc64.
>> Patch 3 and 4 also affect some other architectures.
>> Patches 5 to 7 are adding Power specific additions.
>>
>> Patch 8 is a RFC for a current way to run the docker tests in PPC64LE.
>>
>> Lucas Mateus Castro(alqotel) (8):
>>    tests/docker: Fix alpine dockerfile
>>    scripts/ci/setup: ninja missing from build-environment
>>    scripts/ci/setup: Fix libxen requirements
>>    scripts/ci/setup: spice-server only on x86 aarch64
>>    scripts/ci/setup: Add ppc64le to vars.yml template
>>    scripts/ci/setup: Add Fedora to build-environment.yml
>>    scripts/ci/setup: Added debian to build-environment.yml
>>    tests/docker: Selective line reading by python script
>>
>>   scripts/ci/setup/build-environment.yml | 54 +++++++++++++++++++++-----
>>   scripts/ci/setup/vars.yml.template     |  1 +
>>   tests/docker/docker.py                 | 15 +++++--
>>   tests/docker/dockerfiles/alpine.docker |  3 ++
>>   4 files changed, 61 insertions(+), 12 deletions(-)
>>

-- 
Lucas Mateus M. Araujo e Castro
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>