Re: [PATCH] release/checks/try.nix: init

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 4363 bytes --]

Ryan Lahfa <ryan@lahfa.xyz> writes:

> On Fri, May 26, 2023 at 09:07:58PM +0000, Alyssa Ross wrote:
>> This is a regression test for c7f87f3 ("host/rootfs: allow growing ext
>> partition to fail").  It's the first time we're actually doing
>> automated tests of a Spectrum boot.  For now, I'm using the NixOS test
>> framework, but because we're not using NixOS and not setting any NixOS
>> options, it feels to me like it doesn't actually buy us very much, so
>> if it doesn't start adding more value as we add more (or more complex)
>> tests, it might be simpler to just use a shell/execline script for
>> tests.
>
> Are you only interested into tests that works without any
> instrumentation?
>
> e.g. what if Spectrum added the backdoor.service in their service
> management? That'd repair the ability of the test framework to have
> better interactions with the guest without QEMU Agent.

At least until I really need tests that depend on guest cooperation, yes.
Because to have either the backdoor service or the QEMU agent, I'd
either have to build custom images just for testing (which would mean
the real images were not actually tested), or I'd have to build those
things into all Spectrum images.

At some point, it might not be possible to get away from this, but the
basic tests I've written so far have all gone fine without the need for
any guest cooperation beyond a serial shell.

> On my side, I am interested supporting testing things like Debian + Nix
> via the test framework and saying "here's a disk image or something,
> boot it and connect to it".

You might be interested in replying to this Discourse topic:
https://discourse.nixos.org/t/running-a-nixos-test-on-a-different-image/28458

>> +config.pkgs.nixosTest ({ pkgs, ... }: {
>> +  name = "try-spectrum-test";
>> +  nodes = {};
>> +
>> +  testScript = ''
>> +    import shlex
>> +    import subprocess
>> +
>> +    conf = subprocess.run([
>> +      "${pkgs.mtools}/bin/mcopy",
>> +      "-i",
>> +      "${live}@@1M",
>> +      "::loader/entries/spectrum.conf",
>> +      "-",
>> +    ], stdout=subprocess.PIPE)
>> +    conf.check_returncode()
>> +
>> +    cmdline = None
>> +    for line in conf.stdout.decode('utf-8').splitlines():
>> +      key, value = line.split(' ', 1)
>> +      if key == 'options':
>> +        cmdline = value
>
> Is there any reason to not have `conf` / `cmdline`
> being derived in a derivation?

We can't know it at eval time, because it includes the verity hash.  So
our options are to recompute it here, or reuse the results of the
derivation that already computed it, which is the approach I've taken
here.  It's a bit unweildy, but I blame Python for that.  If we do end
up switching to shell it'd just be:

	mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p'

>> +
>> +    flags = " ".join(map(shlex.quote, [
>> +      "qemu-kvm",
>> +      "-m", "512",
>> +      "-kernel", "${live.rootfs.kernel}/${pkgs.stdenv.hostPlatform.linux-kernel.target}",
>> +      "-initrd", "${live.initramfs}",
>> +      "-device", "qemu-xhci",
>> +      "-device", "usb-storage,drive=drive1,removable=true",
>> +      "-drive", "file=${live},id=drive1,format=raw,if=none,readonly=on",
>
> Does it bring anything that this is real "USB" device? (testing that the
> drivers for USB are present?).
>
> In that case, shouldn't testing covers also NVMe devices, etc. ?

USB is special in that USB devices don't show up until after userspace
has started, so userspace has to handle waiting for them.  This is not
generally the case with other block devices.  When working on the
installer, I always test with USB, both to ensure this extra complexity
is handled correctly, and because it's what I expect to be the most
common device type for the combined installer image to be booted from in
real life.

>> +      "-append", f"panic=-1 {cmdline}",
>> +    ]))
>> +
>> +    machine = create_machine({"startCommand": flags})
>
> `create_machine` is deprecated now, `Machine.create_startcommand` is
> "deprecated" too, but I'm not really happy with the fact we do not have
> serious replacements to that and I also want to better support "testing
> non-NixOS" or non-instrumented systems in the framework.

There's no viable alternative, so I think the best course of action here
is to undeprecated it:
https://github.com/NixOS/nixpkgs/pull/234427

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]