[Merge] ~rbalint/ubuntu/+source/systemd:tests-in-lxd into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-groovy

Dan Streetman ddstreet at canonical.com
Fri May 8 12:42:24 UTC 2020


LGTM with just minor comments inline below.

Diff comments:

> diff --git a/debian/patches/test-Skip-test-boot-timestamps-on-permission-denied.patch b/debian/patches/test-Skip-test-boot-timestamps-on-permission-denied.patch
> new file mode 100644
> index 0000000..1acfe77
> --- /dev/null
> +++ b/debian/patches/test-Skip-test-boot-timestamps-on-permission-denied.patch
> @@ -0,0 +1,41 @@
> +From: Balint Reczey <balint.reczey at canonical.com>
> +Date: Tue, 5 May 2020 21:24:53 +0200
> +Subject: test: Skip test-boot-timestamps on permission denied
> +Forwarded: https://github.com/systemd/systemd/pull/15728
> +
> +In containers even root can be denied to access the needed files.
> +---
> + src/test/test-boot-timestamps.c | 6 +++---
> + 1 file changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/src/test/test-boot-timestamps.c b/src/test/test-boot-timestamps.c
> +index 3c7f7a9..29074ec 100644
> +--- a/src/test/test-boot-timestamps.c
> ++++ b/src/test/test-boot-timestamps.c
> +@@ -17,7 +17,7 @@ static int test_acpi_fpdt(void) {
> + 
> +         r = acpi_get_boot_usec(&loader_start, &loader_exit);
> +         if (r < 0) {
> +-                bool ok = r == -ENOENT || (getuid() != 0 && r == -EACCES) || r == -ENODATA;

should ignoring EACCES for non-root be conditional on detect_container()?

> ++                bool ok = r == -ENOENT || r == -EACCES || r == -ENODATA;
> + 
> +                 log_full_errno(ok ? LOG_DEBUG : LOG_ERR, r, "Failed to read ACPI FPDT: %m");
> +                 return ok ? 0 : r;
> +@@ -37,7 +37,7 @@ static int test_efi_loader(void) {
> + 
> +         r = efi_loader_get_boot_usec(&loader_start, &loader_exit);
> +         if (r < 0) {
> +-                bool ok = r == -ENOENT || (getuid() != 0 && r == -EACCES) || r == -EOPNOTSUPP;
> ++                bool ok = r == -ENOENT || r == -EACCES || r == -EOPNOTSUPP;
> + 
> +                 log_full_errno(ok ? LOG_DEBUG : LOG_ERR, r, "Failed to read EFI loader data: %m");
> +                 return ok ? 0 : r;
> +@@ -59,7 +59,7 @@ static int test_boot_timestamps(void) {
> + 
> +         r = boot_timestamps(NULL, &fw, &l);
> +         if (r < 0) {
> +-                bool ok = r == -ENOENT || (getuid() != 0 && r == -EACCES) || r == -EOPNOTSUPP;
> ++                bool ok = r == -ENOENT || r == -EACCES || r == -EOPNOTSUPP;
> + 
> +                 log_full_errno(ok ? LOG_DEBUG : LOG_ERR, r, "Failed to read variables: %m");
> +                 return ok ? 0 : r;
> diff --git a/debian/tests/control b/debian/tests/control
> index c6aef85..febf6a9 100644
> --- a/debian/tests/control
> +++ b/debian/tests/control
> @@ -120,6 +120,15 @@ Depends: systemd-tests,
>    dbus-user-session,
>  Restrictions: needs-root, allow-stderr, isolation-container, breaks-testbed
>  
> +Tests: tests-in-lxd

should be fine to locate it here in the ubuntu pkg, but if it goes in debian it should go below the "# NOUPSTREAM: Do not run these tests for upstream builds" marker (unless it really should be run for upstream ci)

> +Depends: systemd-tests,
> +  udev,
> +  libpam-systemd,
> +  autopkgtest,
> +  lsb-release,
> +  lxd
> +Restrictions: needs-root, allow-stderr, isolation-machine, breaks-testbed, skip-not-installable
> +
>  Tests: upstream
>  Depends: libsystemd-dev,
>    tree,
> diff --git a/debian/tests/tests-in-lxd b/debian/tests/tests-in-lxd
> new file mode 100644
> index 0000000..9617587
> --- /dev/null
> +++ b/debian/tests/tests-in-lxd
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +# run root-unittests in lxd
> +
> +set -eu
> +
> +lxd init --auto
> +
> +IMAGE=autopkgtest/ubuntu/$(lsb_release -cs)/$(dpkg --print-architecture)
> +
> +autopkgtest-build-lxd ubuntu-daily:$(lsb_release -cs)
> +
> +# push local apt configuration to match pinning
> +lxc launch $IMAGE systemd-lxc
> +lxc file push -r /etc/apt systemd-lxc/etc/
> +
> +# TODO: also inject locally built systemd packages, if there is any
> +
> +# fix failing systemd-remount-fs.service LP: #1877078
> +lxc file delete systemd-lxc/etc/fstab
> +lxc exec  systemd-lxc touch /etc/fstab
> +
> +lxc stop systemd-lxc
> +lxc publish systemd-lxc --alias $IMAGE
> +
> +for t in root-unittests boot-and-services; do
> +    autopkgtest -U -B systemd --test-name=$t -- lxd $IMAGE

should it run upstream tests?

Also, how much extra time does this add?  I think the majority of test time is in the 'upstream' test, but I believe root-unittests is the #2 longest, though I have not looked at each test's total runtime in a while, so maybe it doesn't add any significant time.  I'd previously requested autopkgtest-cloud to run systemd and systemd-upstream in larger instances (since they currently use the default m1.small 1-cpu flavor), but that was blocked, IIRC.

> +done
> +
> +# also check tests in privileged containers
> +lxc profile set default security.privileged "true"
> +for t in root-unittests boot-and-services; do
> +    autopkgtest -U -B systemd --test-name=$t -- lxd $IMAGE
> +done


-- 
https://code.launchpad.net/~rbalint/ubuntu/+source/systemd/+git/systemd/+merge/383501
Your team Ubuntu Core Development Team is subscribed to branch ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-groovy.



More information about the Ubuntu-reviews mailing list