Change the behavior when cgroupsPath is absent #251
Conversation
|
On Tue, Nov 17, 2015 at 05:59:12AM -0800, Qiang Huang wrote:
I'm +1 on this PR. Users who care about what the cgroup will be or This is related to (but does not conflict with) #247. |
runtime-config-linux.md
Outdated
There was a problem hiding this comment.
Isn't empty string our means of not specifying cgroups?
There was a problem hiding this comment.
How about rephrasing as If cgroups path is not specified, behavior is undefined?
There was a problem hiding this comment.
On Tue, Nov 17, 2015 at 05:43:12PM -0800, Vish Kannan wrote:
-If not specified, cgroups will be created under '/'.
+If not specified or specified as empty string, implementations can define the default cgroup path.How about rephrasing as
If cgroups path is not specified, behavior is undefined?
The behavior is defined: the runtime will create a new cgroup, add the
container process to it, and perform any resource allocation specified
by ‘resources’. The only thing that's not defined when cgroupsPath is
not specified is the name of the cgroup that the runtime uses. So I'd
rather go back to the original phrasing, where the only unspecified
behavior is how the runtime picks the default name used when
cgroupPath is unset.
There was a problem hiding this comment.
@wking Are you suggesting we go back to my original phrasing in this PR?
I think undefined behavior is OK, because I think undefined means we don't restrict this behavior, doesn't mean the behavior is unknown.
There was a problem hiding this comment.
On Wed, Nov 18, 2015 at 03:22:54AM -0800, Qiang Huang wrote:
-If not specified, cgroups will be created under '/'.
+If not specified or specified as empty string, implementations can define the default cgroup path.@wking Are you suggesting we go back to my original phrasing in this PR?
Yes.
I think
undefined behavioris OK, because I think undefined means
we don't restrict this behavior, doesn't mean the behavior is
unknown.
It would mean that the spec has nothing to say about what happens when
cgroupsPath is unset, but I think we do want to define what happens
when cgroupsPath is unset (and just have the runtime pick its favorite
cgroupsPath name and then carry on as if cgroupsPath was set).
There was a problem hiding this comment.
What do you mean by favorite? The fact that its implementation dependent,
makes it undefined from a Specification perspective.
On Wed, Nov 18, 2015 at 10:09 AM, W. Trevor King notifications@github.com
wrote:
In runtime-config-linux.md
#251 (comment):@@ -146,7 +146,7 @@ For more information, see the [kernel cgroups documentation](https://www.kernel.
The path to the cgroups can be specified in the Spec via
cgroupsPath.
cgroupsPathis expected to be relative to the cgroups mount point.
-If not specified, cgroups will be created under '/'.
+If not specified or specified as empty string, implementations can define the default cgroup path.On Wed, Nov 18, 2015 at 03:22:54AM -0800, Qiang Huang wrote: > -If not
specified, cgroups will be created under '/'. > +If not specified or
specified as empty string, implementations can define the default cgroup
path. @wking https://github.com/wking Are you suggesting we go back to
my original phrasing in this PR?
Yes.
I thinkundefined behavioris OK, because I think undefined means we
don't restrict this behavior, doesn't mean the behavior is unknown.
It would mean that the spec has nothing to say about what happens when
cgroupsPath is unset, but I think we do want to define what happens when
cgroupsPath is unset (and just have the runtime pick its favorite
cgroupsPath name and then carry on as if cgroupsPath was set).—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/251/files#r45236291.
There was a problem hiding this comment.
This is specifically for developer workflow. Honestly, I don't see why we
are spending so much energy here @wking. This PR LGTM either ways.
On Wed, Nov 18, 2015 at 10:22 AM, Vishnu Kannan vishnuk@google.com wrote:
What do you mean by favorite? The fact that its implementation
dependent, makes it undefined from a Specification perspective.On Wed, Nov 18, 2015 at 10:09 AM, W. Trevor King <notifications@github.com
wrote:
In runtime-config-linux.md
#251 (comment):@@ -146,7 +146,7 @@ For more information, see the [kernel cgroups documentation](https://www.kernel.
The path to the cgroups can be specified in the Spec via
cgroupsPath.
cgroupsPathis expected to be relative to the cgroups mount point.
-If not specified, cgroups will be created under '/'.
+If not specified or specified as empty string, implementations can define the default cgroup path.On Wed, Nov 18, 2015 at 03:22:54AM -0800, Qiang Huang wrote: > -If not
specified, cgroups will be created under '/'. > +If not specified or
specified as empty string, implementations can define the default cgroup
path. @wking https://github.com/wking Are you suggesting we go back to
my original phrasing in this PR?
Yes.
I thinkundefined behavioris OK, because I think undefined means we
don't restrict this behavior, doesn't mean the behavior is unknown.
It would mean that the spec has nothing to say about what happens when
cgroupsPath is unset, but I think we do want to define what happens when
cgroupsPath is unset (and just have the runtime pick its favorite
cgroupsPath name and then carry on as if cgroupsPath was set).—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/251/files#r45236291.
There was a problem hiding this comment.
On Wed, Nov 18, 2015 at 10:23:39AM -0800, Vish Kannan wrote:
-If not specified, cgroups will be created under '/'.
+If not specified or specified as empty string, implementations can define the default cgroup path.This is specifically for developer workflow.
It doesn't read that way to me. “implementations can define the
default cgroup path” sounds like “if you don't set cgroupsPath, we'll
pick one for you” (which I think makes sense). However, “behavior is
undefined” sounds like “the runtime can do anything it wants,
including just exiting with an error and not launching a container at
all” (which I think makes very little sense).
hqhq
force-pushed
the
hq_change_cgroupsPath
branch
from
187af1d to
517ae40
Compare
|
@vishh You are right, specified as empty string would be the same as not specified, and I changed the description to |
The former definition was join "/" when `cgroupsPath` is absent, it's not clear whether to join the root cgroup or create a sub cgroup under "/". Join root cgroup would be a bad idea cause that's not what a container should do. And craete a sub cgroup would be missing definition about what the cgroup name should that be. So I think we should leave this to implementations what the default cgroup path should be. Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
|
"Undefined behavior" means (it is bug)/(it can corrupt)/etc. when cgroupsPath is absent. see I think we can use |
hqhq
force-pushed
the
hq_change_cgroupsPath
branch
from
517ae40 to
d663a5b
Compare
|
@laijs Thanks for the links, I've changed it the original phrasing. |
|
LGTM @crosbymichael @vishh @philips @vbatts PTAL |
|
LGTM |
Change the behavior when cgroupsPath is absent
6 participants
The former definition was join "/" when
cgroupsPathis absent, it'snot clear whether to join the root cgroup or create a sub cgroup under "/".
Join root cgroup would be a bad idea cause that's not what a container
should do. And craete a sub cgroup would be missing definition about what
the cgroup name should that be.
So I think we should leave this to implementations what the default
cgroup path should be.
Signed-off-by: Qiang Huang h.huangqiang@huawei.com