runc update: implement memory.checkBeforeUpdate#3579
runc update: implement memory.checkBeforeUpdate#3579kolyshkin merged 1 commit intoopencontainers:mainfrom
Conversation
|
|
||
| func genV2ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]systemdDbus.Property, error) { | ||
| func genV2ResourcesProperties(dirPath string, r *configs.Resources, cm *dbusConnManager) ([]systemdDbus.Property, error) { | ||
| if err := fs2.CheckMemoryUsage(dirPath, r); err != nil { |
There was a problem hiding this comment.
We need this check before setting systemd properties, otherwise the container is OOM-killed and the systemd unit is removed before we get to fsMgr.Set()
862beb6 to
4b05d9a
Compare
|
The test case was failing: This is where we set memory limit too low without using Modified the test case to not check the error code from this run. |
|
Rebased; PTAL @opencontainers/runc-maintainers |
|
PTAL @opencontainers/runc-maintainers |
| } | ||
|
|
||
| @test "update memory vs CheckBeforeUpdate" { | ||
| requires cgroups_v2 |
There was a problem hiding this comment.
What should happen on cgroups_v1 ?
There was a problem hiding this comment.
This knob is for cgroup v2, it is ignored (not used) on v1. Since we're testing the knob here, and not the kernel cgroup behavior, the test requires cgroup v2.
I don't know how to write a test to ensure the knob is ignored. In general, runc ignores all the parameters it's not aware of, and cgroup v1 code is not aware of CheckBeforeUpdate.
This is aimed at solving the problem of cgroup v2 memory controller behavior which is not compatible with that of cgroup v1. In cgroup v1, if the new memory limit being set is lower than the current usage, setting the new limit fails. In cgroup v2, same operation succeeds, and the container is OOM killed. Introduce a new setting, memory.checkBeforeUpdate, and use it to mimic cgroup v1 behavior. Note that this is not 100% reliable because of TOCTOU, but this is the best we can do. Add some test cases. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is aimed at solving the problem of cgroup v2 memory controller
behavior which is not compatible with that of cgroup v1.
In cgroup v1, if the new memory limit being set is lower than the
current usage, setting the new limit fails.
In cgroup v2, same operation succeeds, and the container is OOM killed.
Introduce a new setting, memory.checkBeforeUpdate, and use it to mimic
cgroup v1 behavior.
Note that this is not 100% reliable because of TOCTOU, but this is the
best we can do.
Fixes: #3509
Currently a draft, pending opencontainers/runtime-spec#1158 merge.