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

Port (some) unit tests to FreeBSD #7042

Merged
merged 6 commits into from
Jun 10, 2022
Merged

Conversation

samuelkarp
Copy link
Member

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.)

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]>
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]>
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]>
Comment on lines +153 to +154
if runtime.GOOS != "freebsd" {
// On Linux, devices can't have major number 0.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should it be GOOS == linux rather than != freebsd?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I see. The condition and the comment below are not aligned, but I'm fine to keep that as is.

@mxpv mxpv merged commit e71ffdd into containerd:main Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants