-
Notifications
You must be signed in to change notification settings - Fork 316
Conversation
@HuKeping I'm not sure but should be pretty easy to write test for this. At least in docker we have test for oom :) |
ok, test will come soon |
hi @LK4D4 I am not quite sure what you would like this test to do. I think the oom control test would be better in docker/docker? |
do we have plan how often will docker/docker merge libcontainer? it seems a lot of docker's PR depend on libcontainer |
@HuKeping I'm working on move docker to new libcontainer API. There are some problems :) After that we can update libcontainer as often as we need it in docker. |
@LK4D4 OK and by the way what do you want for this PR's test? I didn't see much to do. |
@HuKeping Actually I prefer to have good coverage for libcontainer too. There was pretty bad problems because of tests lack. But if you don't want write a test, then we can leave it for docker birthday party :) |
@LK4D4 yes, of cause I'd like to post a test for this , just don't know what should it be like. Is it to check the writeable of file memory.oom_control? |
2e8cab9
to
4332ffc
Compare
test added, ping @LK4D4 |
LGTM |
1 similar comment
LGTM |
cgroups: add support for oom control
This patch add support for diable OOM Killer. Signed-off-by: Hu Keping <hukeping@huawei.com>
Signed-off-by: Hu Keping <hukeping@huawei.com>
hi @LK4D4 , would you please help me to review the related PR on docker/docker |
It's backed by memory.oom_control, so this commit moves it in with the rest of the memory-controller config. Looking at the history, the initial request landing a setting for this in the Docker/OCI ecosystem seems to be [1], which added Cgroup.OomKillDisable. That commit was carried from libcontainer into runC [2] where it is now Resources.OomKillDisable [3]. From runC it was carried into this repo (with some renaming) in [4]. Subsequent early doc updates landed in [5,6]. In none of those can I find discussion about why the setting is not already under memory. I expect the reason is that the runC structures are flat, so "under memory" is not a thing there. But in this spec, resources has per-controller sub-properties. The fact that disableOOMKiller belonged to the memory controller may have been overlooked in [4] and never revisited until now. [1]: docker-archive/libcontainer#417 Subject: cgroups: add support for oom control [2]: opencontainers/runc@295c708 Subject: cgroups: add support for oom control [3]: /~https://github.com/opencontainers/runc/blob/v1.0.0-rc3/libcontainer/configs/cgroup_unix.go#L113-L114 [4]: opencontainers#51 Subject: Add Go types for specification [5]: opencontainers#137 Subject: Adding cgroups path to the Spec. [6]: opencontainers#199 Subject: runtime: config: linux: add cgroups informations Signed-off-by: W. Trevor King <wking@tremily.us>
Redo #412
This patch add support for diable OOM Killer.
Signed-off-by: Hu Keping hukeping@huawei.com