From 917268485557d4e84c1b95bdfe489759a3c23f43 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Fri, 15 Dec 2023 17:07:51 +0900 Subject: [PATCH] runc update: distinguish nil from zero Prior to this commit, commands like `runc update --cpuset-cpus=1` were implying to set cpu burst to "0" (which does not mean "leave it as is"). This was failing when the kernel does not support cpu burst: `openat2 /sys/fs/cgroup/runc-cgroups-integration-test/test-cgroup-22167/cpu.max.burst: no such file or directory` Signed-off-by: Akihiro Suda --- update.go | 126 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 54 deletions(-) diff --git a/update.go b/update.go index 5bcc441a173..4fef85f71fb 100644 --- a/update.go +++ b/update.go @@ -147,30 +147,13 @@ other options are ignored. } r := specs.LinuxResources{ + // nil and u64Ptr(0) are not interchangeable Memory: &specs.LinuxMemory{ - Limit: i64Ptr(0), - Reservation: i64Ptr(0), - Swap: i64Ptr(0), - Kernel: i64Ptr(0), - KernelTCP: i64Ptr(0), - CheckBeforeUpdate: boolPtr(false), - }, - CPU: &specs.LinuxCPU{ - Shares: u64Ptr(0), - Quota: i64Ptr(0), - Burst: u64Ptr(0), - Period: u64Ptr(0), - RealtimeRuntime: i64Ptr(0), - RealtimePeriod: u64Ptr(0), - Cpus: "", - Mems: "", - }, - BlockIO: &specs.LinuxBlockIO{ - Weight: u16Ptr(0), - }, - Pids: &specs.LinuxPids{ - Limit: 0, + CheckBeforeUpdate: boolPtr(false), // constant }, + CPU: &specs.LinuxCPU{}, + BlockIO: &specs.LinuxBlockIO{}, + Pids: &specs.LinuxPids{}, } config := container.Config() @@ -214,45 +197,45 @@ other options are ignored. for _, pair := range []struct { opt string - dest *uint64 + dest **uint64 }{ - {"cpu-burst", r.CPU.Burst}, - {"cpu-period", r.CPU.Period}, - {"cpu-rt-period", r.CPU.RealtimePeriod}, - {"cpu-share", r.CPU.Shares}, + {"cpu-burst", &r.CPU.Burst}, + {"cpu-period", &r.CPU.Period}, + {"cpu-rt-period", &r.CPU.RealtimePeriod}, + {"cpu-share", &r.CPU.Shares}, } { if val := context.String(pair.opt); val != "" { - var err error - *pair.dest, err = strconv.ParseUint(val, 10, 64) + v, err := strconv.ParseUint(val, 10, 64) if err != nil { return fmt.Errorf("invalid value for %s: %w", pair.opt, err) } + *pair.dest = &v } } for _, pair := range []struct { opt string - dest *int64 + dest **int64 }{ - {"cpu-quota", r.CPU.Quota}, - {"cpu-rt-runtime", r.CPU.RealtimeRuntime}, + {"cpu-quota", &r.CPU.Quota}, + {"cpu-rt-runtime", &r.CPU.RealtimeRuntime}, } { if val := context.String(pair.opt); val != "" { - var err error - *pair.dest, err = strconv.ParseInt(val, 10, 64) + v, err := strconv.ParseInt(val, 10, 64) if err != nil { return fmt.Errorf("invalid value for %s: %w", pair.opt, err) } + *pair.dest = &v } } for _, pair := range []struct { opt string - dest *int64 + dest **int64 }{ - {"memory", r.Memory.Limit}, - {"memory-swap", r.Memory.Swap}, - {"kernel-memory", r.Memory.Kernel}, //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. - {"kernel-memory-tcp", r.Memory.KernelTCP}, - {"memory-reservation", r.Memory.Reservation}, + {"memory", &r.Memory.Limit}, + {"memory-swap", &r.Memory.Swap}, + {"kernel-memory", &r.Memory.Kernel}, //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. + {"kernel-memory-tcp", &r.Memory.KernelTCP}, + {"memory-reservation", &r.Memory.Reservation}, } { if val := context.String(pair.opt); val != "" { var v int64 @@ -265,19 +248,31 @@ other options are ignored. } else { v = -1 } - *pair.dest = v + *pair.dest = &v } } r.Pids.Limit = int64(context.Int("pids-limit")) } - if *r.Memory.Kernel != 0 || *r.Memory.KernelTCP != 0 { //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. + // Fix up values + if r.Memory.Limit != nil && *r.Memory.Limit == -1 && r.Memory.Swap == nil { + // To avoid error "unable to set swap limit without memory limit" + r.Memory.Swap = i64Ptr(0) + } + if r.CPU.Idle != nil && r.CPU.Shares == nil { + // To avoid error "failed to write \"4\": write /sys/fs/cgroup/runc-cgroups-integration-test/test-cgroup-7341/cpu.weight: invalid argument" + r.CPU.Shares = u64Ptr(0) + } + + if (r.Memory.Kernel != nil) || (r.Memory.KernelTCP != nil) { //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. logrus.Warn("Kernel memory settings are ignored and will be removed") } // Update the values - config.Cgroups.Resources.BlkioWeight = *r.BlockIO.Weight + if r.BlockIO.Weight != nil { + config.Cgroups.Resources.BlkioWeight = *r.BlockIO.Weight + } // Setting CPU quota and period independently does not make much sense, // but historically runc allowed it and this needs to be supported @@ -290,7 +285,16 @@ other options are ignored. // Here in update, previously set values are available from config. // If only one of {quota,period} is set and the other is not, leave // the unset parameter at the old value (don't overwrite config). - p, q := *r.CPU.Period, *r.CPU.Quota + var ( + p uint64 + q int64 + ) + if r.CPU.Period != nil { + p = *r.CPU.Period + } + if r.CPU.Quota != nil { + q = *r.CPU.Quota + } if (p == 0 && q == 0) || (p != 0 && q != 0) { // both values are either set or unset (0) config.Cgroups.Resources.CpuPeriod = p @@ -306,19 +310,33 @@ other options are ignored. } } - config.Cgroups.Resources.CpuBurst = r.CPU.Burst - config.Cgroups.Resources.CpuShares = *r.CPU.Shares - // CpuWeight is used for cgroupv2 and should be converted - config.Cgroups.Resources.CpuWeight = cgroups.ConvertCPUSharesToCgroupV2Value(*r.CPU.Shares) - config.Cgroups.Resources.CpuRtPeriod = *r.CPU.RealtimePeriod - config.Cgroups.Resources.CpuRtRuntime = *r.CPU.RealtimeRuntime + config.Cgroups.Resources.CpuBurst = r.CPU.Burst // can be nil + if r.CPU.Shares != nil { + config.Cgroups.Resources.CpuShares = *r.CPU.Shares + // CpuWeight is used for cgroupv2 and should be converted + config.Cgroups.Resources.CpuWeight = cgroups.ConvertCPUSharesToCgroupV2Value(*r.CPU.Shares) + } + if r.CPU.RealtimePeriod != nil { + config.Cgroups.Resources.CpuRtPeriod = *r.CPU.RealtimePeriod + } + if r.CPU.RealtimeRuntime != nil { + config.Cgroups.Resources.CpuRtRuntime = *r.CPU.RealtimeRuntime + } config.Cgroups.Resources.CpusetCpus = r.CPU.Cpus config.Cgroups.Resources.CpusetMems = r.CPU.Mems - config.Cgroups.Resources.Memory = *r.Memory.Limit + if r.Memory.Limit != nil { + config.Cgroups.Resources.Memory = *r.Memory.Limit + } config.Cgroups.Resources.CPUIdle = r.CPU.Idle - config.Cgroups.Resources.MemoryReservation = *r.Memory.Reservation - config.Cgroups.Resources.MemorySwap = *r.Memory.Swap - config.Cgroups.Resources.MemoryCheckBeforeUpdate = *r.Memory.CheckBeforeUpdate + if r.Memory.Reservation != nil { + config.Cgroups.Resources.MemoryReservation = *r.Memory.Reservation + } + if r.Memory.Swap != nil { + config.Cgroups.Resources.MemorySwap = *r.Memory.Swap + } + if r.Memory.CheckBeforeUpdate != nil { + config.Cgroups.Resources.MemoryCheckBeforeUpdate = *r.Memory.CheckBeforeUpdate + } config.Cgroups.Resources.PidsLimit = r.Pids.Limit config.Cgroups.Resources.Unified = r.Unified