From nobody Wed Apr 03 14:13:56 2024 X-Original-To: freebsd-hackers@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4V8mvf0wgMz5Gscm for ; Wed, 3 Apr 2024 14:14:06 +0000 (UTC) (envelope-from igor.ostapenko@pm.me) Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "protonmail.com", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4V8mvd1tzCz4Zj3 for ; Wed, 3 Apr 2024 14:14:05 +0000 (UTC) (envelope-from igor.ostapenko@pm.me) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=pm.me header.s=protonmail3 header.b=QmyF9ODX; dmarc=pass (policy=quarantine) header.from=pm.me; spf=pass (mx1.freebsd.org: domain of igor.ostapenko@pm.me designates 185.70.40.131 as permitted sender) smtp.mailfrom=igor.ostapenko@pm.me DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pm.me; s=protonmail3; t=1712153640; x=1712412840; bh=b6miHEyY4QID+K9kaFacfCIvrqCaLmx0ntbJ11ovLdo=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=QmyF9ODXXdcKrAOXT3bPZUIubu3vlrY1ZE8s9+O3DPTKNVMxXt9zIwQqBavaXL+yU JJysyCUM8lV1ZtHLWS9/CpwvCxif1EFeJ2GHsMVPS0n6mKT7kDV3sCEs5tjoa11/UV cdRMMSiN98c7hL4vk4jStC7D3rmz0+LVAVbp+HxF6tngZDnBEaRCWon5BDSHkXPFzH QeTNX6WLkpDCvhAomIp1PTT47YswgDn1y6aiOYjwks1JiTIF1Y2Dp1HmZSbcEWITQ8 rEoQXCfYzGXl9GRpdda6FOmM3wgoZ9T9xjFYSoibEjfqNd2WCiqCmOkYRbmjv58HmF w5ZH6IQBH5kEg== Date: Wed, 03 Apr 2024 14:13:56 +0000 To: freebsd-hackers@freebsd.org From: Igor Ostapenko Cc: "freebsd-testing@FreeBSD.org" , Kristof Provost Subject: Re: Add jail execution environment support to the FreeBSD test suite Message-ID: <8H_refsJfTozkldDQEkQFQY-RBauBRmKOJRWFfdSSZkxJ6KOmarYb8Fk97FjQ1yGgZhKGmkbZWgsgbaLgPhExtasz8OLJujk-54AO_erA5U=@pm.me> In-Reply-To: References: <2bjQNp1msrv-_AqyamMun6kY-SCqbgPm3Q7DqVQHAYlqvFkiE1i85svfIT-QQdUG1cg3cKippyTyv8Z-5nbLu4WaMutgZQ7KT-YYo_5Pbro=@pm.me> Feedback-ID: 8300135:user:proton List-Id: Technical discussions relating to FreeBSD List-Archive: https://lists.freebsd.org/archives/freebsd-hackers List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-hackers@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.20 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-0.997]; DMARC_POLICY_ALLOW(-0.50)[pm.me,quarantine]; RWL_MAILSPIKE_VERYGOOD(-0.20)[185.70.40.131:from]; R_DKIM_ALLOW(-0.20)[pm.me:s=protonmail3]; R_SPF_ALLOW(-0.20)[+ip4:185.70.40.0/24:c]; MIME_GOOD(-0.10)[text/plain]; MISSING_XM_UA(0.00)[]; TO_DN_EQ_ADDR_SOME(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:62371, ipnet:185.70.40.0/24, country:CH]; TO_DN_SOME(0.00)[]; ARC_NA(0.00)[]; RCVD_COUNT_ZERO(0.00)[0]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; FROM_EQ_ENVFROM(0.00)[]; MLMMJ_DEST(0.00)[freebsd-hackers@freebsd.org]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DKIM_TRACE(0.00)[pm.me:+] X-Rspamd-Queue-Id: 4V8mvd1tzCz4Zj3 Hi, Thanks for testing. My current vision of the action points is as follows: The first phase: - [x] igoro: Separate FreeBSD specifics and fix existing Kyua tests broken = by the change - [x] igoro: Migrate some of the existing tests for the start, e.g. netpfil= /pf - [x] igoro: Cover Paul's use case mentioned in this email thread - [x] igoro: Cover Olivier's use case mentioned in this email thread - [x] igoro: Provide the respective documentation updates (manual pages) - [~] community: Review, testing, comments -- probably we want to change th= e design - [ ] committers: Help with the main commit -- it should hit freebsd/kyua GitHub fork first, then vendor branch, and merge to main = after The next phases: - [ ] igoro: Provide the PRs to add brand new tests of Kyua itself to cover the new feature - [ ] igoro: Help with the Handbook(s) update to cover the new concept for test authors The future phases: - [ ] igoro: Work on the related improvements and ideas like required_klds = etc If there is nothing to change or add at this stage then the next step could= be to merge the GitHub PR: https://github.com/freebsd/kyua/pull/224 Thanks the community for your time invested, I hope it will be eventually payed back with better test run time during development. Best regards, Igor. On Thursday, March 21st, 2024 at 4:59 AM, Kristof Provost = wrote: > >=20 > I=E2=80=99ve been toying with this patch. Adding only the following patch= I can get Kyua to run the pf tests in parallel: >=20 > diff --git a/tests/sys/netpfil/pf/Makefile b/tests/sys/netpfil/pf/Mak= efile > index 867b98e5f6c2..c2f0f15fa798 100644 > --- a/tests/sys/netpfil/pf/Makefile > +++ b/tests/sys/netpfil/pf/Makefile > @@ -51,8 +51,8 @@ ATF_TESTS_PYTEST+=3D frag6.py > ATF_TESTS_PYTEST+=3D nat66.py > ATF_TESTS_PYTEST+=3D sctp.py > =20 > -# Tests reuse jail names and so cannot run in parallel. > -TEST_METADATA+=3D is_exclusive=3Dtrue > +TEST_METADATA+=3D execenv=3D"jail" > +TEST_METADATA+=3D execenv_jail=3D"vnet allow.raw_sockets" > =20 > PROGS=3D divapp > =20 >=20 > That gets the test time, with parallelism=3D5, down from 22 minutes to 5m= 40s. > So I=E2=80=99m rather keen to see this work land. >=20 > My read from the reactions here is that people are generally okay with th= e approach, especially (I assume) given that the current version lets us tu= rn this on on a per-test basis. >=20 > Is there anything else anyone wants to raise before we land this? >=20 > Best regards, > Kristof >=20 > On 28 Feb 2024, at 2:32, Igor Ostapenko wrote: >=20 > > Hi, > >=20 > > The patch was updated after the recent discussion. > >=20 > > Currently, the patch provides the following new functionality, from bot= tom to > > top: > >=20 > > 1 ATF based tests > >=20 > > - The new "execenv" metadata property can be set to explicitly ask for = an > > execution environment: "host" or "jail". If it's not defined, as all > > existing tests do, then it implicitly means "host". > >=20 > > - The new "execenv.jail" metadata property can be optionally defined to= ask > > Kyua to use specific jail(8) parameters during creation of a temporary > > jail. An example is "vnet allow.raw_sockets". > >=20 > > 2 Kyuafile > >=20 > > - The same new metadata properties can be defined on Kyuafile level: > > "execenv" and "execenv_jail". > >=20 > > - Note that historically ATF uses dotted style of metadata naming, whil= e > > Kyua uses underscore style. Hence "execenv.jail" vs. "execenv_jail". > >=20 > > 3 kyua.conf, kyua CLI > >=20 > > - The new "execenv" engine configuration variable can be set to a list = of > > execution environments to run only tests designed for. Tests of not lis= ted > > environments are skipped. > >=20 > > - By default, this variable lists all execution environments supported = by a > > Kyua binary, e.g. execenv=3D"host jail". > >=20 > > - This variable can be changed via "kyua.conf" or via kyua CLI's "-v" > > parameter. For example, "kyua -v execenv=3Dhost test" will run only > > host-based tests and skip jail-based ones. > >=20 > > - Current value of this variable can be examined with "kyua config". > >=20 > > The patch is https://reviews.freebsd.org/D42350. > >=20 > > Any help with review and testing is welcome. Its test plan covers the > > details and refers to the demo patch of how existing tests could be > > converted to be run in a jail. > >=20 > > Best regards, Igor. > >=20 > > On Thursday, February 22nd, 2024 at 10:57 PM, igor.ostapenko@pm.me wrote: > >=20 > > > Hi FreeBSD developers, > > >=20 > > > There is a proposal to improve the FreeBSD test suite. > > >=20 > > > 1 The Problem > > >=20 > > > The FreeBSD test suite is based on the Kyua framework. The latter sup= ports > > > running tests in parallel. However, some tests cannot be run in paral= lel and > > > are marked with is_exclusive=3D"true" metadata, which makes Kyua run = such tests > > > in sequence. > > >=20 > > > Many tests are not meant to be exclusive conceptually, they are so fo= r very > > > simple technical reasons. For instance, some network related tests ar= e based > > > on jail and vnet usage. It's convenient for such tests and it provide= s a lot > > > of isolation already not to conflict with other tests. But they are s= till > > > marked as exclusive due to the shared space of jail names, routing, e= tc. > > >=20 > > > The project seeks more tests, and it's kind of a trend for new tests = like > > > jail/vnet based ones to be created as is_exclusive=3D"true" from the = very > > > beginning. It only piles up the suite with exclusive tests, e.g. new = tests > > > from my side faced a fair question from a reviewer whether they could= be > > > re-designed for a parallel run. [1] > > >=20 > > > If such tests were 100% isolated they would be able to run in paralle= l and > > > decrease the test time for CI runs and for the runs within the develo= pment > > > process. > > >=20 > > > And the problem is that trying to add more isolation by a test itself= looks to > > > be a doable task from a glance, but it would add a lot of complexity = to a test > > > code, or could be found as an impossible task in a specific case. > > >=20 > > > 2 The Idea > > >=20 > > > The idea is not new. A test could be running in a jail -- it provides= the > > > required isolation with minimum or zero effort from a test. > > >=20 > > > 3 The Implementation > > >=20 > > > There is a lot of work done already and the working patch passed the = initial > > > review (thanks to markj@ and ngie@). [2] > > >=20 > > > It adds a new concept to the Kyua framework -- an execution environme= nt. Two > > > new metadata were added for that: execenv and execenv_jail. > > >=20 > > > execenv is a switch to select an environment. If a test's metadata de= fines > > > execenv=3D"jail" then Kyua will create a temporary jail, run such tes= t within > > > it, and remove the jail. If execenv=3D"host" is provided or execenv m= etadata is > > > undefined then Kyua will run such test as it does today. > > >=20 > > > execenv_jail metadata takes effect only in case of execenv=3D"jail". = It allows a > > > test to request specific parameters for its jail. These parameters ar= e simply > > > arguments to jail(8), e.g. execenv_jail=3D"vnet allow.raw_sockets". > > >=20 > > > 4 The Adoption > > >=20 > > > ATF based tests can easily define this new metadata via Kyuafile or d= irectly, > > > e.g. for atf-sh based tests: > > >=20 > > > test_head() > > > { > > > atf_set descr "Test foo in case of bar" > > > atf_set require.user root > > > atf_set execenv jail > > > atf_set execenv.jail vnet allow.raw_sockets > > > } > > >=20 > > > Non-ATF based ones will do it via Kyuafile. Our test suite does it th= rough a > > > Makefile: > > >=20 > > > TEST_METADATA+=3D execenv=3D"jail" > > > TEST_METADATA+=3D execenv_jail=3D"vnet allow.raw_sockets" > > >=20 > > > The patch got some little evolution, I started with a single execenv_= jail > > > metadata, and during the patch discussion and review, I ended up with= two > > > knobs: execenv and execenv_jail. It turned out to be a cleaner and le= ss tricky > > > interface such way. The evolution reasoning can be found in the histo= ry of the > > > respective Differential. [2] > > >=20 > > > 5 MFC Concerns > > >=20 > > > For now, I see at least one issue from the usual project workflow per= spective. > > > Let's imagine that the Kyua framework got this execenv feature commit= ted to > > > 15-CURRENT, we started to convert existing tests and create new ones = to use > > > execenv=3D"jail". If some feature or a bug fix needs to be ported bac= k to > > > 14-STABLE or 13-STABLE, then "old" Kyua without execenv feature will = fail to > > > run such tests: > > >=20 > > > kyua: E: Load of 'Kyuafile' failed: Failed to load Lua file 'Kyuafile= ': Kyuafile:9: Unknown metadata property execenv. > > >=20 > > > From a combinatorics perspective, the first three options pop up to d= eal with > > > that: > > > a) Patch Kyua the same way for the supported STABLE branches so it wi= ll be > > > able to run back ported tests based on execenv=3D"jail" (it's not sys= tem ABI > > > change after all) > > > b) Exclusively patch Kyua framework for the supported STABLE branches= to > > > simply skip such tests (does not look to provide much benefit) > > > c) Do not back port tests, only the fix/feature itself (kind of a bad= idea) > > >=20 > > > 6 The Demo > > >=20 > > > My test environment showed promising run time numbers for almost the = whole > > > test suite (ZFS excluded). One of the tests yielded 36 min with test > > > parallelism improvement versus 1 h 25 min without. In my case with 8 = cores, > > > the suite runs about 2 times faster with the improvement. [3] > > >=20 > > > 7 Action Points > > >=20 > > > My current vision of the plan looks as follows: > > > - [ ] community: Review, testing, comments -- probably we want to cha= nge the > > > design > > > - [ ] committers: Help with the main commit -- it should hit freebsd/= kyua > > > GitHub fork first [4], then vendor branch, and merge to > > > main after > > > - [ ] igoro: Provide the subsequent PRs to separate FreeBSD specifics= and fix > > > existing Kyua tests > > > - [ ] igoro: Provide the PRs to add brand new tests of Kyua itself to= cover > > > the new feature > > > - [ ] igoro: Provide the respective documentation updates > > > - [ ] igoro: Migrate some of the existing tests for the start, e.g. n= etpfil/pf > > > - [ ] committers: Help with review and respective commits/merges > > >=20 > > > The plan is not strict, it depends on the discussion and interest of > > > volunteers. > > >=20 > > > I hope that this proposal is found valuable for the project. If so, a= ny help > > > is appreciated. > > >=20 > > > [1] New tests exclusivity concern: https://reviews.freebsd.org/D42314 > > > [2] The Kyua patch: https://reviews.freebsd.org/D42350 > > > [3] The whole test suite demo: https://reviews.freebsd.org/D42410 > > > [4] The respective PR to the fork: https://github.com/freebsd/kyua/pu= ll/224 >=20 > > > Best regards, Igor.