runtime: config: linux: add cgroups informations #199

runcom · 2015-09-23T13:01:15Z

add information to cgroup resources controllers with examples
add pids cgroup information and example
reflect kernel types

I'm submitting this as a first iteration of specifying structures here in the specs so any help/ideas are well accepted for subsequent PRs

Signed-off-by: Antonio Murdaca runcom@linux.com

wking · 2015-09-23T16:44:26Z

runtime-config-linux.md

+    * **`mount`** the container will have an isolated mount table
+    * **`ipc`** processes inside the container will only be able to communicate to other processes inside the same container via system level IPC
+    * **`uts`** the container will be able to have its own hostname and domain name
+    * **`user`** the container will be able to remap user and group IDs from the host to local users and groups within the container


I'd consider just echoing the “isolates” column of namespaces(7) for these entries. For example:

ipc isolates System V IPC, POSIX message queues

Ah, I see this is cut/pasted/reformatted from below. In that case, it's probably best to leave the text as it stands ;).

wking · 2015-09-23T17:44:07Z

I left a number of minor copy-edit suggestion, but overall this looks
great to me :). I'm especially fond of the linkable examples
(e.g. 1).

On Wed, Sep 23, 2015 at 06:01:17AM -0700, Antonio Murdaca wrote:

reflect kernel types

This would be good to get right ;). I haven't dug through the kernel
docs/code to double-check your changes (which are mostly int64 →
uint64), but they all sound reasonable to me. I'm suspicious about
int64 → int for Pids.Limit. Does the kernel really use an
implementation-specific size there?

runcom · 2015-09-23T17:48:53Z

I'm suspicious about
int64 → int for Pids.Limit. Does the kernel really use an
implementation-specific size there?

it's wrong, mistake

runcom · 2015-09-27T12:21:01Z

ping @vbatts @crosbymichael @mrunalp

mrunalp · 2015-09-29T17:05:07Z

runtime-config-linux.md

 ```json
    "disableOOMKiller": false
 ```

 #### Memory

+`memory` represents the cgroup subsystem `memory` and it's used to set limits on the container's memory use.


s/use/usage ?

mrunalp · 2015-10-01T20:24:59Z

LGTM

mrunalp · 2015-10-01T20:25:17Z

@vishh you want to take a look?

wking · 2015-10-01T20:28:54Z

runtime-config-linux.md

+
+* **`gid`** *(uint32, optional)* - gid of device owner
+
+**`fileMode`**, **`uid`** and **`gid`** are required if **`path`** is given, otherwise not allowed


This line needs at least a trailing period, and maybe an end like “and are otherwise not allowed.”

wking · 2015-10-01T20:38:37Z

There are a few ‘cfq’ → ‘CFS’ changes, but those aren't the same thing
[1,2]. I expect block limits should really be CFQ, while CPU limits
should be CFS.

mrunalp · 2015-10-01T20:41:39Z

Yeah, for block it should be CFQ.

runcom · 2015-10-01T20:42:14Z

Yeah, for block it should be CFQ.

sorry I mixed both valules, will fix

- add information to cgroup resources controllers with examples - add pids cgroup information and example - reflect kernel types Signed-off-by: Antonio Murdaca <runcom@linux.com>

runcom · 2015-10-02T21:37:11Z

Updated PTAL

wking · 2015-10-02T21:44:35Z

2ce2c86 looks good to me.

runcom · 2015-10-27T08:26:03Z

is there anything missing here?

vbatts · 2015-10-27T14:50:30Z

LGTM

runtime: config: linux: add cgroups informations

vishh · 2015-10-27T17:01:32Z

runtime_config_linux.go

 	// Total memory usage (memory + swap); set `-1' to disable swap
-	Swap int64 `json:"swap"`
+	Swap uint64 `json:"swap"`


Why uint64 here? How can we set it to -1? I'd much rather retain int64 since negative values have special meaning in the kernel for some of these fields.

I will send a PR to clean this up.

@vishh Pretty sure I went thou the kernel source and checked this can't be negative, but maybe I made some mistakes checking, thanks

definitely made a mistake based on the comment :)

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>

runcom force-pushed the rework-runtime-config-linux branch 2 times, most recently from 8e6ac74 to 7ca9693 Compare September 23, 2015 16:34

wking reviewed Sep 23, 2015
View reviewed changes

runcom force-pushed the rework-runtime-config-linux branch 3 times, most recently from 6f3600e to 7e4de1b Compare September 27, 2015 12:17

mrunalp reviewed Sep 29, 2015
View reviewed changes

runcom force-pushed the rework-runtime-config-linux branch from 7e4de1b to c7bc89c Compare September 29, 2015 17:10

wking reviewed Oct 1, 2015
View reviewed changes

runcom force-pushed the rework-runtime-config-linux branch from c7bc89c to 55f4b81 Compare October 2, 2015 09:57

runtime: config: linux: add cgroups information

2ce2c86

- add information to cgroup resources controllers with examples - add pids cgroup information and example - reflect kernel types Signed-off-by: Antonio Murdaca <runcom@linux.com>

runcom force-pushed the rework-runtime-config-linux branch from 55f4b81 to 2ce2c86 Compare October 2, 2015 21:36

vbatts added a commit that referenced this pull request Oct 27, 2015

Merge pull request #199 from runcom/rework-runtime-config-linux

ab4acc0

runtime: config: linux: add cgroups informations

vbatts merged commit ab4acc0 into opencontainers:master Oct 27, 2015

runcom deleted the rework-runtime-config-linux branch October 27, 2015 14:52

vishh reviewed Oct 27, 2015
View reviewed changes

wking mentioned this pull request Oct 27, 2015

Fix cgroups value types in the spec. #233

Merged

wking mentioned this pull request Jan 21, 2016

config: Single, unified config file #284

Merged

wking mentioned this pull request Jun 16, 2016

The default value of cfs_quota_us should be -1, which is negative #499

Closed

wking mentioned this pull request May 11, 2017

config-linux: Move 'disableOOMKiller' under 'memory'? #806

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: config: linux: add cgroups informations #199

runtime: config: linux: add cgroups informations #199

runcom commented Sep 23, 2015

wking Sep 23, 2015

wking Sep 23, 2015

wking commented Sep 23, 2015

runcom commented Sep 23, 2015

runcom commented Sep 27, 2015

mrunalp Sep 29, 2015

runcom Sep 29, 2015

mrunalp commented Oct 1, 2015

mrunalp commented Oct 1, 2015

wking Oct 1, 2015

wking commented Oct 1, 2015

mrunalp commented Oct 1, 2015

runcom commented Oct 1, 2015

runcom commented Oct 2, 2015

wking commented Oct 2, 2015 via email

runcom commented Oct 27, 2015

vbatts commented Oct 27, 2015

vishh Oct 27, 2015

vishh Oct 27, 2015

runcom Oct 27, 2015

runcom Oct 27, 2015


		* `gid` (uint32, optional) - gid of device owner

		`fileMode`, `uid` and `gid` are required if `path` is given, otherwise not allowed

runtime: config: linux: add cgroups informations #199

runtime: config: linux: add cgroups informations #199

Conversation

runcom commented Sep 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Sep 23, 2015

runcom commented Sep 23, 2015

runcom commented Sep 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrunalp commented Oct 1, 2015

mrunalp commented Oct 1, 2015

Choose a reason for hiding this comment

wking commented Oct 1, 2015

mrunalp commented Oct 1, 2015

runcom commented Oct 1, 2015

runcom commented Oct 2, 2015

wking commented Oct 2, 2015 via email

runcom commented Oct 27, 2015

vbatts commented Oct 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment