*: add support for cgroup namespace #397

cyphar · 2016-04-25T15:14:22Z

The cgroup namespace is a new kernel feature available in 4.6+ that
allows a container to isolate its cgroup hierarchy. This currently only
allows you to hide information from /proc/self/cgroup. But I'm currently
working with upstream on expanding it so that you can modify your
hierarchy inside a user namespace (even a rootless container).

Signed-off-by: Aleksa Sarai asarai@suse.de

vbatts · 2016-04-25T15:20:22Z

seems fine. Is this needing to wait on upstream linux or runc?

cyphar · 2016-04-25T15:35:18Z

This has been merged into Linux and will be released in 4.6. I'd still recommend waiting on the release. The great thing about this API is that its compile-time switch is CONFIG_CGROUPS, so if you have a >=4.6 kernel with cgroups you automatically get this namespace.

vbatts · 2016-04-25T15:36:24Z

oh nice. That smooth the transition for sure.

On Mon, Apr 25, 2016 at 11:35 AM, Aleksa Sarai notifications@github.com
wrote:

This has been merged into Linux and will be released in 4.6. I'd still
recommend waiting on the release. The great thing about this API is that
its compile-time switch is CONFIG_CGROUPS, so if you have a >=4.6 kernel
with cgroups you automatically get this namespace.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#397 (comment)

mrunalp · 2016-04-25T15:42:36Z

👍 We can merge this and add it to runc once released.

wking · 2016-04-25T16:26:54Z

On Mon, Apr 25, 2016 at 08:14:24AM -0700, Aleksa Sarai wrote:

M schema/defs-linux.json (3)
M specs-go/config.go (2)

We also want docs in config-linux.md#namespaces, and possibly an
addition to the example in that section and the full example in
config.md.

cyphar · 2016-04-25T16:42:20Z

@wking I've added examples and explanations in the docs.

wking · 2016-04-25T18:02:26Z

config-linux.md

@@ -33,6 +33,7 @@ The following parameters can be specified to setup namespaces:
    * **`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
+    * **`cgroup`** the container will have an isolated cgroup hierarchy that it can manage


Reference 4.6 as the release that landed this feature?

vishh · 2016-04-25T20:12:56Z

How do we handle incompatibility with new kernel features as of now? Is the Spec attempting to provide some means of discovery of the features that a given kernel supports?

mrunalp · 2016-04-25T21:30:29Z

@vishh I think probably leave that to the runtime.

vishh · 2016-04-25T22:11:21Z

@mrunalp: That would affect client portability right? Is cross-runtime
portability a goal?

On Mon, Apr 25, 2016 at 2:30 PM, Mrunal Patel notifications@github.com
wrote:

@vishh https://github.com/vishh I think probably leave that to the
runtime.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#397 (comment)

cyphar · 2016-04-25T22:59:16Z

@vishh We can make it clear in the spec that a runtime implementation must fail if it can't create all of the requested namespaces. Anything outside of that should be left to the runtime to handle (such as figuring out whether or not it can create the namespaces).

wking · 2016-04-25T23:27:47Z

9926619 looks good to me. I don't mind whether the
fail-if-not-supported language 1 lands via this PR or a separate PR.

mrunalp · 2016-05-16T17:04:26Z

config-linux.md

+    * **`ipc`** processes inside the container will only be able to communicate to other processes inside the same container via system level IPC. Support for this was added in Linux 2.6.19.
+    * **`uts`** the container will be able to have its own hostname and domain name. Support for this was added in Linux 2.6.19.
+    * **`user`** the container will be able to remap user and group IDs from the host to local users and groups within the container. Support for this was added in Linux 3.8.
+    * **`cgroup`** the container will have an isolated cgroup hierarchy that it can manage. Support for this was added in Linux 4.6.


Can we modify the text here - that it sees an isolated/virtualized view of the cgroup heirarchy? The manage portion doesn't quite work well yet ;)

We can use text from here https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d4021f6cd41f03017f831b3d40b0067bed54893d

Heh, it was a bit ambitious for a description. ;)

cyphar · 2016-05-19T14:15:42Z

@mrunalp Updated. I've also added the "must fail" wording as @wking requested.

mrunalp · 2016-05-19T14:33:08Z

config-linux.md


 * **`path`** *(string, optional)* - path to namespace file in the [runtime mount namespace](glossary.md#runtime-namespace)

 If a path is specified, that particular file is used to join that type of namespace.
 Also, when a path is specified, a runtime MUST assume that the setup for that particular namespace has already been done and error out if the config specifies anything else related to that namespace.

+If the runtime is unable to create or join all of the requested namespaces, it
+MUST fail so as not to lull the user into a false sense of security.


I think we can drop 'so as not to lull the user into a false sense of security'. Otherwise looks fine.

mrunalp · 2016-05-19T15:46:14Z

LGTM

wking · 2016-05-19T16:49:56Z

config-linux.md


 * **`path`** *(string, optional)* - path to namespace file in the [runtime mount namespace](glossary.md#runtime-namespace)

 If a path is specified, that particular file is used to join that type of namespace.
 Also, when a path is specified, a runtime MUST assume that the setup for that particular namespace has already been done and error out if the config specifies anything else related to that namespace.

+If the runtime is unable to create or join ALL of the requested namespaces, it
+MUST fail.


For consistency withruntime.md`, we should probably use “generate an error”. Although @vishh's concern was broader than namespaces, so I think we might want to put this language in step 2 of the lifecycle.

Also, one line per sentence ;).

Also, one line per sentence ;).

Eww. 80 characters is ~~objectively~~ the right wrapping. ;)
But yes, I'll update the lifecycle documentation then.

cyphar · 2016-05-27T12:02:17Z

@wking @mrunalp I've updated things as requested. The documentation is now accurate about what the cgroup namespace does -- and I've added an extra line to the lifecycle documentation to make clear that a runtime must generate an error if it can't set up the environment requested by a container.

wking · 2016-05-27T18:52:10Z

On Fri, May 27, 2016 at 05:02:18AM -0700, Aleksa Sarai wrote:

@wking @mrunalp I've updated things as requested.

Heh, f0e4d33 → a021e63 becomes much more excited about isolation (from
“only really hides” to “invaluable for hiding” ;).

Anyhow, the tip commit (058860b) looks good (and can be cherry-picked
into a new PR and landed now as far in my view, although I'm not a
maintainer).

I'm not sure about the penultimate commit; will leave an inline comment.

wking · 2016-05-27T18:56:14Z

config-linux.md


 * **`path`** *(string, optional)* - path to namespace file in the [runtime mount namespace](glossary.md#runtime-namespace)

 If a path is specified, that particular file is used to join that type of namespace.
-Also, when a path is specified, a runtime MUST assume that the setup for that particular namespace has already been done and error out if the config specifies anything else related to that namespace.


This is new with your recent reroll, and rolls back #158. What has changed since #158? The motivation for this line was “we may support join-and-tweak if someone gives a convincing use-case, but we're banning it for now”, but you're removing it (in a drive-by change?) while adding a cgroup namespace, and I don't see the connection.

Urgh, looks like it was a dodgy rebase. I'll fix this up.

EDIT: Fixed.

wking · 2016-05-28T20:17:16Z

runtime.md

@@ -35,6 +35,7 @@ The lifecycle describes the timeline of events that happen from when a container
 1. OCI compliant runtime's `create` command is invoked with a reference to the location of the bundle and a unique identifier.
   How these references are passed to the runtime is an implementation detail.
 2. The container's runtime environment MUST be created according to the configuration in [`config.json`](config.md).
+   If the runtime is unable to create the runtimme environment specified in the [`config.json`](config.md) it MUST generate an error.


“runtimme” → “runtime”.

Or maybe just drop that second “runtime” instances and say “… the environment specified…”.

Also, you probably want a comma after config.json: “If … config.json, it MUST generate an error.”

Fixed. And I fixed up the email madness.

wking · 2016-05-28T20:24:16Z

Everything through 5d3c351 looks pretty good to me (just a few copy-edit suggestions for the tip commit). And you're using two different emails, in case you wanted to adjust those before this lands.

wking · 2016-05-29T19:56:01Z

Everything through 3ce281c looks good to me.

mrunalp · 2016-06-01T17:12:15Z

config-linux.md

-    * **`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
+    * **`pid`** processes inside the container will only be able to see other processes inside the same container. Support for this was added in Linux 2.6.24.


One sentence per line. Otherwise looks good.

Damn, I swore I fixed this.

I don't know if you can do this with lists in md @mrunalp

tianon · 2016-06-01T17:17:20Z

As discussed in the call, my $0.02 on whether to include kernel versions of required features in the spec, I don't think it's really strictly necessary for the spec -- having a separate "cheat sheet" might make sense (mapping features to minimum kernel versions), but if the information is reasonably readily available elsewhere, then I think it's sufficient for the tools to simply error out when unsupported features are used (relying more on the runtime check than fuzzy kernel version matching, which RedHat has a habit of throwing a wrench in 😄).

crosbymichael · 2016-06-01T21:41:30Z

Yes, we are not recording the history of the linux kernel in this spec.

mrunalp · 2016-06-02T15:35:29Z

@cyphar could you update the PR to address the comments? We kinda need this for 1.0 :)
Thanks!

crosbymichael · 2016-06-02T17:26:22Z

I wouldn't say that this is a blocker for 1.0. Its adding an option and does not affect schema or anything so it can come at any time.

mrunalp · 2016-06-02T17:44:32Z

Yeah, not a hard blocker but will be nice to have since it is so close to being merged.

cyphar · 2016-06-02T23:35:15Z

So you want me to remove all of the "This is available since Linux X.Y.Z" sentences?

mrunalp · 2016-06-03T00:00:22Z

@cyphar Yes. That's what we discussed and agreed to in the OCI call yesterday.

philips · 2016-06-03T04:35:07Z

LGTM

Agreed on removing the kernel versions. It is meaningless given some Linux distros do tons of backporting.

cyphar · 2016-06-03T09:08:57Z

@opencontainers/runtime-spec-maintainers There, I've removed the kernel versions.

mrunalp · 2016-06-03T14:07:59Z

LGTM

The cgroup namespace is a new kernel feature available in 4.6+ that allows a container to isolate its cgroup hierarchy. This currently only allows for hiding information from /proc/self/cgroup, and mounting cgroupfs as an unprivileged user. In the future, this namespace may allow for subtree management by a container. Signed-off-by: Aleksa Sarai <asarai@suse.de>

Make it clear that if a runtime cannot set up an environment that *precisely* matches the config.json provided, it must generate an error. This is important because not doing this can cause security issues. Signed-off-by: Aleksa Sarai <asarai@suse.de>

crosbymichael · 2016-06-03T17:42:00Z

LGTM

crosbymichael · 2016-06-03T17:43:13Z

@mrunalp @philips you have to LGTM again because the commits were updated

vbatts · 2016-06-03T17:45:24Z

LGTM

cyphar mentioned this pull request Apr 25, 2016

Rootless Containers opencontainers/runc#774

Merged

46 tasks

cyphar force-pushed the add-cgroup-namespace branch from 3e7752b to bc0d866 Compare April 25, 2016 16:42

wking reviewed Apr 25, 2016
View reviewed changes

cyphar force-pushed the add-cgroup-namespace branch from bc0d866 to 9926619 Compare April 25, 2016 23:03

cyphar mentioned this pull request Apr 26, 2016

Add support for cgroup namespaces opencontainers/runc#781

Closed

1 task

crosbymichael added the platforms/linux/blocked-upstream label May 4, 2016

mrunalp reviewed May 16, 2016
View reviewed changes

mrunalp removed the platforms/linux/blocked-upstream label May 18, 2016

cyphar force-pushed the add-cgroup-namespace branch from 9926619 to f0e4d33 Compare May 19, 2016 14:11

mrunalp reviewed May 19, 2016
View reviewed changes

cyphar force-pushed the add-cgroup-namespace branch from 79313a4 to 76205d0 Compare May 19, 2016 14:38

wking reviewed May 19, 2016
View reviewed changes

mrunalp added this to the 1.0.0 milestone May 25, 2016

cyphar force-pushed the add-cgroup-namespace branch from 76205d0 to 058860b Compare May 27, 2016 12:01

wking reviewed May 27, 2016
View reviewed changes

wking reviewed May 28, 2016
View reviewed changes

cyphar force-pushed the add-cgroup-namespace branch from 5d3c351 to 3ce281c Compare May 29, 2016 04:39

wking mentioned this pull request May 31, 2016

[Config | Hooks] Add "if supported by the platform" #472

Closed

mrunalp reviewed Jun 1, 2016
View reviewed changes

cyphar force-pushed the add-cgroup-namespace branch from 4eb65e7 to d514aad Compare June 3, 2016 14:10

cyphar added 2 commits June 4, 2016 00:14

vbatts merged commit 831d961 into opencontainers:master Jun 3, 2016

cyphar deleted the add-cgroup-namespace branch June 3, 2016 17:45

This was referenced Jun 22, 2016

[Proof of concept] Add Windows fields #504

Closed

validation: add cgroup namespace check opencontainers/runtime-tools#118

Merged

hqhq mentioned this pull request Dec 24, 2016

Mounting cgroups in a container #66

Closed

jasontedor mentioned this pull request Jan 24, 2017

Add hack for obtaining cgroup stats elastic/elasticsearch-docker#25

Merged

This was referenced Jan 26, 2018

generate/seccomp: platform independent values opencontainers/runtime-tools#561

Merged

Cgroup (namespace) Delegation #948

Open

*: add support for cgroup namespace #397

*: add support for cgroup namespace #397

Conversation

cyphar commented Apr 25, 2016 • edited Loading

vbatts commented Apr 25, 2016

cyphar commented Apr 25, 2016

vbatts commented Apr 25, 2016

mrunalp commented Apr 25, 2016 • edited Loading

wking commented Apr 25, 2016

cyphar commented Apr 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented Apr 25, 2016

mrunalp commented Apr 25, 2016

vishh commented Apr 25, 2016

cyphar commented Apr 25, 2016

wking commented Apr 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar commented May 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrunalp commented May 19, 2016

wking May 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar commented May 27, 2016

wking commented May 27, 2016

wking May 27, 2016 • edited Loading

Choose a reason for hiding this comment

cyphar May 28, 2016 • edited Loading

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

Choose a reason for hiding this comment

wking commented May 28, 2016 via email

wking commented May 29, 2016 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tianon commented Jun 1, 2016

crosbymichael commented Jun 1, 2016

mrunalp commented Jun 2, 2016

crosbymichael commented Jun 2, 2016

mrunalp commented Jun 2, 2016

cyphar commented Jun 2, 2016

mrunalp commented Jun 3, 2016

philips commented Jun 3, 2016 • edited by caniszczyk Loading

cyphar commented Jun 3, 2016

mrunalp commented Jun 3, 2016 • edited by caniszczyk Loading

crosbymichael commented Jun 3, 2016 • edited by caniszczyk Loading

crosbymichael commented Jun 3, 2016

vbatts commented Jun 3, 2016 • edited by caniszczyk Loading

cyphar commented Apr 25, 2016 •

edited

Loading

mrunalp commented Apr 25, 2016 •

edited

Loading

wking May 19, 2016 •

edited

Loading

wking May 27, 2016 •

edited

Loading

cyphar May 28, 2016 •

edited

Loading

philips commented Jun 3, 2016 •

edited by caniszczyk

Loading

mrunalp commented Jun 3, 2016 •

edited by caniszczyk

Loading

crosbymichael commented Jun 3, 2016 •

edited by caniszczyk

Loading

vbatts commented Jun 3, 2016 •

edited by caniszczyk

Loading