tests: functional: switch to rsync for directory diffs #12588
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 tasks
|
@cyphar the causes of the failures here is that I'd suggest pulling commit 06947ea in to this PR (or something like it) to resolve these failures. Locally the failing tests pass for me with this change applied. |
ghost
self-requested a review
Ah okay, I assumed that replay would recover the mtime as well as the ctime. That makes a lot of sense. I am a bit confused why rsync reports that there are different xattrs under FreeBSD but I'll apply the commit you mentioned (or try to figure out a slightly cleaner way of doing it -- I think rsync should have a flag that ignores mtimes). |
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
from
adfa05f
to
d2fb618
Compare
|
The failure is reproducible. listing the contents of each dir right before the rsync comparison in slog_replay_fs_001 is a bit more readable than the itemized changes for me. I notice the size differences, but I don't know why they're different. copy: |
behlendorf
reviewed
Sep 28, 2021
There was a problem hiding this comment.
I notice the size differences, but I don't know why they're different.
Interestingly after spot checking a few of the builders I only saw the size differences on FreeBSD. Most of this code is common so that's a bit surprising, I'm not sure what's going on there either.
or try to figure out a slightly cleaner way of doing it
I'm all for it! Although I couldn't find a way myself, it also looks like perhaps --no-times isn't quite sufficient based on the latest CI results.
12 tasks
|
@cyphar to move this forward we're going to need to somehow suppress the mtime/crtime warnings. Those are expected. The size warnings on FreeBSD for the directories are interesting but not harmful so let's update the test for now to ignore them as well. |
|
Agreed, the test change seems good and the failure it exposes can be investigated separately. |
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
from
d2fb618
to
648b512
Compare
|
Sorry, I was busy and only just got back to this now. I've rebased it and cleaned up the ignore_mtimes usage so it's more clear what's going on in the handful of places that use it. Based on some quick tests, different sized directories aren't detected by rsync at all so that shouldn't matter (likely because it can't change directory sizes so it's just ignored). The other issue I saw on FreeBSD was that there were xattr changes mentioned by rsync in the ZIL replays tests, and I couldn't quite figure out how to debug them (maybe I should spin up a FreeBSD VM...). |
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
3 times, most recently
from
0a164e8
to
377e5ac
Compare
|
@behlendorf @freqlabs We are now ignoring all timestamp errors (Linux tests still seem to fail but they seem unrelated since those tests don't use I will need to investigate this a bit more, but this seems to be something we shouldn't ignore? If you'd like me to ignore xattr errors specifically for this test, I can do that too. I'll spin up a FreeBSD VM to figure out what the difference is. |
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
from
377e5ac
to
e402f69
Compare
|
I just tried to reproduce the xattr issue in a FreeBSD 13-RELEASE VM, and I couldn't get it to work. I'm really not sure why this is failing within the tests -- the xattrs are actually identical when I try to reproduce the issue locally. This was done with the built-in ZFS on FreeBSD -- I'm not familiar enough with FreeBSD to compile a custom kernel module to try the latest version of OpenZFS. If someone else could take a look at this, I'd really appreciate it. |
|
Sure, I'll take a look. |
Perhaps this comment was for a previous run, but the latest CI results show the Linux bots failing on these tests which were modified to use |
|
@behlendorf Ah, I was looking at the Debian tests (which aren't failing in any of the There's also a separate issue with zfs_snapshot_009_pos... |
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
2 times, most recently
from
1806ada
to
1d90061
Compare
behlendorf
reviewed
Feb 23, 2022
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
3 times, most recently
from
d6591a3
to
16ac85e
Compare
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
2 times, most recently
from
4b1b40e
to
d3f12a2
Compare
|
@behlendorf After many many many test fixes, all the tests are passing now. 😸 |
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
from
d3f12a2
to
8758401
Compare
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. This fix was suggested by Brian Behlendorf. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
cyphar
force-pushed
the
zfstest-rsync-directory-diff
branch
from
8758401
to
9326492
Compare
behlendorf
approved these changes
Feb 28, 2022
There was a problem hiding this comment.
Looks good. Thanks for sorting out all of those additional test case fixes so we can get this integrated. You probably already saw, but the two FreeBSD failures are known unrelated issues so no need to worry about this.
FAIL cli_root/zfs_receive/receive-o-x_props_override (expected PASS)
FAIL largest_pool/largest_pool_001_pos (expected PASS)
behlendorf
added
Status: Accepted
nicman23
pushed a commit
to nicman23/zfs
that referenced
this pull request
Aug 22, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
nicman23
pushed a commit
to nicman23/zfs
that referenced
this pull request
Aug 22, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Aug 30, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Sep 1, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
andrewc12
pushed a commit
to andrewc12/openzfs
that referenced
this pull request
Sep 23, 2022
While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12588
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This has been split off from #12209 so that it can be reviewed separately and the ZFS test failures (which appear to be real bugs in ZFS or the test suite) can be resolved separately to the renameat2 patchset.
While "diff -r" is the most straightforward way of comparing directory
trees for differences, it has two major issues:
missed even if a test is written that exercises the buggy behaviour.
are always different, which means that a test using diff(1) on
special files will always fail (resulting in such tests not being
added).
rsync can be used in a very similar manner to diff (with the -ni flags),
but has the additional benefit of being able to detect and resolve many
more differences between directory trees. In addition, rsync has a
standard set of features and flags while diffs feature set depends on
whether you're using GNU or BSD binutils.
Signed-off-by: Aleksa Sarai cyphar@cyphar.com
Types of changes
Checklist:
Signed-off-by.