Update cgroupsPath to cgroupName and clarify the semantics around that.

[vishh]

@mrunalp

[wking]

Do we need the explicit allowed-character restrictions if we're punting to “regular filesystem rules” (which are a bit more relaxed than the character restrictions you list here)?

[hqhq]

Yeah, a link would be better.

[vishh]

In terms of characters, only the ones mentioned above are supported. But path construction follow filesystem path rules.
@hqhq: What link do you need?

[wking]

On Mon, Dec 28, 2015 at 11:56:49AM -0800, Vish Kannan wrote:

+In general and unless otherwise specified, regular filesystem path rules apply.

In terms of characters, only the ones mentioned above are
supported. But path construction follow filesystem path rules.
@hqhq: What link do you need?

I can't find a reference to this in the Linux kernel docs, but POSIX
says pretty clearly that everything except the null and slash bytes
are fair game as long as the full byte name isn't ‘.’ or ‘..’ 1.

[hqhq]

@vishh I was thinking of a link about the definition of "what is the allowable characters of a regular filesystem path", if that does exist. Or I'm also OK if we can't find a proper link because I think that's common enough to be accepted.

[mlaventure]

Note that this is false for systemd, / within the parent path are rejected by the dbus service.

[wking]

I left a few nits inline, but except for 1 this PR is a wash for me.

[mrunalp]

s/acceptible/acceptable/

[vishh]

Fixed.

[hqhq]

Any thoughts about how systemd implementation should perform on this? (not a question for this PR, but for the general cgroupsPath/Name definition)

[mrunalp]

@hqhq systemd api work with a slice and a scope name, so would have to split the name into those parts and make the call to create a scope.

[hqhq]

@mrunalp So we want users on systemd cgroup specify the exactly rightcgroupName such as /system.slice/my.slice/app.scope and we need to validate it before apply?
Because for now, we are handling name and parent to assign slice and unitName. I'm facing the problem when working on opencontainers/runc#397 as trying to remove name and parent in runc.

[vishh]

@hqhq@mrunalp: Comment addressed. PTAL.

[wking]

This should keep the *string from #233.

[vishh]

Why? There are only two options here, an empty string "" or a valid name.
There isn't a third system default.

On Mon, Dec 28, 2015 at 12:11 PM, W. Trevor King notifications@github.com
wrote:

In runtime_config_linux.go
#270 (comment):

@@ -25,10 +25,10 @@ type LinuxRuntime struct {
// Resources contain cgroup information for handling resource constraints
// for the container
Resources *Resources json:"resources,omitempty"

• // CgroupsPath specifies the path to cgroups that are created and/or joined by the container.
• // The path is expected to be relative to the cgroups mountpoint.
• // If resources are specified, the cgroups at CgroupsPath will be updated based on resources.
• CgroupsPath *string json:"cgroupsPath,omitempty"
• // CgroupsName specifies the name of to cgroups that are created and/or joined by the container.
• // The name is expected to be absolute.
• // If resources are specified, the CgroupsName cgroups will be updated based on resources.
• CgroupsName string json:"cgroupsName,omitempty"

This should keep the *string from #233
#233.

—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/270/files#r48502724.

[wking]

On Mon, Dec 28, 2015 at 12:14:05PM -0800, Vish Kannan wrote:

• // CgroupsName specifies the name of to cgroups that are created and/or joined by the container.
• // The name is expected to be absolute.
• // If resources are specified, the CgroupsName cgroups will be updated based on resources.
• CgroupsName string json:"cgroupsName,omitempty"

Why? There are only two options here, an empty string "" or a valid name.
There isn't a third system default.

This:

"cgroupsName": "",

seems illegal to me (it's not absolute). If we use missing/null
cgroupsName, we don't have to put in an absolute-path loophole for the
empty string. For more background on nil over empty strings for
“unset”, see 1 and surrounding comments, as well as 2.

[vishh]

@mrunalp@crosbymichael: Can you review this patch if you get a chance?

[hqhq]

The major problem is it's not working on systemd cgroup. Should that be concerned in spec?

[mlaventure]

Couldn't we just state that the format of the path depends on the backend and redirect the user to those for more information?

[vishh]

@hqhq: What part of this PR is not working with systemd?

[vishh]

In the case of systemd, would it be possible to translate the path into a
systemd compatible name?
The Spec currently aligns closely with Linux APIs. I'd like to stick to
that here as well.

On Wed, Jan 27, 2016 at 8:01 AM Kenfe-Mickaël Laventure <
notifications@github.com> wrote:

Couldn't we just state that the format of the path depends on the backend
and redirect the user to those for more information?

—
Reply to this email directly or view it on GitHub
#270 (comment).

[mlaventure]

That's certainly doable, although in the unlikely event that systemd changes its syntax or that a new manager chooses something different we would have to update our implementation.

[wking]

On Wed, Jan 27, 2016 at 01:57:31PM -0800, Kenfe-Mickaël Laventure wrote:

That's certainly doable, although in the unlikely event that systemd
changes its syntax…

I don't know how much stock I'd put in systemd's commitment to
backwards compatibility ;). Decoupling cgroup managers from the
runtime, so you can use a systemd-driving manager or a
filesystem-driving-manager as you see fit still sounds good to me 1.
As far as I know, progress in that direction is still blocking on a
way to opt-out from runtime cgroup manipulation 2.

 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>
Subject: Make runtime cgroups optional (was: removal of cgroups from the OCI Linux spec)
Date: Thu, 29 Oct 2015 12:44:27 -0700
Message-ID: <20151029194427.GA30073@odin.tremily.us>

[hqhq]

The intention of cgroupsName is to specify which cgroup will the container be in, for this purpose, cgroupfs API and systemd API need different information, a path can be used by cgroupfs API, but all systemd API needs is a slice name and a unit name (a scope according to the rule of systemd).

I'm not sure what to be defined in specs is the best to cover these. Maybe add a new entry like slice?

[mrunalp]

We should probably leave this out of the spec and handle this in the runtime.

Sent from my iPhone

On Jan 28, 2016, at 3:53 AM, Qiang Huang notifications@github.com wrote:

The intention of cgroupName is to specify which cgroup will the container be in, for this purpose, cgroupfs API and systemd API need different information, a path can be used by cgroupfs API, but all systemd API needs is a slice name and a unit name (a scope according to the rule of systemd).

I'm not sure what to be defined in specs is the best to cover these. Maybe add a new entry like slice?

—
Reply to this email directly or view it on GitHub.

[vishh]

Leave what out of the Spec? We have to include cgroup names right?

On Thu, Jan 28, 2016 at 7:13 AM, Mrunal Patel notifications@github.com
wrote:

We should probably leave this out of the spec and handle this in the
runtime.

Sent from my iPhone

On Jan 28, 2016, at 3:53 AM, Qiang Huang notifications@github.com
wrote:

The intention of cgroupName is to specify which cgroup will the
container be in, for this purpose, cgroupfs API and systemd API need
different information, a path can be used by cgroupfs API, but all systemd
API needs is a slice name and a unit name (a scope according to the rule of
systemd).

I'm not sure what to be defined in specs is the best to cover these.
Maybe add a new entry like slice?

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub
#270 (comment).

[mlaventure]

@vishh: I think @mrunalp meant to leave the actual format taken by cgroupsName out of the spec, not the field itself. I tend to agree with this.

[vishh]

@mrunalp@mlaventure@crosbymichael: How do we make progress here? The spec needs to define a standard way to specify cgroups. Systemd is an implementation detail. We need to standardize on the naming format. Otherwise the Spec and the corresponding bundle will not be portable.
I'd really like for us to not tie the Spec to an specific implementation. We can always construct implementation specific names from a standardized naming format.

[mlaventure]

The issue is that atm both implementations are hugely different.

systemd requires a name and a scope (which can't not include any / character) whereas the default linux usage is based on filepath.

So far the only way I can see us reconciliating both of them is by requiring a cgroup name of the form x-y-z then systemd would take the last element as the scope (i.e. z in that example) and everything else as the slice name (i.e. x-y) whereas for fs we would just replace the - with /.

But I do not find this particularly elegant.

[vishh]

What about converting the Linux format to systemd compatible one when
required within the runtime?

On Fri, Mar 4, 2016, 11:54 AM Kenfe-Mickaël Laventure <
notifications@github.com> wrote:

The issue is that atm both implementations are hugely different.

systemd requires a name and a scope (which can't not include any /
character) whereas the default linux usage is based on filepath.

So far the only way I can see us reconciliating both of them is by
requiring a cgroup name of the form x-y-z then systemd would take the
last element as the scope (i.e. z in that example) and everything else as
the slice name (i.e. x-y) whereas for fs we would just replace the - with
/.

But I do not find this particularly elegant.

—
Reply to this email directly or view it on GitHub
#270 (comment).