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&gt; --- 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 9141d5cedf..f9903cdbb4 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.0

On Mon, Jan 23, 2023 at 12:06:21PM +0000, Daniel P. Berrangé wrote: Yeah, I don't think that's going to happen. Then again, we'd be probably the only consumers of such thing by far, so adding an explicit script which everyone can read and run without anything else is for the best. Regards, Erik

We no longer support Fedora 35. While at it, we can also tailor the rest of the check to CentOS Stream 8 since all other supported Red Hat like platforms have moved onto modular daemons setup already. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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.0

This is a purely cosmetic change dropping the majority of 'sudo' invocations which we need to do to make final preparations on the test environment. This patch visually divides the part where we need to run as root from the rest of the operations. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- ci/integration.sh | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/ci/integration.sh b/ci/integration.sh index 160aecf00d..5cee536442 100644 --- a/ci/integration.sh +++ b/ci/integration.sh @@ -1,10 +1,13 @@ #!/bin/sh +# BEGIN AS ROOT +sudo su - + # 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 +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 +echo "DefaultLimitCORE=infinity" >> /etc/systemd/system.conf # Explicitly allow storing cores globally +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" -eq 8 @@ -17,13 +20,16 @@ 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 + augtool set /files/etc/libvirt/${daemon}.conf/log_filters "'$LOG_FILTERS'" &>/dev/null + augtool set /files/etc/libvirt/${daemon}.conf/log_outputs "'$LOG_OUTPUTS'" &>/dev/null + systemctl --quiet stop ${daemon}.service + systemctl restart ${daemon}.socket done -sudo virsh net-start default &>/dev/null || true +virsh net-start default &>/dev/null || true + +# END AS ROOT +exit cd "$SCRATCH_DIR" git clone --depth 1 https://gitlab.com/libvirt/libvirt-tck.git -- 2.39.0

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&gt; --- ci/integration.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/ci/integration.sh b/ci/integration.sh index 5cee536442..cbd55d4253 100644 --- a/ci/integration.sh +++ b/ci/integration.sh @@ -6,10 +6,15 @@ sudo su - # Avocado >98.0 fails with the nwfilter TCK tests, so stick with 98.0 for now pip3 install --prefix=/usr avocado-framework==98.0 -echo "DefaultLimitCORE=infinity" >> /etc/systemd/system.conf # Explicitly allow storing cores globally -systemctl daemon-reexec # need to reexec systemd after changing config +# Explicitly allow storing cores globally +echo "DefaultLimitCORE=infinity" >> /etc/systemd/system.conf + +# Need to reexec systemd after changing config +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" @@ -26,6 +31,7 @@ do systemctl restart ${daemon}.socket done +# Make sure the default network is started on all platforms virsh net-start default &>/dev/null || true # END AS ROOT -- 2.39.0

On Fri, Jan 20, 2023 at 04:06:02PM +0100, Erik Skultety wrote: Or simply: if [ "$ID" = "centos" -a "$VERSION_ID" -eq 8 ]

On Fri, Jan 20, 2023 at 04:06:03PM +0100, Erik Skultety wrote: This could fail if someone has this directory already. Which is a good thing as otherwise it could override some of it. But wouldn't it be nicer to use mktemp -d and print the result?

On Mon, Jan 23, 2023 at 10:50:31AM +0100, Erik Skultety wrote: I don't really understand how that affects a change from: SCRATCH_DIR="/tmp/scratch" mkdir "$SCRATCH_DIR" to something like SCRATCH_DIR=$(mktemp -d) or possibly SCRATCH_DIR=$(mktemp -d "/tmp/scratch.XXX") especially when only used without the SCRATCH_DIR variable.

On Mon, Jan 23, 2023 at 12:08:23PM +0100, Erik Skultety wrote: Since running as root you might just mount tmpfs over /tmp/scratch. That is if you are fine with the RAM being used for storage, but I presume that not much is needed. So I guess I misunderstood and I need some clarification. This script will run inside the VM used for testing and is not in any case meant to be run on a machine used for other purposes since it has side effects, right? If that's the case (and looking at it again it seems like it is) I'm fine with both solutions. And I'm guessing the /tmp/scratch is either hardcoded somewhere else or it is expected that someone can diff some outputs with the full path, then (possibly in the future)?

On Mon, Jan 23, 2023 at 10:50:31AM +0100, Erik Skultety wrote: I'm wondering why I put it at /tmp/scratch in the first place when we started using gitlab, and struggling to come up with a justification. In fact I'm not entirely convinced we really need a SCRATCH_DIR env variable at all, since AFAICT, we only use it one place for creating $VROOT. In terms of standalone reproduction of build env, it would be saner to use VOORT=$CWD/vroot, which I think would probably work under GitLab ok too, and do away with SCRATCH_DIR. 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 :|

On Mon, Jan 23, 2023 at 12:02:44PM +0000, Daniel P. Berrangé wrote: I think I may have found the answer to ^this - variables can only be static strings, so $PWD/vroot would not be an option in such case. The obvious benefit is that the variables are inherited to all jobs which extend a template, so even if we currently use SCRATCH_DIR in a single YAML file only, I'd like to keep it that way. Like I mentioned in my previous response, seems like we'd only rename the variable and even had to re-define it in all (.*_)?script sections along with all inherited templates. So, to keep being consistent with other libvirt projects where I believe we still use SCRATCH_DIR in this form. Let's just fix the ramdisk mount issue and move it to /var/tmp - it's a static path, everyone has access there (EUID depends on how the runner was configured), and it's persistent storage Regards, Erik