[PATCH v2 00/32] Improve handling of test temporary files
The tests create temporary files and fifos in a number of places. Some of them can interfere with later test runs, and are awkward to clean up. Consolidate them in a single per-run directory in /tmp which gets cleaned up automatically. This is based on the earlier userns cleanup series. Chances since v1: * Fixed a number of straightforward bugs where things were missed * Also removed files we're no longer putting into the source tree from .gitignore * Added an extra patch moving the video processing files David Gibson (32): test: Correctly match "background" with "wait" commands test: Context execution helpers test: Allow a tmux pane to watch commands executed in contexts test: Integration of old-style pane execution and new context execution test: Issue host commands via context for most tests test: Use new-style contexts for passt pane in the pasta and passt tests test: Add nsholder utility test: Extend context system to run commands in namespace for pasta tests test: Use context system for guest commands test: Use context system for two_guests tests test: Use new-style command issue for passt_in_ns tests Don't store UID & GID persistently in the context structure Split checking for root from dropping root privilege Consolidate determination of UID/GID to run as Safer handling if we can't open /proc/self/uid_map Move self-isolation code into a separate file Consolidate validation of pasta namespace options Clean up and rename conf_ns_open() Correctly handle --netns-only in pasta_start_ns() Handle userns isolation and dropping root at the same time Allow --userns when pasta spawns a command test: Group tests by context then protocol, rather than the reverse test: Remove unused variable FFMPEG_PID_FILE test: Actually run cleanup function test: Create common state directories for temporary files test: Move context temporary files to state dir test: Dont regnerate small test file in pasta/tcp test: Use paths in __STATEDIR__ instead of 'temp' and 'tempdir' directives test: Move pause temporary file to state directory test: Store pcap files in $LOGDIR instead of /tmp test: Move pidfiles and nsholder sockets into state directory test: Move video processing files to $STATEBASE .gitignore | 1 - Makefile | 8 +- conf.c | 236 +++++++------- isolation.c | 210 +++++++++++++ isolation.h | 15 + passt.1 | 5 +- passt.c | 116 +------ passt.h | 9 - pasta.c | 91 ++++-- pasta.h | 1 + test/.gitignore | 6 +- test/Makefile | 13 +- test/build/all | 31 +- test/demo/passt | 9 +- test/demo/pasta | 12 +- test/demo/podman | 11 +- test/distro/debian | 2 +- test/distro/fedora | 2 +- test/distro/opensuse | 16 +- test/distro/ubuntu | 8 +- test/lib/context | 128 ++++++++ test/lib/layout | 61 ++-- test/lib/setup | 296 +++++++++--------- test/lib/term | 98 +++++- test/lib/test | 152 ++++----- test/lib/video | 20 +- test/nsholder.c | 117 +++++++ test/passt.mbuto | 32 +- test/{dhcp/passt => passt/dhcp} | 2 +- test/{ndp/passt => passt/ndp} | 2 +- test/{shutdown/passt => passt/shutdown} | 8 +- test/{tcp/passt => passt/tcp} | 8 +- test/{udp/passt => passt/udp} | 9 +- test/{icmp/passt_in_ns => passt_in_ns/icmp} | 2 +- .../passt_in_ns => passt_in_ns/shutdown} | 8 +- test/{tcp/passt_in_ns => passt_in_ns/tcp} | 27 +- test/{udp/passt_in_ns => passt_in_ns/udp} | 13 +- test/{dhcp/pasta => pasta/dhcp} | 2 +- test/{ndp/pasta => pasta/ndp} | 2 +- test/{tcp/pasta => pasta/tcp} | 14 +- test/{udp/pasta => pasta/udp} | 21 +- test/run | 42 +-- util.c | 83 ----- util.h | 2 - 44 files changed, 1159 insertions(+), 792 deletions(-) create mode 100644 isolation.c create mode 100644 isolation.h create mode 100644 test/lib/context create mode 100644 test/nsholder.c rename test/{dhcp/passt => passt/dhcp} (98%) rename test/{ndp/passt => passt/ndp} (95%) rename test/{shutdown/passt => passt/shutdown} (75%) rename test/{tcp/passt => passt/tcp} (95%) rename test/{udp/passt => passt/udp} (88%) rename test/{icmp/passt_in_ns => passt_in_ns/icmp} (94%) rename test/{shutdown/passt_in_ns => passt_in_ns/shutdown} (75%) rename test/{tcp/passt_in_ns => passt_in_ns/tcp} (97%) rename test/{udp/passt_in_ns => passt_in_ns/udp} (97%) rename test/{dhcp/pasta => pasta/dhcp} (96%) rename test/{ndp/pasta => pasta/ndp} (95%) rename test/{tcp/pasta => pasta/tcp} (95%) rename test/{udp/pasta => pasta/udp} (74%) -- 2.37.3
Our test DSL has a number of paired commands to run something in the
background in a pane, then later to wait for it to complete. However, in
some of the tests we have these mismatched - starting a command in one
pane, then waiting for it in another.
We appear to get away with this for some reason, but it's not correct and
future changes make it cause more problems.
Signed-off-by: David Gibson
For the tests, we need to run commands in various contexts: in the host,
in a guest or in a namespace. Currently we do this by running each context
in a tmux pane, and using tmux commands to type the commands into the
relevant pane, then screen-scrape the output for the results if we need
them.
This is very fragile, because we have to make various assumptions to parse
the output. Those can break if a shell doesn't have the prompt we expect,
if the tmux pane is too small or in various other conditions.
This starts some library functions for a new "context" system, that
provides a common way to invoke commands in a given context, in a way that
properly preserves stdout, stderr and the process return code.
Signed-off-by: David Gibson
We're moving to a new way of the tests dispatching commands to running in
contexts (host, guest, namespace, etc.). As we make this transition,
though, we still want the user to be able to watch the commands running
in a context, as they previously could from the commands issued in the
pane.
Add a helper to set up a pane to watch a context's log to allow this. In
some cases we currently issue commands from several different logical
contexts in the same pane, so allow a pane to watch several contexts at
once. Also use tail's --retry option to allow starting the watch before
we've initialized the context which will be useful in some cases.
Signed-off-by: David Gibson
We're creating a system for tests to more reliably execute commands in
various contexts (e.g. host, guest, namespace). That transition is going
to happen over a number of steps though, so in the meantime we need to deal
with both the old-style issuing of commands via typing into and screen
scraping tmux panels, and the new-style system for executing commands in
context.
Introduce some transitional helpers which will issue a command via context
if the requested context is initialized, but will otherwise fall back to
the old style tmux panel based method. Re-implement the various test DSL
commands in terms of these new helpers.
Signed-off-by: David Gibson
Convert most of the tests to use the new-style system for issuing commands
for all host commands. We leave the distro tests for now: they use
the same pane for both host and guest commands which we'll need some more
things to deal with.
Signed-off-by: David Gibson
Convert the pasta and passt tests to use new-style context execution
for the things that run in the "passt" frame. Don't touch the
passt_in_ns or two_guests tests yet, because they run passt inside a
namespace which introduces some additional complications we have yet
to handle.
Signed-off-by: David Gibson
In our test scripts we need to do some ugly parsing of /proc and/or pstree
output in order to get the PIDs of processes running in namespaces so that
we can connect to those namespaces with nsenter or pasta.
This is actually a pretty tricky problem with standard tools. To determine
the PID from the outside of the namespace we need to know how the process
of interest is related to the unshare or pasta process (child? one of
several children? grandchild?) as well as then parsing /proc or ps output.
This is slightly awkward now, and will get worse with future changes I'd
like to make to have processes are dispatched.
The obvious solution would be to have the process of interest (which we
control) report its own PID, but that doesn't work easily, because it is in
a PID namepace and sees only its local PID not the global PID we need to
address it from outside.
To handle this, add a small custom tool, "nsholder". This takes a path
and a mode parameter. In "hold" mode it will create a unix domain socket
bound to the path and listening. In "pid" mode it will get the "hold"ing
process's pid via the unix socket using SO_PEERCRED, which translates
between PID namespaces. In "stop" mode it will send a message to the
socket causing the "hold"ing process to clean up and exit.
Signed-off-by: David Gibson
Extend the context system to allow commands to be run in a namespace
created with unshare, and use it for the namespace used in the pasta tests.
Signed-off-by: David Gibson
Extends the context system in the test scripts to allow executing commands
within a guest. Do this without requiring an existing network in the guest
by using socat to run ssh via a vsock connection.
We do need some additional "sleep"s in the tests, because the new
faster dispatch means that sometimes we attempt to connect before
socat has managed to listen.
For now, only use this for the plain "passt" tests. The "passt_in_ns" and
other tests have additional complications we still need to deal with.
Signed-off-by: David Gibson
Now that we have all the pieces we need for issuing commands both into
namespaces and into guests, we can use those to convert the two_guests to
using only the new style context command issue.
Signed-off-by: David Gibson
Put the pieces together to use the new style context based dispatch for
the passt_in_pasta tests.
Signed-off-by: David Gibson
c->uid and c->gid are first set in conf(), and last used in check_root()
itself called from conf(). Therefore these don't need to be fields in the
long lived context structure and can instead be locals in conf().
Signed-off-by: David Gibson
check_root() both checks to see if we are root (in the init namespace),
and if we are drops to an unprivileged user. To make future cleanups
simpler, split the checking for root (now in check_root()) from the actual
dropping of privilege (now in drop_root()).
Note that this does slightly alter semantics. Previously we would only
setuid() if we were originally root (in the init namespace). Now we will
always setuid() and setgid(), though it won't actually change anything if
we weren't privileged to begin with. This also means that we will now
always attempt to switch to the user specified with --runas, even if we
aren't (init namespace) root to begin with. Obviously this will fail with
an error if we weren't privileged to start with. --help and the man page
are updated accordingly.
Signed-off-by: David Gibson
Currently the logic to work out what UID and GID we will run as is spread
across conf(). If --runas is specified it's handled in conf_runas(),
otherwise it's handled by check_root(), which depends on initialization of
the uid and gid variables by either conf() itself or conf_runas().
Make this clearer by putting all the UID and GID logic into a single
conf_ugid() function.
Signed-off-by: David Gibson
passt is allowed to run as "root" (UID 0) in a user namespace, but notas
real root in the init namespace. We read /proc/self/uid_map to determine
if we're in the init namespace or not.
If we're unable to open /proc/self/uid_map we assume we're ok and continue
running as UID 0. This seems unwise: AFAIK the only instance in which
uid_map won't be available is if we're running on a kernel which doesn't
support user namespaces, in which case we won't be able to sandbox
ourselves as we want and fail anyway. If there are other circumstances
where it can't be opened it seems marginally more likely that we *are*
in the init namespace.
Therefore, fail with an error in this case, instead of carrying on.
Signed-off-by: David Gibson
passt/pasta contains a number of routines designed to isolate passt from
the rest of the system for security. These are spread through util.c and
passt.c. Move them together into a new isolation.c file.
Signed-off-by: David Gibson
There are a number of different ways to specify namespaces for pasta to
use. Some combinations are valid and some are not. Currently validation
for these is spread across several places: conf_ns_pid() validates PID
options specifically. Near its callsite in conf() several other checks
are made. Some additional checks are made in conf_ns_open() and finally
theres a check just before the call to pasta_start_ns().
This is quite hard to follow. Make it easier by putting all the validation
logic together in a new conf_pasta_ns() function, which subsumes
conf_ns_pid(). This reveals that some of the checks were redundant with
each other, so remove those.
For good measure, rename conf_netns() to conf_netns_opt() to make it
clearer its handling just the --netns option specifically, not overall
configuration of the netns.
Signed-off-by: David Gibson
conf_ns_open() opens file descriptors for the namespaces pasta needs, but
it doesnt really have anything to do with configuration any more. For
better clarity, move it to pasta.c and rename it pasta_open_ns(). This
makes the symmetry between it and pasta_start_ns() more clear, since these
represent the two basic ways that pasta can operate, either attaching to
an existing namespace/process or spawning a new one.
Since its no longer validating options, the errors it could return
shouldn't cause a usage message. Just exit directly with an error instead.
Signed-off-by: David Gibson
--netns-only is supposed to make pasta use only a network namespace, not
a user namespace. However, pasta_start_ns() has this backwards, and if
--netns-only is specified it creates a user namespace but *not* a network
namespace. Correct this.
Signed-off-by: David Gibson
passt/pasta can interact with user namespaces in a number of ways:
1) With --netns-only we'll remain in our original user namespace
2) With --userns or a PID option to pasta we'll join either the given
user namespace or that of the PID
3) When pasta spawns a shell or command we'll start a new user namespace
for the command and then join it
4) With passt we'll create a new user namespace when we sandbox()
ourself
However (3) and (4) turn out to have essentially the same effect. In both
cases we create one new user namespace. The spawned command starts there,
and passt/pasta itself will live there from sandbox() onwards.
Because of this, we can simplify user namespace handling by moving the
userns handling earlier, to the same point we drop root in the original
namespace. Extend the drop_user() function to isolate_user() which does
both.
After switching UID and GID in the original userns, isolate_user() will
either join or create the userns we require. When we spawn a command with
pasta_start_ns()/pasta_setup_ns() we no longer need to create a userns,
because we're already made one. sandbox() likewise no longer needs to
create (or join) an userns because we're already in the one we need.
We no longer need c->pasta_userns_fd, since the fd is only used locally
in isolate_user(). Likewise we can replace c->netns_only with a local
in conf(), since it's not used outside there.
Signed-off-by: David Gibson
Currently --userns is only allowed when pasta is attaching to an existing
netns or PID, and is prohibited when creating a new netns by spawning a
command or shell.
With the new handling of userns, this check isn't neccessary. I'm not sure
if there's any use case for --userns with a spawned command, but it's
strictly more flexible and requires zero extra code, so we might as well.
Signed-off-by: David Gibson
e.g. passt/dhcp rather than dhcp/passt. This is more consistent with the
two_guests and other test groups, and makes some other cleanups simpler.
Signed-off-by: David Gibson
FFPMPEG_PID_FILE is set (creating a temporary file), then never used.
Signed-off-by: David Gibson
We install a cleanup() function with 'trap' in order to clean up temporary
files we generate during the tests. However, we deinstall it after
run_term, which means it won't run in most of the cases where it would be
useful. Even if "run from_term" exits with an error, that error will be
hidden from the run_term wrapper because it's within a tmux session, so we
will return from run_term normally, uninstall the trap and never clean up.
In fact there's no reason to uninstall the trap at all, it works just as
well on the success exit path as an error exit path.
Signed-off-by: David Gibson
The test scripts create a bunch of temporary files to keep track of
internal state. Some are made in /tmp with individual mktemp calls, some
go in the passt source directory, and some go in $LOGDIR. This can
sometimes make it messy to clean up after failed test runs.
Start cleaning this up by creating a single "state" directory ($STATEBASE)
in /tmp for all the state or temporary files used by a single test run.
Clean it up automatically in cleanup() - except when DEBUG==1, because
those files can be useful for debugging test script failures.
We create subdirectories under $STATEBASE for each setup function, exposed
as $STATESETUP. We also create subdirectories for each test script and
expose those to the scripts as __STATEDIR__.
Signed-off-by: David Gibson
Currently the context command dispatch subsystem creates a bunch of
temporary files in $LOGDIR, which is messy. Store them in $STATEDIR which
is for precisely this purpose. The logs from each context still go into
$LOGDIR.
Signed-off-by: David Gibson
In what looks like a copy/paste error, pasta/tcp generates its small test
file twice.
Signed-off-by: David Gibson
Instead of using the 'temp' and 'tempdir' DSL directives to create
temporary files, use fixed paths relative to __STATEDIR__. This has two
advantages:
1) The files are automatically cleaned up if the tests fail (and even if
that doesn't work they're easier to clean up manuall)
2) When debugging tests it's easier to figure out which of the temporary
files are relevant to whatever's going wrong
Signed-off-by: David Gibson
Signed-off-by: David Gibson
The capture files are more or less a different form of log output from the
tests, so place them in $LOGDIR.
Signed-off-by: David Gibson
Currently they go in the passt source tree with a fixed names, which means
their presence can mess with subsequent test runs.
Signed-off-by: David Gibson
The asciinema video handling creates a number of temporary files (.uncat,
.start, .stop) which currently go into the source tree. Put them in the
temporary state directory to avoid clutter. Put the final processed video
into the test_logs/ directory, since it's essentially a test output
artefact.
Signed-off-by: David Gibson
participants (1)
-
David Gibson