Merge pull request #2423 from kolyshkin/systemd-v2-pids-max

Fix setting some systemd limits, add more tests

libcontainer/cgroups/systemd/v1.go

@@ -4,6 +4,7 @@ package systemd
 
 import (
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"math"
 	"os"
@@ -89,7 +90,18 @@ func genV1ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error)
 	}
 
 	// cpu.cfs_quota_us and cpu.cfs_period_us are controlled by systemd.
-	if c.Resources.CpuQuota != 0 && c.Resources.CpuPeriod != 0 {
+	if c.Resources.CpuQuota != 0 || c.Resources.CpuPeriod != 0 {
+		if c.Resources.CpuQuota < -1 {
+			return nil, fmt.Errorf("Invalid CPU quota value: %d", c.Resources.CpuQuota)
+		}
+		if c.Resources.CpuQuota != -1 {
+			if c.Resources.CpuQuota == 0 || c.Resources.CpuPeriod == 0 {
+				return nil, errors.New("CPU quota and period should both be set")
+			}
+			if c.Resources.CpuPeriod < 0 {
+				return nil, fmt.Errorf("Invalid CPU period value: %d", c.Resources.CpuPeriod)
+			}
+		}
 		// corresponds to USEC_INFINITY in systemd
 		// if USEC_INFINITY is provided, CPUQuota is left unbound by systemd
 		// always setting a property value ensures we can apply a quota and remove it later
@@ -113,7 +125,7 @@ func genV1ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error)
 			newProp("BlockIOWeight", uint64(c.Resources.BlkioWeight)))
 	}
 
-	if c.Resources.PidsLimit > 0 {
+	if c.Resources.PidsLimit > 0 || c.Resources.PidsLimit == -1 {
 		properties = append(properties,
 			newProp("TasksAccounting", true),
 			newProp("TasksMax", uint64(c.Resources.PidsLimit)))

libcontainer/cgroups/systemd/v2.go

@@ -53,6 +53,10 @@ func genV2ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error)
 		properties = append(properties,
 			newProp("MemoryMax", uint64(c.Resources.Memory)))
 	}
+	if c.Resources.MemoryReservation != 0 {
+		properties = append(properties,
+			newProp("MemoryLow", uint64(c.Resources.MemoryReservation)))
+	}
 	// swap is set
 	if c.Resources.MemorySwap != 0 {
 		swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(c.Resources.MemorySwap, c.Resources.Memory)
@@ -88,7 +92,7 @@ func genV2ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error)
 			newProp("CPUQuotaPerSecUSec", cpuQuotaPerSecUSec))
 	}
 
-	if c.Resources.PidsLimit > 0 {
+	if c.Resources.PidsLimit > 0 || c.Resources.PidsLimit == -1 {
 		properties = append(properties,
 			newProp("TasksAccounting", true),
 			newProp("TasksMax", uint64(c.Resources.PidsLimit)))

tests/integration/helpers.bash

@@ -110,11 +110,12 @@ function init_cgroup_paths() {
 	test -n "$CGROUP_UNIFIED" && return
 
 	if [ -n "${RUNC_USE_SYSTEMD}" ] ; then
+		SD_UNIT_NAME="runc-cgroups-integration-test.scope"
 		if [ $(id -u) = "0" ]; then
-			REL_CGROUPS_PATH="/machine.slice/runc-cgroups-integration-test.scope"
+			REL_CGROUPS_PATH="/machine.slice/$SD_UNIT_NAME"
 			OCI_CGROUPS_PATH="machine.slice:runc-cgroups:integration-test"
 		else
-			REL_CGROUPS_PATH="/user.slice/user-$(id -u).slice/user@$(id -u).service/machine.slice/runc-cgroups-integration-test.scope"
+			REL_CGROUPS_PATH="/user.slice/user-$(id -u).slice/user@$(id -u).service/machine.slice/$SD_UNIT_NAME"
 			# OCI path doesn't contain "/user.slice/user-$(id -u).slice/user@$(id -u).service/" prefix
 			OCI_CGROUPS_PATH="machine.slice:runc-cgroups:integration-test"
 		fi
@@ -178,16 +179,15 @@ function check_cgroup_value() {
 
 # Helper to check a value in systemd.
 function check_systemd_value() {
-	unitname=$1
+	[ -z "${RUNC_USE_SYSTEMD}" ] && return
-	source=$2
+	source=$1
-	expected=$3
+	[ "$source" = "unsupported" ] && return
+	expected="$2"
+	user=""
+	[ $(id -u) != "0" ] && user="--user"
 
-	if [ $(id -u) = "0" ]; then
+	current=$(systemctl show $user --property $source $SD_UNIT_NAME | awk -F= '{print $2}')
-		current=$(systemctl show $unitname | grep $source)
+	echo "systemd $source: current $current !? $expected"
-	else
-		current=$(systemctl --user show $unitname | grep $source)
-	fi
-	echo "current" $current "!?" "$expected"
 	[ "$current" = "$expected" ]
 }
 

tests/integration/update.bats

@@ -63,21 +63,28 @@ EOF
     case $CGROUP_UNIFIED in
     no)
         MEM_LIMIT="memory.limit_in_bytes"
+        SD_MEM_LIMIT="MemoryLimit"
         MEM_RESERVE="memory.soft_limit_in_bytes"
+        SD_MEM_RESERVE="unsupported"
         MEM_SWAP="memory.memsw.limit_in_bytes"
+        SD_MEM_SWAP="unsupported"
         SYSTEM_MEM=$(cat "${CGROUP_MEMORY_BASE_PATH}/${MEM_LIMIT}")
         SYSTEM_MEM_SWAP=$(cat "${CGROUP_MEMORY_BASE_PATH}/$MEM_SWAP")
         ;;
     yes)
         MEM_LIMIT="memory.max"
+        SD_MEM_LIMIT="MemoryMax"
         MEM_RESERVE="memory.low"
+        SD_MEM_RESERVE="MemoryLow"
         MEM_SWAP="memory.swap.max"
+        SD_MEM_SWAP="MemorySwapMax"
         SYSTEM_MEM="max"
         SYSTEM_MEM_SWAP="max"
         # checking swap is currently disabled for v2
         #CGROUP_MEMORY=$CGROUP_PATH
         ;;
     esac
+    SD_UNLIMITED="infinity"
 
     # check that initial values were properly set
     check_cgroup_value "cpuset.cpus" 0
@@ -86,8 +93,13 @@ EOF
         skip "memory controller not available"
     fi
     check_cgroup_value $MEM_LIMIT 33554432
+    check_systemd_value $SD_MEM_LIMIT 33554432
+
     check_cgroup_value $MEM_RESERVE 25165824
+    check_systemd_value $SD_MEM_RESERVE 25165824
+
     check_cgroup_value "pids.max" 20
+    check_systemd_value "TasksMax" 20
 
     # update cpuset if supported (i.e. we're running on a multicore cpu)
     cpu_count=$(grep -c '^processor' /proc/cpuinfo)
@@ -101,29 +113,18 @@ EOF
     runc update test_update --memory 67108864
     [ "$status" -eq 0 ]
     check_cgroup_value $MEM_LIMIT 67108864
-    if [[ -n "${RUNC_USE_SYSTEMD}" ]] ; then
+    check_systemd_value $SD_MEM_LIMIT 67108864
-        if [ "$CGROUP_UNIFIED" != "yes" ]; then
-            check_systemd_value "runc-cgroups-integration-test.scope" "MemoryLimit=" "MemoryLimit=67108864"
-        else
-            check_systemd_value "runc-cgroups-integration-test.scope" "MemoryMax=" "MemoryMax=67108864"
-        fi
-    fi
 
     runc update test_update --memory 50M
     [ "$status" -eq 0 ]
     check_cgroup_value $MEM_LIMIT 52428800
-    if [[ -n "${RUNC_USE_SYSTEMD}" ]] ; then
+    check_systemd_value $SD_MEM_LIMIT 52428800
-        if [ "$CGROUP_UNIFIED" != "yes" ]; then
-            check_systemd_value "runc-cgroups-integration-test.scope" "MemoryLimit=" "MemoryLimit=52428800"
-        else
-            check_systemd_value "runc-cgroups-integration-test.scope" "MemoryMax=" "MemoryMax=52428800"
-        fi
-    fi
 
     # update memory soft limit
     runc update test_update --memory-reservation 33554432
     [ "$status" -eq 0 ]
     check_cgroup_value "$MEM_RESERVE" 33554432
+    check_systemd_value "$SD_MEM_RESERVE" 33554432
 
     # Run swap memory tests if swap is available
     if [ -f "$CGROUP_MEMORY/$MEM_SWAP" ]; then
@@ -131,11 +132,13 @@ EOF
         runc update test_update --memory-swap -1
         [ "$status" -eq 0 ]
         check_cgroup_value "$MEM_SWAP" $SYSTEM_MEM_SWAP
+        check_systemd_value "$SD_MEM_SWAP" $SD_UNLIMITED
 
         # update memory swap
         runc update test_update --memory-swap 96468992
         [ "$status" -eq 0 ]
         check_cgroup_value "$MEM_SWAP" 96468992
+        check_systemd_value "$SD_MEM_SWAP" 96468992
     fi
 
     # try to remove memory limit
@@ -144,19 +147,25 @@ EOF
 
     # check memory limit is gone
     check_cgroup_value $MEM_LIMIT $SYSTEM_MEM
+    check_systemd_value $SD_MEM_LIMIT $SD_UNLIMITED
 
     # check swap memory limited is gone
     if [ -f "$CGROUP_MEMORY/$MEM_SWAP" ]; then
         check_cgroup_value $MEM_SWAP $SYSTEM_MEM
+        check_systemd_value "$SD_MEM_SWAP" $SD_UNLIMITED
     fi
 
     # update pids limit
     runc update test_update --pids-limit 10
     [ "$status" -eq 0 ]
     check_cgroup_value "pids.max" 10
-    if [[ -n "${RUNC_USE_SYSTEMD}" ]] ; then
+    check_systemd_value "TasksMax" 10
-        check_systemd_value "runc-cgroups-integration-test.scope" "TasksMax=" "TasksMax=10"
+
-    fi
+    # unlimited
+    runc update test_update --pids-limit -1
+    [ "$status" -eq 0 ]
+    check_cgroup_value "pids.max" max
+    check_systemd_value "TasksMax" $SD_UNLIMITED
 
     # Revert to the test initial value via json on stdin
     runc update  -r - test_update <<EOF
@@ -178,9 +187,15 @@ EOF
 EOF
     [ "$status" -eq 0 ]
     check_cgroup_value "cpuset.cpus" 0
+
     check_cgroup_value $MEM_LIMIT 33554432
+    check_systemd_value $SD_MEM_LIMIT 33554432
+
     check_cgroup_value $MEM_RESERVE 25165824
+    check_systemd_value $SD_MEM_RESERVE 25165824
+
     check_cgroup_value "pids.max" 20
+    check_systemd_value "TasksMax" 20
 
     # redo all the changes at once
     runc update test_update \
@@ -189,8 +204,13 @@ EOF
         --pids-limit 10
     [ "$status" -eq 0 ]
     check_cgroup_value $MEM_LIMIT 67108864
+    check_systemd_value $SD_MEM_LIMIT 67108864
+
     check_cgroup_value $MEM_RESERVE 33554432
+    check_systemd_value $SD_MEM_RESERVE 33554432
+
     check_cgroup_value "pids.max" 10
+    check_systemd_value "TasksMax" 10
 
     # reset to initial test value via json file
     cat << EOF > $BATS_TMPDIR/runc-cgroups-integration-test.json
@@ -214,9 +234,15 @@ EOF
     runc update  -r $BATS_TMPDIR/runc-cgroups-integration-test.json test_update
     [ "$status" -eq 0 ]
     check_cgroup_value "cpuset.cpus" 0
+
     check_cgroup_value $MEM_LIMIT 33554432
+    check_systemd_value $SD_MEM_LIMIT 33554432
+
     check_cgroup_value $MEM_RESERVE 25165824
+    check_systemd_value $SD_MEM_RESERVE 25165824
+
     check_cgroup_value "pids.max" 20
+    check_systemd_value "TasksMax" 20
 }
 
 @test "update cgroup v1 cpu limits" {
@@ -230,22 +256,36 @@ EOF
     # check that initial values were properly set
     check_cgroup_value "cpu.cfs_period_us" 1000000
     check_cgroup_value "cpu.cfs_quota_us" 500000
+    check_systemd_value "CPUQuotaPerSecUSec" 500ms
+
     check_cgroup_value "cpu.shares" 100
+    check_systemd_value "CPUShares" 100
 
-    # update cpu-period
+    # systemd driver does not allow to update quota and period separately
-    runc update test_update --cpu-period 900000
+    if [ -z "$RUNC_USE_SYSTEMD" ]; then
-    [ "$status" -eq 0 ]
+        # update cpu period
-    check_cgroup_value "cpu.cfs_period_us" 900000
+        runc update test_update --cpu-period 900000
+        [ "$status" -eq 0 ]
+        check_cgroup_value "cpu.cfs_period_us" 900000
 
-    # update cpu-quota
+        # update cpu quota
-    runc update test_update --cpu-quota 600000
+        runc update test_update --cpu-quota 600000
-    [ "$status" -eq 0 ]
+        [ "$status" -eq 0 ]
-    check_cgroup_value "cpu.cfs_quota_us" 600000
+        check_cgroup_value "cpu.cfs_quota_us" 600000
+    else
+        # update cpu quota and period together
+        runc update test_update --cpu-period 900000 --cpu-quota 600000
+        [ "$status" -eq 0 ]
+        check_cgroup_value "cpu.cfs_period_us" 900000
+        check_cgroup_value "cpu.cfs_quota_us" 600000
+        check_systemd_value "CPUQuotaPerSecUSec" 670ms
+    fi
 
     # update cpu-shares
     runc update test_update --cpu-share 200
     [ "$status" -eq 0 ]
     check_cgroup_value "cpu.shares" 200
+    check_systemd_value "CPUShares" 200
 
     # Revert to the test initial value via json on stding
     runc update  -r - test_update <<EOF
@@ -260,7 +300,10 @@ EOF
     [ "$status" -eq 0 ]
     check_cgroup_value "cpu.cfs_period_us" 1000000
     check_cgroup_value "cpu.cfs_quota_us" 500000
+    check_systemd_value "CPUQuotaPerSecUSec" 500ms
+
     check_cgroup_value "cpu.shares" 100
+    check_systemd_value "CPUShares" 100
 
     # redo all the changes at once
     runc update test_update \
@@ -268,7 +311,10 @@ EOF
     [ "$status" -eq 0 ]
     check_cgroup_value "cpu.cfs_period_us" 900000
     check_cgroup_value "cpu.cfs_quota_us" 600000
+    check_systemd_value "CPUQuotaPerSecUSec" 670ms
+
     check_cgroup_value "cpu.shares" 200
+    check_systemd_value "CPUShares" 200
 
     # reset to initial test value via json file
     cat << EOF > $BATS_TMPDIR/runc-cgroups-integration-test.json
@@ -285,7 +331,10 @@ EOF
     [ "$status" -eq 0 ]
     check_cgroup_value "cpu.cfs_period_us" 1000000
     check_cgroup_value "cpu.cfs_quota_us" 500000
+    check_systemd_value "CPUQuotaPerSecUSec" 500ms
+
     check_cgroup_value "cpu.shares" 100
+    check_systemd_value "CPUShares" 100
 }
 
 @test "update rt period and runtime" {