Skip to content

Commit

Permalink
runc update: distinguish nil from zero
Browse files Browse the repository at this point in the history 
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 <akihiro.suda.cz@hco.ntt.co.jp>
  • Loading branch information
@AkihiroSuda
AkihiroSuda committed Feb 15, 2024
1 parent f48a27c commit c9ba576
Showing 1 changed file with 72 additions and 54 deletions.
126 changes: 72 additions & 54 deletions update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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},
{"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},
{"kernel-memory-tcp", &r.Memory.KernelTCP},
{"memory-reservation", &r.Memory.Reservation},
} {
if val := context.String(pair.opt); val != "" {
var v int64
Expand All @@ -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 {
// 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) {
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
Expand All @@ -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
Expand All @@ -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

Expand Down

0 comments on commit c9ba576

Please sign in to comment.