From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=CpFLvr+r; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 214835A0279 for ; Wed, 20 Aug 2025 14:13:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1755691987; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bgcVttshBaxmSRcdKreLHDp/8CsiVBmpFCXx/0fBLWI=; b=CpFLvr+rftVmAYQCvoXW0P4OEthnt//v1XE/tGayxNC9/MseZsQQGgFHw/3Fr0T14pxXfo 5fJDjMmOqzUmK5nB2jVIWKAhHLRtyhnq7njapRA/gwtsUosev7sYHMJmqoSPv31NI+GtmV HkxKUjthOPzo/eXxC/B8CbkioJ7PWO0= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-517-JmUdhbqUMtaKc1h9WfBf7Q-1; Wed, 20 Aug 2025 08:13:05 -0400 X-MC-Unique: JmUdhbqUMtaKc1h9WfBf7Q-1 X-Mimecast-MFC-AGG-ID: JmUdhbqUMtaKc1h9WfBf7Q_1755691985 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-45a1b05d8d0so40433495e9.1 for ; Wed, 20 Aug 2025 05:13:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1755691984; x=1756296784; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=bgcVttshBaxmSRcdKreLHDp/8CsiVBmpFCXx/0fBLWI=; b=ELLMHFz1vvnpmNA6XAawJQLuluSzY+TPNSyM1EkvtlqNQ81PyHFOgQuAWmN+yJxQOC FvXz5Whw/b7F0fZUN1z/JvkfiqBGblV9ob9nV28W8UGmu1I8PdVkIXA29BYYuwo7D7SL LR69gA+20+P8ZEIaeRofukJVmiza5xqah1jr/wyUPYAFggANOtcZadm82BFKtOKDHRJX 5vlu44HI+dNEaacf3OyBrtLLBgoMilUHlt/ewPXqmrNmUIcp9EG3ZOEIHpE0AJ97hoan 6WcHf2Z3oRjMD4E4STI9CCTufPlZrY3dNi8AwpfE21q/9wlyp+tCCXrMfTo4OaRfwAHt XPYA== X-Gm-Message-State: AOJu0YziLlG55vS8+h3Im8t4tBaMS2JWSOCOwaGqXI9OEhTvfkkY7/h6 Pz1LUDumOK8NTl9C3GJSl7ZVyqLUKRf8bzr7yXT7MQGA8cE6Yqa8upDvg94e421y5oNnT+Dio6t +eNgcdKTxYWjXz4aTUYHBjAwk19qTsZDfLVJnYmz5iOmd3dUJ6B2Z/Q== X-Gm-Gg: ASbGnctqfa+/CQcaAviNTv1lDs6518ePuUf0ADYxvKgPjQrgBsh2cznRcGjXQ1MLrkC 4Js6Lsv+HUv5ulRC3UeNGVBS2ISz+0XjunAXYqdiAPDgxr9nAAsaT3X4bUPLoIogzj2q0Pqg75l +pMHG2jbgRZFMJZj1yPKCvgSAnMGb1MiDNhZ4TSOl+jLqVPgTWBeaBLbJIUSzfdesW2hqAuJ6A7 d9yFC6mkZNTt3HLpvVK47gZyxggx+m2lK5e0oBbaqYk9HwOMyNKW27s8BslY4U3TAhcnmGgoxy6 qMKYrw2jZDlygAObGpy+R+KaQTDwud2tLNt8hNXpY7XVBAmssac= X-Received: by 2002:a05:600c:4309:b0:459:df48:3b19 with SMTP id 5b1f17b1804b1-45b479ea726mr15356585e9.18.1755691984332; Wed, 20 Aug 2025 05:13:04 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHpYCTOJO7Yx4u5azCONEqkYtyyIwmMQ1uDC7RIR91+nwWO1pxeTOZKHvL4HwMSNXtBxkwZNQ== X-Received: by 2002:a05:600c:4309:b0:459:df48:3b19 with SMTP id 5b1f17b1804b1-45b479ea726mr15356275e9.18.1755691983697; Wed, 20 Aug 2025 05:13:03 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-45b47c5bfdasm29465235e9.19.2025.08.20.05.13.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Aug 2025 05:13:03 -0700 (PDT) Date: Wed, 20 Aug 2025 14:13:01 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v4 3/3] test: Convert build tests to exeter Message-ID: <20250820141301.5568a184@elisabeth> In-Reply-To: References: <20250807113237.548294-1-david@gibson.dropbear.id.au> <20250807113237.548294-4-david@gibson.dropbear.id.au> <20250819162754.0fdb4c86@elisabeth> <20250820124044.7f1bd32c@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: J5mnuC1Z8J6XJ9-0Vl4nAv-jYNDMfU11QuUiXl8VnYs_1755691985 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: PEGJ3QXOKIRTKNCQ7QQYU55JQIYB7FD7 X-Message-ID-Hash: PEGJ3QXOKIRTKNCQ7QQYU55JQIYB7FD7 X-MailFrom: sbrivio@redhat.com 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: On Wed, 20 Aug 2025 21:19:40 +1000 David Gibson 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 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 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 > > > > > --- > > > > > 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 > > > > > - > > > > > -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 > > > > > + > > > > > +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