Skip to content
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.

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

config: Shift Linux-namespace no-tweaking rule to a generic rule #540

Closed
wants to merge 1 commit into from

Conversation

wking
Copy link
Contributor

@wking wking commented Aug 26, 2016

There are more background references for the Linux-namespaces no-tweaking rule in 01c2d55 (config-linux: Extend no-tweak requirement to runtime namespaces, 2016-08-24, #538). But the old rule's:

... error out if the config specifies anything else related to that namespace.

was overly broad. For example, it arguably blocked you from setting network interface priorities for interfaces belonging to an old network namespace even if you were setting those priorities in a new cgroup (because the interfaces and therefore priorities for them are related to the old network namespace).

The new rule tries to apply the spirit of the old rule (“don't touch things that already exist”) more generally so we have a consistent approach that clearly does allow you to configure a new cgroup without having to care about new/old namespaces.

I'm personally fine with join-and-tweak, but the maintainer consensus is that it's too complicated to allow (at least for now, some folks want it).

There are more background references for the Linux-namespaces
no-tweaking rule in 01c2d55 (config-linux: Extend no-tweak
requirement to runtime namespaces, 2016-08-24, opencontainers#538).  But the old
rule's:

> ... error out if the config specifies anything else related to that
> namespace.

was overly broad.  For example, it arguably blocked you from setting
network interface priorities for interfaces belonging to an old
network namespace even if you were setting those priorities in a new
cgroup (because the interfaces and therefore priorities for them are
related to the old network namespace).

The new rule tries to apply the spirit of the old rule ("don't touch
things that already exist") more generally so we have a consistent
approach that clearly *does* allow you to configure a new cgroup
without having to care about new/old namespaces.

I'm personally fine with join-and-tweak, but the maintainer consensus
is that it's too complicated to allow (at least for now) [1,2].

[1]: opencontainers#158
     Subject: Clarify behavior around namespaces paths
[2]: opencontainers#537 (comment)
     Subject: [linux] Tweaking host namespaces?

Signed-off-by: W. Trevor King <wking@tremily.us>
While the following are invalid:

* A Linux configuration that sets [`hostname`](#hostname) but does not create a new [UTS namespace](config-linux.md#namespaces).
* A Linux configuration that sets [network limits][config-linux.md#network] with an existing [control group][config-linux.md#control-groups].

Choose a reason for hiding this comment

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

How to judge a cgroup is an existing one or a new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Fri, Aug 26, 2016 at 01:24:04AM -0700, Ma Shimiao wrote:

+* A Linux configuration that sets [network limits][config-linux.md#network] with an existing [control group][config-linux.md#control-groups].

How to judge a cgroup is an existing one or a new one?

If cgroupsPath is absolute, you can require no cgroups to exist at
that relative path from the controller mount points 1. You can also
compare cgroupsPath with values set for other running containers 2,
if you happen to know which other containers are running and which
runtime was used to launch them. In other cases, external tooling
like ocitools should probably skip this test.

Runtimes creating a container, on the other hand, have an easy way to
tell: they're using a new cgroup if they try to mkdir the leaf group
and it does not return EEXIST or other error.

@mrunalp mrunalp closed this Jan 11, 2017
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jan 11, 2017
This restriction originally landed via 02b456e (Clarify behavior
around namespaces paths, 2015-09-08, opencontainers#158).  The hostname case landed
via 66a0543 (config: Require a new UTS namespace for config.json's
hostname, 2015-10-05, opencontainers#214) citing the namespace restriction.  The
restriciton extended to runtime namespaces in 01c2d55 (config-linux:
Extend no-tweak requirement to runtime namespaces, 2016-08-24, opencontainers#538).
There was a proposal in-flight to get config-wide consistency around
the no-tweaking concept [1].

In today's meeting, the maintainer consensus was to strike the
no-tweaking restriction [2], which is what I've done here.

The hostname entry still mentions the UTS namespace to provide a guard
against accidental foot-gunning.  There was no no-tweaking language
for properties related to other namespaces (e.g. 'mounts').
Maybe the other namespaces have more obvious names.

[1]: opencontainers#540
[2]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2017/opencontainers.2017-01-11-22.04.log.html#l-117

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jan 11, 2017
This restriction originally landed via 02b456e (Clarify behavior
around namespaces paths, 2015-09-08, opencontainers#158).  The hostname case landed
via 66a0543 (config: Require a new UTS namespace for config.json's
hostname, 2015-10-05, opencontainers#214) citing the namespace restriction.  The
restriciton extended to runtime namespaces in 01c2d55 (config-linux:
Extend no-tweak requirement to runtime namespaces, 2016-08-24, opencontainers#538).
There was a proposal in-flight to get config-wide consistency around
the no-tweaking concept [1].

In today's meeting, the maintainer consensus was to strike the
no-tweaking restriction [2], which is what I've done here.  I've
removed the ROADMAP entry because this gives folks a way to adjust
existing containers (launch a new container which joins and tweaks the
original).

The hostname entry still mentions the UTS namespace to provide a guard
against accidental foot-gunning.  There was no no-tweaking language
for properties related to other namespaces (e.g. 'mounts').
Maybe the other namespaces have more obvious names.

[1]: opencontainers#540
[2]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2017/opencontainers.2017-01-11-22.04.log.html#l-117

Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants