diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go index 5c00c0bb..cdf656b8 100644 --- a/libcontainer/cgroups/systemd/common.go +++ b/libcontainer/cgroups/systemd/common.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "os" + "strconv" "strings" "sync" "time" @@ -21,6 +22,10 @@ var ( connOnce sync.Once connDbus *systemdDbus.Conn connErr error + + versionOnce sync.Once + version int + versionErr error ) // NOTE: This function comes from package github.com/coreos/go-systemd/util @@ -351,21 +356,56 @@ func stopUnit(dbusConnection *systemdDbus.Conn, unitName string) error { return nil } -func addCpuQuota(properties *[]systemdDbus.Property, quota int64, period uint64) { +func systemdVersion(conn *systemdDbus.Conn) (int, error) { + versionOnce.Do(func() { + version = -1 + verStr, err := conn.GetManagerProperty("Version") + if err != nil { + versionErr = err + return + } + + // verStr is like "v245.4-1.fc32" (including quotes) + if !strings.HasPrefix(verStr, `"v`) { + versionErr = fmt.Errorf("can't parse version %s", verStr) + return + } + // remove `"v` prefix and everything after the first dot + ver, err := strconv.Atoi(strings.SplitN(verStr[2:], ".", 2)[0]) + if err != nil { + versionErr = err + } + version = ver + }) + + return version, versionErr +} + +func addCpuQuota(conn *systemdDbus.Conn, properties *[]systemdDbus.Property, quota int64, period uint64) { + if period != 0 { + // systemd only supports CPUQuotaPeriodUSec since v242 + sdVer, err := systemdVersion(conn) + if err != nil { + logrus.Warnf("systemdVersion: %s", err) + } else if sdVer >= 242 { + *properties = append(*properties, + newProp("CPUQuotaPeriodUSec", period)) + } + } if quota != 0 || period != 0 { // corresponds to USEC_INFINITY in systemd cpuQuotaPerSecUSec := uint64(math.MaxUint64) if quota > 0 { - // systemd converts CPUQuotaPerSecUSec (microseconds per CPU second) to CPUQuota - // (integer percentage of CPU) internally. This means that if a fractional percent of - // CPU is indicated by Resources.CpuQuota, we need to round up to the nearest - // 10ms (1% of a second) such that child cgroups can set the cpu.cfs_quota_us they expect. if period == 0 { // assume the default kernel value of 100000 us (100 ms), same for v1 and v2. // v1: https://www.kernel.org/doc/html/latest/scheduler/sched-bwc.html and // v2: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html period = 100000 } + // systemd converts CPUQuotaPerSecUSec (microseconds per CPU second) to CPUQuota + // (integer percentage of CPU) internally. This means that if a fractional percent of + // CPU is indicated by Resources.CpuQuota, we need to round up to the nearest + // 10ms (1% of a second) such that child cgroups can set the cpu.cfs_quota_us they expect. cpuQuotaPerSecUSec = uint64(quota*1000000) / period if cpuQuotaPerSecUSec%10000 != 0 { cpuQuotaPerSecUSec = ((cpuQuotaPerSecUSec / 10000) + 1) * 10000 diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index b3e91f30..c4d5e526 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -68,7 +68,7 @@ var legacySubsystems = subsystemSet{ &fs.NameGroup{GroupName: "name=systemd"}, } -func genV1ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error) { +func genV1ResourcesProperties(c *configs.Cgroup, conn *systemdDbus.Conn) ([]systemdDbus.Property, error) { var properties []systemdDbus.Property r := c.Resources @@ -88,7 +88,7 @@ func genV1ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error) newProp("CPUShares", r.CpuShares)) } - addCpuQuota(&properties, r.CpuQuota, r.CpuPeriod) + addCpuQuota(conn, &properties, r.CpuQuota, r.CpuPeriod) if r.BlkioWeight != 0 { properties = append(properties, @@ -165,7 +165,11 @@ func (m *legacyManager) Apply(pid int) error { properties = append(properties, newProp("DefaultDependencies", false)) - resourcesProperties, err := genV1ResourcesProperties(c) + dbusConnection, err := getDbusConnection(false) + if err != nil { + return err + } + resourcesProperties, err := genV1ResourcesProperties(c, dbusConnection) if err != nil { return err } @@ -180,10 +184,6 @@ func (m *legacyManager) Apply(pid int) error { } } - dbusConnection, err := getDbusConnection(false) - if err != nil { - return err - } if err := startUnit(dbusConnection, unitName, properties); err != nil { return err } @@ -368,7 +368,11 @@ func (m *legacyManager) Set(container *configs.Config) error { if m.cgroups.Paths != nil { return nil } - properties, err := genV1ResourcesProperties(container.Cgroups) + dbusConnection, err := getDbusConnection(false) + if err != nil { + return err + } + properties, err := genV1ResourcesProperties(container.Cgroups, dbusConnection) if err != nil { return err } @@ -393,11 +397,6 @@ func (m *legacyManager) Set(container *configs.Config) error { logrus.Infof("freeze container before SetUnitProperties failed: %v", err) } - dbusConnection, err := getDbusConnection(false) - if err != nil { - _ = m.Freeze(targetFreezerState) - return err - } if err := dbusConnection.SetUnitProperties(getUnitName(container.Cgroups), true, properties...); err != nil { _ = m.Freeze(targetFreezerState) return err diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index d11b8abb..c44d7734 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -34,7 +34,7 @@ func NewUnifiedManager(config *configs.Cgroup, path string, rootless bool) cgrou } } -func genV2ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error) { +func genV2ResourcesProperties(c *configs.Cgroup, conn *systemdDbus.Conn) ([]systemdDbus.Property, error) { var properties []systemdDbus.Property r := c.Resources @@ -72,7 +72,7 @@ func genV2ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error) newProp("CPUWeight", r.CpuWeight)) } - addCpuQuota(&properties, r.CpuQuota, r.CpuPeriod) + addCpuQuota(conn, &properties, r.CpuQuota, r.CpuPeriod) if r.PidsLimit > 0 || r.PidsLimit == -1 { properties = append(properties, @@ -136,17 +136,17 @@ func (m *unifiedManager) Apply(pid int) error { properties = append(properties, newProp("DefaultDependencies", false)) - resourcesProperties, err := genV2ResourcesProperties(c) + dbusConnection, err := getDbusConnection(m.rootless) + if err != nil { + return err + } + resourcesProperties, err := genV2ResourcesProperties(c, dbusConnection) if err != nil { return err } properties = append(properties, resourcesProperties...) properties = append(properties, c.SystemdProps...) - dbusConnection, err := getDbusConnection(m.rootless) - if err != nil { - return err - } if err := startUnit(dbusConnection, unitName, properties); err != nil { return errors.Wrapf(err, "error while starting unit %q with properties %+v", unitName, properties) } @@ -289,7 +289,11 @@ func (m *unifiedManager) GetStats() (*cgroups.Stats, error) { } func (m *unifiedManager) Set(container *configs.Config) error { - properties, err := genV2ResourcesProperties(m.cgroups) + dbusConnection, err := getDbusConnection(m.rootless) + if err != nil { + return err + } + properties, err := genV2ResourcesProperties(m.cgroups, dbusConnection) if err != nil { return err } @@ -314,11 +318,6 @@ func (m *unifiedManager) Set(container *configs.Config) error { logrus.Infof("freeze container before SetUnitProperties failed: %v", err) } - dbusConnection, err := getDbusConnection(m.rootless) - if err != nil { - _ = m.Freeze(targetFreezerState) - return err - } if err := dbusConnection.SetUnitProperties(getUnitName(m.cgroups), true, properties...); err != nil { _ = m.Freeze(targetFreezerState) return errors.Wrap(err, "error while setting unit properties") diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index 5db99320..7cf3580a 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -105,6 +105,16 @@ function runc_rootless_cgroup() { update_config '.linux.resources += {"memory":{},"cpu":{},"blockio":{},"pids":{}}' $bundle } +# Returns systemd version as a number (-1 if systemd is not enabled/supported). +function systemd_version() { + if [ -n "${RUNC_USE_SYSTEMD}" ]; then + systemctl --version | awk '/^systemd / {print $2; exit}' + return + fi + + echo "-1" +} + function init_cgroup_paths() { # init once test -n "$CGROUP_UNIFIED" && return @@ -180,15 +190,16 @@ function check_cgroup_value() { # Helper to check a value in systemd. function check_systemd_value() { [ -z "${RUNC_USE_SYSTEMD}" ] && return - source=$1 + local source=$1 [ "$source" = "unsupported" ] && return - expected="$2" - user="" + local expected="$2" + local expected2="$3" + local user="" [ $(id -u) != "0" ] && user="--user" current=$(systemctl show $user --property $source $SD_UNIT_NAME | awk -F= '{print $2}') - echo "systemd $source: current $current !? $expected" - [ "$current" = "$expected" ] + echo "systemd $source: current $current !? $expected $expected2" + [ "$current" = "$expected" ] || [ -n "$expected2" -a "$current" = "$expected2" ] } # Helper function to set a resources limit diff --git a/tests/integration/update.bats b/tests/integration/update.bats index 261ed99c..72902f4a 100644 --- a/tests/integration/update.bats +++ b/tests/integration/update.bats @@ -250,8 +250,18 @@ function check_cpu_quota() { check_cgroup_value "cpu.cfs_quota_us" $quota check_cgroup_value "cpu.cfs_period_us" $period fi - # systemd value is the same for v1 and v2 + # systemd values are the same for v1 and v2 check_systemd_value "CPUQuotaPerSecUSec" $sd_quota + + # CPUQuotaPeriodUSec requires systemd >= v242 + [ "$(systemd_version)" -lt 242 ] && return + + local sd_period=$(( period/1000 ))ms + [ "$sd_period" = "1000ms" ] && sd_period="1s" + local sd_infinity="" + # 100ms is the default value, and if not set, shown as infinity + [ "$sd_period" = "100ms" ] && sd_infinity="infinity" + check_systemd_value "CPUQuotaPeriodUSec" $sd_period $sd_infinity } function check_cpu_shares() {