Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/unit/go: use tests.session wrapper for running tests as a user #9976

Merged
merged 5 commits into from Mar 5, 2021

Conversation

bboozzoo
Copy link
Collaborator

@bboozzoo bboozzoo commented Mar 2, 2021

We occasionally see the tests invariant check incorrectly pick up a 'test' user
session dbus:

2021-03-01T08:55:18.3295048Z tests.invariant: root-files-in-home ok
2021-03-01T08:55:18.3296599Z tests.invariant: crashed-snap-confine ok
2021-03-01T08:55:18.3298081Z tests.invariant: lxcfs-mounted ok
2021-03-01T08:55:18.3299546Z tests.invariant: more than one dbus-daemon running
2021-03-01T08:55:18.3301790Z pid:67160 user:test cmdline:/usr/bin/dbus-daemon --session --address=systemd: --nofork --nopidfile --systemd-activation --syslog-only
2021-03-01T08:55:18.3303861Z tests.invariant: stray-dbus-daemon not-ok
2021-03-01T08:55:18.3305558Z tests.invariant: leftover-defer-sh ok
2021-03-01T08:55:18.3306643Z tests.invariant: system is corrupted

Use the tests.session wrapper for running things as the 'test' user so that we
can ensure a proper session cleanup afterwards.

We occasionally see the tests invariant check incorrectly pick up a 'test' user
session dbus:

2021-03-01T08:55:18.3295048Z tests.invariant: root-files-in-home ok
2021-03-01T08:55:18.3296599Z tests.invariant: crashed-snap-confine ok
2021-03-01T08:55:18.3298081Z tests.invariant: lxcfs-mounted ok
2021-03-01T08:55:18.3299546Z tests.invariant: more than one dbus-daemon running
2021-03-01T08:55:18.3301790Z pid:67160 user:test cmdline:/usr/bin/dbus-daemon --session --address=systemd: --nofork --nopidfile --systemd-activation --syslog-only
2021-03-01T08:55:18.3303861Z tests.invariant: stray-dbus-daemon not-ok
2021-03-01T08:55:18.3305558Z tests.invariant: leftover-defer-sh ok
2021-03-01T08:55:18.3306643Z tests.invariant: system is corrupted

Use the tests.session wrapper for running things as the 'test' user so that we
can ensure a proper session cleanup afterwards.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
@bboozzoo bboozzoo added Simple 😃 A small PR which can be reviewed quickly Test Robustness labels Mar 2, 2021
Copy link
Contributor

@stolowski stolowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

On 18.04, the following error is observed in secboot tests using fde hooks:

"cannot run fde-reveal-key: Failed to start transient service unit: Invalid Type
setting: exec"

This is confirmed by running the following command on 18.04 using the same
parameters as the actual FDE reveal key command:

google:ubuntu-18.04-64 $ systemd-run --user --wait --collect --service-type=exec /bin/true
Failed to start transient service unit: Invalid Type setting: exec

The --help output of systemd-run includes --service-type, but the manpage does
not, suggesting this may be a bug in systemd or its packaging.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
@bboozzoo
Copy link
Collaborator Author

bboozzoo commented Mar 3, 2021

The secboot unit tests failed on 18.04. Looks like systemd-run from 237 does not accept --service-type=exec in the command line and unit tests would fail with cannot run fde-reveal-key: Failed to start transient service unit: Invalid Type setting: exec.

We have some logic to skip the tests, which likely works on 16.04, but the logic used a different systemd-run invocation from what the code does. I've pushed a commit to do the check correctly.

It can even be confirmed in the VMs:

google:ubuntu-18.04-64 $ systemd-run --user --wait --collect --service-type=exec /bin/true
Failed to start transient service unit: Invalid Type setting: exec
google:ubuntu-20.04-64 .../mini/hello# systemd-run --user --wait --collect --service-type=exec /bin/true
Running as unit: run-u0.service
Finished with result: success
Main processes terminated with: code=exited/status=0
Service runtime: 32ms

@@ -25,13 +29,13 @@ execute: |
skip="${skip:-} SKIP_NAKEDRET=1"
fi

su -l -c "cd /tmp/static-unit-tests/src/github.com/snapcore/snapd && \
tests.session -u test exec sh -c "cd /tmp/static-unit-tests/src/github.com/snapcore/snapd && \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have a switch here for trusty, to use su -l -c on trusty so that we can still get unit test coverage on trusty, but all the rest of the systems which support tests.session can benefit from not leaking the processes as you noticed

The tests.session helper requires busctl which is not available on 14.04,
however we still want to run tests on those systems.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
@mvo5 mvo5 merged commit 3db531a into snapcore:master Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simple 😃 A small PR which can be reviewed quickly Test Robustness
Projects
None yet
4 participants