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

[Ryan Lahfa <ryan@lahfa.xyz>]

On Sat, May 27, 2023 at 02:38:22PM +0000, Alyssa Ross wrote:
> 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.

Makes sense, supporting "minimal" friction for this usecase is
desirable.

I guess we could have Python *and* Bash/execline as "test scripts" if we want.

But I would need to see more about what would Bash/execline-oriented
test scripts would look like in practice.

Python is suboptimal for various things but the "batteries included" are
very useful for getting a OK-grade thing running.

> > 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'

Makes sense, but do you need it at evaltime?

Wouldn't this be a possibility:

```nix
let optionsFile = pkgs.runCommand "extract-options" {} ''mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p' > $out'';
in
''
  with open(${optionsFile}, "r") as f:
    options = f.read()
''
```

Ideally, with the https://github.com/NixOS/nixpkgs/issues/156563
proposal, this could even become easier IMHO.

> 
> >> +
> >> +    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.

Got it. :)

> 
> >> +      "-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

Agreed.


Kind regards,
-- 
Ryan Lahfa