-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nixosTests.installer: Port installer and ZFS test to python #78670
Conversation
@GrahamcOfBorg test installer zfs |
Done @worldofpeace |
7cc4a1d
to
ab649e2
Compare
Umm I don't see a different commit message for ab649e2? |
ab649e2
to
380134d
Compare
Sorry, my fault. Please have a look again @worldofpeace |
@@ -566,6 +567,42 @@ def screenshot(self, filename: str) -> None: | |||
if ret.returncode != 0: | |||
raise Exception("Cannot convert screenshot") | |||
|
|||
def copy_from_host_via_shell(self, source: str, target: str) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we have this?
Looking at 510ada7, these could probably also just use copy_from_host
…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first i am using copy_from_host
, but with the machine configs that follow later in the script, it doesn't work any longer as the VMs do not seem to support this. For that reason i added the shell variant as an addition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is because /tmp/shared
from nixos/modules/virtualisation/qemu-vm.nix
isn't setup in the installer.
Can we just invoke the mount
command manually before running copy_from_host
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, but i really don't like the implications of this: Currently, tests are agnostic of how this is all implemented and this allows for transparently changing the virt backend etc (apart from the create_machine
parameters which are pretty much qemu-specific, but these are also used by only a minority of tests).
As soon as tests begin doing such things, they would become testdriver-implementation specific and i am pretty sure we don't want that. I'd rather have two different functions that communicate which preconditions they assume and let the user choose from them, before users hard-code testdriver-details into their tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had an offline discussion on that. Agreed on not leaking implementation-specific details about how things are shared into the actual test code. Ideally, this should be encapsulated behind the corresponding methods.
nixos/lib/test-driver/test-driver.py
Outdated
def copy_from_host_via_shell(self, source: str, target: str) -> None: | ||
"""Copy a file from the host into the guest by piping it over the | ||
shell into the destination file. Works without host-guest shared folder. | ||
Prefer copy_from_host for large files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer copy_from_host for large files. | |
Prefer copy_from_host whenever possible. |
This implies it's okay to use it for small files.
In fact, we should use copy_from_host
whenever possible, and only use the base64 and shell dance if we have to IMHO.
7a47b7c
to
11de1ae
Compare
@tfc The zfs tests are still missing from |
11de1ae
to
c2864b8
Compare
@GrahamcOfBorg test zfs |
@GrahamcOfBorg build nixosTests.installer.simpleUefiSystemdBoot |
@tfc the above test seems to fail: https://logs.nix.ci/?attempt_id=0cfe37cd-ae01-49d3-9997-aa34ffe60ae9&key=nixos%2Fnixpkgs.78670 |
the above test case also fails on master btw https://hydra.nixos.org/build/111477951, but I can confirm it currently fails for an related reasons to that one. |
As the log indicates, this currently fails in the black validator, so it's more broken than before. We should at least fix that. |
Also not sure if the installer tests block channel releases, so we should be extra careful here. |
Don't worry, i am on it. It's just that i had no extra time yesterday. |
c2864b8
to
0495eae
Compare
@GrahamcOfBorg test installer.simpleUefiSystemdBoot |
So the current state is that the
machine # copying channel...
machine # installing the boot loader...
machine # setting up /etc...
machine # Initializing machine ID from random generator.
machine # Failed to open file system "/dev/block/253:1": No such file or directory
machine # Traceback (most recent call last):
machine # File "/nix/store/4lzpcmzwdi73ma34vx6hiff80an1r050-systemd-boot-builder.py", line 240, in <module>
machine # main()
machine # File "/nix/store/4lzpcmzwdi73ma34vx6hiff80an1r050-systemd-boot-builder.py", line 199, in main
machine # subprocess.check_call(["/nix/store/c54w4pb6g627ix9d7pyrqfv9qvgjbg6n-systemd-243.4/bin/bootctl", "--path=/boot", "--no-variables", "install"])
machine # File "/nix/store/4c4c5ig32lqhsgmcncnb72c3a4bwj66n-python3-3.7.6/lib/python3.7/subprocess.py", line 363, in check_call
machine # raise CalledProcessError(retcode, cmd)
machine # subprocess.CalledProcessError: Command '['/nix/store/c54w4pb6g627ix9d7pyrqfv9qvgjbg6n-systemd-243.4/bin/bootctl', '--path=/boot', '--no-variables', 'install']' returned non-zero exit status 1.
machine: output:
error: command `nixos-install < /dev/null >&2` failed (exit code 1)
cleaning up
killing machine (pid 9)
(0.00 seconds)
builder for '/nix/store/7jad11fkfs1rqql86zx5c84i1dqf7b1m-vm-test-run-installer-simpleUefiSystemdBoot.drv' failed with exit code 1
error: build of '/nix/store/7jad11fkfs1rqql86zx5c84i1dqf7b1m-vm-test-run-installer-simpleUefiSystemdBoot.drv' failed |
0495eae
to
81172e1
Compare
@GrahamcOfBorg test installer.btrfsSimple installer.luksroot-format1 installer.simpleClone installer.swraid installer.btrfsSubvolDefault installer.luksroot-format2 installer.simpleLabels installer.zfsroot installer.btrfsSubvols installer.lvm installer.simpleProvided installer.encryptedFSWithKeyfile installer.separateBoot installer.simpleUefiGrub installer.grub1 installer.separateBootFat installer.simpleUefiGrubClone installer.luksroot installer.simple installer.simpleUefiSystemdBoot |
81172e1
to
30d8d1d
Compare
@GrahamcOfBorg test installer-btrfsSimple installer-luksroot-format1 installer-simpleClone installer-swraid installer-btrfsSubvolDefault installer-luksroot-format2 installer-simpleLabels installer-zfsroot installer-btrfsSubvols installer-lvm installer-simpleProvided installer-encryptedFSWithKeyfile installer-separateBoot installer-simpleUefiGrub installer-grub1 installer-separateBootFat installer-simpleUefiGrubClone installer-luksroot installer-simple installer-simpleUefiSystemdBoot |
30d8d1d
to
942f0fd
Compare
@GrahamcOfBorg test installer |
Judging from hydra, |
Good for me as well. |
nixosTests.installer: Port installer and ZFS test to python (cherry picked from commit dd5f92f)
Motivation for this change
#72828
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)cc @flokli @worldofpeace