[libvirt PATCH 0/7] ci: Integration test suite template fixes
Failed job to demonstrate fixed log collecting: https://gitlab.com/eskultety/libvirt/-/pipelines/666789669 Successfull job with all patches applied: https://gitlab.com/eskultety/libvirt/-/pipelines/666906197 Erik Skultety (7): ci: integration: Add virtnetworkd to the list of restarted services ci: integration: Add an extra level of quoting for augeas set options ci: integration: Fix the test on avocado directory existence ci: integration: Only create a coredump log when a core exists ci: integration: Use 'set +e' in the log collecting job ci: integration: Create the avocado logs artifact dir conditionally ci: integration: Bundle only failed Avocado test results ci/integration-template.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) -- 2.37.3
It was missing from the set. While at it, order the daemon set alphabetically. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/integration-template.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/integration-template.yml b/ci/integration-template.yml index acb51a0b27..4ca8a0bb24 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -39,9 +39,9 @@ - if test "$ID" = "centos" && test "$VERSION_ID" -lt 9 || test "$ID" = "fedora" && test "$VERSION_ID" -lt 35; then - DAEMONS="libvirtd virtlogd virtlockd"; + DAEMONS="libvirtd virtlockd virtlogd"; else - DAEMONS="virtproxyd virtqemud virtinterfaced virtsecretd virtstoraged virtnwfilterd virtnodedevd virtlogd virtlockd"; + DAEMONS="virtinterfaced virtlockd virtlogd virtnetworkd virtnodedevd virtnwfilterd virtproxyd virtqemud virtsecretd virtstoraged"; fi - for daemon in $DAEMONS; do -- 2.37.3
Both log filters and log outputs expect string values, however, augeas apparently requires an extra level of quotes apart from the ones we pass via shell (see comment [1]) to work properly, otherwise augeas ignores the value and returns 0. Without this fix we don't set libvirt's log level to debug, we don't set logging to a file and hence we don't include the logs in CI artifacts in case the test suite fails. [1] https://github.com/hercules-team/augeas/issues/301#issuecomment-143699880 Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/integration-template.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/integration-template.yml b/ci/integration-template.yml index 4ca8a0bb24..bb8969da19 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -47,8 +47,8 @@ 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 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 -- 2.37.3
It's a directory, so -d should be used with 'test'. Signed-off-by: Erik Skultety <eskultet@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 bb8969da19..2e7802dbe0 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -56,7 +56,7 @@ .collect-logs: &collect-logs - mkdir logs - - test -e "$SCRATCH_DIR"/avocado && sudo mv "$SCRATCH_DIR"/avocado/latest/test-results logs/avocado; + - test -d "$SCRATCH_DIR"/avocado && sudo mv "$SCRATCH_DIR"/avocado/latest/test-results logs/avocado; - sudo coredumpctl info --no-pager > logs/coredumpctl.txt - sudo mv /var/log/libvirt logs/libvirt - sudo chown -R $(whoami):$(whoami) logs -- 2.37.3
It could be quite confusing looking at the job log artifacts and having an empty coredump log in there, IOW it doesn't really give much confidence that the reporting mechanism actually works. Signed-off-by: Erik Skultety <eskultet@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 2e7802dbe0..4321f0d29e 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -57,7 +57,7 @@ .collect-logs: &collect-logs - mkdir logs - test -d "$SCRATCH_DIR"/avocado && sudo mv "$SCRATCH_DIR"/avocado/latest/test-results logs/avocado; - - sudo coredumpctl info --no-pager > logs/coredumpctl.txt + - sudo coredumpctl &>/dev/null && sudo coredumpctl info --no-pager > logs/coredumpctl.txt - sudo mv /var/log/libvirt logs/libvirt - sudo chown -R $(whoami):$(whoami) logs # rename all Avocado stderr/stdout logs to *.log so that GitLab's web UI doesn't mangle the MIME type -- 2.37.3
All 'script' blocks are defined as 'set -e' and so a single failed return value means we won't collect some of the logs. Because of the nature of the original job's failure some of the log sources might not be available, but that's fine, however, the gitlab after_script job cannot finish prematurely. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/integration-template.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/integration-template.yml b/ci/integration-template.yml index 4321f0d29e..67982c5167 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -55,6 +55,7 @@ .collect-logs: &collect-logs + - set +e - mkdir logs - test -d "$SCRATCH_DIR"/avocado && sudo mv "$SCRATCH_DIR"/avocado/latest/test-results logs/avocado; - sudo coredumpctl &>/dev/null && sudo coredumpctl info --no-pager > logs/coredumpctl.txt -- 2.37.3
Don't create an avocado directory in the resulting log artifacts if Avocado didn't even run (e.g. libvirt errored out on service restart). Signed-off-by: Erik Skultety <eskultet@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 67982c5167..48e71e2dd9 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -57,7 +57,7 @@ .collect-logs: &collect-logs - set +e - mkdir logs - - test -d "$SCRATCH_DIR"/avocado && sudo mv "$SCRATCH_DIR"/avocado/latest/test-results logs/avocado; + - test -d "$SCRATCH_DIR"/avocado && mkdir logs/avocado - sudo coredumpctl &>/dev/null && sudo coredumpctl info --no-pager > logs/coredumpctl.txt - sudo mv /var/log/libvirt logs/libvirt - sudo chown -R $(whoami):$(whoami) logs -- 2.37.3
Latest versions of Avocado create 'by-status' symlink shortcuts to test results, IOW: # this is the main test results directory containing all data $ ls <path>/avocado/job-results/latest/test-results/ 01-scripts_networks_050-transient-lifecycle.t 02-scripts_networks_051-transient-autostart.t ... 22-scripts_networks_400-guest-bandwidth.t by_status/ # list only the failed tests $ ls -l <path>/avocado/job-results/latest/test-results/by-status/FAIL 19-scripts_networks_360-guest-network-vepa.t -> <path>/avocado/job-results/latest/test-results/19-scripts_networks_360-guest-network-vepa.t Therefore, let's bundle only the failed ones, it's going to make the log artifacts more obvious when looking for libvirt errors. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/integration-template.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci/integration-template.yml b/ci/integration-template.yml index 48e71e2dd9..f01de7845d 100644 --- a/ci/integration-template.yml +++ b/ci/integration-template.yml @@ -56,10 +56,16 @@ .collect-logs: &collect-logs - set +e + - shopt -s nullglob - mkdir logs - test -d "$SCRATCH_DIR"/avocado && mkdir logs/avocado - sudo coredumpctl &>/dev/null && sudo coredumpctl info --no-pager > logs/coredumpctl.txt - sudo mv /var/log/libvirt logs/libvirt + # filter only the failed tests, omitting successful job logs + - for test_log in "$SCRATCH_DIR"/avocado/latest/test-results/by-status/{ERROR,FAIL}/*; + do + sudo mv "$(realpath $test_log)" logs/avocado; + done; - sudo chown -R $(whoami):$(whoami) logs # rename all Avocado stderr/stdout logs to *.log so that GitLab's web UI doesn't mangle the MIME type - find logs/avocado/ -type f ! -name "*.log" -exec -- 2.37.3
On Fri, Oct 14, 2022 at 01:28:38PM +0200, Erik Skultety wrote:
Failed job to demonstrate fixed log collecting: https://gitlab.com/eskultety/libvirt/-/pipelines/666789669
Successfull job with all patches applied: https://gitlab.com/eskultety/libvirt/-/pipelines/666906197
Erik Skultety (7): ci: integration: Add virtnetworkd to the list of restarted services ci: integration: Add an extra level of quoting for augeas set options ci: integration: Fix the test on avocado directory existence ci: integration: Only create a coredump log when a core exists ci: integration: Use 'set +e' in the log collecting job ci: integration: Create the avocado logs artifact dir conditionally ci: integration: Bundle only failed Avocado test results
ci/integration-template.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
-- 2.37.3
ping
On 10/14/22 13:28, Erik Skultety wrote:
Failed job to demonstrate fixed log collecting: https://gitlab.com/eskultety/libvirt/-/pipelines/666789669
Successfull job with all patches applied: https://gitlab.com/eskultety/libvirt/-/pipelines/666906197
Erik Skultety (7): ci: integration: Add virtnetworkd to the list of restarted services ci: integration: Add an extra level of quoting for augeas set options ci: integration: Fix the test on avocado directory existence ci: integration: Only create a coredump log when a core exists ci: integration: Use 'set +e' in the log collecting job ci: integration: Create the avocado logs artifact dir conditionally ci: integration: Bundle only failed Avocado test results
ci/integration-template.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Erik Skultety -
Michal Prívozník