runsc: update sentry CPU count on live cgroup CPU changes - #14277
Open
mayur-tolexo wants to merge 4 commits into
Open
runsc: update sentry CPU count on live cgroup CPU changes#14277mayur-tolexo wants to merge 4 commits into
mayur-tolexo wants to merge 4 commits into
Conversation
When a sandbox's CPU cgroup quota changes without a restart (e.g. a
Kubernetes in-place pod resize), the sentry's ApplicationCores stayed
frozen at the boot-time value, so anything that sizes its parallelism
to nproc (GOMAXPROCS, make -j, the JVM, nginx worker_processes auto)
couldn't use the added CPU without recreating the sandbox.
This adds Kernel.SetApplicationCores, wired through the existing runsc
update path (already invoked by containerd on in-place resize), so
ApplicationCores and GOMAXPROCS track the live cgroup quota. It's
grow-only: shrinking is rejected, since it would leave already-created
tasks' AllowedCPUMask referencing now-invalid CPUs. It's also rejected
on platforms with a fixed vCPU pool sized at boot (HasCPUNumbers(),
e.g. KVM).
Tested on a kind cluster (systrap platform, cgroup v2): boot a pod at
cpu=1, resize in place to cpu=8, no restart.
$ kubectl exec gv-resize-test -- nproc
2
$ kubectl patch pod gv-resize-test --subresource resize --patch \
'{"spec":{"containers":[{"name":"c","resources":{"requests":{"cpu":"8","memory":"128Mi"},"limits":{"cpu":"8","memory":"128Mi"}}}]}}'
$ kubectl exec gv-resize-test -- nproc
8
$ kubectl get pod gv-resize-test -o jsonpath='{.status.containerStatuses[0].restartCount}'
0
Sentry log:
I0822 03:09:37.398523 kernel.go:1962] ApplicationCores changed from 2 to 8
D0822 03:09:37.398545 gomaxprocs.go:76] Setting GOMAXPROCS to 8
An 8-thread busy-loop workload, measured via the pod's cgroup cpu.stat
over a 5s window after the resize, actually used all 8 cores rather
than staying capped at the boot-time 2:
delta_usec=40010083
effective_cores=8.00
A shrink is rejected without disrupting the sandbox:
W0822 03:10:20.658484 urpc.go:368] urpc: RPC call for method
containerManager.SetCPUCount failed: SetApplicationCores(2) would
shrink ApplicationCores from 8: not yet supported
$ kubectl get pod gv-resize-test
NAME READY STATUS RESTARTS AGE
gv-resize-test 1/1 Running 0 54s
Making ApplicationCores mutable at runtime (previous commit) exposed an
existing sched_setaffinity/SetCPUMask race: the syscall handler sizes
its mask from one ApplicationCores() read, then SetCPUMask re-reads it
and panicked on any size mismatch. CPUSetSize rounds to 64-CPU
boundaries, so a live grow crossing one between those two reads could
panic the whole sentry from an unprivileged guest syscall. SetCPUMask
now zero-extends a stale, smaller mask instead of panicking; a mask
larger than currently allowed is still a caller bug and still panics.
Covered by TestSetCPUMaskToleratesGrowthAcrossSizeBoundary and
TestSetCPUMaskRejectsOversizedMask, which reproduce the exact 32-to-96
core crossing deterministically rather than relying on real timing.
//pkg/sentry/kernel:kernel_test PASSED in 0.0s
Executed 1 out of 1 test: 1 test passes.
Making applicationCores an atomicbitops.Uint32 changed how it lands in
the statefile: atomicbitops.Uint32 is itself a savable struct, so the
field encoded as a nested struct where it used to be a plain uint.
That is a gratuitous statefile format break — a checkpoint written
before this series could no longer be read after it, for a field whose
saved value is just an integer.
Save it through saveApplicationCores/loadApplicationCores so the wire
type stays uint while the in-memory field remains atomic, matching how
Task's atomic.Pointer fields are already handled. Kernel.StateFields()
and the slot-5 encoding are then byte-identical to before the series.
Not state:"nosave" — the value must survive save/restore. Applications
size per-cpu structures from it, so a restore has to keep the
checkpointed count rather than adopt the new boot's --cpu-num.
$ diff schema_before.txt schema_after.txt # Kernel.StateFields()
(no output)
# before: Save(5, &k.applicationCores) // field was uint
# after: SaveValue(5, applicationCoresValue)
# LoadValue(5, new(uint), func(y any) { ... })
//pkg/sentry/kernel:kernel_test PASSED in 0.0s
//runsc/sandbox:sandbox_test PASSED in 0.0s
Executed 2 out of 2 tests: 2 tests pass.
SetCPUCount re-read the CPU quota from the sandbox cgroup. Under Kubernetes pod-level resources the kubelet writes the quota to the pod cgroup and leaves the sandbox's own at max, so the re-read found no quota, fell back to the cpuset, and the count never moved -- the update was silently dropped for exactly the case it exists to serve. The quota is already in hand: containerd passes it in the update request. Prefer it over the cgroup's, keeping the cgroup for the cpuset ceiling, which a quota change does not move. When the request carries no usable quota the cgroup decides as before, so a memory-only update is not read as "quota removed". Boot behaviour is unchanged. The quota-to-count arithmetic moves into cpuNumFromQuota, shared by boot and update so the two cannot derive different counts from the same quota. Verified on a kind cluster whose cgroup layout matches the failing case (pod cgroup 50000/100000, sandbox cgroup max). Against one running sandbox, "runsc update --cpu-quota=400000": before the fix nproc stays 2 after the fix nproc 2 -> 4, then 8 at --cpu-quota=800000 Note /proc/cpuinfo cannot show this: it is a static file generated once at boot, so sched_getaffinity is the observable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a sandbox's CPU quota changes without a restart (a Kubernetes in-place pod
resize, say), ApplicationCores stays at its boot-time value, so anything sizing
parallelism to nproc — GOMAXPROCS,
make -j, the JVM — can't use the added CPUwithout recreating the sandbox.
runsc updatealready writes the host cgroup(#12790); nothing told the sentry. Guest-visibility half of #14141.
Adds
Kernel.SetApplicationCores, wired through the existingrunsc updatepath, so ApplicationCores and GOMAXPROCS follow the live quota. Grow-only:
shrinking would leave existing tasks' AllowedCPUMask pointing at CPUs that no
longer exist, and is rejected. Also rejected on platforms whose vCPU pool is
fixed at boot (
HasCPUNumbers()); those need hot-add, which is separate work.The count is taken from the update request rather than re-read from the cgroup.
Container.Updatewrites the cgroup only for the root container, so re-readingit made
runsc update --cpu-quotaon a subcontainer a silent no-op. The cgroupstill supplies the cpuset ceiling.
This also exposed a sched_setaffinity crash: the syscall handler sizes its mask
from one
ApplicationCores()read andSetCPUMaskre-reads it, and CPUSetSizerounds to 64-CPU boundaries, so a grow crossing one between the two reads could
panic the sentry from an unprivileged guest syscall.
SetCPUMasknowzero-extends a stale, smaller mask.
kind, systrap, cgroup v2 — pod booted at cpu=1, resized in place to cpu=8:
An 8-thread busy loop then used all 8 cores (pod cgroup cpu.stat over 5s:
delta_usec=40010083,effective_cores=8.00) instead of staying capped at 2. Ashrink is refused without disrupting the sandbox. The affinity fix has two unit
tests;
Kernel.StateFields()diffs clean against master, with applicationCoresstill encoded as a uint via saveApplicationCores/loadApplicationCores — it has to
survive restore, so
state:"nosave"was not an option.Two notes for anyone reproducing. Kubernetes works with or without the
request-vs-cgroup change, because the kubelet writes the pod cgroup itself; the
last commit's message says otherwise and I can't amend it (fork ruleset).
/proc/cpuinfoand/sys/devices/system/cpu/onlineare generated once at bootand never move — read the per-cpu lines in
/proc/stator sched_getaffinity.