ZTS: init zfs test suite nixos tests (WIP) #110873

Mindavi · 2021-01-26T19:11:12Z

Motivation for this change

Using the official test suite is nice. Currently some random tests are enabled, the hardest part of this whole story seems to be: which tests should be enabled?

Note that running the 'sanity' tests already takes over 40 minutes, which is unacceptable. @adisbladis and me had a quick chat over on IRC (https://logs.nix.samueldr.com/nixos/2021-01-10, search for Mindavi and adisbladis), and we determined that ~10 mins is a good amount of time to shoot for (this is quite arbitrary, and based on the fact that ZFS is a fundamental part of the system).

There are some things I'd like feedback on:

Do we want to combine the zfs.nix and this one (zfs-tests.nix currently)?
Is there a way to conditionally not remove the test suite files? I've commented out the line and tried some things with a variable as function argument, but couldn't figure out how to combine override and overrideAttrs (they're both needed).
Is it acceptable that ZFS is rebuilt? This is currently done because the build scripts are changed (due to changes required to the test scripts).

All the currently enabled tests are passing, but (as can be seen in the test script) some of the tests fail. I'm not sure why, and the failing tests may be contributing to the long sanity test time as well.

This one in particular seems to be a culprit:

file_wait_event exceeded 120.289 seconds
ERROR: file_wait_event /var/tmp/zed/zed.debug.log sysevent.fs.zfs.scrub_start exited 1

It can be seen multiple times in a failure log.

I think the first step for this should be that we decide on where to put this and how to handle the ZFS conditional building / inclusion of the test suite. After this is merged, it's possible to look into what tests are best to run. This set will at least do something in the good direction. If you have suggestions, please let me know. I don't want to be bikeshedding about which ones to run exactly now though. I'll trust any good advice on this, as long as the runtime stays acceptable.

Things done

@adisbladis
@Atemu

Attached is a test log of some of the failing tests, with the data of the logfiles that the ZTS generates included.

zfs-tests-failing-tests-dump.txt

hmenke · 2021-01-26T19:29:39Z

It looks like there is a test set linux-fast upstream. Maybe that would be suitable?
https://github.com/openzfs/zfs/blob/f4f50a70488ebad3fa96c2fe4142924215733113/scripts/zfs-tests.sh#L338-L339

Mindavi · 2021-01-26T20:14:46Z

It does mention a linux-fast test, but it doesn't exist. A linux test does exist though, but I've started it an it has been running for 1830 seconds already now -- so that's too long.

pkgs/os-specific/linux/zfs/default.nix

stale · 2021-08-21T02:54:43Z

I marked this as stale due to inactivity. → More info

nixos/tests/zfs-tests.nix

pkgs/os-specific/linux/zfs/default.nix

nixos/tests/zfs-tests.nix

Atemu · 2021-08-23T18:37:32Z

pkgs/os-specific/linux/zfs/default.nix

@@ -17,6 +17,14 @@

 # for determining the latest compatible linuxPackages
 , linuxPackages_5_13
+
+# By default, do not include tests as they cause gcc to be in the closure


They're also quite large, multiple dozen MiB.

Atemu · 2021-08-23T18:39:17Z

pkgs/os-specific/linux/zfs/default.nix

+      '' + optionalString (buildUser && includeTests) ''
+        # TODO(ricsch): Is overriding with /run/current-system/sw/bin acceptable?
+        substituteInPlace scripts/zfs-tests.sh \
+          --replace '$STF_PATH/ksh' ${ksh}/bin/ksh \
+          --replace 'LOSETUP=' 'LOSETUP=${util-linux}/bin/losetup #' \
+          --replace 'export PATH=$STF_PATH' '#' \
+          --replace 'DMSETUP=' 'DMSETUP=${lvm2}/bin/dmsetup #' \
+          --replace '$STF_PATH/gzip' '${gzip}/bin/gzip' \
+          --replace '$STF_PATH/gunzip' '${gzip}/bin/gunzip' \
+          --replace '/sbin/fsck.ext4' '${e2fsprogs}/bin/fsck.ext4' \
+          --replace '/sbin/mkfs.ext4' '${e2fsprogs}/bin/mkfs.ext4' \
+          --replace '$STF_PATH/awk' '${gawk}/bin/awk' \
+          --replace 'SYSTEM_DIRS="/usr/local/bin /usr/local/sbin"' 'SYSTEM_DIRS="/run/current-system/sw/bin"'
+
+        substituteInPlace scripts/common.sh.in \
+          --replace 'export ZTS_DIR=' 'export ZTS_DIR=${placeholder "out"}/share/zfs #' \
+          --replace 'export SCRIPT_DIR=' 'export SCRIPT_DIR=${placeholder "out"}/share/zfs #' \
+          --replace 'export ZDB=' 'export ZDB=${placeholder "out"}/bin/zdb #' \
+          --replace 'export ZFS=' 'export ZFS=${placeholder "out"}/bin/zfs #' \
+          --replace 'export ZPOOL=' 'export ZPOOL=${placeholder "out"}/bin/zpool #' \
+          --replace 'export ZTEST=' 'export ZTEST=${placeholder "out"}/bin/ztest #' \
+          --replace 'export ZFS_SH=' 'export ZFS_SH=${placeholder "out"}/share/zfs/zfs.sh #'


Is there no better way to provide these dependencies? How do upstream testers declare these binaries? It'd probably be best to just get them from $PATH which is hermetic anyways.

Hmm, the PATH is being modified when these tests are run, which is slighly hocus-pocus to me.

Mindavi · 2021-08-23T18:40:33Z

@Atemu do you have a tip on how to run those tests in parallel on my machine? Is there some way to run them all instead of having to nix-build -A <testname> for each of them?

Atemu · 2021-08-24T13:23:27Z

Usually, you can build a set of drvs with nix-build but the tests evaluate to lambdas which nix-build is somehow able to eval to a drv? I don't understand this magic well enough.

An easy way of doing this is to generate a list instead of a set; just map mkTest tests.

Mindavi · 2021-10-07T19:20:16Z

I did some experiments with making multiple outputs and by doing that properly, there was no loop in the references anymore. I should look into that some time (this PR isn't high-priority for me, just a nice to have).

hmenke · 2021-11-11T12:31:03Z

It would be amazing if we had the full ZTS as a multiple output derivation. This would allow us to run the full ZTS as a single-shot test when bumping the ZFS version. With zfsUnstable there are often demands to “unlock” it for kernel version that have not been marked as compatible by upstream. If such a configuration passes the ZTS I would be much more comfortable approving such a merge, see also #145458

Mindavi · 2021-12-06T07:22:10Z

Rebased on top of latest master, but the deadman test now crashes the kernel and hangs indefinitely.

I think this is starting to be ready to go though. The tests are now put in a separate 'zfs_tests' output, to ensure they're properly separated.

Only thing that'd still be nice is if we could remove all hardcoded paths to the tools, but since the ZTS does a little dance for setting up the PATH that I don't fully understand, I'm not 100% sure how to approach that properly.

Mindavi · 2022-05-26T17:32:36Z

@ofborg test zfs

Mindavi · 2022-05-26T17:33:13Z

@ofborg build zfs

Mindavi · 2022-05-26T17:34:04Z

Hmm, so I should add it to passthru and to the toplevel tests file still, I think.

Atemu

Two things:

The test isn't discoverable in nixosTests
We need to be able to test a matrix of zfs and kernel versions.

hmenke · 2022-11-18T13:30:48Z

pkgs/os-specific/linux/zfs/default.nix

+      '' + optionalString (buildUser) ''
+        # TODO(ricsch): Is overriding with /run/current-system/sw/bin acceptable?
+        substituteInPlace scripts/zfs-tests.sh \
+          --replace '$STF_PATH/ksh' ${ksh}/bin/ksh \
+          --replace 'LOSETUP=' 'LOSETUP=${util-linux}/bin/losetup #' \
+          --replace 'export PATH=$STF_PATH' '#' \
+          --replace 'DMSETUP=' 'DMSETUP=${lvm2}/bin/dmsetup #' \
+          --replace '$STF_PATH/gzip' '${gzip}/bin/gzip' \
+          --replace '$STF_PATH/gunzip' '${gzip}/bin/gunzip' \
+          --replace '/sbin/fsck.ext4' '${e2fsprogs}/bin/fsck.ext4' \
+          --replace '/sbin/mkfs.ext4' '${e2fsprogs}/bin/mkfs.ext4' \
+          --replace '$STF_PATH/awk' '${gawk}/bin/awk' \
+          --replace 'SYSTEM_DIRS="/usr/local/bin /usr/local/sbin"' 'SYSTEM_DIRS="/run/current-system/sw/bin"'
+
+        substituteInPlace scripts/common.sh.in \
+          --replace 'export ZTS_DIR=' 'export ZTS_DIR=${placeholder "zfs_tests"}/share/zfs #' \
+          --replace 'export SCRIPT_DIR=' 'export SCRIPT_DIR=${placeholder "zfs_tests"}/share/zfs #' \
+          --replace 'export ZDB=' 'export ZDB=${placeholder "out"}/bin/zdb #' \
+          --replace 'export ZFS=' 'export ZFS=${placeholder "out"}/bin/zfs #' \
+          --replace 'export ZPOOL=' 'export ZPOOL=${placeholder "out"}/bin/zpool #' \
+          --replace 'export ZTEST=' 'export ZTEST=${placeholder "out"}/bin/ztest #' \
+          --replace 'export ZFS_SH=' 'export ZFS_SH=${placeholder "zfs_tests"}/share/zfs/zfs.sh #'
+
+        # Fix ksh paths in test suite.
+        # patchShebangs doesn't work due to the scripts not being executable.
+        # It doesn't seem logical to make them executable either.
+        find -name "*.ksh" -exec sed -i 's,/bin/ksh,${ksh}/bin/ksh,' {} \;
+
+        # Patching maybe required for more binaries
+        # Maybe FHS environment would be better?


Oh boy, look at the time! It's time to resurrect this PR yet again :)

I think these scripts could use makeWrapper instead of awkwardly patching them. Most variables use the pattern LOSETUP=${LOSETUP:-/sbin/losetup}, so they can be overridden using makeWrapper ... --set LOSETUP "${util-linux}/bin/losetup".

Also what's up with all the ksh stuff? I've read through zfs-tests.sh and to me it looks like ksh is only used on FreeBSD.

I agree! Hopefully I can find some time soon to maybe clean this up :). Of course, feel free to contribute if you feel like it.

Mindavi · 2022-11-22T19:27:36Z

I rebased this on top of master, didn't change anything else for now (other than solve a minor merge conflict). Seems to be still working in this form.

Mindavi · 2024-03-25T20:26:05Z

I'm not really interested in getting this working anymore. I'm not that big of a ZFS user and I did this at some point to scratch an itch. It seems like the tests are now broken and there were some features I couldn't figure out how to implement (test all zfs versions). Hopefully someone can pick up from where I left, if someone's still interested in this.

ofborg bot added the 6.topic: nixos label Jan 26, 2021

ofborg bot requested review from wizeman, fpletz, jonringer, Mic92, globin, hmenke and jcumming January 26, 2021 19:20

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Jan 26, 2021

dotlambda reviewed Jan 26, 2021

View reviewed changes

pkgs/os-specific/linux/zfs/default.nix Outdated Show resolved Hide resolved

Mindavi mentioned this pull request Feb 13, 2021

ZTS: ZFS Test Suite #108649

Open

Mindavi force-pushed the feature/zfs-tests branch from ef215be to a6dfc11 Compare February 20, 2021 07:39

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 21, 2021

Mindavi force-pushed the feature/zfs-tests branch from a6dfc11 to 365a4df Compare August 21, 2021 08:03

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 21, 2021

Mindavi force-pushed the feature/zfs-tests branch from 365a4df to fe8bc20 Compare August 21, 2021 08:04

ofborg bot added the 10.rebuild-darwin: 1 label Aug 21, 2021

Atemu requested changes Aug 22, 2021

View reviewed changes

Atemu reviewed Aug 23, 2021

View reviewed changes

ofborg bot added the 2.status: merge conflict label Dec 4, 2021

Mindavi force-pushed the feature/zfs-tests branch from 5552c0b to b0f2219 Compare December 4, 2021 19:43

ofborg bot removed the 2.status: merge conflict label Dec 4, 2021

Mindavi force-pushed the feature/zfs-tests branch from b0f2219 to 2058d26 Compare May 9, 2022 06:17

ofborg bot removed the 10.rebuild-darwin: 1 label May 9, 2022

Mindavi marked this pull request as ready for review May 26, 2022 17:31

Atemu requested changes May 27, 2022

View reviewed changes

hmenke reviewed Nov 18, 2022

View reviewed changes

Mindavi force-pushed the feature/zfs-tests branch from 17a6018 to 47314aa Compare November 22, 2022 19:26

ofborg bot requested a review from hmenke November 22, 2022 19:39

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label May 21, 2023

wegank added the 2.status: merge conflict label Mar 20, 2024

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

wegank marked this pull request as draft March 20, 2024 22:53

Mindavi force-pushed the feature/zfs-tests branch 2 times, most recently from d0483ba to 496808d Compare March 25, 2024 19:58

nixos/zfs-tests: ZTS: init zfs test suite nixos tests

cb04d13

ofborg bot removed the 2.status: merge conflict label Mar 25, 2024

nixos/zfs-tests: zts: add some debug logging and enable some more tests

f82ce38

Mindavi force-pushed the feature/zfs-tests branch from 496808d to f82ce38 Compare March 25, 2024 20:25

Mindavi closed this Mar 25, 2024

ofborg bot requested review from amarshall, RaitoBezarius and adamcstephens March 25, 2024 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZTS: init zfs test suite nixos tests (WIP) #110873

ZTS: init zfs test suite nixos tests (WIP) #110873

Mindavi commented Jan 26, 2021

hmenke commented Jan 26, 2021

Mindavi commented Jan 26, 2021

stale bot commented Aug 21, 2021

Atemu Aug 23, 2021

Atemu Aug 23, 2021

Mindavi Oct 25, 2021

Mindavi commented Aug 23, 2021

Atemu commented Aug 24, 2021

Mindavi commented Oct 7, 2021

hmenke commented Nov 11, 2021

Mindavi commented Dec 6, 2021

Mindavi commented May 26, 2022

Mindavi commented May 26, 2022

Mindavi commented May 26, 2022

Atemu left a comment

hmenke Nov 18, 2022

Mindavi Nov 21, 2022

Mindavi commented Nov 22, 2022 •

edited

Mindavi commented Mar 25, 2024

ZTS: init zfs test suite nixos tests (WIP) #110873

ZTS: init zfs test suite nixos tests (WIP) #110873

Conversation

Mindavi commented Jan 26, 2021

Motivation for this change

Things done

hmenke commented Jan 26, 2021

Mindavi commented Jan 26, 2021

stale bot commented Aug 21, 2021

Atemu Aug 23, 2021

Choose a reason for hiding this comment

Atemu Aug 23, 2021

Choose a reason for hiding this comment

Mindavi Oct 25, 2021

Choose a reason for hiding this comment

Mindavi commented Aug 23, 2021

Atemu commented Aug 24, 2021

Mindavi commented Oct 7, 2021

hmenke commented Nov 11, 2021

Mindavi commented Dec 6, 2021

Mindavi commented May 26, 2022

Mindavi commented May 26, 2022

Mindavi commented May 26, 2022

Atemu left a comment

Choose a reason for hiding this comment

hmenke Nov 18, 2022

Choose a reason for hiding this comment

Mindavi Nov 21, 2022

Choose a reason for hiding this comment

Mindavi commented Nov 22, 2022 • edited

Mindavi commented Mar 25, 2024

Mindavi commented Nov 22, 2022 •

edited