kyua: Add "prepare" command

[igoro]

Summary

The general "requirement preparation handler" concept is introduced and
the very first handler named "kmods" is implemented.

Running without arguments lists all available handlers:

  kyua prepare

Currently there are only two handlers:

  all: runs all available handlers
  kmods: loads the modules declared by required_kmods metadata

The dry run option lists the required modules without actual loading:

  kyua prepare { --dry-run | -n } kmods

The "prepare" command traverses only the given tree of tests, i.e., the
following invocation lists all required modules for the whole test
suite:

  kyua prepare -k /usr/tests/Kyuafile -n kmods

, while this one is limited to the pf tests only:

  kyua prepare -k /usr/tests/sys/netpfil/pf/Kyuafile -n kmods

[igoro]

This is the very first working version of the Project B: https://lists.freebsd.org/archives/freebsd-testing/2024-November/000395.html.

rr stands for:

• requirement resolver
• resolve requirements
• requirement resolving

Currently the interface is as follows:

• A new command kyua rr lists all available requirement resolvers
• kyua rr resolver1 resolver2 ... runs the resolvers listed
• kyua rr all runs all available resolvers
• kyua rr accepts the same --kyuafile and --build-root options as kyua test does
• A new option kyua test --rr=resolvers is added as a handy shortcut to resolve requirements and run tests, e.g. kyua test --rr kmods
• Whitespace-separated list of required modules can be declared as required_kmods on Kyua side and as require.kmods on ATF side (this is what is expected to be extended by the Project A to add skipping mechanism)
• A new resolver kmods is added exclusively for FreeBSD, it consumes required_kmods/require.kmods

At least the following things are in my todo:

• Add/update man pages
• Add new tests
• Analyze cases flagged by olivier@ where specific tests do kldunload
• Final commit log message
• GitHub freebsd/kyua fork PR

[markj]

As usual, I will quibble a bit about naming - the kyua subcommands are mostly english words, so "rr" is a bit odd. Why not "resolve" or even "resolve-requirements" (or "resolve-reqs")?

Another comment is about candidate requirements: this mechanism is useful for loading kmods, but I'm not sure how it'll work with other requirements. What does it mean to resolve require.memory? The only other resolvable requirement I can think of (and which you mentioned in the ML post) is packages, but tests just enumerate required programs, which doesn't tell you how to install them. For example, required.programs = nping requires one to install the nmap package, but we can't know that without additional metadata. Plus, the test environment might not have access to pkg repos.

Having this mechanism to load kernel modules is indeed useful, but I have a hard time seeing how it can be generalized. Please don't take that as an objection. This feature doesn't require a lot of code, so isn't too intrusive.

[igoro]

In D48087#1096189, @markj wrote:

As usual, I will quibble a bit about naming - the kyua subcommands are mostly english words, so "rr" is a bit odd. Why not "resolve" or even "resolve-requirements" (or "resolve-reqs")?

Thanks for that, we do need to stress the design here. Initially, I was thinking about something like "kyua resolve-required", where "required" is like an established word due to required_* metadata. That's fine for unattended cases like CI scripts or bricoler, but I was thinking it might end up being tedious to type, and shell completions might not always be available. Hence, I started the discussion with less obvious option to cover my doubts. What if we have a long name and a short alias as it frequently happens with CLI? Like "checkout" -> "co" and so on. As long as the subcommand expects a list of resolvers, we could consider another naming variant as well:

> kyua run-resolvers kmods pkgs
> kyua rr kmods
> kyua test --run-resolvers pkgs
> kyua test --rr kmods

Another comment is about candidate requirements: this mechanism is useful for loading kmods, but I'm not sure how it'll work with other requirements. What does it mean to resolve require.memory? The only other resolvable requirement I can think of (and which you mentioned in the ML post) is packages, but tests just enumerate required programs, which doesn't tell you how to install them. For example, required.programs = nping requires one to install the nmap package, but we can't know that without additional metadata. Plus, the test environment might not have access to pkg repos.

I was thinking about a new "required_pkgs" metadata. Also OS agnostic as "required_kmods" is. Just a whitespace-separated list of names. And the FreeBSD resolver named "pkgs" could read it and do the usual pkg install. Chasing packages via required_programs does not seem to be deterministic. If we provide an optional mechanism a specific test host may still use custom third-party software/builds and avoid "pkgs" resolver usage, e.g. some pre-cooked VM or OCI image etc. Thus, required_programs and required_pkgs could stay as separate mechanisms which could be used together if needed. Probably, "required_pkgs" should not provide any check-and-skip mechanism and should stay as a pure declaration of needs -- some tests may use both "required_pkgs" and "required_programs".

Having this mechanism to load kernel modules is indeed useful, but I have a hard time seeing how it can be generalized. Please don't take that as an objection. This feature doesn't require a lot of code, so isn't too intrusive.

Yes, good news is that it does not require a lot of code, and it's great to follow the Kyua architecture which tries to provide extensible sub-systems.

One of the key points I had in mind is that it should not run all resolvers by default. For instance, I have a prepared system and the only thing I need is "kyua test --rr kmods testprogram" due to modules are not there after reboot, and I want to avoid "pkgs" resolver spending my precious seconds to check that I have everything installed (or it may disrupt my setup with updates, or the test host has no Internet connection, etc). Such cases also tend to have some generic approach here. And the special "all" resolver runs all of them if we do need it.

There is no intention to add resolvers w/o actual demand, but I had the following resolvers in my mind, without giving it thorough thought yet:

• kmods
• pkgs
• pip? # e.g. to have scapy installed
• sysctls # some tests may want to have specific settings like turn-on-a-feature, e.g. sysctl kern.ipc.tls.enable=1 and sysctl kern.crypto.allow_soft=1
• thinking out loud: in the future we may want to prefetch some OCI images, similar to pkg install idea

[igoro]

By the way, now I'm leaning towards resolve subcommand as you mentioned:

> kyua resolve kmods
> kyua test { --resolve | -r } kmods testprog

Let's gather more opinions/votes before the next version of the patch.

[kp]

I generally share Mark's views. I was tempted to suggest we should rename it to 'load-kmods' or similar, but there's a somewhat reasonable path to it also being able to install required packages (even if that will require test modifications), so keeping the generic name seems sensible. If we do get to that point we should consider adding a 'kyua prepare' or something to take care of all of the requirements (i.e. kmods, packages) it knows about. That should let us run (most) tests with kyua prepare && kyua test on a new install. (That's very much a maybe someday sort of thing, we do not need that to proceed here.)

One trivial remark about 'pip? # e.g. to have scapy installed'. We have a scapy package and that's how I've always installed it. Right now it's py311-scapy-2.6.1.

[igoro]

Rename "kyua rr" to "kyua prepare"; Drop "kyua test --rr"

[igoro]

My current favorite is the prepare term proposed by Kristof. So, I've changed it in the latest version of the patch for the sake of further testing and discussion. Everything is open for renaming. The internals still use requirement resolver terminology, I could not quickly find the replacement and probably it's not needed due to "prepare" is kind of higher level, anyway this is the encapsulated part with the only exposure via CLI help output.

Also, I've dropped any change to kyua test. I think it's too soon to provide shortcuts there due to non-established yet general concept, or probably it's better to postpone it until actual demand. It might be better to keep prepare and test as separate things as I envisioned from the very beginning.

I've checked the points raised by Olivier (https://lists.freebsd.org/archives/freebsd-testing/2024-November/000396.html). It seems we change nothing here, it's like instead of listing the required modules manually as the maintainers of the CI configuration do today we will get the list from the declarations by the tests themselves. And those specific ipsec and opencrypto tests will do the usual kldload/kldunload things as today. The sys/netipsec/tunnel tests are already marked as is_exclusive, I guess we could do the same for the sys/opencrypto tests -- so that we make sure they do not play "kld[un]load" cards while there are others running. Of course, it means that these ipsec/opencrypto tests will not use required_kmods for modules they handle on their own. It seems that my current vision correlates with Kristof's "perhaps we don’t need to worry about that just yet" (https://lists.freebsd.org/archives/freebsd-testing/2024-November/000397.html).

A quick recap of the UI provided by the latest patch:

> kyua prepare
all                     Run all requirement resolvers
kmods                   Resolve required_kmods for FreeBSD

> grep -r kmods *
Kyuafile:atf_test_program{name="divert-to", execenv="jail", execenv_jail_params="vnet allow.raw_sockets", required_kmods="pf ipdivert iflib"}
Kyuafile:atf_test_program{name="mbuf", execenv="jail", execenv_jail_params="vnet allow.raw_sockets", required_kmods="ether pflog if_bridge"}
mbuf:   atf_set require.kmods if_epair if_bridge pf

> kyua prepare --dry-run all
kldload ether if_bridge if_epair iflib ipdivert pf pflog

> kyua prepare kmods
kldload ether if_bridge if_epair iflib ipdivert pf pflog

The above demonstrates:

• Kernel module requirement declaration on Kyuafile and ATF test case levels
• Built-in modules support (the iflib and ether are used just for the demo)
• The all resolver in addition to the kmods
• The --dry-run | -n option of kyua prepare command

[igoro]

Now the wording is away from the internals and closer to the end users:

> ./kyua help | grep prepare
  prepare                 Prepare env and resolve requirements before testing.

> ./kyua prepare
all                     Run all preparations
kmods                   FreeBSD: kldload modules declared using required_kmods metadata

[igoro]

In D48087#1096440, @kp wrote:

One trivial remark about 'pip? # e.g. to have scapy installed'. We have a scapy package and that's how I've always installed it. Right now it's py311-scapy-2.6.1.

Great! I have not checked yet the details for the pip case, thanks for saving my time by providing the best way to cover it.

[igoro]

@ngie , it would be appreciated if you could find time to consider this. It seems that your opinion is necessary to move forward.

[ngie]

Could requires be used instead of the term prepare? I think that would align the concept and the command more with what is being reported on.

[igoro]

In D48087#1117288, @ngie wrote:

Could requires be used instead of the term prepare? I think that would align the concept and the command more with what is being reported on.

Thanks for your time.

Yes, the initial idea was to play around the word require due to the existing metadata naming approach: required_kmods, required_files, etc. But this word itself is more about declaration not the implementation. Also, there are good comments reminding that kyua sub-commands are verbs like debug, report, test, and so on. And a verb like prepare fits quite well, it depicts the action itself which would be taken, and it is generic enough to cover possible future cases when it is not only about kernel modules or packages: kyua prepare kmods, kyua prepare pkgs, kyua prepare all, kyua prepare <anything-else-in-the-future>.

What do you think about the internal design and code? It seems rr is short enough for typing with less obvious meaning (in general it stands for requirement resolver) covered with the code comments. Also, all these are internals which can be easily renamed/refactoring any time.

P.S. I guess, I could rename all those rr_* classes to prepare_* ones, even if it's a verb instead of a noun. It should make everything talk the same terminology for clarity. But it would be great to get some agreement first to minimize the amount of re-work and re-testing.

[igoro]

Rebase onto the latest kyua changes and rework from rr to prepare

[ngie]

1. This probably won't work in jails. This scenario needs to be detected and handled correctly.
2. Having a way to override the behavior of automatically loading in kyua.conf might be desired, similar to require.config="allow_sysctl_side_effects". I think it should default to on in the configuration on FreeBSD, but be opt-out if needed.

contrib/kyua/doc/kyuafile.5.in

303

Just trying to think of a slightly more extended explanation for how the variable works.. my description could stand wordsmithing.
Also, this appears to be the only case where this is true (appending occurs). Is there value in doing that, or would it be better to just specify the explicit requirements each time--despite it being redundant potentially?

contrib/kyua/engine/prepare/prepare_all.cpp

56–64

It's nicer to understand what's going on with the variable fully spelled out.

contrib/kyua/os/freebsd/prepare_kmods.cpp

2

50–51

*shrug*

[ngie]

Oh, also.. this probably should be serialized. I don't think it would be terribly wise to try parallelizing efforts (seems like you'd run into potential TOCTTOU bugs..).
Finally, for loadable subsystems compiled into the kernel the name can differ between the module and the static subsystem name. It might be worth adding support to handle that eventuality.

[igoro]

Tiny improvements

[igoro]

Thanks for your time and review.

In D48087#1197102, @ngie wrote:

1. This probably won't work in jails. This scenario needs to be detected and handled correctly.

Yes, it will not. Only root is expected to run kyua prepare kmods. It's much easier not to introduce extra code and let it error as it usually does.

2. Having a way to override the behavior of automatically loading in kyua.conf might be desired, similar to require.config="allow_sysctl_side_effects". I think it should default to on in the configuration on FreeBSD, but be opt-out if needed.

The idea is to introduce a separate opt-in feature and implement more use cases in demand in the future. Unfortunately , loading kernel modules automatically by default will be POLA violation. Also, some test runs may require not loading all the modules intentionally and keeps some cases skipped. It is better to keep this opportunity for later, after we get all tests declaring their needs and some test of time. Currently, it is expected that FreeBSD CI will eventually replace all those “kldload…” lines with a single “kyua prepare all”.

In D48087#1197107, @ngie wrote:

Oh, also.. this probably should be serialized. I don't think it would be terribly wise to try parallelizing efforts (seems like you'd run into potential TOCTTOU bugs..).
Finally, for loadable subsystems compiled into the kernel the name can differ between the module and the static subsystem name. It might be worth adding support to handle that eventuality.

What do you mean? Currently, this is a separate kyua prepare kmods command which is expected to be run once before the actual testing, i.e. it gathers all required_kmods declarations and loads the requested modules as a single step. The idea was not about doing it per test just before test execution.

contrib/kyua/doc/kyuafile.5.in

303

The required_kmods metadata has been recently introduced, this is not the first time it is documented here. See the unchanged lines above -- it is already documented. The .Va require.kmods sub-section simply depicts how this metadata is named on ATF side. If you remember I introduced this change months ago to make it easier to map Kyua names to ATF names of the metadata. This small addition simply explains a case when this metadata is present on Kyuafile level and on ATF-based test level. The whole idea of this small comment added is to make sure the manual reader understands that it behaves useful way -- appending instead of overriding. The reason of the comment is that usually metadata declared on Kyuafile level is overridden on ATF level.

contrib/kyua/engine/prepare/prepare_all.cpp

56–64

Agree, fixed.

contrib/kyua/os/freebsd/prepare_kmods.cpp

2

It was implemented back in 2024, but we were waiting for des@ changes first as the prerequisite (to introduce required_kmods metadata), and now we are just trying to land it finally.

50–51

Thanks, I like it. Improved.