From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202508 header.b=fDxy6e0A; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id CE1A05A0279 for ; Thu, 21 Aug 2025 02:41:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1755736888; bh=rl/kF1gGFJKF12CgaicMRQ2hjl/szbXorxra5H6DEuM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fDxy6e0ARBZyVzexe8RaZTpAy/0UcYWaCzI3PkZZWtwloSheUS+JpT5MXW3I3/bJz /IFuqLD7WQxDOFS3ZxKnKnmDhONSZCf1kJVxWi/t4fSLRIdRrYjKBoa1VPot2HAbha THWw7l5gPFGf7ei/8g5XMIuVGHn4jjugxgFUKNYUlnZ+sO1RiOAIcIDk7V0UwAEdcf 9UjFkrym0SHg7TYq/eo1trudZFvYId95wm6jcarEBa4CkBxe3zt6Cxzv6p15It+IB2 1qAcU8hTYzifD9ZC0QSEgC2/cGRq12UET5vfBAMlOF6hvszmECarMrNlOzZi1KZt9Z EtpMG5r1vkL7Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4c6kyw4BZPz4wbr; Thu, 21 Aug 2025 10:41:28 +1000 (AEST) Date: Thu, 21 Aug 2025 10:41:23 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v4 3/3] test: Convert build tests to exeter Message-ID: References: <20250807113237.548294-1-david@gibson.dropbear.id.au> <20250807113237.548294-4-david@gibson.dropbear.id.au> <20250819162754.0fdb4c86@elisabeth> <20250820124044.7f1bd32c@elisabeth> <20250820141301.5568a184@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="n8tjZ+X3O//Ul5hC" Content-Disposition: inline In-Reply-To: <20250820141301.5568a184@elisabeth> Message-ID-Hash: 4ZQGOTCINWL2KKYUNMXV5SFQ4MXIKPE2 X-Message-ID-Hash: 4ZQGOTCINWL2KKYUNMXV5SFQ4MXIKPE2 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --n8tjZ+X3O//Ul5hC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 20, 2025 at 02:13:01PM +0200, Stefano Brivio wrote: > On Wed, 20 Aug 2025 21:19:40 +1000 > David Gibson wrote: >=20 > > On Wed, Aug 20, 2025 at 12:40:44PM +0200, Stefano Brivio wrote: > > > On Wed, 20 Aug 2025 13:10:02 +1000 > > > David Gibson wrote: > > > =20 > > > > On Tue, Aug 19, 2025 at 04:27:54PM +0200, Stefano Brivio wrote: =20 > > > > > On Thu, 7 Aug 2025 21:32:37 +1000 > > > > > David Gibson wrote: > > > > > =20 > > > > > > 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 te= mporary copy > > > > > > of the source tree so will not be affected by concurrent manual= builds. > > > > > >=20 > > > > > > Signed-off-by: David Gibson > > > > > > --- > > > > > > 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 > > > > > >=20 > > > > > > 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 togethe= r, check output > > > > > > -# > > > > > > -# Copyright (c) 2021 Red Hat GmbH > > > > > > -# Author: Stefano Brivio > > > > > > - > > > > > > -htools make cc rm uname getconf mkdir cp rm man > > > > > > - > > > > > > -test Build passt > > > > > > -host make clean > > > > > > -check ! [ -e passt ] > > > > > > -host CFLAGS=3D"-Werror" make passt > > > > > > -check [ -f passt ] > > > > > > - > > > > > > -test Build pasta > > > > > > -host make clean > > > > > > -check ! [ -e pasta ] > > > > > > -host CFLAGS=3D"-Werror" make pasta > > > > > > -check [ -h pasta ] > > > > > > - > > > > > > -test Build qrap > > > > > > -host make clean > > > > > > -check ! [ -e qrap ] > > > > > > -host CFLAGS=3D"-Werror" make qrap > > > > > > -check [ -f qrap ] > > > > > > - > > > > > > -test Build all > > > > > > -host make clean > > > > > > -check ! [ -e passt ] > > > > > > -check ! [ -e pasta ] > > > > > > -check ! [ -e qrap ] > > > > > > -host CFLAGS=3D"-Werror" make > > > > > > -check [ -f passt ] > > > > > > -check [ -h pasta ] > > > > > > -check [ -f qrap ] > > > > > > - > > > > > > -test Install > > > > > > -host mkdir __STATEDIR__/prefix > > > > > > -host prefix=3D__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=3D__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/n= ull > > > > > > -check ! man -M __STATEDIR__/prefix/share/man -W pasta 2>/dev/n= ull > > > > > > -check ! man -M __STATEDIR__/prefix/share/man -W qrap 2>/dev/nu= ll > > > > > > 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 > > > > > > + > > > > > > +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=3DTrue) > > > > > > + > > > > > > + > > > > > > +@contextlib.contextmanager > > > > > > +def clone_sources() -> Iterator[str]: > > > > > > + os.chdir('..') # Move from test/ to repo base > > > > > > + with tempfile.TemporaryDirectory(ignore_cleanup_errors=3DF= alse) as tmpdir: =20 > > > > >=20 > > > > > 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 clean= up > > > > > function. =20 > > > >=20 > > > > 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 ca= ses > > > > of an interrupted test (not a SIGKILL, obviously). > > > >=20 > > > > Arguably, it's not a good trade off in this simple case. However o= nce > > > > 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. =20 > > >=20 > > > 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. = =20 > >=20 > > 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. >=20 > 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. Right. To be clear, I'm planning for tunbridge to do _network_ sandboxing. I think that makes sense, since it's a tool for setting up simulated network configurations. It might do a bit more than that, if it's convenient to do so there, but as you say comprehensive sandboxing doesn't really belong there. It can't really go in exeter either. exeter is too lightweight to even know what kind of sandboxing would make sense for the program using it. An attempt would both make it much too complex, and also make it inflexible, because the sandboxing could interfere with certain tests you might want to write. Really, comprehensive sandboxing belongs in the runner - the thing that runs the exeter tests. Indeed, Avocado offers this - it can run tests in containers. Likewise things like the Git* CIs run things in various containerized environments. I wasn't about to try to implement that in the quick and dirty integration with our existing test scripts, of course. > 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 mean.. that's a nice idea, but not really in scope for what I currently have planned. > > 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. >=20 > ...you could take care of this kind of problem as well (at least in > some cases?). Maybe. > 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). >=20 > 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). >=20 > [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. Again, could be nice, but not really in scope for the immediate future. Also, TBH, I'd probably implement something like this with context managers (though in the wrapper/runner rather than in the test code itself). Or with RAII if using a non-Python, which is loosely equivalent. > > > 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. =20 > >=20 > > Right, it can sometimes make things more confusing during debugging, > > though. >=20 > On that subject, the test contexts stuff you implemented for the > current test framework almost entirely eliminates this confusion, at > least for my usage. Well, I'll see how it turns out. > > > Add mount namespaces on top, with tmpfs, and we probably don't even > > > need to clean up after ourselves in build tests. > > >=20 > > > Yes, it could be called a container but it's probably useful to retain > > > tight control of what namespace we detach and when. > > >=20 > > > 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. =20 > >=20 > > I don't really want to. I consider the test cases being entirely cut > > off from the outside internet an advantage, in most cases. >=20 > That wasn't about internet access, more about network access and a > quick way to do / draft sandboxing. Right, but if you don't need external network access you can do the same thing with unshare(1). --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --n8tjZ+X3O//Ul5hC Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmimazIACgkQzQJF27ox 2Ge2aQ/9ETzsTtnHwlJX2fCGAjbN2ria5R09YIc2MDHVZ+Dq/lh36N2mbsu3Kwab ff6h4ikicGzMOqxw1NaW2yah3lYX7TgfDWB+H3nuWZd9/SnKpXhuYjp2Q+hO41EC 1s4Lz+YJa0PODPIp3YQCKYAE4FAo4Wf3g6YYjzzNYLqHzCT/i0e+2qrJQ/FPQxtY hGzH3oGOwgK4XhRTLhX9RgbtHJ0Ka4soMfnlIEbKMHSfO0mmD8o1JFFaWh64XRY3 XPuOH4SU6hp8G2IyUr2RzFAtdgQWwpyXz5SzBjVU0mv536c8lolCwNsZfSyF9VQQ MR0d/ym/z5RqcJ9f/6ewHLTgURW0gdvYZrugLs3pgqGA3dkuCsWKzP+KFxqH2zJH uQGLG5iilZO/DUiU8rRbfOp9avbKL1kzEHSpvSSWi2mIBVAiYFPaqHBUkkOVfo0i 9itKQrI2+X9NFDG4NHv7C4DUEkCYZJ4wVgfqZaMY89QIgc+ApmfRYuhbRXpC832R zrDgi2jBDR2ugsIEf0Y7HfjtWzhO9/dfgFTgFtZ+F/onZrvsBQ4vJmQjCGXwFbjR LP+tHK7kZZrrkjzx5KQzwFDcmBBY/t8HefyX6bVzsIGNm/xTsISumEH7XKEe1HwY odPv6FBGhokRI7CmINPx9B5x9FZ6DdN4w+iCbzvHqDTBwGhPNf4= =OUAj -----END PGP SIGNATURE----- --n8tjZ+X3O//Ul5hC--