Port (some) unit tests to FreeBSD#7042
Merged
mxpv merged 6 commits intocontainerd:mainfrom Jun 10, 2022
Merged
Conversation
Calling link(2) with a symlink as the target will cause FreeBSD to attempt to create a new hard link pointing to the target of the symlink rather than a hardlink to the symlink itself. By contrast, Linux creates a hardlink to the symlink. Use linkat(2) instead, which accepts a flag controlling this behavior. If linkat(2) is called with AT_SYMLINK_FOLLOW, it will behave the same as link(2). If linkat(2) is called without AT_SYMLINK_FOLLOW, it will behave the same as Linux's link(2) instead. See FreeBSD's implementation of ln(1), which uses linkat(2) and controls this behavior by way of the -P and -L flags: https://github.com/freebsd/freebsd-src/blob/30031172534c22695ab7b26a9420bda7b20b0824/bin/ln/ln.c#L342-L343 Signed-off-by: Samuel Karp <[email protected]>
Signed-off-by: Samuel Karp <[email protected]>
Mount options are marked `json:omitempty`. An empty slice in the default object caused TestWithSpecFromFile to fail. Signed-off-by: Samuel Karp <[email protected]>
The TestPodAnnotationPassthroughContainerSpec test and the TestContainerAnnotationPassthroughContainerSpec test both depend on a platform-specific implementation of criService.containerSpec, which is unimplemented on FreeBSD. The TestSandboxContainerSpec depends on a platform-specific implementation oc criService.sandboxContainerSpec, which is unimplemented on FreeBSD. Signed-off-by: Samuel Karp <[email protected]>
Signed-off-by: Samuel Karp <[email protected]>
Different tar(1) implementations default to different input and output locations when none is specified. This can include tape devices like /dev/st0 (on Linux) or /dev/sa0 (on FreeBSD), but may be overridden by compilation options or environment variables. Using the f option with the special value of - instructs tar(1) to read from stdin and write to stdout instead of the default. Signed-off-by: Samuel Karp <[email protected]>
a7995d6 to
5560b62
Compare
kzys
reviewed
Jun 10, 2022
Comment on lines
+153
to
+154
| if runtime.GOOS != "freebsd" { | ||
| // On Linux, devices can't have major number 0. |
Member
There was a problem hiding this comment.
Nit: should it be GOOS == linux rather than != freebsd?
Member
Author
There was a problem hiding this comment.
I'm not sure. The file right now is built for !windows && !darwin and I know this assertion won't apply to FreeBSD, but I'm not sure about other Unix implementations (and whether anyone is actively looking at a Solaris/Illumos, OpenBSD, or NetBSD port of containerd). I'm happy to change this to GOOS == linux if you think that's more appropriate here.
Member
There was a problem hiding this comment.
I see. The condition and the comment below are not aligned, but I'm fine to keep that as is.
kzys
approved these changes
Jun 10, 2022
mxpv
approved these changes
Jun 10, 2022
This file contains hidden or 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
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 patch series ports a number of unit tests to FreeBSD and removes a few that test features that haven't been ported to FreeBSD yet (CRI-related).
I have not yet added a new Cirrus-CI task for these unit tests as not all tests yet pass in that environment. Notably there are differences in behavior between ZFS and UFS which have not yet been accounted for in the tests (I haven't dug into them yet), but may require changes in continuity to be resolved. When run in ZFS, all tests currently pass; when run in UFS (which is the default filesystem in use in Cirrus) there are a few failures. (You can see a sample of failures here if you're interested.)