All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] Few CI improvements
@ 2025-02-13  1:23 Marek Marczykowski-Górecki
  2025-02-13  1:23 ` [PATCH v1 1/3] automation: skip building domU if there is no test defined for it Marek Marczykowski-Górecki
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-13  1:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Marek Marczykowski-Górecki

- Add some more test jobs
- Allow selecting individual jobs, without editing yaml files

I don't think it needs to be included in 4.20, but may be considered later for
backporting.

Marek Marczykowski-Górecki (3):
  automation: skip building domU if there is no test defined for it
  automation: add jobs running tests from tools/tests/*
  automation: allow selecting individual jobs via CI variables

 automation/gitlab-ci/build.yaml    |  6 ++-
 automation/gitlab-ci/test.yaml     | 37 +++++++++++++++-
 automation/scripts/build           |  1 +-
 automation/scripts/qubes-x86-64.sh | 77 ++++++++++++++++++++++---------
 automation/scripts/run-tools-tests | 47 +++++++++++++++++++-
 5 files changed, 148 insertions(+), 20 deletions(-)
 create mode 100755 automation/scripts/run-tools-tests

base-commit: 819c3cb186a86ef3e04fb5af4d9f9f6de032c3ee
-- 
git-series 0.9.1


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

* [PATCH v1 1/3] automation: skip building domU if there is no test defined for it
  2025-02-13  1:23 [PATCH v1 0/3] Few CI improvements Marek Marczykowski-Górecki
@ 2025-02-13  1:23 ` Marek Marczykowski-Górecki
  2025-02-13  1:47   ` Stefano Stabellini
  2025-02-13  1:23 ` [PATCH v1 2/3] automation: add jobs running tests from tools/tests/* Marek Marczykowski-Górecki
  2025-02-13  1:23 ` [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables Marek Marczykowski-Górecki
  2 siblings, 1 reply; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-13  1:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Marek Marczykowski-Górecki, Doug Goldstein,
	Stefano Stabellini

This will be useful for later tests not using generic domU (unit tests,
xtf etc).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/qubes-x86-64.sh | 50 +++++++++++++++++++------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 8a0b7bfbc0d0..7eb3ce1bf703 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -144,26 +144,28 @@ disk = [ ]
 ${domU_extra_config}
 "
 
-# DomU
-mkdir -p rootfs
-cd rootfs
-# fakeroot is needed to preserve device nodes in rootless podman container
-fakeroot -s ../fakeroot-save tar xzf ../binaries/initrd.tar.gz
-mkdir proc
-mkdir run
-mkdir srv
-mkdir sys
-rm var/run
-echo "#!/bin/sh
+if [ -n "$domU_check" ]; then
+    # DomU
+    mkdir -p rootfs
+    cd rootfs
+    # fakeroot is needed to preserve device nodes in rootless podman container
+    fakeroot -s ../fakeroot-save tar xzf ../binaries/initrd.tar.gz
+    mkdir proc
+    mkdir run
+    mkdir srv
+    mkdir sys
+    rm var/run
+    echo "#!/bin/sh
 
 ${domU_check}
 " > etc/local.d/xen.start
-chmod +x etc/local.d/xen.start
-echo "rc_verbose=yes" >> etc/rc.conf
-sed -i -e 's/^Welcome/domU \0/' etc/issue
-find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
-cd ..
-rm -rf rootfs
+    chmod +x etc/local.d/xen.start
+    echo "rc_verbose=yes" >> etc/rc.conf
+    sed -i -e 's/^Welcome/domU \0/' etc/issue
+    find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
+    cd ..
+    rm -rf rootfs
+fi
 
 # DOM0 rootfs
 mkdir -p rootfs
@@ -188,11 +190,19 @@ ifconfig eth0 up
 ifconfig xenbr0 up
 ifconfig xenbr0 192.168.0.1
 
+" > etc/local.d/xen.start
+
+if [ -n "$domU_check" ]; then
+    echo "
 # get domU console content into test log
 tail -F /var/log/xen/console/guest-domU.log 2>/dev/null | sed -e \"s/^/(domU) /\" &
 xl create /etc/xen/domU.cfg
 ${dom0_check}
-" > etc/local.d/xen.start
+" >> etc/local.d/xen.start
+else
+    echo "${dom0_check}" >> etc/local.d/xen.start
+fi
+
 chmod +x etc/local.d/xen.start
 echo "$domU_config" > etc/xen/domU.cfg
 
@@ -201,7 +211,9 @@ echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
 echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
 mkdir -p var/log/xen/console
 cp ../binaries/bzImage boot/vmlinuz
-cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
+if [ -n "$domU_check" ]; then
+    cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
+fi
 find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > ../binaries/dom0-rootfs.cpio.gz
 cd ..
 
-- 
git-series 0.9.1


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

* [PATCH v1 2/3] automation: add jobs running tests from tools/tests/*
  2025-02-13  1:23 [PATCH v1 0/3] Few CI improvements Marek Marczykowski-Górecki
  2025-02-13  1:23 ` [PATCH v1 1/3] automation: skip building domU if there is no test defined for it Marek Marczykowski-Górecki
@ 2025-02-13  1:23 ` Marek Marczykowski-Górecki
  2025-02-13  2:07   ` Stefano Stabellini
  2025-02-13  1:23 ` [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables Marek Marczykowski-Górecki
  2 siblings, 1 reply; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-13  1:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Marek Marczykowski-Górecki, Doug Goldstein,
	Stefano Stabellini

There are a bunch of tests in tools/tests/, let them run in CI.
For each subdirectory expect "make run" will run the test, and observe
its exit code. This way, adding new tests is easy, and they will be
automatically picked up.

For better visibility, log test output to junit xml format, and let
gitlab ingest it. Set SUT_ADDR variable with name/address of the system
under test, so a network can be used to extract the file. The actual
address is set using DHCP. And for the test internal network, still add
the 192.168.0.1 IP (but don't replace the DHCP-provided one).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/gitlab-ci/test.yaml     | 23 +++++++++++++++-
 automation/scripts/build           |  1 +-
 automation/scripts/qubes-x86-64.sh | 27 +++++++++++++++++-
 automation/scripts/run-tools-tests | 47 +++++++++++++++++++++++++++++++-
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/run-tools-tests

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 1822e3ea5fd7..c21a37933881 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -130,6 +130,7 @@
     PCIDEV: "03:00.0"
     PCIDEV_INTR: "MSI-X"
     CONSOLE_OPTS: "console=com1 com1=115200,8n1"
+    SUT_ADDR: test-2.testnet
   artifacts:
     paths:
       - smoke.serial
@@ -263,6 +264,28 @@ adl-pvshim-x86-64-gcc-debug:
     - *x86-64-test-needs
     - alpine-3.18-gcc-debug
 
+adl-tools-tests-pv-x86-64-gcc-debug:
+  extends: .adl-x86-64
+  script:
+    - ./automation/scripts/qubes-x86-64.sh tools-tests-pv 2>&1 | tee ${LOGFILE}
+  artifacts:
+    reports:
+      junit: tests-junit.xml
+  needs:
+    - *x86-64-test-needs
+    - alpine-3.18-gcc-debug
+
+adl-tools-tests-pvh-x86-64-gcc-debug:
+  extends: .adl-x86-64
+  script:
+    - ./automation/scripts/qubes-x86-64.sh tools-tests-pvh 2>&1 | tee ${LOGFILE}
+  artifacts:
+    reports:
+      junit: tests-junit.xml
+  needs:
+    - *x86-64-test-needs
+    - alpine-3.18-gcc-debug
+
 zen3p-smoke-x86-64-gcc-debug:
   extends: .zen3p-x86-64
   script:
diff --git a/automation/scripts/build b/automation/scripts/build
index 952599cc25c2..522efe774ef3 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -109,5 +109,6 @@ else
     # even though dist/ contains everything, while some containers don't even
     # build Xen
     cp -r dist binaries/
+    cp -r tools/tests binaries/
     collect_xen_artefacts
 fi
diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 7eb3ce1bf703..81d239cc8b75 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -10,6 +10,8 @@ set -ex
 #  - pci-pv         PV dom0,  PV domU + PCI Passthrough
 #  - pvshim         PV dom0,  PVSHIM domU
 #  - s3             PV dom0,  S3 suspend/resume
+#  - tools-tests-pv PV dom0, run tests from tools/tests/*
+#  - tools-tests-pvh PVH dom0, run tests from tools/tests/*
 test_variant=$1
 
 ### defaults
@@ -19,6 +21,7 @@ timeout=120
 domU_type="pvh"
 domU_vif="'bridge=xenbr0',"
 domU_extra_config=
+retrieve_xml=
 
 case "${test_variant}" in
     ### test: smoke test & smoke test PVH & smoke test HVM & smoke test PVSHIM
@@ -126,6 +129,21 @@ done
 "
         ;;
 
+    ### tests: tools-tests-pv, tools-tests-pvh
+    "tools-tests-pv"|"tools-tests-pvh")
+        retrieve_xml=1
+        passed="test passed"
+        domU_check=""
+        dom0_check="
+/tests/run-tools-tests /tests /tmp/tests-junit.xml && echo \"${passed}\"
+nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
+"
+        if [ "${test_variant}" = "tools-tests-pvh" ]; then
+            extra_xen_opts="dom0=pvh"
+        fi
+
+        ;;
+
     *)
         echo "Unrecognised test_variant '${test_variant}'" >&2
         exit 1
@@ -178,6 +196,8 @@ mkdir srv
 mkdir sys
 rm var/run
 cp -ar ../binaries/dist/install/* .
+cp -ar ../binaries/tests .
+cp -a ../automation/scripts/run-tools-tests tests/
 
 echo "#!/bin/bash
 
@@ -188,7 +208,8 @@ brctl addbr xenbr0
 brctl addif xenbr0 eth0
 ifconfig eth0 up
 ifconfig xenbr0 up
-ifconfig xenbr0 192.168.0.1
+timeout 30s udhcpc -i xenbr0
+ip addr add dev xenbr0 192.168.0.1/24
 
 " > etc/local.d/xen.start
 
@@ -272,6 +293,10 @@ if [ $timeout -le 0 ]; then
     exit 1
 fi
 
+if [ -n "$retrieve_xml" ]; then
+    nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
+fi
+
 sleep 1
 
 (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests
new file mode 100755
index 000000000000..242a9edad941
--- /dev/null
+++ b/automation/scripts/run-tools-tests
@@ -0,0 +1,47 @@
+#!/bin/sh
+
+usage() {
+    echo "Usage: $0 tests-dir xml-out"
+}
+
+xml_out=$2
+if [ -z "$xml_out" ]; then
+  xml_out=/dev/null
+fi
+printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out"
+printf '<testsuites name="tools.tests">\n' >> "$xml_out"
+printf ' <testsuite name="tools.tests">\n' >> "$xml_out"
+failed=
+for dir in "$1"/*; do
+    [ -d "$dir" ] || continue
+    echo "Running test in $dir"
+    printf '  <testcase name="%s">\n' "$dir" >> "$xml_out"
+    ret=
+    for f in "$dir"/*; do
+        [ -f "$f" ] || continue
+        [ -x "$f" ] || continue
+        "$f" 2>&1 | tee /tmp/out
+        ret=$?
+        if [ "$ret" -ne 0 ]; then
+            echo "FAILED"
+            failed+=" $dir"
+            printf '   <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out"
+            # TODO: could use xml escaping... but current tests seems to
+            # produce sane output
+            cat /tmp/out >> "$xml_out"
+            printf '   </failure>\n' "$f" "$ret" >> "$xml_out"
+        else
+            echo "PASSED"
+        fi
+    done
+    if [ -z "$ret" ]; then
+        printf '   <skipped type="skipped" message="test not found"/>\n' >> "$xml_out"
+    fi
+    printf '  </testcase>\n' "$dir" >> "$xml_out"
+done
+printf ' </testsuite>\n' >> "$xml_out"
+printf '</testsuites>\n' >> "$xml_out"
+
+if [ -n "$failed" ]; then
+    exit 1
+fi
-- 
git-series 0.9.1


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

* [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables
  2025-02-13  1:23 [PATCH v1 0/3] Few CI improvements Marek Marczykowski-Górecki
  2025-02-13  1:23 ` [PATCH v1 1/3] automation: skip building domU if there is no test defined for it Marek Marczykowski-Górecki
  2025-02-13  1:23 ` [PATCH v1 2/3] automation: add jobs running tests from tools/tests/* Marek Marczykowski-Górecki
@ 2025-02-13  1:23 ` Marek Marczykowski-Górecki
  2025-02-14  1:36   ` Stefano Stabellini
  2 siblings, 1 reply; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-13  1:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Marek Marczykowski-Górecki, Doug Goldstein,
	Stefano Stabellini

Debugging sometimes involves running specific jobs on different
versions. It's useful to easily avoid running all of the not interesting
ones (for given case) to save both time and CI resources. Doing so used
to require changing the yaml files, usually in several places.
Ease this step by adding SELECTED_JOBS_ONLY variable that takes a regex.
Note that one needs to satisfy job dependencies on their own (for
example if a test job needs a build job, that specific build job
needs to be included too).

The variable can be specified via Gitlab web UI when scheduling a
pipeline, but it can be also set when doing git push directly:

    git push -o ci.variable=SELECTED_JOBS_ONLY="/job1|job2/"

More details at https://docs.gitlab.co.jp/ee/user/project/push_options.html

The variable needs to include regex for selecting jobs, including
enclosing slashes.
A coma/space separated list of jobs to select would be friendlier UX,
but unfortunately that is not supported:
https://gitlab.com/gitlab-org/gitlab/-/issues/209904 (note the proposed
workaround doesn't work for job-level CI_JOB_NAME).
On the other hand, the regex is more flexible (one can select for
example all arm32 jobs).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
This probably wants documenting beyond this commit message. I don't
think we have any CI-related docs anywhere, do we? Some new file in
docs/misc?

And also, it's possible to extend web ui for starting pipelines to
include pre-defined variables. I use it in qubes here if you want to
see:
https://gitlab.com/QubesOS/qubes-continuous-integration/-/pipelines/new
Does it make sense to include SELECTED_JOBS_ONLY this way too?
Personally, I'll probably use it via cmdline push only anyway, but I
don't know what workflows other people have.
---
 automation/gitlab-ci/build.yaml |  6 ++++++
 automation/gitlab-ci/test.yaml  | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 35e224366f62..f12de00a164a 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -12,6 +12,12 @@
       - '*/*.log'
     when: always
   needs: []
+  rules:
+  - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
+    when: always
+  - if: $SELECTED_JOBS_ONLY
+    when: never
+  - when: on_success
 
 .gcc-tmpl:
   variables: &gcc
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index c21a37933881..93632f1f9204 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -1,6 +1,11 @@
 .test-jobs-common:
   stage: test
   image: ${XEN_REGISTRY}/${CONTAINER}
+  rules:
+  - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
+  - if: $SELECTED_JOBS_ONLY
+    when: never
+  - when: on_success
 
 .arm64-test-needs: &arm64-test-needs
   - alpine-3.18-arm64-rootfs-export
@@ -99,6 +104,9 @@
       - '*.dtb'
     when: always
   rules:
+    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
+    - if: $SELECTED_JOBS_ONLY
+      when: never
     - if: $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
   tags:
     - xilinx
@@ -117,6 +125,9 @@
       - '*.log'
     when: always
   rules:
+    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
+    - if: $SELECTED_JOBS_ONLY
+      when: never
     - if: $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
   tags:
     - xilinx
@@ -137,6 +148,9 @@
       - '*.log'
     when: always
   rules:
+    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
+    - if: $SELECTED_JOBS_ONLY
+      when: never
     - if: $QUBES_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
   tags:
     - qubes-hw2
-- 
git-series 0.9.1


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

* Re: [PATCH v1 1/3] automation: skip building domU if there is no test defined for it
  2025-02-13  1:23 ` [PATCH v1 1/3] automation: skip building domU if there is no test defined for it Marek Marczykowski-Górecki
@ 2025-02-13  1:47   ` Stefano Stabellini
  0 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2025-02-13  1:47 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Doug Goldstein, Stefano Stabellini

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

On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> This will be useful for later tests not using generic domU (unit tests,
> xtf etc).
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  automation/scripts/qubes-x86-64.sh | 50 +++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 8a0b7bfbc0d0..7eb3ce1bf703 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -144,26 +144,28 @@ disk = [ ]
>  ${domU_extra_config}
>  "
>  
> -# DomU
> -mkdir -p rootfs
> -cd rootfs
> -# fakeroot is needed to preserve device nodes in rootless podman container
> -fakeroot -s ../fakeroot-save tar xzf ../binaries/initrd.tar.gz
> -mkdir proc
> -mkdir run
> -mkdir srv
> -mkdir sys
> -rm var/run
> -echo "#!/bin/sh
> +if [ -n "$domU_check" ]; then
> +    # DomU
> +    mkdir -p rootfs
> +    cd rootfs
> +    # fakeroot is needed to preserve device nodes in rootless podman container
> +    fakeroot -s ../fakeroot-save tar xzf ../binaries/initrd.tar.gz
> +    mkdir proc
> +    mkdir run
> +    mkdir srv
> +    mkdir sys
> +    rm var/run
> +    echo "#!/bin/sh
>  
>  ${domU_check}
>  " > etc/local.d/xen.start
> -chmod +x etc/local.d/xen.start
> -echo "rc_verbose=yes" >> etc/rc.conf
> -sed -i -e 's/^Welcome/domU \0/' etc/issue
> -find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
> -cd ..
> -rm -rf rootfs
> +    chmod +x etc/local.d/xen.start
> +    echo "rc_verbose=yes" >> etc/rc.conf
> +    sed -i -e 's/^Welcome/domU \0/' etc/issue
> +    find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
> +    cd ..
> +    rm -rf rootfs
> +fi
>  
>  # DOM0 rootfs
>  mkdir -p rootfs
> @@ -188,11 +190,19 @@ ifconfig eth0 up
>  ifconfig xenbr0 up
>  ifconfig xenbr0 192.168.0.1
>  
> +" > etc/local.d/xen.start
> +
> +if [ -n "$domU_check" ]; then
> +    echo "
>  # get domU console content into test log
>  tail -F /var/log/xen/console/guest-domU.log 2>/dev/null | sed -e \"s/^/(domU) /\" &
>  xl create /etc/xen/domU.cfg
>  ${dom0_check}
> -" > etc/local.d/xen.start
> +" >> etc/local.d/xen.start
> +else
> +    echo "${dom0_check}" >> etc/local.d/xen.start
> +fi
> +
>  chmod +x etc/local.d/xen.start
>  echo "$domU_config" > etc/xen/domU.cfg
>  
> @@ -201,7 +211,9 @@ echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
>  echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
>  mkdir -p var/log/xen/console
>  cp ../binaries/bzImage boot/vmlinuz
> -cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
> +if [ -n "$domU_check" ]; then
> +    cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
> +fi
>  find . | fakeroot -i ../fakeroot-save cpio -H newc -o | gzip > ../binaries/dom0-rootfs.cpio.gz
>  cd ..
>  
> -- 
> git-series 0.9.1
> 

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

* Re: [PATCH v1 2/3] automation: add jobs running tests from tools/tests/*
  2025-02-13  1:23 ` [PATCH v1 2/3] automation: add jobs running tests from tools/tests/* Marek Marczykowski-Górecki
@ 2025-02-13  2:07   ` Stefano Stabellini
  2025-02-13  2:28     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2025-02-13  2:07 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Doug Goldstein, Stefano Stabellini

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

On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> There are a bunch of tests in tools/tests/, let them run in CI.
> For each subdirectory expect "make run" will run the test, and observe
> its exit code. This way, adding new tests is easy, and they will be
> automatically picked up.
> 
> For better visibility, log test output to junit xml format, and let
> gitlab ingest it. Set SUT_ADDR variable with name/address of the system
> under test, so a network can be used to extract the file. The actual
> address is set using DHCP. And for the test internal network, still add
> the 192.168.0.1 IP (but don't replace the DHCP-provided one).
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Very nice!!

Only one comment below


> ---
>  automation/gitlab-ci/test.yaml     | 23 +++++++++++++++-
>  automation/scripts/build           |  1 +-
>  automation/scripts/qubes-x86-64.sh | 27 +++++++++++++++++-
>  automation/scripts/run-tools-tests | 47 +++++++++++++++++++++++++++++++-
>  4 files changed, 97 insertions(+), 1 deletion(-)
>  create mode 100755 automation/scripts/run-tools-tests
> 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 1822e3ea5fd7..c21a37933881 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -130,6 +130,7 @@
>      PCIDEV: "03:00.0"
>      PCIDEV_INTR: "MSI-X"
>      CONSOLE_OPTS: "console=com1 com1=115200,8n1"
> +    SUT_ADDR: test-2.testnet
>    artifacts:
>      paths:
>        - smoke.serial
> @@ -263,6 +264,28 @@ adl-pvshim-x86-64-gcc-debug:
>      - *x86-64-test-needs
>      - alpine-3.18-gcc-debug
>  
> +adl-tools-tests-pv-x86-64-gcc-debug:
> +  extends: .adl-x86-64
> +  script:
> +    - ./automation/scripts/qubes-x86-64.sh tools-tests-pv 2>&1 | tee ${LOGFILE}
> +  artifacts:
> +    reports:
> +      junit: tests-junit.xml
> +  needs:
> +    - *x86-64-test-needs
> +    - alpine-3.18-gcc-debug
> +
> +adl-tools-tests-pvh-x86-64-gcc-debug:
> +  extends: .adl-x86-64
> +  script:
> +    - ./automation/scripts/qubes-x86-64.sh tools-tests-pvh 2>&1 | tee ${LOGFILE}
> +  artifacts:
> +    reports:
> +      junit: tests-junit.xml
> +  needs:
> +    - *x86-64-test-needs
> +    - alpine-3.18-gcc-debug
> +
>  zen3p-smoke-x86-64-gcc-debug:
>    extends: .zen3p-x86-64
>    script:
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 952599cc25c2..522efe774ef3 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -109,5 +109,6 @@ else
>      # even though dist/ contains everything, while some containers don't even
>      # build Xen
>      cp -r dist binaries/
> +    cp -r tools/tests binaries/
>      collect_xen_artefacts
>  fi
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 7eb3ce1bf703..81d239cc8b75 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -10,6 +10,8 @@ set -ex
>  #  - pci-pv         PV dom0,  PV domU + PCI Passthrough
>  #  - pvshim         PV dom0,  PVSHIM domU
>  #  - s3             PV dom0,  S3 suspend/resume
> +#  - tools-tests-pv PV dom0, run tests from tools/tests/*
> +#  - tools-tests-pvh PVH dom0, run tests from tools/tests/*
>  test_variant=$1
>  
>  ### defaults
> @@ -19,6 +21,7 @@ timeout=120
>  domU_type="pvh"
>  domU_vif="'bridge=xenbr0',"
>  domU_extra_config=
> +retrieve_xml=
>  
>  case "${test_variant}" in
>      ### test: smoke test & smoke test PVH & smoke test HVM & smoke test PVSHIM
> @@ -126,6 +129,21 @@ done
>  "
>          ;;
>  
> +    ### tests: tools-tests-pv, tools-tests-pvh
> +    "tools-tests-pv"|"tools-tests-pvh")
> +        retrieve_xml=1
> +        passed="test passed"
> +        domU_check=""
> +        dom0_check="
> +/tests/run-tools-tests /tests /tmp/tests-junit.xml && echo \"${passed}\"
> +nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
> +"
> +        if [ "${test_variant}" = "tools-tests-pvh" ]; then
> +            extra_xen_opts="dom0=pvh"
> +        fi
> +
> +        ;;
> +
>      *)
>          echo "Unrecognised test_variant '${test_variant}'" >&2
>          exit 1
> @@ -178,6 +196,8 @@ mkdir srv
>  mkdir sys
>  rm var/run
>  cp -ar ../binaries/dist/install/* .
> +cp -ar ../binaries/tests .
> +cp -a ../automation/scripts/run-tools-tests tests/
>  
>  echo "#!/bin/bash
>  
> @@ -188,7 +208,8 @@ brctl addbr xenbr0
>  brctl addif xenbr0 eth0
>  ifconfig eth0 up
>  ifconfig xenbr0 up
> -ifconfig xenbr0 192.168.0.1
> +timeout 30s udhcpc -i xenbr0
> +ip addr add dev xenbr0 192.168.0.1/24
>  
>  " > etc/local.d/xen.start
>  
> @@ -272,6 +293,10 @@ if [ $timeout -le 0 ]; then
>      exit 1
>  fi
>  
> +if [ -n "$retrieve_xml" ]; then
> +    nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
> +fi
> +
>  sleep 1
>  
>  (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
> diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests
> new file mode 100755
> index 000000000000..242a9edad941
> --- /dev/null
> +++ b/automation/scripts/run-tools-tests
> @@ -0,0 +1,47 @@
> +#!/bin/sh

It should be /bin/bash

You could also consider -e and maybe -x


> +usage() {
> +    echo "Usage: $0 tests-dir xml-out"
> +}
> +
> +xml_out=$2
> +if [ -z "$xml_out" ]; then
> +  xml_out=/dev/null
> +fi
> +printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out"
> +printf '<testsuites name="tools.tests">\n' >> "$xml_out"
> +printf ' <testsuite name="tools.tests">\n' >> "$xml_out"
> +failed=
> +for dir in "$1"/*; do
> +    [ -d "$dir" ] || continue
> +    echo "Running test in $dir"
> +    printf '  <testcase name="%s">\n' "$dir" >> "$xml_out"
> +    ret=
> +    for f in "$dir"/*; do
> +        [ -f "$f" ] || continue
> +        [ -x "$f" ] || continue
> +        "$f" 2>&1 | tee /tmp/out
> +        ret=$?
> +        if [ "$ret" -ne 0 ]; then
> +            echo "FAILED"
> +            failed+=" $dir"
> +            printf '   <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out"
> +            # TODO: could use xml escaping... but current tests seems to
> +            # produce sane output
> +            cat /tmp/out >> "$xml_out"
> +            printf '   </failure>\n' "$f" "$ret" >> "$xml_out"
> +        else
> +            echo "PASSED"
> +        fi
> +    done
> +    if [ -z "$ret" ]; then
> +        printf '   <skipped type="skipped" message="test not found"/>\n' >> "$xml_out"
> +    fi
> +    printf '  </testcase>\n' "$dir" >> "$xml_out"
> +done
> +printf ' </testsuite>\n' >> "$xml_out"
> +printf '</testsuites>\n' >> "$xml_out"
> +
> +if [ -n "$failed" ]; then
> +    exit 1
> +fi
> -- 
> git-series 0.9.1
> 

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

* Re: [PATCH v1 2/3] automation: add jobs running tests from tools/tests/*
  2025-02-13  2:07   ` Stefano Stabellini
@ 2025-02-13  2:28     ` Marek Marczykowski-Górecki
  2025-02-13 19:14       ` Stefano Stabellini
  0 siblings, 1 reply; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-13  2:28 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Doug Goldstein

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

On Wed, Feb 12, 2025 at 06:07:37PM -0800, Stefano Stabellini wrote:
> On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > There are a bunch of tests in tools/tests/, let them run in CI.
> > For each subdirectory expect "make run" will run the test, and observe
> > its exit code. This way, adding new tests is easy, and they will be
> > automatically picked up.
> > 
> > For better visibility, log test output to junit xml format, and let
> > gitlab ingest it. Set SUT_ADDR variable with name/address of the system
> > under test, so a network can be used to extract the file. The actual
> > address is set using DHCP. And for the test internal network, still add
> > the 192.168.0.1 IP (but don't replace the DHCP-provided one).
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> 
> Very nice!!
> 
> Only one comment below
> 
> 
> > ---
> >  automation/gitlab-ci/test.yaml     | 23 +++++++++++++++-
> >  automation/scripts/build           |  1 +-
> >  automation/scripts/qubes-x86-64.sh | 27 +++++++++++++++++-
> >  automation/scripts/run-tools-tests | 47 +++++++++++++++++++++++++++++++-
> >  4 files changed, 97 insertions(+), 1 deletion(-)
> >  create mode 100755 automation/scripts/run-tools-tests
> > 
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 1822e3ea5fd7..c21a37933881 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -130,6 +130,7 @@
> >      PCIDEV: "03:00.0"
> >      PCIDEV_INTR: "MSI-X"
> >      CONSOLE_OPTS: "console=com1 com1=115200,8n1"
> > +    SUT_ADDR: test-2.testnet
> >    artifacts:
> >      paths:
> >        - smoke.serial
> > @@ -263,6 +264,28 @@ adl-pvshim-x86-64-gcc-debug:
> >      - *x86-64-test-needs
> >      - alpine-3.18-gcc-debug
> >  
> > +adl-tools-tests-pv-x86-64-gcc-debug:
> > +  extends: .adl-x86-64
> > +  script:
> > +    - ./automation/scripts/qubes-x86-64.sh tools-tests-pv 2>&1 | tee ${LOGFILE}
> > +  artifacts:
> > +    reports:
> > +      junit: tests-junit.xml
> > +  needs:
> > +    - *x86-64-test-needs
> > +    - alpine-3.18-gcc-debug
> > +
> > +adl-tools-tests-pvh-x86-64-gcc-debug:
> > +  extends: .adl-x86-64
> > +  script:
> > +    - ./automation/scripts/qubes-x86-64.sh tools-tests-pvh 2>&1 | tee ${LOGFILE}
> > +  artifacts:
> > +    reports:
> > +      junit: tests-junit.xml
> > +  needs:
> > +    - *x86-64-test-needs
> > +    - alpine-3.18-gcc-debug
> > +
> >  zen3p-smoke-x86-64-gcc-debug:
> >    extends: .zen3p-x86-64
> >    script:
> > diff --git a/automation/scripts/build b/automation/scripts/build
> > index 952599cc25c2..522efe774ef3 100755
> > --- a/automation/scripts/build
> > +++ b/automation/scripts/build
> > @@ -109,5 +109,6 @@ else
> >      # even though dist/ contains everything, while some containers don't even
> >      # build Xen
> >      cp -r dist binaries/
> > +    cp -r tools/tests binaries/
> >      collect_xen_artefacts
> >  fi
> > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> > index 7eb3ce1bf703..81d239cc8b75 100755
> > --- a/automation/scripts/qubes-x86-64.sh
> > +++ b/automation/scripts/qubes-x86-64.sh
> > @@ -10,6 +10,8 @@ set -ex
> >  #  - pci-pv         PV dom0,  PV domU + PCI Passthrough
> >  #  - pvshim         PV dom0,  PVSHIM domU
> >  #  - s3             PV dom0,  S3 suspend/resume
> > +#  - tools-tests-pv PV dom0, run tests from tools/tests/*
> > +#  - tools-tests-pvh PVH dom0, run tests from tools/tests/*
> >  test_variant=$1
> >  
> >  ### defaults
> > @@ -19,6 +21,7 @@ timeout=120
> >  domU_type="pvh"
> >  domU_vif="'bridge=xenbr0',"
> >  domU_extra_config=
> > +retrieve_xml=
> >  
> >  case "${test_variant}" in
> >      ### test: smoke test & smoke test PVH & smoke test HVM & smoke test PVSHIM
> > @@ -126,6 +129,21 @@ done
> >  "
> >          ;;
> >  
> > +    ### tests: tools-tests-pv, tools-tests-pvh
> > +    "tools-tests-pv"|"tools-tests-pvh")
> > +        retrieve_xml=1
> > +        passed="test passed"
> > +        domU_check=""
> > +        dom0_check="
> > +/tests/run-tools-tests /tests /tmp/tests-junit.xml && echo \"${passed}\"
> > +nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
> > +"
> > +        if [ "${test_variant}" = "tools-tests-pvh" ]; then
> > +            extra_xen_opts="dom0=pvh"
> > +        fi
> > +
> > +        ;;
> > +
> >      *)
> >          echo "Unrecognised test_variant '${test_variant}'" >&2
> >          exit 1
> > @@ -178,6 +196,8 @@ mkdir srv
> >  mkdir sys
> >  rm var/run
> >  cp -ar ../binaries/dist/install/* .
> > +cp -ar ../binaries/tests .
> > +cp -a ../automation/scripts/run-tools-tests tests/
> >  
> >  echo "#!/bin/bash
> >  
> > @@ -188,7 +208,8 @@ brctl addbr xenbr0
> >  brctl addif xenbr0 eth0
> >  ifconfig eth0 up
> >  ifconfig xenbr0 up
> > -ifconfig xenbr0 192.168.0.1
> > +timeout 30s udhcpc -i xenbr0

This is actually wrong with tests doing passthrough. I'll send v2 that
limits it.

> > +ip addr add dev xenbr0 192.168.0.1/24
> >  
> >  " > etc/local.d/xen.start
> >  
> > @@ -272,6 +293,10 @@ if [ $timeout -le 0 ]; then
> >      exit 1
> >  fi
> >  
> > +if [ -n "$retrieve_xml" ]; then
> > +    nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
> > +fi
> > +
> >  sleep 1
> >  
> >  (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
> > diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests
> > new file mode 100755
> > index 000000000000..242a9edad941
> > --- /dev/null
> > +++ b/automation/scripts/run-tools-tests
> > @@ -0,0 +1,47 @@
> > +#!/bin/sh
> 
> It should be /bin/bash

That script is running inside SUT (started from initramfs) which is
rather minimal. I think it currently has bash, but with the initramfs at
over 200MB (compressed) I can see trimming it in the future...

> You could also consider -e and maybe -x

That is a good idea (but also failures need to not break the XML
structure, so it will clutter the script a bit).

> > +usage() {
> > +    echo "Usage: $0 tests-dir xml-out"
> > +}
> > +
> > +xml_out=$2
> > +if [ -z "$xml_out" ]; then
> > +  xml_out=/dev/null
> > +fi
> > +printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out"
> > +printf '<testsuites name="tools.tests">\n' >> "$xml_out"
> > +printf ' <testsuite name="tools.tests">\n' >> "$xml_out"
> > +failed=
> > +for dir in "$1"/*; do
> > +    [ -d "$dir" ] || continue
> > +    echo "Running test in $dir"
> > +    printf '  <testcase name="%s">\n' "$dir" >> "$xml_out"
> > +    ret=
> > +    for f in "$dir"/*; do
> > +        [ -f "$f" ] || continue
> > +        [ -x "$f" ] || continue
> > +        "$f" 2>&1 | tee /tmp/out
> > +        ret=$?
> > +        if [ "$ret" -ne 0 ]; then
> > +            echo "FAILED"
> > +            failed+=" $dir"
> > +            printf '   <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out"
> > +            # TODO: could use xml escaping... but current tests seems to
> > +            # produce sane output
> > +            cat /tmp/out >> "$xml_out"
> > +            printf '   </failure>\n' "$f" "$ret" >> "$xml_out"
> > +        else
> > +            echo "PASSED"
> > +        fi
> > +    done
> > +    if [ -z "$ret" ]; then
> > +        printf '   <skipped type="skipped" message="test not found"/>\n' >> "$xml_out"
> > +    fi
> > +    printf '  </testcase>\n' "$dir" >> "$xml_out"
> > +done
> > +printf ' </testsuite>\n' >> "$xml_out"
> > +printf '</testsuites>\n' >> "$xml_out"
> > +
> > +if [ -n "$failed" ]; then
> > +    exit 1
> > +fi
> > -- 
> > git-series 0.9.1
> > 


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v1 2/3] automation: add jobs running tests from tools/tests/*
  2025-02-13  2:28     ` Marek Marczykowski-Górecki
@ 2025-02-13 19:14       ` Stefano Stabellini
  2025-02-13 21:47         ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2025-02-13 19:14 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, xen-devel, Doug Goldstein

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

On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > > diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests
> > > new file mode 100755
> > > index 000000000000..242a9edad941
> > > --- /dev/null
> > > +++ b/automation/scripts/run-tools-tests
> > > @@ -0,0 +1,47 @@
> > > +#!/bin/sh
> > 
> > It should be /bin/bash
> 
> That script is running inside SUT (started from initramfs) which is
> rather minimal. I think it currently has bash, but with the initramfs at
> over 200MB (compressed) I can see trimming it in the future...

Hi Marek, let me clarify a bit more my comment.

While I have a preference for bash because that is what we are using for
all the other shell scripts, it is OK to use /bin/sh but then we need to
make sure the script is actually /bin/sh compatible and doesn't have any
bash-isms. Eye-balling the script I had the impression it was using
bash-isms, so I made the comment about using /bin/bash.

But in my experience most /bin/sh implementations today they are
actually somewhat bash compatible, so in general it is easier to declare
/bin/bash instead of /bin/sh.

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

* Re: [PATCH v1 2/3] automation: add jobs running tests from tools/tests/*
  2025-02-13 19:14       ` Stefano Stabellini
@ 2025-02-13 21:47         ` Marek Marczykowski-Górecki
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-13 21:47 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Doug Goldstein

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

On Thu, Feb 13, 2025 at 11:14:43AM -0800, Stefano Stabellini wrote:
> On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > > > diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests
> > > > new file mode 100755
> > > > index 000000000000..242a9edad941
> > > > --- /dev/null
> > > > +++ b/automation/scripts/run-tools-tests
> > > > @@ -0,0 +1,47 @@
> > > > +#!/bin/sh
> > > 
> > > It should be /bin/bash
> > 
> > That script is running inside SUT (started from initramfs) which is
> > rather minimal. I think it currently has bash, but with the initramfs at
> > over 200MB (compressed) I can see trimming it in the future...
> 
> Hi Marek, let me clarify a bit more my comment.
> 
> While I have a preference for bash because that is what we are using for
> all the other shell scripts, it is OK to use /bin/sh but then we need to
> make sure the script is actually /bin/sh compatible and doesn't have any
> bash-isms. Eye-balling the script I had the impression it was using
> bash-isms, so I made the comment about using /bin/bash.

Indeed += is bash-ism. But since I generate xml report now, I don't even
need it anymore.

> But in my experience most /bin/sh implementations today they are
> actually somewhat bash compatible, so in general it is easier to declare
> /bin/bash instead of /bin/sh.

I guess that's fine with the current initramfs. If somebody wants to
reduce it, this can be changed later.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables
  2025-02-13  1:23 ` [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables Marek Marczykowski-Górecki
@ 2025-02-14  1:36   ` Stefano Stabellini
  2025-02-14  2:32     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2025-02-14  1:36 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Doug Goldstein, Stefano Stabellini

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

On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> Debugging sometimes involves running specific jobs on different
> versions. It's useful to easily avoid running all of the not interesting
> ones (for given case) to save both time and CI resources. Doing so used
> to require changing the yaml files, usually in several places.
> Ease this step by adding SELECTED_JOBS_ONLY variable that takes a regex.
> Note that one needs to satisfy job dependencies on their own (for
> example if a test job needs a build job, that specific build job
> needs to be included too).
> 
> The variable can be specified via Gitlab web UI when scheduling a
> pipeline, but it can be also set when doing git push directly:
> 
>     git push -o ci.variable=SELECTED_JOBS_ONLY="/job1|job2/"
> 
> More details at https://docs.gitlab.co.jp/ee/user/project/push_options.html
> 
> The variable needs to include regex for selecting jobs, including
> enclosing slashes.
> A coma/space separated list of jobs to select would be friendlier UX,
> but unfortunately that is not supported:
> https://gitlab.com/gitlab-org/gitlab/-/issues/209904 (note the proposed
> workaround doesn't work for job-level CI_JOB_NAME).
> On the other hand, the regex is more flexible (one can select for
> example all arm32 jobs).

I was trying to find workarounds so that we could also support the
simple list of comma-separated jobs you mentioned, but I couldn't find
an easy way to do that.

However, one thing we can do is to support writing SELECTED_JOBS_ONLY in
.gitlab-ci.yml as a commit in xen.git, for people that don't know or
don't remember how to set ci.variable using the git command line.

Given that this is for testing, I think it would be no problem to adding
a special commit on top of your tree. We are just trying to make it
easier compared to having to manually delete the list of jobs we don't
need.

But even with the special commit, I couldn't think of an easy way to
achieve the nicer comma-separated list of jobs...


> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> This probably wants documenting beyond this commit message. I don't
> think we have any CI-related docs anywhere, do we? Some new file in
> docs/misc?

Yes please :-)

It would be also worth documenting how to create a simple pipeline by
removing everything that you don't need for a single test


> And also, it's possible to extend web ui for starting pipelines to
> include pre-defined variables. I use it in qubes here if you want to
> see:
> https://gitlab.com/QubesOS/qubes-continuous-integration/-/pipelines/new

I don't have access to this


> Does it make sense to include SELECTED_JOBS_ONLY this way too?
> Personally, I'll probably use it via cmdline push only anyway, but I
> don't know what workflows other people have.
> ---
>  automation/gitlab-ci/build.yaml |  6 ++++++
>  automation/gitlab-ci/test.yaml  | 14 ++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 35e224366f62..f12de00a164a 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -12,6 +12,12 @@
>        - '*/*.log'
>      when: always
>    needs: []
> +  rules:
> +  - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> +    when: always
> +  - if: $SELECTED_JOBS_ONLY
> +    when: never
> +  - when: on_success
>  
>  .gcc-tmpl:
>    variables: &gcc
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index c21a37933881..93632f1f9204 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -1,6 +1,11 @@
>  .test-jobs-common:
>    stage: test
>    image: ${XEN_REGISTRY}/${CONTAINER}
> +  rules:
> +  - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> +  - if: $SELECTED_JOBS_ONLY
> +    when: never
> +  - when: on_success
>  
>  .arm64-test-needs: &arm64-test-needs
>    - alpine-3.18-arm64-rootfs-export
> @@ -99,6 +104,9 @@
>        - '*.dtb'
>      when: always
>    rules:
> +    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> +    - if: $SELECTED_JOBS_ONLY
> +      when: never
>      - if: $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
>    tags:
>      - xilinx
> @@ -117,6 +125,9 @@
>        - '*.log'
>      when: always
>    rules:
> +    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> +    - if: $SELECTED_JOBS_ONLY
> +      when: never
>      - if: $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
>    tags:
>      - xilinx
> @@ -137,6 +148,9 @@
>        - '*.log'
>      when: always
>    rules:
> +    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> +    - if: $SELECTED_JOBS_ONLY
> +      when: never
>      - if: $QUBES_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
>    tags:
>      - qubes-hw2
> -- 
> git-series 0.9.1
> 

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

* Re: [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables
  2025-02-14  1:36   ` Stefano Stabellini
@ 2025-02-14  2:32     ` Marek Marczykowski-Górecki
  2025-02-14 21:27       ` Stefano Stabellini
  0 siblings, 1 reply; 13+ messages in thread
From: Marek Marczykowski-Górecki @ 2025-02-14  2:32 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Doug Goldstein


[-- Attachment #1.1: Type: text/plain, Size: 5843 bytes --]

On Thu, Feb 13, 2025 at 05:36:47PM -0800, Stefano Stabellini wrote:
> On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > Debugging sometimes involves running specific jobs on different
> > versions. It's useful to easily avoid running all of the not interesting
> > ones (for given case) to save both time and CI resources. Doing so used
> > to require changing the yaml files, usually in several places.
> > Ease this step by adding SELECTED_JOBS_ONLY variable that takes a regex.
> > Note that one needs to satisfy job dependencies on their own (for
> > example if a test job needs a build job, that specific build job
> > needs to be included too).
> > 
> > The variable can be specified via Gitlab web UI when scheduling a
> > pipeline, but it can be also set when doing git push directly:
> > 
> >     git push -o ci.variable=SELECTED_JOBS_ONLY="/job1|job2/"
> > 
> > More details at https://docs.gitlab.co.jp/ee/user/project/push_options.html
> > 
> > The variable needs to include regex for selecting jobs, including
> > enclosing slashes.
> > A coma/space separated list of jobs to select would be friendlier UX,
> > but unfortunately that is not supported:
> > https://gitlab.com/gitlab-org/gitlab/-/issues/209904 (note the proposed
> > workaround doesn't work for job-level CI_JOB_NAME).
> > On the other hand, the regex is more flexible (one can select for
> > example all arm32 jobs).
> 
> I was trying to find workarounds so that we could also support the
> simple list of comma-separated jobs you mentioned, but I couldn't find
> an easy way to do that.
> 
> However, one thing we can do is to support writing SELECTED_JOBS_ONLY in
> .gitlab-ci.yml as a commit in xen.git, for people that don't know or
> don't remember how to set ci.variable using the git command line.

You can always do it, in `variables` setting AFAIR.

> Given that this is for testing, I think it would be no problem to adding
> a special commit on top of your tree. We are just trying to make it
> easier compared to having to manually delete the list of jobs we don't
> need.

Yes, manually delete was awful. In practice I usually added always-false
rules, but still.

> But even with the special commit, I couldn't think of an easy way to
> achieve the nicer comma-separated list of jobs...
> 
> 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> > This probably wants documenting beyond this commit message. I don't
> > think we have any CI-related docs anywhere, do we? Some new file in
> > docs/misc?
> 
> Yes please :-)
> 
> It would be also worth documenting how to create a simple pipeline by
> removing everything that you don't need for a single test

You mean how to find what jobs you need?

> > And also, it's possible to extend web ui for starting pipelines to
> > include pre-defined variables. I use it in qubes here if you want to
> > see:
> > https://gitlab.com/QubesOS/qubes-continuous-integration/-/pipelines/new
> 
> I don't have access to this

Oh, sorry. Screenshot attached.

And its definition looks like this:
https://gitlab.com/QubesOS/qubes-continuous-integration/-/blob/main/.gitlab-ci.yml?ref_type=heads#L15-26

> > Does it make sense to include SELECTED_JOBS_ONLY this way too?
> > Personally, I'll probably use it via cmdline push only anyway, but I
> > don't know what workflows other people have.
> > ---
> >  automation/gitlab-ci/build.yaml |  6 ++++++
> >  automation/gitlab-ci/test.yaml  | 14 ++++++++++++++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> > index 35e224366f62..f12de00a164a 100644
> > --- a/automation/gitlab-ci/build.yaml
> > +++ b/automation/gitlab-ci/build.yaml
> > @@ -12,6 +12,12 @@
> >        - '*/*.log'
> >      when: always
> >    needs: []
> > +  rules:
> > +  - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> > +    when: always
> > +  - if: $SELECTED_JOBS_ONLY
> > +    when: never
> > +  - when: on_success
> >  
> >  .gcc-tmpl:
> >    variables: &gcc
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index c21a37933881..93632f1f9204 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -1,6 +1,11 @@
> >  .test-jobs-common:
> >    stage: test
> >    image: ${XEN_REGISTRY}/${CONTAINER}
> > +  rules:
> > +  - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> > +  - if: $SELECTED_JOBS_ONLY
> > +    when: never
> > +  - when: on_success
> >  
> >  .arm64-test-needs: &arm64-test-needs
> >    - alpine-3.18-arm64-rootfs-export
> > @@ -99,6 +104,9 @@
> >        - '*.dtb'
> >      when: always
> >    rules:
> > +    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> > +    - if: $SELECTED_JOBS_ONLY
> > +      when: never
> >      - if: $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> >    tags:
> >      - xilinx
> > @@ -117,6 +125,9 @@
> >        - '*.log'
> >      when: always
> >    rules:
> > +    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> > +    - if: $SELECTED_JOBS_ONLY
> > +      when: never
> >      - if: $XILINX_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> >    tags:
> >      - xilinx
> > @@ -137,6 +148,9 @@
> >        - '*.log'
> >      when: always
> >    rules:
> > +    - if: $SELECTED_JOBS_ONLY && $CI_JOB_NAME =~ $SELECTED_JOBS_ONLY
> > +    - if: $SELECTED_JOBS_ONLY
> > +      when: never
> >      - if: $QUBES_JOBS == "true" && $CI_COMMIT_REF_PROTECTED == "true"
> >    tags:
> >      - qubes-hw2
> > -- 
> > git-series 0.9.1
> > 


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #1.2: Screenshot 2025-02-14 at 03-29-11 New pipeline · QubesOS _ qubes-continuous-integration · GitLab.png --]
[-- Type: image/png, Size: 94575 bytes --]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables
  2025-02-14  2:32     ` Marek Marczykowski-Górecki
@ 2025-02-14 21:27       ` Stefano Stabellini
  2025-02-14 21:37         ` Stefano Stabellini
  0 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2025-02-14 21:27 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, xen-devel, Doug Goldstein

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

On Fri, 14 Feb 2025, Marek Marczykowski-Górecki wrote:
> On Thu, Feb 13, 2025 at 05:36:47PM -0800, Stefano Stabellini wrote:
> > On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > > Debugging sometimes involves running specific jobs on different
> > > versions. It's useful to easily avoid running all of the not interesting
> > > ones (for given case) to save both time and CI resources. Doing so used
> > > to require changing the yaml files, usually in several places.
> > > Ease this step by adding SELECTED_JOBS_ONLY variable that takes a regex.
> > > Note that one needs to satisfy job dependencies on their own (for
> > > example if a test job needs a build job, that specific build job
> > > needs to be included too).
> > > 
> > > The variable can be specified via Gitlab web UI when scheduling a
> > > pipeline, but it can be also set when doing git push directly:
> > > 
> > >     git push -o ci.variable=SELECTED_JOBS_ONLY="/job1|job2/"
> > > 
> > > More details at https://docs.gitlab.co.jp/ee/user/project/push_options.html
> > > 
> > > The variable needs to include regex for selecting jobs, including
> > > enclosing slashes.
> > > A coma/space separated list of jobs to select would be friendlier UX,
> > > but unfortunately that is not supported:
> > > https://gitlab.com/gitlab-org/gitlab/-/issues/209904 (note the proposed
> > > workaround doesn't work for job-level CI_JOB_NAME).
> > > On the other hand, the regex is more flexible (one can select for
> > > example all arm32 jobs).
> > 
> > I was trying to find workarounds so that we could also support the
> > simple list of comma-separated jobs you mentioned, but I couldn't find
> > an easy way to do that.
> > 
> > However, one thing we can do is to support writing SELECTED_JOBS_ONLY in
> > .gitlab-ci.yml as a commit in xen.git, for people that don't know or
> > don't remember how to set ci.variable using the git command line.
> 
> You can always do it, in `variables` setting AFAIR.
> 
> > Given that this is for testing, I think it would be no problem to adding
> > a special commit on top of your tree. We are just trying to make it
> > easier compared to having to manually delete the list of jobs we don't
> > need.
> 
> Yes, manually delete was awful. In practice I usually added always-false
> rules, but still.
> 
> > But even with the special commit, I couldn't think of an easy way to
> > achieve the nicer comma-separated list of jobs...
> > 
> > 
> > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > > ---
> > > This probably wants documenting beyond this commit message. I don't
> > > think we have any CI-related docs anywhere, do we? Some new file in
> > > docs/misc?
> > 
> > Yes please :-)
> > 
> > It would be also worth documenting how to create a simple pipeline by
> > removing everything that you don't need for a single test
> 
> You mean how to find what jobs you need?
> 
> > > And also, it's possible to extend web ui for starting pipelines to
> > > include pre-defined variables. I use it in qubes here if you want to
> > > see:
> > > https://gitlab.com/QubesOS/qubes-continuous-integration/-/pipelines/new
> > 
> > I don't have access to this
> 
> Oh, sorry. Screenshot attached.
> 
> And its definition looks like this:
> https://gitlab.com/QubesOS/qubes-continuous-integration/-/blob/main/.gitlab-ci.yml?ref_type=heads#L15-26

Actually this gave me an idea. What if we flip the default? Normally we
want to run all the jobs.

When doing development, we typically want to run one specific job
attached to the work we are currently doing. This patch introduces a
blacklist, but it looks like we want a whitelist instead?

Wouldn't it be easier to say: "delete everything except for
adl-smoke-x86-64-dom0pvh-hvm-gcc-debug"

Then we can arrange it so adl-smoke-x86-64-dom0pvh-hvm-gcc-debug and its
dependencies are left enabled. Everything else is disabled?

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

* Re: [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables
  2025-02-14 21:27       ` Stefano Stabellini
@ 2025-02-14 21:37         ` Stefano Stabellini
  0 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2025-02-14 21:37 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Marek Marczykowski-Górecki, xen-devel, Doug Goldstein

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

On Fri, 14 Feb 2025, Stefano Stabellini wrote:
> On Fri, 14 Feb 2025, Marek Marczykowski-Górecki wrote:
> > On Thu, Feb 13, 2025 at 05:36:47PM -0800, Stefano Stabellini wrote:
> > > On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > > > Debugging sometimes involves running specific jobs on different
> > > > versions. It's useful to easily avoid running all of the not interesting
> > > > ones (for given case) to save both time and CI resources. Doing so used
> > > > to require changing the yaml files, usually in several places.
> > > > Ease this step by adding SELECTED_JOBS_ONLY variable that takes a regex.
> > > > Note that one needs to satisfy job dependencies on their own (for
> > > > example if a test job needs a build job, that specific build job
> > > > needs to be included too).
> > > > 
> > > > The variable can be specified via Gitlab web UI when scheduling a
> > > > pipeline, but it can be also set when doing git push directly:
> > > > 
> > > >     git push -o ci.variable=SELECTED_JOBS_ONLY="/job1|job2/"
> > > > 
> > > > More details at https://docs.gitlab.co.jp/ee/user/project/push_options.html
> > > > 
> > > > The variable needs to include regex for selecting jobs, including
> > > > enclosing slashes.
> > > > A coma/space separated list of jobs to select would be friendlier UX,
> > > > but unfortunately that is not supported:
> > > > https://gitlab.com/gitlab-org/gitlab/-/issues/209904 (note the proposed
> > > > workaround doesn't work for job-level CI_JOB_NAME).
> > > > On the other hand, the regex is more flexible (one can select for
> > > > example all arm32 jobs).
> > > 
> > > I was trying to find workarounds so that we could also support the
> > > simple list of comma-separated jobs you mentioned, but I couldn't find
> > > an easy way to do that.
> > > 
> > > However, one thing we can do is to support writing SELECTED_JOBS_ONLY in
> > > .gitlab-ci.yml as a commit in xen.git, for people that don't know or
> > > don't remember how to set ci.variable using the git command line.
> > 
> > You can always do it, in `variables` setting AFAIR.
> > 
> > > Given that this is for testing, I think it would be no problem to adding
> > > a special commit on top of your tree. We are just trying to make it
> > > easier compared to having to manually delete the list of jobs we don't
> > > need.
> > 
> > Yes, manually delete was awful. In practice I usually added always-false
> > rules, but still.
> > 
> > > But even with the special commit, I couldn't think of an easy way to
> > > achieve the nicer comma-separated list of jobs...
> > > 
> > > 
> > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > > > ---
> > > > This probably wants documenting beyond this commit message. I don't
> > > > think we have any CI-related docs anywhere, do we? Some new file in
> > > > docs/misc?
> > > 
> > > Yes please :-)
> > > 
> > > It would be also worth documenting how to create a simple pipeline by
> > > removing everything that you don't need for a single test
> > 
> > You mean how to find what jobs you need?
> > 
> > > > And also, it's possible to extend web ui for starting pipelines to
> > > > include pre-defined variables. I use it in qubes here if you want to
> > > > see:
> > > > https://gitlab.com/QubesOS/qubes-continuous-integration/-/pipelines/new
> > > 
> > > I don't have access to this
> > 
> > Oh, sorry. Screenshot attached.
> > 
> > And its definition looks like this:
> > https://gitlab.com/QubesOS/qubes-continuous-integration/-/blob/main/.gitlab-ci.yml?ref_type=heads#L15-26
> 
> Actually this gave me an idea. What if we flip the default? Normally we
> want to run all the jobs.
> 
> When doing development, we typically want to run one specific job
> attached to the work we are currently doing. This patch introduces a
> blacklist, but it looks like we want a whitelist instead?
> 
> Wouldn't it be easier to say: "delete everything except for
> adl-smoke-x86-64-dom0pvh-hvm-gcc-debug"
> 
> Then we can arrange it so adl-smoke-x86-64-dom0pvh-hvm-gcc-debug and its
> dependencies are left enabled. Everything else is disabled?

Sorry I sent it too fast without thinking. This patch is already
implementing the whitelist. The only thing we are really missing is the
automatic dependency enablement, but I don't know how that could be
implemented.

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

end of thread, other threads:[~2025-02-14 21:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-13  1:23 [PATCH v1 0/3] Few CI improvements Marek Marczykowski-Górecki
2025-02-13  1:23 ` [PATCH v1 1/3] automation: skip building domU if there is no test defined for it Marek Marczykowski-Górecki
2025-02-13  1:47   ` Stefano Stabellini
2025-02-13  1:23 ` [PATCH v1 2/3] automation: add jobs running tests from tools/tests/* Marek Marczykowski-Górecki
2025-02-13  2:07   ` Stefano Stabellini
2025-02-13  2:28     ` Marek Marczykowski-Górecki
2025-02-13 19:14       ` Stefano Stabellini
2025-02-13 21:47         ` Marek Marczykowski-Górecki
2025-02-13  1:23 ` [PATCH v1 3/3] automation: allow selecting individual jobs via CI variables Marek Marczykowski-Górecki
2025-02-14  1:36   ` Stefano Stabellini
2025-02-14  2:32     ` Marek Marczykowski-Górecki
2025-02-14 21:27       ` Stefano Stabellini
2025-02-14 21:37         ` Stefano Stabellini

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.