Skip to content
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

ZTS: allow alternate work and output dir #17059

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

robn
Copy link
Member

@robn robn commented Feb 15, 2025

Motivation and Context

I realised that part of the reason my test rig takes about 15 hours for a full ZTS run is at least in part to do with most of the tests on file vdevs, on ext4, in a VM, on a zvol. Learned a few things, some patches coming soon etc. But the best thing is to just dig out a spare NVMe drive, direct attach it to the VM, and make ZTS use that for everything.

Turns out it's awkward to mount a separate drive at /var/tmp because a typical Linux wants to use that for some of its housekeeping and there's a mild boot ordering issue, so fine, I'll put it somewhere else. But then it turns out that some of the path handling through zfs-tests and test-runner and the tests themselves were a bit messed up, so it wasn't possible to get everything where I wanted it.

So this PR makes it so that if you tell the test runners to use a different path, you get (mostly) get what you pay for. And now all the test work goes to /zts on my test rig, and a full run takes 4 hours, which is very much more manageable.

Description

There's three separate changes here.

First, we make sure that the work dir (default /var/tmp) is threaded through properly. As before, it starts life as $FILEDIR in zfs-tests.sh, settable with -d. That becomes $TEST_BASE_DIR once it hits the test scripts. Some of those had /var/tmp hardcoded, and have been updated. There's also a small handful of setup things like the default vdevs, the constrained_path command dir and the generated runfile for single tests that were also hardcoded, and now go to $FILEDIR.

Next, we remove the explicit outputdir from the runfiles, and properly thread through test-runner's default of /var/tmp/test_results. This was buggy; adding the timestamp suffix was not done consistently for all the different sources of outputdir, so I've fixed that up. Not that we use it, but if the code is going to exist it should probably work right.

Finally, I've made the use of /tmp and $TMPDIR a lot more consistent. In many places, individual tests create work files (including large vdev backing files) using their best guess for a good temporary dir, but it was done very inconsistently and most often ended up with files in /tmp, which I wanted to avoid. So I've made sure $TMPDIR is set to $FILEDIR/$TEST_BASE_DIR, and updated uses of mktemp to use it (usually simplifying the call) and uses of $TMPDIR to use mktemp.

Taken together, this brings the vast majority of the work output under the path specified by the operator in zfs-tests.sh -d, and I'm much happier.

Worth noting that there's still a handful of outputs to /tmp, which I've left for now, mostly because I ran out of day and I'd rather get this posted than leave it lying around for weeks while I find time to finish it off properly.

How Has This Been Tested?

Two successful full ZTS runs completed pointed to an alternate work dir. The handful of tests that were modified for mktemp/$TMPDIR were run directly both on the default /var/tmp and an alternate dir, all passed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

robn added 7 commits February 15, 2025 13:29
The operator can override TEST_BASE_DIR by setting its source var
FILEDIR through zfs-tests.sh -d. There were a handful of cases where
this was not honoured.

By default FILEDIR (and so TEST_BASE_DIR) is /var/tmp, so there should
be no functional change if the operator does nothing.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
The default file vdevs, constrained binpath and temporary runfiles were
all explicitly places in /var/tmp. Instead, put them under FILEDIR,
which is set from -d and defaults to /var/tmp. TEST_BASE_DIR is also
initialised from FILEDIR, which means all data for the run will now end
up under the operator-specified data dir.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
The config file value overrides any set by the operator, making it quite
difficult to put the test output elsewhere. The default is
/var/tmp/test_results (via BASEDIR in test-runner) so this shouldn't
change anything for the default case.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
The default outputdir had a timestamp appended in TestRun.__init__, and
then the timestamp was unconditionally applied again after the runfile
had been loaded, assuming that an outputdir would be set in the runfile
too. If the runfile didn't have an outputdir, then the outputdir would
get a second timestamp appended.

Further, if test groups or individual tests themselves specificed an
outputdir, those would be set on their config, but would not get a
timestamp appended. It's not entirely clear if that's wrong or not, but
it is certainly not consistent with the rest.

To clean all this up, change things to append a timestamp to a received
outputdir (from arg or runfile) before setting it in any TestRun,
TestGroup or Test object.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
Many tests use mktemp to create temporary files and dirs, which will
usually put them in /tmp unless instructed otherwise. This had led to
many tests trying to give mktemp a useful temp path in ad-hoc ways, and
others just using it directly without knowing they're potentially
leaving stuff lying around.

So we set TMPDIR to FILEDIR, which makes the simplest uses of mktemp put
things in the wanted work dir.

Included here is a hack to get TMPDIR into the test. If a test has to be
run as a different user (most of them), it is run through sudo. ld.so
from glibc will not pass TMPDIR to a setuid program, so instead we
re-set TMPDIR after sudo before running the target command.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
In all cases, rely on mktemp itself to make the best decision about
where to place the file or directory. In all cases, that decision will
be $TMPDIR, which we have set globally.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
Most of these are trying to use TMPDIR to put their work files somewhere
sensible. Now that we've set up correctly, they can all just use mktemp
to do the job.

In a couple of places cleaning up temp files wasn't being done
correctly, which has been fixed.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <robn@despairlabs.com>
@@ -130,7 +130,7 @@ function test_posix_mode # base
}

# Sanity check on tmpfs first
tmpdir=$(TMPDIR=$TEST_BASE_DIR mktemp -d)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code makes it easier to clean up the temporary ZTS files after a failed run, since you'd know they'd be in a fixed location ($TEST_BASE_DIR aka /var/tmp). I would keep this as-is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous commit defaults TMPDIR to FILEDIR, which is TEST_BASE_DIR, making this specific line a no-op change for the default case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants