Add FreeBSD Jail execution environment support #224

ihoro · 2024-02-03T16:59:52Z

The main work and review was conducted here: https://reviews.freebsd.org/D42350.
The respective mailing list discussion: https://lists.freebsd.org/archives/freebsd-hackers/2024-February/003032.html.

ihoro · 2024-03-22T17:41:27Z

The following topics are closed by current state of the patch:

FreeBSD specifics are separated with conditional build.
The existing tests, broken by the change, are fixed.
The documentation update is added. kyua.conf.5 and kyuafile.5 man pages got the respective upgrades.

ngie-eign

As much as I'm not a fan of the term "container" because it focuses on the Linux/Solaris technological implementations, I believe that it would be wise to use "container" in lieu of "jail". "jail"s are a concept that are very FreeBSD specific and don't exist outside of FreeBSD/FreeBSD derivatives. I realize this project is hosted in the freebsd GitHub org, but it's a good idea to keep things as generic as possible to avoid having to rearchitect things later if someone contributes an actual Linux or Solaris container implementation (for instance).

Cross-references to jail(8), et al, are missing from the manpage edits (this seems to be where most of the jail kernel subsystem documentation lives). This is important because you're adding a feature which isn't fully documented and not providing enough information for more casual FreeBSD users to use the feature.

doc/kyua.conf.5.in

doc/kyuafile.5.in

model/metadata.hpp

utils/config/nodes.ipp

utils/process/executor.cpp

ngie-eign · 2024-05-04T23:15:51Z

utils/process/executor.cpp

+    executor::exit_handle
+    reap(const int original_pid)
+    {
+        const exec_handles_map::iterator iter = all_exec_handles.find(


Error handling is missing for values like 0, -1, etc. These values have special meanings and can result in the process either committing suicide OR a system-wide reboot/shutdown/etc. Neither case is desired.
From kill(2):

If pid is zero: The sig signal is sent to all processes whose group ID is equal to the process group ID of the sender, and for which the process has permission; this is a variant of killpg(2). If pid is -1: If the user has super-user privileges, the signal is sent to all processes excluding system processes (with P_SYSTEM flag set), process with ID 1 (usually init(8)), and the process sending the signal. If the user is not the super user, the signal is sent to all processes which the caller has permissions to, excluding the process sending the signal. No error is returned if any process could be signaled.

It looks the function name is misleading, it's not the same reaping as in the general process management context. During my work on this patch back in Oct, I found a defect in the existing Kyua mechanism of abrupt termination handling -- it was not collecting all exit handles of its children. It was easier to reproduce with parallelism>1, and the parallelism is what I was actively working on. The pid passed here as a parameter is a pid of a spawn child previously saved by kyua, the kyua simply expects to see an exit handle object per each child spawn and this "kyua specifics" reaping process helps with that. That is, it's named after the same term of the general context, but this is about kyua internals.

What I was driving at before is that some invariants should be added to make sure -1/0 are never passed to this function.

The follow-up: #231

ngie-eign · 2024-05-04T23:17:15Z

freebsd/utils/jail.cpp

+    // invoke jail
+    std::auto_ptr< process::child > child = child::fork_capture(
+        run(fs::path("/usr/sbin/jail"), av));


It seems like sanity checking that the jail(8) program exists and skipping with a helpful message would be wise--instead of just marking all of the tests broken.
This is especially important for systems where WITHOUT_JAIL= is specified in src.conf(5).

I was working on a concern from Olivier: https://lists.freebsd.org/archives/freebsd-hackers/2024-February/003016.html. Currently it's covered such way that for the WITHOUT_JAIL system kyua is built without execenv=jail support. It makes kyua config to report execenv list with a single host item. As a result it triggers the existing mechanism of requirements checking and marks execenv=jail based tests as skipped with execenv="jail" requires FreeBSD with jail feature. message. It looks to be simpler, on the higher level, without additional checks of the system consistency in cases like WITH_JAIL system + missing binary, or WITHOUT_JAIL system + kyua binary copied from WITH_JAIL system, etc. Also, the same mechanism and message is used in case of execenv=jail tests running on Linux and other systems. Any not-covered inconsistency will be reported in a test log with messages like /usr/sbin/jail: file not found and so on.
What do you think?

I think that would be ok.

ngie-eign

Seems like the code warrants adding or modifying isolation_test.cpp, et al.

In general, it seems like the PR is proposing a number of changes without providing sufficient numbers of negative tests and integration tests. This is concerning from a code maintenance/support perspective.

Also, the jail feature creates an exploitable vector for security issues which doesn't seem to have an adequate disclaimer regarding its use. Although it's an implied bad practice to run tests from a potentially untrusted sources as root on production systems, this concern isn't called out in the documentation. In particular, this feature requires root to function, even if required.user='unprivileged' is implemented such that jails are run as unprivileged users. jail(8) on FreeBSD always requires superuser privileges, whereas other container implementations, such as docker, allow specific groups to run administrative commands.

Another warning should be added noting that multiple parallel kyua processes can conflict with one another. This is true if is_exclusive = true is not set, but also if multiple kyua instances are run in parallel, despite other guarantees (sqlite3 db locking) which prevents multiple processes for conflicting with one another.

It would be really good to call this out (as a warning) too.

ngie-eign · 2024-05-04T23:33:10Z

doc/kyuafile.5.in

+atf_test_program{name='network_test',
+                 execenv='jail',
+                 execenv_jail='vnet allow.raw_sockets',
+                 required_user='root'}


(picking a line)
The code below doesn't explicitly check to make sure the user has permissions to create jails.
atf_test_program allows a config file to incorrectly specify execenv='jail' and required_user='unprivileged_user'.

I was thinking about this and it looks to be simpler to keep the mechanism without additional built-in policy. Who knows, probably in future we will have a capability to create some sort of jails by non-superuser.

For now I propose to improve the documentation (kyuafile.5). I've added the key message that root is needed and the examples section was extended with extra explanation. The non-working jail+unprivileged combination is mentioned explicitly. What do you think?

Anyway, explicit documentation is a good change here.

That sounds great to me!

ihoro · 2024-05-05T19:12:38Z

@ngie-eign, thank you very much for your time. Going to work on the outcome of the review.

ihoro · 2024-05-06T20:24:45Z

As much as I'm not a fan of the term "container" because it focuses on the Linux/Solaris technological implementations, I believe that it would be wise to use "container" in lieu of "jail". "jail"s are a concept that are very FreeBSD specific and don't exist outside of FreeBSD/FreeBSD derivatives. I realize this project is hosted in the freebsd GitHub org, but it's a good idea to keep things as generic as possible to avoid having to rearchitect things later if someone contributes an actual Linux or Solaris container implementation (for instance).

Sounds interesting to consider. I have concerns that two levels of abstraction could add extra misunderstanding from end user perspective and extra interface limitations for the possible future execution environments added. The execenv is already meant to be the first abstraction added. It's abstracted from the concepts and details of a specific execution environment, i.e. it does not imply a containerization only. And currently it's designed as a switch, it could be anything (just thinking out loud): execenv=host, execenv=jail, execenv=bhyve, execenv=docker, execenv=kubernetes, execenv=python-virtualenv, execenv=aws, etc. And the key abstraction is that parameters are not generalized, due to it can be hard to do and it may introduce limitations only. For example, jail has a single metadata property -- execenv.jail, what is a string of parameters passed to the jail(8) program. Docker could have multiple properties like execenv.docker.imageurl and execenv.docker.runparams. AWS could have more execenv.aws.* properties to cover authorization and details of the exact service to be used.

There was no intention to provide an automatic containerization feature which uses the local capabilities like jail for FreeBSD or cgroups et al. for Linux. It simply provides a new concept, execution environment, and implements the first non-default (aka host) environment -- FreeBSD jail based one. It's merely an outcome that it can be used to add extra isolation of some FreeBSD tests to improve parallelism of the test suite. A test author is expected to understand the background if jail execution environment is picked for a test.

This is a short elaboration of the current vision. What do you think? Probably I have not caught accurately the idea with execenv=container and execenv.container.params.

ihoro · 2024-05-07T07:45:18Z

Anyway, now I think that execenv.jail could be re-worked into execenv.jail.params to keep the "namespace" for possible future improvements of jail execenv with extra execenv.jail.* metadata.

ihoro · 2024-05-09T12:28:57Z

@ngie-eign, I'm leaving the "Resolve conversation" button for you. I guess it must be convenient for you.

ihoro · 2024-05-13T08:49:19Z

@ngie-eign, I think it's ready for the next round of your review.

ihoro · 2024-05-27T12:28:50Z

The latest push introduces the following changes:

Rebase and update the code accordingly -- align with libtool usage
Rename execenv.jail to execenv.jail.params -- to keep execenv.jail.* open for future improvements
Rename execenv engine conf variable to execenvs -- to keep it consistent with the existing Kyua interface
Fix autotools based linkage due to recent introduction of new constants in the code

ihoro · 2024-05-27T17:13:14Z

I've moved freebsd/* to os/freebsd/. There is no os namespace added, it can be done in future if it's really needed. The main task is to ease work with the history -- by putting the files at the desired location from the very beginning.

markjdb · 2024-06-04T17:49:56Z

I agree with Igor's view about the container/jail naming. There are lots of interface differences between jails, cgroups, zones, etc.., and it's premature to generalize from a single backend implementation. If in the future someone shows up with an equivalent backend for a different OS, it may have constraints which mean that a generic "container" engine will need backwards-incompatible changes to support both jails and the new container type. Meanwhile, this feature provides major benefits to FreeBSD src developers today - with very little effort we can use this to speed up network tests substantially.

As a test, I integrated this PR into FreeBSD and tried running the pf tests with and without execenv=jail. In bhyve with parallelism=4, it takes ~960s without this patch (most tests are serialized because of jail name clashes), and ~360s with the patch. All I had to do was add two lines to a makefile. I believe kp@ has seen similar improvements.

@ngie-eign is there anything I can do to help speed up review here? I spent a fair bit of time looking at early iterations of the patch and would quite like to see this feature progress into FreeBSD. Should we perhaps import it directly into FreeBSD and refine it there, then reconcile the two copies of kyua later?

markjdb · 2024-06-05T02:20:02Z

@ihoro I believe there is some kind of memory leak in the patch. So far I narrowed it down to something in the FreeBSD sbin tests. If I run them with -v parallelism=4, something causes kyua to consume a huge amount of memory, triggering the OOM killer. In particular, nothing there is using execenv=jail. Please let me know if I can help narrow it down further.

ihoro · 2024-06-05T19:39:21Z

@ihoro I believe there is some kind of memory leak in the patch. So far I narrowed it down to something in the FreeBSD sbin tests. If I run them with -v parallelism=4, something causes kyua to consume a huge amount of memory, triggering the OOM killer. In particular, nothing there is using execenv=jail. Please let me know if I can help narrow it down further.

Thank you for the extra attention and additional testing. I guess there is no kyua report --verbose to share respectively.

I've tried main on aarch64@qemu with kldload pf and have not re-produced it yet with the following result:
Test cases: 637 total, 46 skipped, 0 expected failures, 1 broken, 1 failed
Working on the amd64@qemu trial.

Any detail of the case's environment would be helpful. Let me guess that probably bricoler has been used here then probably I will have higher chances to reproduce the same if you provide me with the invocation details. It could be an interesting testing of many things at the same time.

markjdb · 2024-06-05T19:58:46Z

Any detail of the case's environment would be helpful. Let me guess that probably bricoler has been used here then probably I will have higher chances to reproduce the same if you provide me with the invocation details. It could be an interesting testing of many things at the same time.

Please try cloning bricoler, then run make. Then:

$ ./bricoler run freebsd-src-regression-suite-run --param freebsd-src:url=https://github.com/markjdb/freebsd --param freebsd-src:branch=main-kyua-parallel --param freebsd-src-regression-suite-run:hypervisor=bhyve --param freebsd-src-regression-suite-run:tests=sbin

Note you'll need sudo to use bhyve for now (I just configure bhyve and bhyvectl in sudoers for now on my build system). If you omit the hypervisor parameter, it'll use QEMU instead, which also works, but it's slower.

While tests are running, you can ssh into the VM with bricoler run freebsd-src-regression-suite-run ssh. If I do that, I see this in top:

 2045 root        122    0    14G  5226M CPU2     2   0:15 100.00% kyua -v parallelism=4

There are some sharp corners here, please feel free to report issues or make suggestions. If you can't make progress, email me and we can debug further. Maybe I made a mistake somewhere.

markjdb · 2024-06-07T19:20:12Z

@ihoro I believe there is some kind of memory leak in the patch.

Hmm, there is something strange going on. It seems to be related to incremental buildworlds that I use by default in my test runner. If I do a clean build, I can't reproduce any problems, but I still don't understand the root cause.

I will keep digging, but please ignore this for now. Sorry for the noise.

markjdb · 2024-06-10T23:17:17Z

@ihoro I believe there is some kind of memory leak in the patch.
I will keep digging, but please ignore this for now. Sorry for the noise.

I'm pretty sure there is a latent bug in kyua somewhere, related to -v parallelization. I've been running the FreeBSD src test suite in a loop with parallel tests, collecting bug fixes (mostly test bugs/hidden assumptions), and I've seen a couple of OOM kills. Again, sorry for the noise, it seems unrelated to this patch.

I would still really like to see this land soon. On my test systems, with -v parallelization=4, the full test suite takes 3-4 hours to run, which isn't terrible but is rather long. I expect that to drop significantly with this patch and a small amount of work.

ihoro · 2024-06-12T07:40:23Z

I'm pretty sure there is a latent bug in kyua somewhere, related to -v parallelization. I've been running the FreeBSD src test suite in a loop with parallel tests, collecting bug fixes (mostly test bugs/hidden assumptions), and I've seen a couple of OOM kills. Again, sorry for the noise, it seems unrelated to this patch.

Thanks for the additional analysis and the test runs.

Just thinking out loud: probably, we could add an extra logic to bricoler to capture and report such cases, with collecting of kyua core dump and related files if necessary (like kyua and kyua.debug binaries). It could provide us with a clue to build hypothesis of the lurking defect. Kyua is an important tool for the FreeBSD Test Suite, hence it should not be a bad thing to have some logic hard-coded specifically for the kyua test runs.

In addition to that, we could limit the memory of the kyua process with 200MB or something. Just to make it crash sooner without waste of resources and time. Just a quick idea, currently I do not have intuition of normal memory usage spikes kyua may have, I saw something steady like 11MB for the sbin tests.

ihoro · 2024-06-12T08:35:20Z

I would still really like to see this land soon. On my test systems, with -v parallelization=4, the full test suite takes 3-4 hours to run, which isn't terrible but is rather long. I expect that to drop significantly with this patch and a small amount of work.

From the organizational point of view, my current vision is that only the following topics need closure before the merge:

The comments raised by @ngie-eign . I have a feeling that I covered most of them in a single iteration, and around 10% of them may require another iteration of discussion/change.
The execenv.container idea discussion is not resolved yet. My intuition is that @ngie-eign could accept our vision, i.e. it would not require additional work.

And I propose to move the following topics to the subsequent PRs:

syntax(N) version bump question. I have a proposal to mark such tests as failed/whatever. Having kyua process failure during Kyuafile parsing stage looks less practical, when the whole test suite is rendered unusable. There are no troubles expected from the FreeBSD project perspective, @kprovost and I had a discussion of "MFC dilemma" and it looks like the MFC of the latest Kyua version should cover it, i.e. we won't have to balance between old and new Kyua.
New tests of the Kyua itself to cover the changes.
Kyua version bump. To allow downstreams to sync respectively, e.g. homebrew formula's maintainer could provide the respective upgrade if there is the next official version GA. But I have a feeling of some debt collected so far like Linux support repairing need, etc. This is definitely a separate topic.

markjdb · 2024-06-14T14:27:07Z

I'm pretty sure there is a latent bug in kyua somewhere, related to -v parallelization. I've been running the FreeBSD src test suite in a loop with parallel tests, collecting bug fixes (mostly test bugs/hidden assumptions), and I've seen a couple of OOM kills. Again, sorry for the noise, it seems unrelated to this patch.

Thanks for the additional analysis and the test runs.

Just thinking out loud: probably, we could add an extra logic to bricoler to capture and report such cases, with collecting of kyua core dump and related files if necessary (like kyua and kyua.debug binaries). It could provide us with a clue to build hypothesis of the lurking defect. Kyua is an important tool for the FreeBSD Test Suite, hence it should not be a bad thing to have some logic hard-coded specifically for the kyua test runs.

Absolutely. In fact, I occasionally see kyua crash with one of several assertions on arm64, also not understood yet. :(

sbin/nvmecontrol/basic:io_passthru  ->  passed  [2.780s]                                                                                                                                                                                                                                                                      
sbin/mdconfig/mdconfig_test:attach_vnode_non_explicit_type  ->  passed  [3.087s]                                                                                                                                                                                                                                              
*** /usr/home/markj/bricoler/freebsd-src/src/contrib/kyua/utils/optional.ipp:175: Precondition check failed: _data != NULL                                                                                                                                                                                                    
*** Fatal signal 6 received

I plan to extend the freebsd-src-regression-suite job to automatically fetch kyua.core and open it in a debugger. Similarly, the VM console watcher will soon be able to automatically detect kernel panics and generate a report of that.

In addition to that, we could limit the memory of the kyua process with 200MB or something. Just to make it crash sooner without waste of resources and time. Just a quick idea, currently I do not have intuition of normal memory usage spikes kyua may have, I saw something steady like 11MB for the sbin tests.

From watching it in top(1), I think there is some specific race condition which triggers a quick blowup of kyua's memory consumption. That is, memory usage is usually below ~12MB, and at some point it quickly starts growing.

A new Kyua concept is added -- "execution environment". A test can be configured to be run within a specific environment. The test case lifecycle is extended respectively: - execenv init (creates a jail or does nothing for default execenv="host") - test exec - cleanup exec (optional) - execenv cleanup (removes a jail or does nothing for default execenv="host") The following new functionality is provided, from bottom to top: 1 ATF based tests - 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". - The new "execenv.jail.params" 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". Kyua implicitly adds "children.max" to "execenv_jail_params" parameters with the maximum possible value. A test case can override it. 2 Kyuafile - The same new metadata properties can be defined on Kyuafile level: "execenv" and "execenv_jail_params". - Note that historically ATF uses dotted style of metadata naming, while Kyua uses underscore style. Hence "execenv.jail.params" vs. "execenv_jail_params". 3 kyua.conf, kyua CLI - The new "execenvs" engine configuration variable can be set to a list of execution environments to run only tests designed for. Tests of not listed environments are skipped. - By default, this variable lists all execution environments supported by a Kyua binary, e.g. execenvs="host jail". - This variable can be changed via "kyua.conf" or via kyua CLI's "-v" parameter. For example, "kyua -v execenvs=host test" will run only host-based tests and skip jail-based ones. - Current value of this variable can be examined with "kyua config". [markj] This feature has not landed upstream yet. See the discussion in freebsd/kyua#224 . Having the ability to automatically jail tests allows many network tests to run in parallel, giving a drastic speedup. So, let's import the feature and start using it in main. Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: markj, kp Tested by: markj, kp MFC after: 3 months Differential Revision: https://reviews.freebsd.org/D45865

A new Kyua concept is added -- "execution environment". A test can be configured to be run within a specific environment. The test case lifecycle is extended respectively: - execenv init (creates a jail or does nothing for default execenv="host") - test exec - cleanup exec (optional) - execenv cleanup (removes a jail or does nothing for default execenv="host") The following new functionality is provided, from bottom to top: 1 ATF based tests - 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". - The new "execenv.jail.params" 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". Kyua implicitly adds "children.max" to "execenv_jail_params" parameters with the maximum possible value. A test case can override it. 2 Kyuafile - The same new metadata properties can be defined on Kyuafile level: "execenv" and "execenv_jail_params". - Note that historically ATF uses dotted style of metadata naming, while Kyua uses underscore style. Hence "execenv.jail.params" vs. "execenv_jail_params". 3 kyua.conf, kyua CLI - The new "execenvs" engine configuration variable can be set to a list of execution environments to run only tests designed for. Tests of not listed environments are skipped. - By default, this variable lists all execution environments supported by a Kyua binary, e.g. execenvs="host jail". - This variable can be changed via "kyua.conf" or via kyua CLI's "-v" parameter. For example, "kyua -v execenvs=host test" will run only host-based tests and skip jail-based ones. - Current value of this variable can be examined with "kyua config". Signed-off-by: Igor Ostapenko <pm@igoro.pro>

ihoro · 2024-08-04T10:42:00Z

@ngie-eign do you have a vision how we could proceed with the Pull Request? Currently it's aligned with the code committed to the freebsd-src/main. It looks less productive to continue with the related improvements due to this is not the only diff now between freebsd-src:contrib/kyua and kyua.

ngie-eign · 2024-09-08T05:05:09Z

Apologies for being so incommunicado. I'll merge this change now to sync freebsd:main and this repo. It would be good to have followup issues for any items pointed out that haven't been resolved in this PR.
Thanks!

ihoro · 2024-09-09T07:45:50Z

Thanks for your time and volunteer effort. The subsequent PRs should be much smaller and easier to discuss.

A new Kyua concept is added -- "execution environment". A test can be configured to be run within a specific environment. The test case lifecycle is extended respectively: - execenv init (creates a jail or does nothing for default execenv="host") - test exec - cleanup exec (optional) - execenv cleanup (removes a jail or does nothing for default execenv="host") The following new functionality is provided, from bottom to top: 1 ATF based tests - 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". - The new "execenv.jail.params" 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". Kyua implicitly adds "children.max" to "execenv_jail_params" parameters with the maximum possible value. A test case can override it. 2 Kyuafile - The same new metadata properties can be defined on Kyuafile level: "execenv" and "execenv_jail_params". - Note that historically ATF uses dotted style of metadata naming, while Kyua uses underscore style. Hence "execenv.jail.params" vs. "execenv_jail_params". 3 kyua.conf, kyua CLI - The new "execenvs" engine configuration variable can be set to a list of execution environments to run only tests designed for. Tests of not listed environments are skipped. - By default, this variable lists all execution environments supported by a Kyua binary, e.g. execenvs="host jail". - This variable can be changed via "kyua.conf" or via kyua CLI's "-v" parameter. For example, "kyua -v execenvs=host test" will run only host-based tests and skip jail-based ones. - Current value of this variable can be examined with "kyua config". [markj] This feature has not landed upstream yet. See the discussion in freebsd/kyua#224 . Having the ability to automatically jail tests allows many network tests to run in parallel, giving a drastic speedup. So, let's import the feature and start using it in main. Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: markj, kp Tested by: markj, kp MFC after: 3 months Differential Revision: https://reviews.freebsd.org/D45865 (cherry picked from commit 257e70f) Approved by: kp (mentor), markj (mentor)

A new Kyua concept is added -- "execution environment". A test can be configured to be run within a specific environment. The test case lifecycle is extended respectively: - execenv init (creates a jail or does nothing for default execenv="host") - test exec - cleanup exec (optional) - execenv cleanup (removes a jail or does nothing for default execenv="host") The following new functionality is provided, from bottom to top: 1 ATF based tests - 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". - The new "execenv.jail.params" 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". Kyua implicitly adds "children.max" to "execenv_jail_params" parameters with the maximum possible value. A test case can override it. 2 Kyuafile - The same new metadata properties can be defined on Kyuafile level: "execenv" and "execenv_jail_params". - Note that historically ATF uses dotted style of metadata naming, while Kyua uses underscore style. Hence "execenv.jail.params" vs. "execenv_jail_params". 3 kyua.conf, kyua CLI - The new "execenvs" engine configuration variable can be set to a list of execution environments to run only tests designed for. Tests of not listed environments are skipped. - By default, this variable lists all execution environments supported by a Kyua binary, e.g. execenvs="host jail". - This variable can be changed via "kyua.conf" or via kyua CLI's "-v" parameter. For example, "kyua -v execenvs=host test" will run only host-based tests and skip jail-based ones. - Current value of this variable can be examined with "kyua config". [markj] This feature has not landed upstream yet. See the discussion in freebsd/kyua#224 . Having the ability to automatically jail tests allows many network tests to run in parallel, giving a drastic speedup. So, let's import the feature and start using it in main. Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: markj, kp Tested by: markj, kp MFC after: 3 months Differential Revision: https://reviews.freebsd.org/D45865 (cherry picked from commit 257e70f1d5ee61037c8c59b116538d3b6b1427a2) Approved by: kp (mentor), markj (mentor)

A new Kyua concept is added -- "execution environment". A test can be configured to be run within a specific environment. The test case lifecycle is extended respectively: - execenv init (creates a jail or does nothing for default execenv="host") - test exec - cleanup exec (optional) - execenv cleanup (removes a jail or does nothing for default execenv="host") The following new functionality is provided, from bottom to top: 1 ATF based tests - 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". - The new "execenv.jail.params" 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". Kyua implicitly adds "children.max" to "execenv_jail_params" parameters with the maximum possible value. A test case can override it. 2 Kyuafile - The same new metadata properties can be defined on Kyuafile level: "execenv" and "execenv_jail_params". - Note that historically ATF uses dotted style of metadata naming, while Kyua uses underscore style. Hence "execenv.jail.params" vs. "execenv_jail_params". 3 kyua.conf, kyua CLI - The new "execenvs" engine configuration variable can be set to a list of execution environments to run only tests designed for. Tests of not listed environments are skipped. - By default, this variable lists all execution environments supported by a Kyua binary, e.g. execenvs="host jail". - This variable can be changed via "kyua.conf" or via kyua CLI's "-v" parameter. For example, "kyua -v execenvs=host test" will run only host-based tests and skip jail-based ones. - Current value of this variable can be examined with "kyua config". [markj] This feature has not landed upstream yet. See the discussion in freebsd/kyua#224 . Having the ability to automatically jail tests allows many network tests to run in parallel, giving a drastic speedup. So, let's import the feature and start using it in main. Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: markj, kp Tested by: markj, kp MFC after: 3 months Differential Revision: https://reviews.freebsd.org/D45865

This comment was marked as outdated.

Sign in to view

ihoro marked this pull request as draft February 25, 2024 14:45

ihoro closed this Mar 22, 2024

ihoro force-pushed the freebsd-jail-execenv branch from 99692a8 to 0a43bb8 Compare March 22, 2024 16:53

ihoro reopened this Mar 22, 2024

ihoro marked this pull request as ready for review March 22, 2024 17:41

ihoro force-pushed the freebsd-jail-execenv branch from a33e429 to 08699b3 Compare March 23, 2024 11:28

ihoro force-pushed the freebsd-jail-execenv branch from 08699b3 to 9721be7 Compare April 1, 2024 13:05

ngie-eign requested changes May 4, 2024

View reviewed changes

ngie-eign reviewed May 4, 2024

View reviewed changes

ihoro force-pushed the freebsd-jail-execenv branch 2 times, most recently from 10330c3 to 6553ba0 Compare May 9, 2024 12:17

ihoro force-pushed the freebsd-jail-execenv branch 4 times, most recently from 2b9f217 to 4fe2a93 Compare May 10, 2024 15:53

ihoro requested a review from ngie-eign May 16, 2024 11:59

ihoro force-pushed the freebsd-jail-execenv branch 2 times, most recently from 3d51835 to 3ef1a3a Compare May 27, 2024 12:22

ihoro force-pushed the freebsd-jail-execenv branch from 3ef1a3a to d8212aa Compare May 27, 2024 17:08

ihoro force-pushed the freebsd-jail-execenv branch from d8212aa to a4737b1 Compare August 4, 2024 10:27

ngie-eign approved these changes Sep 8, 2024

View reviewed changes

ngie-eign merged commit ffd1158 into freebsd:master Sep 8, 2024

ihoro mentioned this pull request Sep 27, 2024

executor_handle: Add missing precondition to reap() function #231

Merged

Add FreeBSD Jail execution environment support #224

Add FreeBSD Jail execution environment support #224

Conversation

ihoro commented Feb 3, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ihoro commented Mar 22, 2024

ngie-eign left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngie-eign left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihoro commented May 5, 2024

ihoro commented May 6, 2024

ihoro commented May 7, 2024

ihoro commented May 9, 2024

ihoro commented May 13, 2024

ihoro commented May 27, 2024

ihoro commented May 27, 2024

markjdb commented Jun 4, 2024

markjdb commented Jun 5, 2024

ihoro commented Jun 5, 2024

markjdb commented Jun 5, 2024 • edited Loading

markjdb commented Jun 7, 2024

markjdb commented Jun 10, 2024

ihoro commented Jun 12, 2024

ihoro commented Jun 12, 2024

markjdb commented Jun 14, 2024

ihoro commented Aug 4, 2024

ngie-eign commented Sep 8, 2024

ihoro commented Sep 9, 2024

ihoro commented Feb 3, 2024 •

edited

Loading

ngie-eign left a comment •

edited

Loading

ngie-eign left a comment •

edited

Loading

markjdb commented Jun 5, 2024 •

edited

Loading