Merge pull request #2584 from kolyshkin/unified-support

libct/cgroups: support Cgroups.Resources.Unified

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/golang/protobuf v1.4.2
 	github.com/moby/sys/mountinfo v0.2.0
 	github.com/mrunalp/fileutils v0.5.0
-	github.com/opencontainers/runtime-spec v1.0.3-0.20200728170252-4d89ac9fbff6
+	github.com/opencontainers/runtime-spec v1.0.3-0.20200817204227-f9c09b4ea1df
 	github.com/opencontainers/selinux v1.6.0
 	github.com/pkg/errors v0.9.1
 	github.com/seccomp/libseccomp-golang v0.9.1

go.sum

@@ -36,8 +36,8 @@ github.com/moby/sys/mountinfo v0.2.0 h1:HgYSHMWCj8D7w7TE/cQJfWrY6W3TUxs3pwGFyC5q
 github.com/moby/sys/mountinfo v0.2.0/go.mod h1:rEr8tzG/lsIZHBtN/JjGG+LMYx9eXgW2JI+6q0qou+A=
 github.com/mrunalp/fileutils v0.5.0 h1:NKzVxiH7eSk+OQ4M+ZYW1K6h27RUV3MI6NUTsHhU6Z4=
 github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ=
-github.com/opencontainers/runtime-spec v1.0.3-0.20200728170252-4d89ac9fbff6 h1:NhsM2gc769rVWDqJvapK37r+7+CBXI8xHhnfnt8uQsg=
-github.com/opencontainers/runtime-spec v1.0.3-0.20200728170252-4d89ac9fbff6/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
+github.com/opencontainers/runtime-spec v1.0.3-0.20200817204227-f9c09b4ea1df h1:5AW5dMFSXVH4Mg3WYe4z7ui64bK8n66IoWK8i6T4QZ8=
+github.com/opencontainers/runtime-spec v1.0.3-0.20200817204227-f9c09b4ea1df/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/selinux v1.6.0 h1:+bIAS/Za3q5FTwWym4fTB0vObnfCf3G/NC7K6Jx62mY=
 github.com/opencontainers/selinux v1.6.0/go.mod h1:VVGKuOLlE7v4PJyT6h7mNWvq1rzqiriPsEqVhc+svHE=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

libcontainer/cgroups/fs/fs.go

@@ -218,7 +218,10 @@ func (m *manager) Apply(pid int) (err error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	var c = m.cgroups
+	c := m.cgroups
+	if c.Resources.Unified != nil {
+		return cgroups.ErrV1NoUnified
+	}
 
 	m.paths = make(map[string]string)
 	if c.Paths != nil {
@@ -309,6 +312,9 @@ func (m *manager) Set(container *configs.Config) error {
 	if m.cgroups != nil && m.cgroups.Paths != nil {
 		return nil
 	}
+	if container.Cgroups.Resources.Unified != nil {
+		return cgroups.ErrV1NoUnified
+	}
 
 	m.mu.Lock()
 	defer m.mu.Unlock()

libcontainer/cgroups/fs2/fs2.go

@@ -3,11 +3,14 @@
 package fs2
 
 import (
+	"fmt"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
+	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/pkg/errors"
 )
@@ -206,10 +209,40 @@ func (m *manager) Set(container *configs.Config) error {
 	if err := setFreezer(m.dirPath, container.Cgroups.Freezer); err != nil {
 		return err
 	}
+	if err := m.setUnified(container.Cgroups.Unified); err != nil {
+		return err
+	}
 	m.config = container.Cgroups
 	return nil
 }
 
+func (m *manager) setUnified(res map[string]string) error {
+	for k, v := range res {
+		if strings.Contains(k, "/") {
+			return fmt.Errorf("unified resource %q must be a file name (no slashes)", k)
+		}
+		if err := fscommon.WriteFile(m.dirPath, k, v); err != nil {
+			errC := errors.Cause(err)
+			// Check for both EPERM and ENOENT since O_CREAT is used by WriteFile.
+			if errors.Is(errC, os.ErrPermission) || errors.Is(errC, os.ErrNotExist) {
+				// Check if a controller is available,
+				// to give more specific error if not.
+				sk := strings.SplitN(k, ".", 2)
+				if len(sk) != 2 {
+					return fmt.Errorf("unified resource %q must be in the form CONTROLLER.PARAMETER", k)
+				}
+				c := sk[0]
+				if _, ok := m.controllers[c]; !ok && c != "cgroup" {
+					return fmt.Errorf("unified resource %q can't be set: controller %q not available", k, c)
+				}
+			}
+			return errors.Wrapf(err, "can't set unified resource %q", k)
+		}
+	}
+
+	return nil
+}
+
 func (m *manager) GetPaths() map[string]string {
 	paths := make(map[string]string, 1)
 	paths[""] = m.dirPath

libcontainer/cgroups/fscommon/fscommon.go

@@ -21,7 +21,7 @@ func WriteFile(dir, file, data string) error {
 		return err
 	}
 	if err := retryingWriteFile(path, []byte(data), 0700); err != nil {
-		return errors.Wrapf(err, "failed to write %q to %q", data, path)
+		return errors.Wrapf(err, "failed to write %q", data)
 	}
 	return nil
 }

libcontainer/cgroups/systemd/v1.go

@@ -101,6 +101,10 @@ func (m *legacyManager) Apply(pid int) error {
 		properties []systemdDbus.Property
 	)
 
+	if c.Resources.Unified != nil {
+		return cgroups.ErrV1NoUnified
+	}
+
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	if c.Paths != nil {
@@ -342,6 +346,9 @@ func (m *legacyManager) Set(container *configs.Config) error {
 	if m.cgroups.Paths != nil {
 		return nil
 	}
+	if container.Cgroups.Resources.Unified != nil {
+		return cgroups.ErrV1NoUnified
+	}
 	dbusConnection, err := getDbusConnection(false)
 	if err != nil {
 		return err

libcontainer/cgroups/v1_utils.go

@@ -23,7 +23,8 @@ const (
 )
 
 var (
-	errUnified = errors.New("not implemented for cgroup v2 unified hierarchy")
+	errUnified     = errors.New("not implemented for cgroup v2 unified hierarchy")
+	ErrV1NoUnified = errors.New("invalid configuration: cannot use unified on cgroup v1")
 )
 
 type NotFoundError struct {

libcontainer/configs/cgroup_linux.go

@@ -127,6 +127,9 @@ type Resources struct {
 	// CpuWeight sets a proportional bandwidth limit.
 	CpuWeight uint64 `json:"cpu_weight"`
 
+	// Unified is cgroupv2-only key-value map.
+	Unified map[string]string `json:"unified"`
+
 	// SkipDevices allows to skip configuring device permissions.
 	// Used by e.g. kubelet while creating a parent cgroup (kubepods)
 	// common for many containers.

libcontainer/integration/exec_test.go

@@ -705,6 +705,155 @@ func testRunWithKernelMemory(t *testing.T, systemd bool) {
 	}
 }
 
+func TestCgroupResourcesUnifiedErrorOnV1(t *testing.T) {
+	testCgroupResourcesUnifiedErrorOnV1(t, false)
+}
+
+func TestCgroupResourcesUnifiedErrorOnV1Systemd(t *testing.T) {
+	if !systemd.IsRunningSystemd() {
+		t.Skip("Systemd is unsupported")
+	}
+	testCgroupResourcesUnifiedErrorOnV1(t, true)
+}
+
+func testCgroupResourcesUnifiedErrorOnV1(t *testing.T, systemd bool) {
+	if testing.Short() {
+		return
+	}
+	if cgroups.IsCgroup2UnifiedMode() {
+		t.Skip("requires cgroup v1")
+	}
+	rootfs, err := newRootfs()
+	ok(t, err)
+	defer remove(rootfs)
+
+	config := newTemplateConfig(rootfs)
+	if systemd {
+		config.Cgroups.Parent = "system.slice"
+	}
+	config.Cgroups.Resources.Unified = map[string]string{
+		"memory.min": "10240",
+	}
+	_, _, err = runContainer(config, "", "true")
+	if !strings.Contains(err.Error(), cgroups.ErrV1NoUnified.Error()) {
+		t.Fatalf("expected error to contain %v, got %v", cgroups.ErrV1NoUnified, err)
+	}
+}
+
+func TestCgroupResourcesUnified(t *testing.T) {
+	testCgroupResourcesUnified(t, false)
+}
+
+func TestCgroupResourcesUnifiedSystemd(t *testing.T) {
+	if !systemd.IsRunningSystemd() {
+		t.Skip("Systemd is unsupported")
+	}
+	testCgroupResourcesUnified(t, true)
+}
+
+func testCgroupResourcesUnified(t *testing.T, systemd bool) {
+	if testing.Short() {
+		return
+	}
+	if !cgroups.IsCgroup2UnifiedMode() {
+		t.Skip("requires cgroup v2")
+	}
+	rootfs, err := newRootfs()
+	ok(t, err)
+	defer remove(rootfs)
+
+	config := newTemplateConfig(rootfs)
+	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"
+	}
+
+	testCases := []struct {
+		name     string
+		cfg      map[string]string
+		expError string
+		cmd      []string
+		exp      string
+	}{
+		{
+			name: "dummy",
+			cmd:  []string{"true"},
+			exp:  "",
+		},
+		{
+			name: "set memory.min",
+			cfg:  map[string]string{"memory.min": "131072"},
+			cmd:  []string{"cat", "/sys/fs/cgroup/memory.min"},
+			exp:  "131072\n",
+		},
+		{
+			name: "check memory.max",
+			cmd:  []string{"cat", "/sys/fs/cgroup/memory.max"},
+			exp:  strconv.Itoa(int(config.Cgroups.Resources.Memory)) + "\n",
+		},
+
+		{
+			name: "overwrite memory.max",
+			cfg:  map[string]string{"memory.max": "268435456"},
+			cmd:  []string{"cat", "/sys/fs/cgroup/memory.max"},
+			exp:  "268435456\n",
+		},
+		{
+			name:     "no such controller error",
+			cfg:      map[string]string{"privet.vsem": "vam"},
+			expError: "controller \"privet\" not available",
+		},
+		{
+			name:     "slash in key error",
+			cfg:      map[string]string{"bad/key": "val"},
+			expError: "must be a file name (no slashes)",
+		},
+		{
+			name:     "no dot in key error",
+			cfg:      map[string]string{"badkey": "val"},
+			expError: "must be in the form CONTROLLER.PARAMETER",
+		},
+		{
+			name:     "read-only parameter",
+			cfg:      map[string]string{"pids.current": "42"},
+			expError: "failed to write",
+		},
+	}
+
+	for _, tc := range testCases {
+		config.Cgroups.Resources.Unified = tc.cfg
+		buffers, ret, err := runContainer(config, "", tc.cmd...)
+		if tc.expError != "" {
+			if err == nil {
+				t.Errorf("case %q failed: expected error, got nil", tc.name)
+				continue
+			}
+			if !strings.Contains(err.Error(), tc.expError) {
+				t.Errorf("case %q failed: expected error to contain %q, got %q", tc.name, tc.expError, err)
+			}
+			continue
+		}
+		if err != nil {
+			t.Errorf("case %q failed: expected no error, got %v (command: %v, status: %d, stderr: %q)",
+				tc.name, err, tc.cmd, ret, buffers.Stderr.String())
+			continue
+		}
+		if tc.exp != "" {
+			out := buffers.Stdout.String()
+			if out != tc.exp {
+				t.Errorf("expected %q, got %q", tc.exp, out)
+			}
+		}
+	}
+}
+
 func TestContainerState(t *testing.T) {
 	if testing.Short() {
 		return

libcontainer/specconv/spec_linux.go

@@ -619,6 +619,13 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*configs.Device) (*confi
 					})
 				}
 			}
+			if len(r.Unified) > 0 {
+				// copy the map
+				c.Resources.Unified = make(map[string]string, len(r.Unified))
+				for k, v := range r.Unified {
+					c.Resources.Unified[k] = v
+				}
+			}
 		}
 	}
 

tests/integration/cgroups.bats

@@ -6,6 +6,8 @@ function teardown() {
     rm -f "$BATS_TMPDIR"/runc-cgroups-integration-test.json
     teardown_running_container test_cgroups_kmem
     teardown_running_container test_cgroups_permissions
+    teardown_running_container test_cgroups_group
+    teardown_running_container test_cgroups_unified
     teardown_busybox
 }
 
@@ -176,3 +178,64 @@ function setup() {
     [ "$status" -eq 0 ]
 #
 }
+
+@test "runc run (cgroup v1 + unified resources should fail)" {
+    requires root cgroups_v1
+
+    set_cgroups_path "$BUSYBOX_BUNDLE"
+    set_resources_limit "$BUSYBOX_BUNDLE"
+    update_config '.linux.resources.unified |= {"memory.min": "131072"}' "$BUSYBOX_BUNDLE"
+
+    runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_unified
+    [ "$status" -ne 0 ]
+    [[ "$output" == *'invalid configuration'* ]]
+}
+
+@test "runc run (cgroup v2 resources.unified only)" {
+    requires root cgroups_v2
+
+    set_cgroups_path "$BUSYBOX_BUNDLE"
+    update_config ' .linux.resources.unified |= {
+                      "memory.min":   "131072",
+                      "memory.low":   "524288",
+                      "memory.high": "5242880",
+                      "memory.max": "10485760",
+                      "pids.max": "99",
+                      "cpu.max": "10000 100000"
+                     }' "$BUSYBOX_BUNDLE"
+
+    runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_unified
+    [ "$status" -eq 0 ]
+
+    runc exec test_cgroups_unified sh -c 'cd /sys/fs/cgroup && grep . *.min *.max *.low *.high'
+    [ "$status" -eq 0 ]
+    echo "$output"
+
+    echo "$output" | grep -q '^memory.min:131072$'
+    echo "$output" | grep -q '^memory.low:524288$'
+    echo "$output" | grep -q '^memory.high:5242880$'
+    echo "$output" | grep -q '^memory.max:10485760$'
+    echo "$output" | grep -q '^pids.max:99$'
+    echo "$output" | grep -q '^cpu.max:10000 100000$'
+}
+@test "runc run (cgroup v2 resources.unified override)" {
+    requires root cgroups_v2
+
+    set_cgroups_path "$BUSYBOX_BUNDLE"
+    update_config ' .linux.resources.memory |= {"limit": 33554432}
+                  | .linux.resources.memorySwap |= {"limit": 33554432}
+                  | .linux.resources.unified |=
+                      {"memory.min": "131072", "memory.max": "10485760" }' \
+        "$BUSYBOX_BUNDLE"
+
+    runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_unified
+    [ "$status" -eq 0 ]
+
+    runc exec test_cgroups_unified cat /sys/fs/cgroup/memory.min
+    [ "$status" -eq 0 ]
+    [ "$output" = '131072' ]
+
+    runc exec test_cgroups_unified cat /sys/fs/cgroup/memory.max
+    [ "$status" -eq 0 ]
+    [ "$output" = '10485760' ]
+}

vendor/github.com/opencontainers/runtime-spec/specs-go/config.go

@@ -60,7 +60,7 @@ type Process struct {
 	SelinuxLabel string `json:"selinuxLabel,omitempty" platform:"linux"`
 }
 
-// LinuxCapabilities specifies the whitelist of capabilities that are kept for a process.
+// LinuxCapabilities specifies the list of allowed capabilities that are kept for a process.
 // http://man7.org/linux/man-pages/man7/capabilities.7.html
 type LinuxCapabilities struct {
 	// Bounding is the set of capabilities checked by the kernel.
@@ -354,7 +354,7 @@ type LinuxRdma struct {
 
 // LinuxResources has container runtime resource constraints
 type LinuxResources struct {
-	// Devices configures the device whitelist.
+	// Devices configures the device allowlist.
 	Devices []LinuxDeviceCgroup `json:"devices,omitempty"`
 	// Memory restriction configuration
 	Memory *LinuxMemory `json:"memory,omitempty"`
@@ -372,6 +372,8 @@ type LinuxResources struct {
 	// Limits are a set of key value pairs that define RDMA resource limits,
 	// where the key is device name and value is resource limits.
 	Rdma map[string]LinuxRdma `json:"rdma,omitempty"`
+	// Unified resources.
+	Unified map[string]string `json:"unified,omitempty"`
 }
 
 // LinuxDevice represents the mknod information for a Linux special device file
@@ -392,7 +394,8 @@ type LinuxDevice struct {
 	GID *uint32 `json:"gid,omitempty"`
 }
 
-// LinuxDeviceCgroup represents a device rule for the whitelist controller
+// LinuxDeviceCgroup represents a device rule for the devices specified to
+// the device controller
 type LinuxDeviceCgroup struct {
 	// Allow or deny
 	Allow bool `json:"allow"`
@@ -628,6 +631,7 @@ const (
 	ArchS390X       Arch = "SCMP_ARCH_S390X"
 	ArchPARISC      Arch = "SCMP_ARCH_PARISC"
 	ArchPARISC64    Arch = "SCMP_ARCH_PARISC64"
+	ArchRISCV64     Arch = "SCMP_ARCH_RISCV64"
 )
 
 // LinuxSeccompAction taken upon Seccomp rule match

vendor/modules.txt

@@ -38,7 +38,7 @@ github.com/moby/sys/mountinfo
 # github.com/mrunalp/fileutils v0.5.0
 ## explicit
 github.com/mrunalp/fileutils
-# github.com/opencontainers/runtime-spec v1.0.3-0.20200728170252-4d89ac9fbff6
+# github.com/opencontainers/runtime-spec v1.0.3-0.20200817204227-f9c09b4ea1df
 ## explicit
 github.com/opencontainers/runtime-spec/specs-go
 # github.com/opencontainers/selinux v1.6.0