Skip to content

Port (some) unit tests to FreeBSD#7042

Merged
mxpv merged 6 commits intocontainerd:mainfrom
samuelkarp:freebsd-unit-tests
Jun 10, 2022
Merged

Port (some) unit tests to FreeBSD#7042
mxpv merged 6 commits intocontainerd:mainfrom
samuelkarp:freebsd-unit-tests

Conversation

@samuelkarp
Copy link
Copy Markdown
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]>
@samuelkarp samuelkarp force-pushed the freebsd-unit-tests branch from a7995d6 to 5560b62 Compare June 10, 2022 01:54
Comment thread oci/utils_unix_test.go
Comment on lines +153 to +154
if runtime.GOOS != "freebsd" {
// On Linux, devices can't have major number 0.
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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