Re: [PATCH v4 3/3] test: Convert build tests to exeter

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 20 Aug 2025 21:19:40 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Aug 20, 2025 at 12:40:44PM +0200, Stefano Brivio wrote:
> > On Wed, 20 Aug 2025 13:10:02 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Tue, Aug 19, 2025 at 04:27:54PM +0200, Stefano Brivio wrote:  
> > > > On Thu,  7 Aug 2025 21:32:37 +1000
> > > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > >     
> > > > > Convert the tests in build/all to be based on exeter.  The new version of
> > > > > the tests is more robust than the original, since it makes a temporary copy
> > > > > of the source tree so will not be affected by concurrent manual builds.
> > > > > 
> > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > > ---
> > > > >  test/build/all      | 61 --------------------------------
> > > > >  test/build/build.py | 84 +++++++++++++++++++++++++++++++++++++++++++++
> > > > >  test/run            |  8 ++---
> > > > >  3 files changed, 88 insertions(+), 65 deletions(-)
> > > > >  delete mode 100644 test/build/all
> > > > >  create mode 100755 test/build/build.py
> > > > > 
> > > > > diff --git a/test/build/all b/test/build/all
> > > > > deleted file mode 100644
> > > > > index 1f79e0d8..00000000
> > > > > --- a/test/build/all
> > > > > +++ /dev/null
> > > > > @@ -1,61 +0,0 @@
> > > > > -# SPDX-License-Identifier: GPL-2.0-or-later
> > > > > -#
> > > > > -# PASST - Plug A Simple Socket Transport
> > > > > -#  for qemu/UNIX domain socket mode
> > > > > -#
> > > > > -# PASTA - Pack A Subtle Tap Abstraction
> > > > > -#  for network namespace/tap device mode
> > > > > -#
> > > > > -# test/build/all - Build targets, one by one, then all together, check output
> > > > > -#
> > > > > -# Copyright (c) 2021 Red Hat GmbH
> > > > > -# Author: Stefano Brivio <sbrivio@redhat.com>
> > > > > -
> > > > > -htools	make cc rm uname getconf mkdir cp rm man
> > > > > -
> > > > > -test	Build passt
> > > > > -host	make clean
> > > > > -check	! [ -e passt ]
> > > > > -host	CFLAGS="-Werror" make passt
> > > > > -check	[ -f passt ]
> > > > > -
> > > > > -test	Build pasta
> > > > > -host	make clean
> > > > > -check	! [ -e pasta ]
> > > > > -host	CFLAGS="-Werror" make pasta
> > > > > -check	[ -h pasta ]
> > > > > -
> > > > > -test	Build qrap
> > > > > -host	make clean
> > > > > -check	! [ -e qrap ]
> > > > > -host	CFLAGS="-Werror" make qrap
> > > > > -check	[ -f qrap ]
> > > > > -
> > > > > -test	Build all
> > > > > -host	make clean
> > > > > -check	! [ -e passt ]
> > > > > -check	! [ -e pasta ]
> > > > > -check	! [ -e qrap ]
> > > > > -host	CFLAGS="-Werror" make
> > > > > -check	[ -f passt ]
> > > > > -check	[ -h pasta ]
> > > > > -check	[ -f qrap ]
> > > > > -
> > > > > -test	Install
> > > > > -host	mkdir __STATEDIR__/prefix
> > > > > -host	prefix=__STATEDIR__/prefix make install
> > > > > -check	[ -f __STATEDIR__/prefix/bin/passt ]
> > > > > -check	[ -h __STATEDIR__/prefix/bin/pasta ]
> > > > > -check	[ -f __STATEDIR__/prefix/bin/qrap ]
> > > > > -check	man -M __STATEDIR__/prefix/share/man -W passt
> > > > > -check	man -M __STATEDIR__/prefix/share/man -W pasta
> > > > > -check	man -M __STATEDIR__/prefix/share/man -W qrap
> > > > > -
> > > > > -test	Uninstall
> > > > > -host	prefix=__STATEDIR__/prefix make uninstall
> > > > > -check	! [ -f __STATEDIR__/prefix/bin/passt ]
> > > > > -check	! [ -h __STATEDIR__/prefix/bin/pasta ]
> > > > > -check	! [ -f __STATEDIR__/prefix/bin/qrap ]
> > > > > -check	! man -M __STATEDIR__/prefix/share/man -W passt 2>/dev/null
> > > > > -check	! man -M __STATEDIR__/prefix/share/man -W pasta 2>/dev/null
> > > > > -check	! man -M __STATEDIR__/prefix/share/man -W qrap 2>/dev/null
> > > > > diff --git a/test/build/build.py b/test/build/build.py
> > > > > new file mode 100755
> > > > > index 00000000..12bb82d8
> > > > > --- /dev/null
> > > > > +++ b/test/build/build.py
> > > > > @@ -0,0 +1,84 @@
> > > > > +#! /usr/bin/env python3
> > > > > +#
> > > > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > > > +#
> > > > > +# PASST - Plug A Simple Socket Transport
> > > > > +#  for qemu/UNIX domain socket mode
> > > > > +#
> > > > > +# PASTA - Pack A Subtle Tap Abstraction
> > > > > +#  for network namespace/tap device mode
> > > > > +#
> > > > > +# test/build/build.py - Test build and install targets
> > > > > +#
> > > > > +# Copyright Red Hat
> > > > > +# Author: David Gibson <david@gibson.dropbear.id.au>
> > > > > +
> > > > > +import contextlib
> > > > > +import os
> > > > > +from pathlib import Path
> > > > > +import subprocess
> > > > > +import tempfile
> > > > > +from typing import Iterable, Iterator
> > > > > +
> > > > > +import exeter
> > > > > +
> > > > > +def sh(cmd):
> > > > > +    subprocess.run(cmd, shell=True)
> > > > > +
> > > > > +
> > > > > +@contextlib.contextmanager
> > > > > +def clone_sources() -> Iterator[str]:
> > > > > +    os.chdir('..')  # Move from test/ to repo base
> > > > > +    with tempfile.TemporaryDirectory(ignore_cleanup_errors=False) as tmpdir:    
> > > > 
> > > > I guess I see the advantage of this syntax, it's still a bit less
> > > > obvious to me than a mere sequence of steps and an explicit cleanup
> > > > function.    
> > > 
> > > I agree that it's less obvious, but I think the advantages are worth
> > > it.  Namely that it will correctly run the cleanup in nearly all cases
> > > of an interrupted test (not a SIGKILL, obviously).
> > > 
> > > Arguably, it's not a good trade off in this simple case.  However once
> > > we get to real network tests, it's pretty common to have multiple
> > > nested layers of setup, each with their own teardown.  Sometimes you
> > > need something torn down, but it's only set up partway through the
> > > test, or you want something for the first part of the test but not
> > > after, so it needs to be torn down only if interrupted at certain
> > > points.  Manually keeping track of that quickly becomes really
> > > painful, I really want to use this technique there.  
> > 
> > Another idea (I'm not sure if it makes this useless, but we probably
> > want to implement it anyway) is what nft tests (and some kselftests) do
> > nowadays: every test runs in its own network namespace, so you don't
> > need an explicit teardown. The kernel already tracks things for you.  
> 
> Right, I'm planning to implement that in tunbridge.  Unlike the
> earlier drafts, my intention is that all the namespaces / whatever you
> explicitly create will be nested inside a top-level sandboxing
> namespace.

Ah, neat. One might probably argue that the top-level sandboxing should
be part of a test runner rather than tunbridge itself, but I guess it's
the only practical choice at the moment.

And this is probably enough complexity at this stage, but eventually, I
was thinking, if instead of a single top-level sandboxing, you allow
several levels of nested sandboxing with some kind of descriptive way
to define it, then...

> I don't think it obviates the use for context managers, though.  For
> one thing I think we may have occasional need for things that won't be
> handled by the normal sandbox, but the contextmanager can handle those
> too.  More widely, there are cases where you want to tear something
> down partway through normal test operation.  The context manager will
> do that neatly, while also doing the teardown if interrupted before
> that point.

...you could take care of this kind of problem as well (at least in
some cases?).

Consider the case where you have a group of tests with some expensive
setup that's in common between them (say, setting up a guest image and
starting a guest... even though eventually I think we should move to
libkrun / virtiofs / muvm and get rid of the test image itself).

The single tests could be network-sandboxed between each other, and
share the same mount namespace and tmpfs. Once a test finishes, its
specific network setup is cleaned up, but you still have the guest
image around and possibly the guest running (if we allow that as a
special sandboxing level).

[counts occurrences of 'killall -9 nstool' in shell history :)] to me
this approach looks more robust / enforcing than context managers, even
though I'm not sure how pervasively it can be used.

> > If you combine that with what pasta does (same as container runtimes
> > really), you could get something that's also robust to SIGKILL, by
> > making every test "parent" process (assuming there can be one) PID 1 in
> > its own PID namespace.  
> 
> Right, it can sometimes make things more confusing during debugging,
> though.

On that subject, the test contexts stuff you implemented for the
current test framework almost entirely eliminates this confusion, at
least for my usage.

> > Add mount namespaces on top, with tmpfs, and we probably don't even
> > need to clean up after ourselves in build tests.
> > 
> > Yes, it could be called a container but it's probably useful to retain
> > tight control of what namespace we detach and when.
> > 
> > Given that pasta(1) is now available on any distribution where you
> > might reasonably find Python, by the way, you could consider using it
> > directly (the installed version, that is) if it helps.  
> 
> I don't really want to.  I consider the test cases being entirely cut
> off from the outside internet an advantage, in most cases.

That wasn't about internet access, more about network access and a
quick way to do / draft sandboxing.

-- 
Stefano