From 73d93eeb0192d749b8e419b94743799c2823092e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 28 Sep 2020 16:55:21 -0700 Subject: [PATCH 1/5] libct/int: make newTemplateConfig argument a struct ...so we can add more fields later. This commit is mostly courtesy of sed -i 's/newTemplateConfig(rootfs)/newTemplateConfig(\&tParam{rootfs: rootfs})/g' Signed-off-by: Kir Kolyshkin --- libcontainer/integration/checkpoint_test.go | 2 +- libcontainer/integration/exec_test.go | 72 ++++++++++----------- libcontainer/integration/execin_test.go | 18 +++--- libcontainer/integration/seccomp_test.go | 16 ++--- libcontainer/integration/template_test.go | 8 ++- 5 files changed, 60 insertions(+), 56 deletions(-) diff --git a/libcontainer/integration/checkpoint_test.go b/libcontainer/integration/checkpoint_test.go index 28fdbdb0c..b22f48b9b 100644 --- a/libcontainer/integration/checkpoint_test.go +++ b/libcontainer/integration/checkpoint_test.go @@ -76,7 +76,7 @@ func testCheckpoint(t *testing.T, userns bool) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if userns { config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 9c8652336..935a6c2c3 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -40,7 +40,7 @@ func testExecPS(t *testing.T, userns bool) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if userns { config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} @@ -77,7 +77,7 @@ func TestIPCPrivate(t *testing.T) { l, err := os.Readlink("/proc/1/ns/ipc") ok(t, err) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) buffers, exitCode, err := runContainer(config, "", "readlink", "/proc/self/ns/ipc") ok(t, err) @@ -102,7 +102,7 @@ func TestIPCHost(t *testing.T) { l, err := os.Readlink("/proc/1/ns/ipc") ok(t, err) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Namespaces.Remove(configs.NEWIPC) buffers, exitCode, err := runContainer(config, "", "readlink", "/proc/self/ns/ipc") ok(t, err) @@ -128,7 +128,7 @@ func TestIPCJoinPath(t *testing.T) { l, err := os.Readlink("/proc/1/ns/ipc") ok(t, err) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Namespaces.Add(configs.NEWIPC, "/proc/1/ns/ipc") buffers, exitCode, err := runContainer(config, "", "readlink", "/proc/self/ns/ipc") @@ -152,7 +152,7 @@ func TestIPCBadPath(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Namespaces.Add(configs.NEWIPC, "/proc/1/ns/ipcc") _, _, err = runContainer(config, "", "true") @@ -182,7 +182,7 @@ func testRlimit(t *testing.T, userns bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if userns { config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} @@ -212,7 +212,7 @@ func TestEnter(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainerWithName("test", config) ok(t, err) @@ -299,7 +299,7 @@ func TestProcessEnv(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainerWithName("test", config) ok(t, err) @@ -347,7 +347,7 @@ func TestProcessEmptyCaps(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Capabilities = nil container, err := newContainerWithName("test", config) @@ -396,7 +396,7 @@ func TestProcessCaps(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainerWithName("test", config) ok(t, err) @@ -464,7 +464,7 @@ func TestAdditionalGroups(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainerWithName("test", config) ok(t, err) @@ -518,7 +518,7 @@ func testFreeze(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainerWithName("test", config) ok(t, err) defer container.Destroy() @@ -575,7 +575,7 @@ func testCpuShares(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if systemd { config.Cgroups.Parent = "system.slice" } @@ -607,7 +607,7 @@ func testPids(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if systemd { config.Cgroups.Parent = "system.slice" } @@ -693,7 +693,7 @@ func testRunWithKernelMemory(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if systemd { config.Cgroups.Parent = "system.slice" } @@ -727,7 +727,7 @@ func testCgroupResourcesUnifiedErrorOnV1(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if systemd { config.Cgroups.Parent = "system.slice" } @@ -762,7 +762,7 @@ func testCgroupResourcesUnified(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Cgroups.Resources.Memory = 536870912 // 512M config.Cgroups.Resources.MemorySwap = 536870912 // 512M, i.e. no swap config.Namespaces.Add(configs.NEWCGROUP, "") @@ -870,7 +870,7 @@ func TestContainerState(t *testing.T) { t.Fatal(err) } - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Namespaces = configs.Namespaces([]configs.Namespace{ {Type: configs.NEWNS}, {Type: configs.NEWUTS}, @@ -931,7 +931,7 @@ func TestPassExtraFiles(t *testing.T) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainerWithName("test", config) if err != nil { @@ -1006,7 +1006,7 @@ func TestMountCmds(t *testing.T) { } defer os.RemoveAll(tmpDir) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Mounts = append(config.Mounts, &configs.Mount{ Source: tmpDir, Destination: "/tmp", @@ -1063,7 +1063,7 @@ func TestSysctl(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Sysctl = map[string]string{ "kernel.shmmni": "8192", } @@ -1100,7 +1100,7 @@ func TestMountCgroupRO(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Mounts = append(config.Mounts, &configs.Mount{ Destination: "/sys/fs/cgroup", @@ -1149,7 +1149,7 @@ func TestMountCgroupRW(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Mounts = append(config.Mounts, &configs.Mount{ Destination: "/sys/fs/cgroup", @@ -1200,7 +1200,7 @@ func TestOomScoreAdj(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.OomScoreAdj = ptrInt(200) container, err := newContainerWithName("test", config) @@ -1242,7 +1242,7 @@ func TestHook(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) expectedBundle := bundle config.Labels = append(config.Labels, "bundle="+expectedBundle) @@ -1388,7 +1388,7 @@ func TestSTDIOPermissions(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) buffers, exitCode, err := runContainer(config, "", "sh", "-c", "echo hi > /dev/stderr") ok(t, err) if exitCode != 0 { @@ -1421,7 +1421,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.RootPropagation = unix.MS_SLAVE | unix.MS_REC @@ -1537,7 +1537,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.RootPropagation = unix.MS_PRIVATE // Bind mount a volume @@ -1646,7 +1646,7 @@ func TestPIDHost(t *testing.T) { l, err := os.Readlink("/proc/1/ns/pid") ok(t, err) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Namespaces.Remove(configs.NEWPID) buffers, exitCode, err := runContainer(config, "", "readlink", "/proc/self/ns/pid") ok(t, err) @@ -1669,7 +1669,7 @@ func TestInitJoinPID(t *testing.T) { defer remove(rootfs) // Execute a long-running container - container1, err := newContainer(newTemplateConfig(rootfs)) + container1, err := newContainer(newTemplateConfig(&tParam{rootfs: rootfs})) ok(t, err) defer container1.Destroy() @@ -1693,7 +1693,7 @@ func TestInitJoinPID(t *testing.T) { pidns1 := state1.NamespacePaths[configs.NEWPID] // Run a container inside the existing pidns but with different cgroups - config2 := newTemplateConfig(rootfs) + config2 := newTemplateConfig(&tParam{rootfs: rootfs}) config2.Namespaces.Add(configs.NEWPID, pidns1) config2.Cgroups.Path = "integration/test2" container2, err := newContainerWithName("testCT2", config2) @@ -1772,7 +1772,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) { defer remove(rootfs) // Execute a long-running container - config1 := newTemplateConfig(rootfs) + config1 := newTemplateConfig(&tParam{rootfs: rootfs}) config1.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config1.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config1.Namespaces = append(config1.Namespaces, configs.Namespace{Type: configs.NEWUSER}) @@ -1805,7 +1805,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) { ok(t, err) defer remove(rootfs2) - config2 := newTemplateConfig(rootfs2) + config2 := newTemplateConfig(&tParam{rootfs: rootfs2}) config2.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config2.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config2.Namespaces.Add(configs.NEWNET, netns1) @@ -1865,7 +1865,7 @@ func TestTmpfsCopyUp(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Mounts = append(config.Mounts, &configs.Mount{ Source: "tmpfs", @@ -1915,7 +1915,7 @@ func TestCGROUPPrivate(t *testing.T) { l, err := os.Readlink("/proc/1/ns/cgroup") ok(t, err) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Namespaces.Add(configs.NEWCGROUP, "") buffers, exitCode, err := runContainer(config, "", "readlink", "/proc/self/ns/cgroup") ok(t, err) @@ -1944,7 +1944,7 @@ func TestCGROUPHost(t *testing.T) { l, err := os.Readlink("/proc/1/ns/cgroup") ok(t, err) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) buffers, exitCode, err := runContainer(config, "", "readlink", "/proc/self/ns/cgroup") ok(t, err) diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 14f8a5964..4215f0b92 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -25,7 +25,7 @@ func TestExecIn(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainer(config) ok(t, err) defer container.Destroy() @@ -91,7 +91,7 @@ func testExecInRlimit(t *testing.T, userns bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) if userns { config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} @@ -152,7 +152,7 @@ func TestExecInAdditionalGroups(t *testing.T) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainer(config) ok(t, err) defer container.Destroy() @@ -209,7 +209,7 @@ func TestExecInError(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainer(config) ok(t, err) defer container.Destroy() @@ -262,7 +262,7 @@ func TestExecInTTY(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainer(config) ok(t, err) defer container.Destroy() @@ -359,7 +359,7 @@ func TestExecInEnvironment(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainer(config) ok(t, err) defer container.Destroy() @@ -421,7 +421,7 @@ func TestExecinPassExtraFiles(t *testing.T) { t.Fatal(err) } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) container, err := newContainer(config) if err != nil { t.Fatal(err) @@ -505,7 +505,7 @@ func TestExecInOomScoreAdj(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.OomScoreAdj = ptrInt(200) container, err := newContainer(config) ok(t, err) @@ -557,7 +557,7 @@ func TestExecInUserns(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) diff --git a/libcontainer/integration/seccomp_test.go b/libcontainer/integration/seccomp_test.go index 75319e7e8..b40a7a585 100644 --- a/libcontainer/integration/seccomp_test.go +++ b/libcontainer/integration/seccomp_test.go @@ -25,7 +25,7 @@ func TestSeccompDenyGetcwdWithErrno(t *testing.T) { errnoRet := uint(syscall.ESRCH) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -95,7 +95,7 @@ func TestSeccompDenyGetcwd(t *testing.T) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -164,7 +164,7 @@ func TestSeccompPermitWriteConditional(t *testing.T) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -226,7 +226,7 @@ func TestSeccompDenyWriteConditional(t *testing.T) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -304,7 +304,7 @@ func TestSeccompPermitWriteMultipleConditions(t *testing.T) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -359,7 +359,7 @@ func TestSeccompDenyWriteMultipleConditions(t *testing.T) { } defer remove(rootfs) - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -409,7 +409,7 @@ func TestSeccompMultipleConditionSameArgDeniesStdout(t *testing.T) { defer remove(rootfs) // Prevent writing to both stdout and stderr - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ @@ -457,7 +457,7 @@ func TestSeccompMultipleConditionSameArgDeniesStderr(t *testing.T) { defer remove(rootfs) // Prevent writing to both stdout and stderr - config := newTemplateConfig(rootfs) + config := newTemplateConfig(&tParam{rootfs: rootfs}) config.Seccomp = &configs.Seccomp{ DefaultAction: configs.Allow, Syscalls: []*configs.Syscall{ diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go index 702f918f1..af82a29f4 100644 --- a/libcontainer/integration/template_test.go +++ b/libcontainer/integration/template_test.go @@ -16,17 +16,21 @@ var standardEnvironment = []string{ const defaultMountFlags = unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NODEV +type tParam struct { + rootfs string +} + // newTemplateConfig returns a base template for running a container // // it uses a network strategy of just setting a loopback interface // and the default setup for devices -func newTemplateConfig(rootfs string) *configs.Config { +func newTemplateConfig(p *tParam) *configs.Config { var allowedDevices []*configs.DeviceRule for _, device := range specconv.AllowedDevices { allowedDevices = append(allowedDevices, &device.DeviceRule) } return &configs.Config{ - Rootfs: rootfs, + Rootfs: p.rootfs, Capabilities: &configs.Capabilities{ Bounding: []string{ "CAP_CHOWN", From 9135d99c947a144ba0ccbc0b0f90d8f7032d1d31 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 28 Sep 2020 17:15:44 -0700 Subject: [PATCH 2/5] libct/int/newTemplateConfig: add userns param It seems that a few tests add a cgroup mount in case userns is not set. Let's do it inside newTemplateConfig() for all tests. Signed-off-by: Kir Kolyshkin --- libcontainer/integration/checkpoint_test.go | 21 ++------ libcontainer/integration/exec_test.go | 60 ++++++++------------- libcontainer/integration/execin_test.go | 18 +++---- libcontainer/integration/template_test.go | 17 +++++- 4 files changed, 51 insertions(+), 65 deletions(-) diff --git a/libcontainer/integration/checkpoint_test.go b/libcontainer/integration/checkpoint_test.go index b22f48b9b..86b876664 100644 --- a/libcontainer/integration/checkpoint_test.go +++ b/libcontainer/integration/checkpoint_test.go @@ -11,9 +11,6 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer" - "github.com/opencontainers/runc/libcontainer/configs" - - "golang.org/x/sys/unix" ) func showFile(t *testing.T, fname string) error { @@ -76,20 +73,10 @@ func testCheckpoint(t *testing.T, userns bool) { } defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - - if userns { - config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) - } else { - config.Mounts = append(config.Mounts, &configs.Mount{ - Destination: "/sys/fs/cgroup", - Device: "cgroup", - Flags: defaultMountFlags | unix.MS_RDONLY, - }) - } - + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + userns: userns, + }) factory, err := libcontainer.New(root, libcontainer.Cgroupfs) if err != nil { diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 935a6c2c3..23ac876ca 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -40,12 +40,10 @@ func testExecPS(t *testing.T, userns bool) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if userns { - config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + userns: userns, + }) buffers, exitCode, err := runContainer(config, "", "ps", "-o", "pid,user,comm") if err != nil { @@ -182,12 +180,10 @@ func testRlimit(t *testing.T, userns bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if userns { - config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + userns: userns, + }) // ensure limit is lower than what the config requests to test that in a user namespace // the Setrlimit call happens early enough that we still have permissions to raise the limit. @@ -766,11 +762,6 @@ func testCgroupResourcesUnified(t *testing.T, systemd bool) { config.Cgroups.Resources.Memory = 536870912 // 512M config.Cgroups.Resources.MemorySwap = 536870912 // 512M, i.e. no swap config.Namespaces.Add(configs.NEWCGROUP, "") - config.Mounts = append(config.Mounts, &configs.Mount{ - Destination: "/sys/fs/cgroup", - Device: "cgroup", - Flags: defaultMountFlags | unix.MS_RDONLY, - }) if systemd { config.Cgroups.Parent = "system.slice" } @@ -1101,13 +1092,6 @@ func TestMountCgroupRO(t *testing.T) { ok(t, err) defer remove(rootfs) config := newTemplateConfig(&tParam{rootfs: rootfs}) - - config.Mounts = append(config.Mounts, &configs.Mount{ - Destination: "/sys/fs/cgroup", - Device: "cgroup", - Flags: defaultMountFlags | unix.MS_RDONLY, - }) - buffers, exitCode, err := runContainer(config, "", "mount") if err != nil { t.Fatalf("%s: %s", buffers, err) @@ -1150,12 +1134,13 @@ func TestMountCgroupRW(t *testing.T) { ok(t, err) defer remove(rootfs) config := newTemplateConfig(&tParam{rootfs: rootfs}) - - config.Mounts = append(config.Mounts, &configs.Mount{ - Destination: "/sys/fs/cgroup", - Device: "cgroup", - Flags: defaultMountFlags, - }) + // clear the RO flag from cgroup mount + for _, m := range config.Mounts { + if m.Device == "cgroup" { + m.Flags = defaultMountFlags + break + } + } buffers, exitCode, err := runContainer(config, "", "mount") if err != nil { @@ -1772,10 +1757,10 @@ func TestInitJoinNetworkAndUser(t *testing.T) { defer remove(rootfs) // Execute a long-running container - config1 := newTemplateConfig(&tParam{rootfs: rootfs}) - config1.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config1.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config1.Namespaces = append(config1.Namespaces, configs.Namespace{Type: configs.NEWUSER}) + config1 := newTemplateConfig(&tParam{ + rootfs: rootfs, + userns: true, + }) container1, err := newContainer(config1) ok(t, err) defer container1.Destroy() @@ -1805,9 +1790,10 @@ func TestInitJoinNetworkAndUser(t *testing.T) { ok(t, err) defer remove(rootfs2) - config2 := newTemplateConfig(&tParam{rootfs: rootfs2}) - config2.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config2.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} + config2 := newTemplateConfig(&tParam{ + rootfs: rootfs2, + userns: true, + }) config2.Namespaces.Add(configs.NEWNET, netns1) config2.Namespaces.Add(configs.NEWUSER, userns1) config2.Cgroups.Path = "integration/test2" diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 4215f0b92..583faa29f 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -91,12 +91,10 @@ func testExecInRlimit(t *testing.T, userns bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if userns { - config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + userns: userns, + }) container, err := newContainer(config) ok(t, err) @@ -557,10 +555,10 @@ func TestExecInUserns(t *testing.T) { rootfs, err := newRootfs() ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} - config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + userns: true, + }) container, err := newContainer(config) ok(t, err) defer container.Destroy() diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go index af82a29f4..ac7f1797a 100644 --- a/libcontainer/integration/template_test.go +++ b/libcontainer/integration/template_test.go @@ -18,6 +18,7 @@ const defaultMountFlags = unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NODEV type tParam struct { rootfs string + userns bool } // newTemplateConfig returns a base template for running a container @@ -29,7 +30,7 @@ func newTemplateConfig(p *tParam) *configs.Config { for _, device := range specconv.AllowedDevices { allowedDevices = append(allowedDevices, &device.DeviceRule) } - return &configs.Config{ + config := &configs.Config{ Rootfs: p.rootfs, Capabilities: &configs.Capabilities{ Bounding: []string{ @@ -195,4 +196,18 @@ func newTemplateConfig(p *tParam) *configs.Config { }, }, } + + if p.userns { + config.UidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} + config.GidMappings = []configs.IDMap{{HostID: 0, ContainerID: 0, Size: 1000}} + config.Namespaces = append(config.Namespaces, configs.Namespace{Type: configs.NEWUSER}) + } else { + config.Mounts = append(config.Mounts, &configs.Mount{ + Destination: "/sys/fs/cgroup", + Device: "cgroup", + Flags: defaultMountFlags | unix.MS_RDONLY, + }) + } + + return config } From fa47f95872ed265ff20a1edcc4b0ea0e5ee7fbcf Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 28 Sep 2020 18:43:19 -0700 Subject: [PATCH 3/5] libct/int/newTemplateConfig: add systemd support ... and properly set ScopePrefix, Name and Parent for a systemd case. Signed-off-by: Kir Kolyshkin --- libcontainer/integration/exec_test.go | 45 ++++++++++++----------- libcontainer/integration/template_test.go | 19 ++++++++-- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 23ac876ca..7e8bd014c 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -514,7 +514,10 @@ func testFreeze(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + systemd: systemd, + }) container, err := newContainerWithName("test", config) ok(t, err) defer container.Destroy() @@ -571,10 +574,10 @@ func testCpuShares(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if systemd { - config.Cgroups.Parent = "system.slice" - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + systemd: systemd, + }) config.Cgroups.Resources.CpuShares = 1 _, _, err = runContainer(config, "", "ps") @@ -603,10 +606,10 @@ func testPids(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if systemd { - config.Cgroups.Parent = "system.slice" - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + systemd: systemd, + }) config.Cgroups.Resources.PidsLimit = -1 // Running multiple processes. @@ -689,10 +692,10 @@ func testRunWithKernelMemory(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if systemd { - config.Cgroups.Parent = "system.slice" - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + systemd: systemd, + }) config.Cgroups.Resources.KernelMemory = 52428800 _, _, err = runContainer(config, "", "ps") @@ -723,10 +726,10 @@ func testCgroupResourcesUnifiedErrorOnV1(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) - if systemd { - config.Cgroups.Parent = "system.slice" - } + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + systemd: systemd, + }) config.Cgroups.Resources.Unified = map[string]string{ "memory.min": "10240", } @@ -758,13 +761,13 @@ func testCgroupResourcesUnified(t *testing.T, systemd bool) { ok(t, err) defer remove(rootfs) - config := newTemplateConfig(&tParam{rootfs: rootfs}) + config := newTemplateConfig(&tParam{ + rootfs: rootfs, + systemd: systemd, + }) config.Cgroups.Resources.Memory = 536870912 // 512M config.Cgroups.Resources.MemorySwap = 536870912 // 512M, i.e. no swap config.Namespaces.Add(configs.NEWCGROUP, "") - if systemd { - config.Cgroups.Parent = "system.slice" - } testCases := []struct { name string diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go index ac7f1797a..51a6b5361 100644 --- a/libcontainer/integration/template_test.go +++ b/libcontainer/integration/template_test.go @@ -1,6 +1,9 @@ package integration import ( + "math/rand" + "strconv" + "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/specconv" @@ -17,8 +20,9 @@ var standardEnvironment = []string{ const defaultMountFlags = unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NODEV type tParam struct { - rootfs string - userns bool + rootfs string + userns bool + systemd bool } // newTemplateConfig returns a base template for running a container @@ -122,7 +126,6 @@ func newTemplateConfig(p *tParam) *configs.Config { {Type: configs.NEWNET}, }), Cgroups: &configs.Cgroup{ - Path: "/sys/fs/cgroup/", Resources: &configs.Resources{ MemorySwappiness: nil, Devices: allowedDevices, @@ -209,5 +212,15 @@ func newTemplateConfig(p *tParam) *configs.Config { }) } + if p.systemd { + id := strconv.FormatUint(rand.Uint64(), 36) + config.Cgroups.Name = "test" + id + // do not change Parent (see newContainerWithName) + config.Cgroups.Parent = "system.slice" + config.Cgroups.ScopePrefix = "runc-test" + } else { + config.Cgroups.Path = "/test/integration" + } + return config } From c1bba720f71ff747d9684194fec041baeeb8d372 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 5 Oct 2020 20:06:49 -0700 Subject: [PATCH 4/5] libct/cg/systemd/v1: do not use c.Path Commit a1d5398afa91b0cb10 ("Respect container's cgroup path") added a cgroupPath argument to FindCgroupMountpoint to make runc/libcontainer work in a custom multitenant environment with multiple cgroup mount points. It also added passing c.Path as an argument to FindCgroupMountpoint for systemd (v1) controller. This is wrong, because 1. systemd controller do not use c.Path at all (and c.Path is never set by specconv) -- instead, it uses Name and Parent. 2. c.Path, if set, is not absolute -- it is relative to /sys/fs/cgroup -- but it is used as an absolute path here. Since c.Path is never set, the change did not result in any breakage, so this code sit quietly for some time and the issue might not have been discovered -- until we started running libcontainer/integration tests in a CentOS 7 VM, which resulted in a following weird error: > FAIL: TestPidsSystemd: utils_test.go:55: exec_test.go:630: unexpected error: container_linux.go:353: starting container process caused: process_linux.go:326: applying cgroup configuration for process caused: mountpoint for devices not found The error was "fixed" in commit f57bb2fe3dbe8 by changing the tests' cgroups Path to be "/sys/fs/cgroup/". This actually resulted in creation of cgroup directories like /sys/fs/cgroup/memory/sys/fs/cgroup, /sys/fs/cgroup/devices/sys/fs/cgroup and so on. The proper fix to the test case is implemented in the previous commit, which sets c.Name and c.Parent. This commit just removes the invalid use of c.Path, and tells the whole story. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v1.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 2d85fdefd..25a1f3ef9 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -267,7 +267,7 @@ func (m *legacyManager) joinCgroups(pid int) error { } func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) { - mountpoint, err := cgroups.FindCgroupMountpoint(c.Path, subsystem) + mountpoint, err := cgroups.FindCgroupMountpoint("", subsystem) if err != nil { return "", err } From f0fdde79d2e24db35d2272a9b3244332cab51fde Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 5 Oct 2020 20:41:53 -0700 Subject: [PATCH 5/5] libct/cg/systemd/v1: fix err check in enableKmem Commit 27d3dd3df35 ("don't fail when subsystem not mounted") added ignoring "not found" error to enableKmem, and as a result the function now tries to call Mkdir with an empty path, which results in a weird error message. For example, this is a failure from a libcontainer/integration test: > === RUN TestRunWithKernelMemorySystemd > exec_test.go:704: runContainer failed with kernel memory limit: container_linux.go:370: starting container process caused: process_linux.go:327: applying cgroup configuration for process caused: mkdir : no such file or directory I am not entirely sure if it is a good idea to silently ignore set limits, but at least let's fix the error handling. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v1.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 25a1f3ef9..2679b9993 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -406,7 +406,10 @@ func (m *legacyManager) Set(container *configs.Config) error { func enableKmem(c *configs.Cgroup) error { path, err := getSubsystemPath(c, "memory") - if err != nil && !cgroups.IsNotFound(err) { + if err != nil { + if cgroups.IsNotFound(err) { + return nil + } return err }