Skip to content

Commit

Permalink
Merge pull request #3735 from kolyshkin/int-fix-flake
Browse files Browse the repository at this point in the history
libct/int: make TestFdLeaks more robust
  • Loading branch information
AkihiroSuda authored Mar 20, 2023
2 parents e67dc39 + f2e71b0 commit efad7a3
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 103 deletions.
119 changes: 30 additions & 89 deletions libcontainer/integration/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,7 @@ func testExecPS(t *testing.T, userns bool) {
}
config := newTemplateConfig(t, &tParam{userns: userns})

buffers, exitCode, err := runContainer(t, config, "ps", "-o", "pid,user,comm")
if err != nil {
t.Fatalf("%s: %s", buffers, err)
}
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "ps", "-o", "pid,user,comm")
lines := strings.Split(buffers.Stdout.String(), "\n")
if len(lines) < 2 {
t.Fatalf("more than one process running for output %q", buffers.Stdout.String())
Expand All @@ -67,12 +61,7 @@ func TestIPCPrivate(t *testing.T) {
ok(t, err)

config := newTemplateConfig(t, nil)
buffers, exitCode, err := runContainer(t, config, "readlink", "/proc/self/ns/ipc")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/ipc")

if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual == l {
t.Fatalf("ipc link should be private to the container but equals host %q %q", actual, l)
Expand All @@ -89,12 +78,7 @@ func TestIPCHost(t *testing.T) {

config := newTemplateConfig(t, nil)
config.Namespaces.Remove(configs.NEWIPC)
buffers, exitCode, err := runContainer(t, config, "readlink", "/proc/self/ns/ipc")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/ipc")

if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
t.Fatalf("ipc link not equal to host link %q %q", actual, l)
Expand All @@ -111,13 +95,7 @@ func TestIPCJoinPath(t *testing.T) {

config := newTemplateConfig(t, nil)
config.Namespaces.Add(configs.NEWIPC, "/proc/1/ns/ipc")

buffers, exitCode, err := runContainer(t, config, "readlink", "/proc/self/ns/ipc")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/ipc")

if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
t.Fatalf("ipc link not equal to host link %q %q", actual, l)
Expand Down Expand Up @@ -163,8 +141,7 @@ func testRlimit(t *testing.T, userns bool) {
Cur: 1024,
}))

out, _, err := runContainer(t, config, "/bin/sh", "-c", "ulimit -n")
ok(t, err)
out := runContainerOk(t, config, "/bin/sh", "-c", "ulimit -n")
if limit := strings.TrimSpace(out.Stdout.String()); limit != "1025" {
t.Fatalf("expected rlimit to be 1025, got %s", limit)
}
Expand Down Expand Up @@ -537,7 +514,7 @@ func testCpuShares(t *testing.T, systemd bool) {
config.Cgroups.Resources.CpuShares = 1

if _, _, err := runContainer(t, config, "ps"); err == nil {
t.Fatalf("runContainer should failed with invalid CpuShares")
t.Fatal("runContainer should fail with invalid CpuShares")
}
}

Expand All @@ -560,30 +537,20 @@ func testPids(t *testing.T, systemd bool) {
config := newTemplateConfig(t, &tParam{systemd: systemd})
config.Cgroups.Resources.PidsLimit = -1

// Running multiple processes.
_, ret, err := runContainer(t, config, "/bin/sh", "-c", "/bin/true | /bin/true | /bin/true | /bin/true")
ok(t, err)

if ret != 0 {
t.Fatalf("expected fork() to succeed with no pids limit")
}
// Running multiple processes, expecting it to succeed with no pids limit.
_ = runContainerOk(t, config, "/bin/sh", "-c", "/bin/true | /bin/true | /bin/true | /bin/true")

// Enforce a permissive limit. This needs to be fairly hand-wavey due to the
// issues with running Go binaries with pids restrictions (see below).
config.Cgroups.Resources.PidsLimit = 64
_, ret, err = runContainer(t, config, "/bin/sh", "-c", `
_ = runContainerOk(t, config, "/bin/sh", "-c", `
/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true`)
ok(t, err)

if ret != 0 {
t.Fatalf("expected fork() to succeed with permissive pids limit")
}

// Enforce a restrictive limit. 64 * /bin/true + 1 * shell should cause this
// to fail reliability.
// Enforce a restrictive limit. 64 * /bin/true + 1 * shell should cause
// this to fail reliably.
config.Cgroups.Resources.PidsLimit = 64
out, _, err := runContainer(t, config, "/bin/sh", "-c", `
/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
Expand Down Expand Up @@ -884,13 +851,8 @@ func TestMountCgroupRO(t *testing.T) {
return
}
config := newTemplateConfig(t, nil)
buffers, exitCode, err := runContainer(t, config, "mount")
if err != nil {
t.Fatalf("%s: %s", buffers, err)
}
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "mount")

mountInfo := buffers.Stdout.String()
lines := strings.Split(mountInfo, "\n")
for _, l := range lines {
Expand Down Expand Up @@ -931,13 +893,8 @@ func TestMountCgroupRW(t *testing.T) {
}
}

buffers, exitCode, err := runContainer(t, config, "mount")
if err != nil {
t.Fatalf("%s: %s", buffers, err)
}
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "mount")

mountInfo := buffers.Stdout.String()
lines := strings.Split(mountInfo, "\n")
for _, l := range lines {
Expand Down Expand Up @@ -1148,11 +1105,7 @@ func TestSTDIOPermissions(t *testing.T) {
}

config := newTemplateConfig(t, nil)
buffers, exitCode, err := runContainer(t, config, "sh", "-c", "echo hi > /dev/stderr")
ok(t, err)
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "sh", "-c", "echo hi > /dev/stderr")

if actual := strings.Trim(buffers.Stderr.String(), "\n"); actual != "hi" {
t.Fatalf("stderr should equal be equal %q %q", actual, "hi")
Expand Down Expand Up @@ -1395,12 +1348,7 @@ func TestPIDHost(t *testing.T) {

config := newTemplateConfig(t, nil)
config.Namespaces.Remove(configs.NEWPID)
buffers, exitCode, err := runContainer(t, config, "readlink", "/proc/self/ns/pid")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/pid")

if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
t.Fatalf("ipc link not equal to host link %q %q", actual, l)
Expand Down Expand Up @@ -1689,12 +1637,7 @@ func TestCGROUPPrivate(t *testing.T) {

config := newTemplateConfig(t, nil)
config.Namespaces.Add(configs.NEWCGROUP, "")
buffers, exitCode, err := runContainer(t, config, "readlink", "/proc/self/ns/cgroup")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/cgroup")

if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual == l {
t.Fatalf("cgroup link should be private to the container but equals host %q %q", actual, l)
Expand All @@ -1713,12 +1656,7 @@ func TestCGROUPHost(t *testing.T) {
ok(t, err)

config := newTemplateConfig(t, nil)
buffers, exitCode, err := runContainer(t, config, "readlink", "/proc/self/ns/cgroup")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/cgroup")

if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
t.Fatalf("cgroup link not equal to host link %q %q", actual, l)
Expand All @@ -1741,6 +1679,16 @@ func testFdLeaks(t *testing.T, systemd bool) {
return
}

config := newTemplateConfig(t, &tParam{systemd: systemd})
// Run a container once to exclude file descriptors that are only
// opened once during the process lifetime by the library and are
// never closed. Those are not considered leaks.
//
// Examples of this open-once file descriptors are:
// - /sys/fs/cgroup dirfd opened by prepareOpenat2 in libct/cgroups;
// - dbus connection opened by getConnection in libct/cgroups/systemd.
_ = runContainerOk(t, config, "true")

pfd, err := os.Open("/proc/self/fd")
ok(t, err)
defer pfd.Close()
Expand All @@ -1749,13 +1697,7 @@ func testFdLeaks(t *testing.T, systemd bool) {
_, err = pfd.Seek(0, 0)
ok(t, err)

config := newTemplateConfig(t, &tParam{systemd: systemd})
buffers, exitCode, err := runContainer(t, config, "true")
ok(t, err)

if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}
_ = runContainerOk(t, config, "true")

fds1, err := pfd.Readdirnames(0)
ok(t, err)
Expand All @@ -1766,7 +1708,6 @@ func testFdLeaks(t *testing.T, systemd bool) {
// Show the extra opened files.

excludedPaths := []string{
"/sys/fs/cgroup", // opened once, see prepareOpenat2
"anon_inode:bpf-prog", // FIXME: see https://github.com/opencontainers/runc/issues/2366#issuecomment-776411392
}

Expand Down
16 changes: 2 additions & 14 deletions libcontainer/integration/seccomp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,7 @@ func TestSeccompPermitWriteMultipleConditions(t *testing.T) {
},
}

buffers, exitCode, err := runContainer(t, config, "ls", "/")
if err != nil {
t.Fatalf("%s: %s", buffers, err)
}
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d buffers %s", exitCode, buffers)
}
buffers := runContainerOk(t, config, "ls", "/")
// We don't need to verify the actual thing printed
// Just that something was written to stdout
if len(buffers.Stdout.String()) == 0 {
Expand Down Expand Up @@ -375,13 +369,7 @@ func TestSeccompMultipleConditionSameArgDeniesStdout(t *testing.T) {
},
}

buffers, exitCode, err := runContainer(t, config, "ls", "/")
if err != nil {
t.Fatalf("%s: %s", buffers, err)
}
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d buffers %s", exitCode, buffers)
}
buffers := runContainerOk(t, config, "ls", "/")
// Verify that nothing was printed
if len(buffers.Stdout.String()) != 0 {
t.Fatalf("Something was written to stdout, write call succeeded!\n")
Expand Down
16 changes: 16 additions & 0 deletions libcontainer/integration/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,22 @@ func runContainer(t *testing.T, config *configs.Config, args ...string) (buffers
return
}

// runContainerOk is a wrapper for runContainer, simplifying its use for cases
// when the run is expected to succeed and return exit code of 0.
func runContainerOk(t *testing.T, config *configs.Config, args ...string) *stdBuffers {
buffers, exitCode, err := runContainer(t, config, args...)

t.Helper()
if err != nil {
t.Fatalf("%s: %s", buffers, err)
}
if exitCode != 0 {
t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr)
}

return buffers
}

func destroyContainer(container *libcontainer.Container) {
_ = container.Destroy()
}

0 comments on commit efad7a3

Please sign in to comment.