Skip to content

runtime-config-linux: Clarify resources vs. cgroupsPath #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wking wants to merge 1 commit into from
Closed

runtime-config-linux: Clarify resources vs. cgroupsPath #247

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions runtime-config-linux.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,12 @@ Also known as cgroups, they are used to restrict resource usage for a container
cgroups provide controls to restrict cpu, memory, IO, pids and network for the container.
For more information, see the [kernel cgroups documentation](https://www.kernel.org/doc/Documentation/cgroups/cgroups.txt).

There are a few facets to this:

* Managing the existence of cgroups within the hierarchy (`cgroupsPath`).
Copy link
Contributor

@vishh vishh Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

within what hierarchy?
cgroupsPath is the path to a cgroups hierarchy.

Copy link
Contributor

@jonboulle jonboulle Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume he means the unified hierarchy, since subsequently it mentions the spec does not support split, but would be good to make that explicit

Copy link
Contributor Author

@wking wking Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Dec 16, 2015 at 10:23:07AM -0800, Vish Kannan wrote:

+* Managing the existence of cgroups within the hierarchy (cgroupsPath).

within what hierarchy? cgroupsPath is the path to a cgroups
hierarchy.

This spec requires a unified cgroups hierarchy 1. cgroupsPath
locates the cgroup for this process within that hierarchy. The
runtime creates cgroups as needed to make that happen, and removes any
cgroups it created (but which are no longer used) when it cleans up
after itself. That makes the “within the hierarchy” phrasing clear to
me, but if you have an alternative that you prefer I'll switch to it.

Copy link
Contributor Author

@wking wking Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Dec 16, 2015 at 11:02:59AM -0800, Jonathan Boulle wrote:

+* Managing the existence of cgroups within the hierarchy (cgroupsPath).

I assume he means the unified hierarchy, since subsequently it
mentions the spec does not support split, but would be good to make
that explicit

I can make “the hierarchy” a link to 1. But as far as I know, we
don't actually require you to use that unified-hierarchy
implementation. We just don't allow you to give per-controller paths.
I think that's too complicated an idea for this summary-list, but I'll
happily add more detail to the existing split-hierarchy reference to
clarify this point.

Copy link
Member

@vbatts vbatts Jan 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is, this could allow for either, though can you point me to where unified-hierarchy is explicitly required for this spec?

Copy link
Contributor Author

@wking wking Jan 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Tue, Jan 05, 2016 at 12:14:37PM -0800, Vincent Batts wrote:

As is, this could allow for either, though can you point me to where
unified-hierarchy is explicitly required for this spec?

The wording (from 429f936, #137) is:

The Spec does not support split hierarchy

And the surrounding discussion 1 was (paraphrased):

me: cgexec(1) lets you set per-controller paths, do we want that?
@vishh: Won't this break with unified hierarchy 2?
me: I see. Can we link to those kernel docs?

I could see someone reading that as either:

  • “we require a __DEVEL__sane_behavior mount” or
  • “you only get to specify one cgroup path, not per-controller paths.
    Whether the runtime implements that via an __DEVEL__sane_behavior
    mount or not is undefined”.

Personally I'm in favor of the latter interpretation, but I think the
current wording could go either way. In either case, there is only
one name for all controllers, so I feel comfortable saying “the
hierarchy” here.

* Moving processes to respective cgroups.
Copy link
Contributor

@jonboulle jonboulle Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also affected by cgroupsPath, right? Actually I find this list pretty unclear as-is, I think it should explain in more detail something like: "there are multiple aspects (facets) to cgroups ... the spec interacts with them in different ways based on certain configuration parameters ... bla bla"

Copy link
Contributor Author

@wking wking Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Dec 16, 2015 at 11:06:00AM -0800, Jonathan Boulle wrote:

+* Moving processes to respective cgroups.

This is also affected by cgroupsPath, right?

I don't think so (see my breakdown here 1)

Actually I find this list pretty unclear as-is…

And so has everyone else that has reviewed it ;). So I think we need
to either drop it completely or replace it with something like the
shell example in 2.

* Configuring cgroups to apply resource limits (`resources`).

Copy link
Contributor

@hqhq hqhq Nov 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, if it's general facets for cgroup, I don't think it's typical enough and helps much.
I suggest we remove this hunk, the others look good to me.

ping @vishh @mrunalp WDYT?

Copy link
Contributor Author

@wking wking Nov 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Nov 18, 2015 at 03:35:28AM -0800, Qiang Huang wrote:

+There are a few facets to this:
+
+* Managing the existence of cgroups within the hierarchy (cgroupsPath).
+* Assigning processes to cgroups.
+* Configuring cgroups to apply resource limits (resources).

I don't know, if it's general facets for cgroup, I don't think it's
typical enough and helps much. I suggest we remove this hunk, the
others look good to me.

I think the lack of a distinction between “what the cgroup is called”
and “how the cgroup is configured” is part of the confusion we've seen
around ‘cgroupsPath’ and ‘resources’. I was trying to clarify that
distinction here, but if it doesn't need to be clarified (or there's a
better way to clarify it), I'm happy drop (or replace) this hunk.

Copy link
Contributor

@hqhq hqhq Dec 11, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand what's the meaning of "Managing the existence of cgroups within the hierarchy", does this includes:

  • Assigning processes to cgroups (where to assign to)
  • Configuring cgroups to apply resource limits (what kind of resources limits)

Because above two are almost all we can do to manage cgroup. I think the three facets are not on the same page, that's why I think is not clear.

Copy link
Contributor Author

@wking wking Dec 11, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Dec 10, 2015 at 11:16:03PM -0800, Qiang Huang wrote:

+There are a few facets to this:
+
+* Managing the existence of cgroups within the hierarchy (cgroupsPath).
+* Assigning processes to cgroups.
+* Configuring cgroups to apply resource limits (resources).
+

I don't quite understand what's the meaning of "Managing the
existence of cgroups within the hierarchy", does this includes:

  • Assigning processes to cgroups (where to assign to)
  • Configuring cgroups to apply resource limits (what kind of
    resources limits)

No, those are both separate facets in my list.

Because above two are almost all we can do to manage cgroup. I think
the three facets are not on the same page, that's why I think is not
clear.

“Adding process to a cgroup” and “applying resource limits” are both
independent of where the cgroup lives in the hierarchy
(cgroupsPath). For example, the cgroup setup is something like:

$ CGROUP=/sys/fs/cgroup/cpuset/a/b/c
$ mkdir -p "${CGROUP}"
$ echo 123 >"${CGROUP}/cgroup.procs
$ echo 2-3 >"${CGROUP}/cpuset.cpus

the first two lines are “managing the existence of cgroups within the
hierarchy”, the third line is “assigning processes to cgroups”, and the
fourth line is “configuring cgroups to apply resource limits”.

Copy link
Contributor

@hqhq hqhq Dec 14, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking That explains, thanks. In my understanding, "Assigning processes to cgroups (where to assign to)" includes where you say "managing the existence of cgroups within the hierarchy", but it makes sense to separate them.

The path to the cgroups can be specified in the Spec via `cgroupsPath`.
`cgroupsPath` is expected to be relative to the cgroups mount point.
If `cgroupsPath` is not specified, implementations can define the default cgroup path.
Expand All @@ -156,11 +162,13 @@ The cgroups will be created if they don't exist.
"cgroupsPath": "/myRuntime/myContainer"
```

`cgroupsPath` can be used to either control the cgroups hierarchy for containers or to run a new process in an existing container.
The container process will be added to the cgroups at `cgroupsPath`.

You can configure a container's cgroups via the `resources` field of the Linux configuration.
Do not specify `resources` unless limits have to be updated.
For example, to run a new process in an existing container without updating limits, `resources` need not be specified.
For example, to run a new process in an existing container without updating limits, specify `cgroupsPath` but not `resources`.
To run a new process in an existing container and update limits, specify both `cgroupsPath` and `resources`.
To run a new process in a new container with explicit resource limits, specify `resources` and optionally set `cgroupsPath` to a cgroup that does not yet exist.
Copy link
Contributor

@vishh vishh Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't a user run a new container using existing cgroups?

Copy link
Contributor Author

@wking wking Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Dec 16, 2015 at 11:46:24AM -0800, Vish Kannan wrote:

-For example, to run a new process in an existing container without updating limits, resources need not be specified.
+For example, to run a new process in an existing container without updating limits, specify cgroupsPath but not resources.
+To run a new process in an existing container and update limits, specify both cgroupsPath and resources.
+To run a new process in a new container with explicit resource limits, specify resources and optionally set cgroupsPath to a cgroup that does not yet exist.

Why can't a user run a new container using existing cgroups?

They can, but it wouldn't be a new container (and I tried to cover
that in the “in an existing container without updating limits” and “in
an existing container and update limits” cases, but can add
clarification about both of those requiring cgroupsPath pointing to an
existing cgroup. Would that address your concerns?

Copy link
Contributor

@philips philips Jan 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work when launching under systemd which creates a cgroup before launching a process. I don't understand why we are tying the concept of container to creation of cgroup.

Copy link
Contributor Author

@wking wking Jan 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Jan 06, 2016 at 02:26:40AM -0800, Brandon Philips wrote:

This won't work when launching under systemd which creates a cgroup
before launching a process.

I'm not familiar with systemd and its interaction with OCI-specified
cgroups. What exactly would not work? If systemd created cgroups,
you use the path to the cgroups it will create in cgroupsPath, and
your container joins them. If systemd does not create cgroups with
unified-hierarchy-like naming, we probably need to relax 2 and come
up with a different approach (but that's a big enough change that it
probably deserves mailing-list discussion).

I don't understand why we are tying the concept of container to
creation of cgroup.

I agree, but I think that's what 1 and #237 are about. This PR is
about clarifying behavior until the spec drops cgroup handling, which
may never happen.

 Subject: removal of cgroups from the OCI Linux spec
 Date: Wed, 28 Oct 2015 17:01:59 +0000
 Message-ID: <CAD2oYtO1RMCcUp52w-xXemzDTs+J6t4hS5Mm4mX+uBnVONGDfA@mail.gmail.com>


#### Disable out-of-memory killer

Expand Down