3:26 a.m.
Using shell scripts rather than inlining shell commands to YAML feels more
natural, more readable, and will keep all different variations of execution
consistent. Essentially the only disadvantage is that we won't see each command
listed one-by-one in gitlab's log output (unless we set -x that is), but given
that shell would complain if something was wrong with the script, it's fairly
easy to identify the problem.
Here's a test pipeline after the change:
https://gitlab.com/eskultety/libvirt/-/pipelines/759277200
Since v1:
- 3/7 - reworded commit message as requested
- 4/7 was dropped
- point the SCRATCH_DIR to /var/tmp instead of /tmp to not be limited by the
size of ramdisk mounted in there
Erik Skultety (8):
syntax-check: Drop the shell's 'check for minus' rule
ci: Move the SCRATCH_DIR from /tmp
ci: integration: Extract several hidden job definitions to a script
ci: integration: Drop the 'install-deps' hidden job and reference
ci: integration-template: Drop the '-lt Fedora 35' check
ci: integration.sh: Add/Rewrite/Reformat commentaries
ci: integration.sh: Replace 'test' with '[' operator
ci: integration.sh: Define the SCRATCH_DIR variable for local
execution
build-aux/syntax-check.mk | 9 --------
ci/integration-template.yml | 44 +++--------------------------------
ci/integration.sh | 46 +++++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 50 deletions(-)
create mode 100644 ci/integration.sh
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 1/8] syntax-check: Drop the shell's 'check for minus' rule
Apparently we've only had it because the -[ao] options weren't portable
at the time, but according to
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
both are defined in POSIX.1-2017 revision which is old enough for all
our supported platforms to have adopted it already. Therefore, drop the
rule.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
build-aux/syntax-check.mk | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 96d322ee04..7b07fd4712 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1183,15 +1183,6 @@ sc_prohibit_double_semicolon:
halt="Double semicolon detected" \
$(_sc_search_regexp)
-_ptm1 = use "test C1 && test C2", not "test C1 -''a
C2"
-_ptm2 = use "test C1 || test C2", not "test C1 -''o C2"
-# Using test's -a and -o operators is not portable.
-# We prefer test over [, since the latter is spelled [[ in configure.ac.
-sc_prohibit_test_minus_ao:
- @prohibit='(\<test| \[+) .+ -[ao] ' \
- halt='$(_ptm1); $(_ptm2)' \
- $(_sc_search_regexp)
-
# Avoid a test bashism.
sc_prohibit_test_double_equal:
@prohibit='(\<test| \[+) .+ == ' \
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 2/8] ci: Move the SCRATCH_DIR from /tmp
In case /tmp is mounted as an in-memory file system, we're fairly storage
limited. This might pose a problem in VM scenarios if we build multiple
projects in there or download some data needed for testing. Therefore,
move the scratch to /home. Also, take the opportunity and rename
scratch to 'vroot' as primarily we're building other projects in this
directory, the fact that we also store other data is only a secondary
use case for it.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration-template.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ci/integration-template.yml b/ci/integration-template.yml
index 9141d5cedf..15c36d7a29 100644
--- a/ci/integration-template.yml
+++ b/ci/integration-template.yml
@@ -91,7 +91,7 @@
- test "$CI_JOB_STATUS" = "success" && exit 0;
- *collect-logs
variables:
- SCRATCH_DIR: "/tmp/scratch"
+ SCRATCH_DIR: "/var/tmp/scratch"
artifacts:
name: logs
expire_in: 1 day
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 3/8] ci: integration: Extract several hidden job definitions to a script
This will allow us to re-use the script in other scenarios like local
executions where up until now one had to copy-paste the recipe
line-by-line.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration-template.yml | 36 +-----------------------------------
ci/integration.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 35 deletions(-)
create mode 100644 ci/integration.sh
diff --git a/ci/integration-template.yml b/ci/integration-template.yml
index 15c36d7a29..802cba9420 100644
--- a/ci/integration-template.yml
+++ b/ci/integration-template.yml
@@ -27,34 +27,6 @@
.install-deps: &install-deps
- sudo dnf install -y libvirt-rpms/* libvirt-perl-rpms/* libvirt-python-rpms/*
- # Avocado >98.0 fails with the nwfilter TCK tests, so stick with 98.0 for now
- - sudo pip3 install --prefix=/usr avocado-framework==98.0
-
-
-.enable-core-dumps: &enable-core-dumps
- - sudo sh -c "echo DefaultLimitCORE=infinity >>
/etc/systemd/system.conf" # Explicitly allow storing cores globally
- - sudo systemctl daemon-reexec # need to reexec systemd after changing config
-
-
-.enable-libvirt-debugging: &enable-libvirt-debugging
- - source /etc/os-release # in order to query the vendor-provided variables
- - if test "$ID" = "centos" && test "$VERSION_ID"
-lt 9 ||
- test "$ID" = "fedora" && test "$VERSION_ID"
-lt 35;
- then
- DAEMONS="libvirtd virtlockd virtlogd";
- else
- DAEMONS="virtinterfaced virtlockd virtlogd virtnetworkd virtnodedevd
virtnwfilterd virtproxyd virtqemud virtsecretd virtstoraged";
- fi
- - for daemon in $DAEMONS;
- do
- LOG_OUTPUTS="1:file:/var/log/libvirt/${daemon}.log";
- LOG_FILTERS="3:remote 4:event 3:util.json 3:util.object 3:util.dbus
3:util.netlink 3:node_device 3:rpc 3:access 1:*";
- sudo augtool set /files/etc/libvirt/${daemon}.conf/log_filters
"'$LOG_FILTERS'" &>/dev/null;
- sudo augtool set /files/etc/libvirt/${daemon}.conf/log_outputs
"'$LOG_OUTPUTS'" &>/dev/null;
- sudo systemctl --quiet stop ${daemon}.service;
- sudo systemctl restart ${daemon}.socket;
- done
-
.collect-logs: &collect-logs
- set +e
@@ -79,14 +51,8 @@
before_script:
- mkdir "$SCRATCH_DIR"
- *install-deps
- - *enable-core-dumps
- - *enable-libvirt-debugging
- - sudo virsh net-start default &>/dev/null || true;
script:
- - cd "$SCRATCH_DIR"
- - git clone --depth 1 https://gitlab.com/libvirt/libvirt-tck.git
- - cd libvirt-tck
- - sudo avocado --config avocado.config run --job-results-dir
"$SCRATCH_DIR"/avocado
+ - chmod +x ci/integration.sh && ci/integration.sh
after_script:
- test "$CI_JOB_STATUS" = "success" && exit 0;
- *collect-logs
diff --git a/ci/integration.sh b/ci/integration.sh
new file mode 100644
index 0000000000..21795518c3
--- /dev/null
+++ b/ci/integration.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+# Avocado >98.0 fails with the nwfilter TCK tests, so stick with 98.0 for now
+sudo pip3 install --prefix=/usr avocado-framework==98.0
+
+sudo sh -c "echo DefaultLimitCORE=infinity >> /etc/systemd/system.conf" #
Explicitly allow storing cores globally
+sudo systemctl daemon-reexec # need to reexec systemd after changing config
+
+source /etc/os-release # in order to query the vendor-provided variables
+if test "$ID" = "centos" && test "$VERSION_ID" -lt
9 ||
+ test "$ID" = "fedora" && test "$VERSION_ID" -lt
35
+then
+ DAEMONS="libvirtd virtlockd virtlogd"
+else
+ DAEMONS="virtinterfaced virtlockd virtlogd virtnetworkd virtnodedevd
virtnwfilterd virtproxyd virtqemud virtsecretd virtstoraged"
+fi
+for daemon in $DAEMONS
+do
+ LOG_OUTPUTS="1:file:/var/log/libvirt/${daemon}.log"
+ LOG_FILTERS="3:remote 4:event 3:util.json 3:util.object 3:util.dbus
3:util.netlink 3:node_device 3:rpc 3:access 1:*"
+ sudo augtool set /files/etc/libvirt/${daemon}.conf/log_filters
"'$LOG_FILTERS'" &>/dev/null
+ sudo augtool set /files/etc/libvirt/${daemon}.conf/log_outputs
"'$LOG_OUTPUTS'" &>/dev/null
+ sudo systemctl --quiet stop ${daemon}.service
+ sudo systemctl restart ${daemon}.socket
+done
+
+sudo virsh net-start default &>/dev/null || true
+
+cd "$SCRATCH_DIR"
+git clone --depth 1 https://gitlab.com/libvirt/libvirt-tck.git
+cd libvirt-tck
+sudo avocado --config avocado.config run --job-results-dir
"$SCRATCH_DIR"/avocado
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 4/8] ci: integration: Drop the 'install-deps' hidden job and reference
Since the section now only consists of a single command, we can happily
move the command to the main integration template job body.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration-template.yml | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/ci/integration-template.yml b/ci/integration-template.yml
index 802cba9420..1425510d4f 100644
--- a/ci/integration-template.yml
+++ b/ci/integration-template.yml
@@ -24,10 +24,6 @@
- sudo make install
-.install-deps: &install-deps
- - sudo dnf install -y libvirt-rpms/* libvirt-perl-rpms/* libvirt-python-rpms/*
-
-
.collect-logs: &collect-logs
- set +e
- shopt -s nullglob
@@ -50,7 +46,7 @@
stage: integration_tests
before_script:
- mkdir "$SCRATCH_DIR"
- - *install-deps
+ - sudo dnf install -y libvirt-rpms/* libvirt-perl-rpms/* libvirt-python-rpms/*
script:
- chmod +x ci/integration.sh && ci/integration.sh
after_script:
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 5/8] ci: integration-template: Drop the '-lt Fedora 35' check
We no longer support Fedora 35 or older. While at it, we can also
tailor the rest of the check to CentOS Stream 8 only since all other
supported Red Hat like platforms have moved onto modular daemons setup
already.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/ci/integration.sh b/ci/integration.sh
index 21795518c3..160aecf00d 100644
--- a/ci/integration.sh
+++ b/ci/integration.sh
@@ -7,8 +7,7 @@ sudo sh -c "echo DefaultLimitCORE=infinity >>
/etc/systemd/system.conf" # Explic
sudo systemctl daemon-reexec # need to reexec systemd after changing config
source /etc/os-release # in order to query the vendor-provided variables
-if test "$ID" = "centos" && test "$VERSION_ID" -lt
9 ||
- test "$ID" = "fedora" && test "$VERSION_ID" -lt
35
+if test "$ID" = "centos" && test "$VERSION_ID" -eq
8
then
DAEMONS="libvirtd virtlockd virtlogd"
else
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 6/8] ci: integration.sh: Add/Rewrite/Reformat commentaries
Because of the nature of writing inline shell commands to YAML, most of
the commentaries where inlined with the command not to hinder YAML
readability any further. Since we moved the logic to a standalone
script, we can now do whatever formatting & readability adjustments we
want.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration.sh | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/ci/integration.sh b/ci/integration.sh
index 160aecf00d..cbf2b0f1b4 100644
--- a/ci/integration.sh
+++ b/ci/integration.sh
@@ -3,10 +3,15 @@
# Avocado >98.0 fails with the nwfilter TCK tests, so stick with 98.0 for now
sudo pip3 install --prefix=/usr avocado-framework==98.0
-sudo sh -c "echo DefaultLimitCORE=infinity >> /etc/systemd/system.conf" #
Explicitly allow storing cores globally
-sudo systemctl daemon-reexec # need to reexec systemd after changing config
+# Explicitly allow storing cores globally
+sudo sh -c "echo DefaultLimitCORE=infinity >> /etc/systemd/system.conf"
+
+# Need to reexec systemd after changing config
+sudo systemctl daemon-reexec
+
+# Source the os-release file to query the vendor-provided variables
+source /etc/os-release
-source /etc/os-release # in order to query the vendor-provided variables
if test "$ID" = "centos" && test "$VERSION_ID" -eq
8
then
DAEMONS="libvirtd virtlockd virtlogd"
@@ -23,6 +28,7 @@ do
sudo systemctl restart ${daemon}.socket
done
+# Make sure the default network is started on all platforms
sudo virsh net-start default &>/dev/null || true
cd "$SCRATCH_DIR"
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 7/8] ci: integration.sh: Replace 'test' with '[' operator
Since we now have a standalone script instead of inlined shell commands
in a YAML, we can adopt the common syntax for if-then-else clauses
which involve the '[' test command alias rather than keep using 'test'
explicitly.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ci/integration.sh b/ci/integration.sh
index cbf2b0f1b4..7bf3623809 100644
--- a/ci/integration.sh
+++ b/ci/integration.sh
@@ -12,7 +12,7 @@ sudo systemctl daemon-reexec
# Source the os-release file to query the vendor-provided variables
source /etc/os-release
-if test "$ID" = "centos" && test "$VERSION_ID" -eq
8
+if [ "$ID" = "centos" -a "$VERSION_ID" -eq 8 ]
then
DAEMONS="libvirtd virtlockd virtlogd"
else
--
2.39.1
3:26 a.m.
New subject: [libvirt PATCH v2 8/8] ci: integration.sh: Define the SCRATCH_DIR variable for local execution
Running outside of GitLab will likely not have the variable set and
hence the execution would fail.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/integration.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ci/integration.sh b/ci/integration.sh
index 7bf3623809..e1f461a441 100644
--- a/ci/integration.sh
+++ b/ci/integration.sh
@@ -31,6 +31,15 @@ done
# Make sure the default network is started on all platforms
sudo virsh net-start default &>/dev/null || true
+# If we're running outside of GitLab, this variable will likely not exist, so
+# we need to define it and create the scratch directory
+if [ -z "$SCRATCH_DIR" ]
+then
+ SCRATCH_DIR="/var/tmp/scratch"
+ mkdir "$SCRATCH_DIR" 2>/dev/null
+fi
+
+rm -rf "$SCRATCH_DIR/*"
cd "$SCRATCH_DIR"
git clone --depth 1 https://gitlab.com/libvirt/libvirt-tck.git
cd libvirt-tck
--
2.39.1
7:54 a.m.
On Fri, Jan 27, 2023 at 10:26:48AM +0100, Erik Skultety wrote:
Using shell scripts rather than inlining shell commands to YAML feels
more
natural, more readable, and will keep all different variations of execution
consistent. Essentially the only disadvantage is that we won't see each command
listed one-by-one in gitlab's log output (unless we set -x that is), but given
that shell would complain if something was wrong with the script, it's fairly
easy to identify the problem.
Here's a test pipeline after the change:
https://gitlab.com/eskultety/libvirt/-/pipelines/759277200
Since v1:
- 3/7 - reworded commit message as requested
- 4/7 was dropped
- point the SCRATCH_DIR to /var/tmp instead of /tmp to not be limited by the
size of ramdisk mounted in there
Ping.
654
days inactive
664
days old
9 comments
1 participants
participants (1)
-
Erik Skultety