config-linux: Lift no-tweaking namespace restriction #649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jan 12, 2017
Now that d43fc42 (config-linux: Lift no-tweaking namespace restriction, 2017-01-11, opencontainers#649) allows us to get into this sort of situation. This sort of ownership may also apply to other resources (cgroups?), but we can handle them in follow-up PRs. Also drop "Configuration" from the root header. Everything in that file is a configuration. Signed-off-by: W. Trevor King <[email protected]>
|
Why would this fix update issues? Update is more about tweaking cgroup resources. |
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jan 12, 2017
Now that d43fc42 (config-linux: Lift no-tweaking namespace restriction, 2017-01-11, opencontainers#649) allows us to get into this sort of situation. This sort of ownership may also apply to other resources (cgroups?), but we can handle them in follow-up commits. Also drop "Configuration" from the root header. Everything in that file is a configuration. container-namespace3 (instead of container-namespace) supports the single-page, Pandoc-generated file (see e7be40f, Cleanup the spec a bit to remove WG/git text that's not really part of the spec, 2016-11-14, opencontainers#626). Signed-off-by: W. Trevor King <[email protected]>
|
On Wed, Jan 11, 2017 at 07:46:50PM -0800, Qiang Huang wrote:
Why would this fix update issues? Update is more about tweaking
cgroup resources.
You create a new config like:
{
…
"linux": {
"cgroupsPath": "/my/existing/container"
"resources": {
"pids": {
"limit": 4096
}
}
}
}
and create/delete a container with that config. That will join your
existing container's ‘pids’ controller, adjust the limit, and die
(leaving the adjusted value in place, although this is made clearer by
#651).
There's still no way to use the runtime join and tweak rlimits,
capabilities, etc. but you can call prlimit on the container process
without entering any container namespace, so I don't see a need to
involve the runtime in that. However, I'm not sure if there are
prlimit analogs for capabilities, etc., and folks without a container
process ID (#459) might need some special tooling around this. So
there's still work to be done, but *cgroup resources* seem like
they're sufficiently handled by this PR and #651.
|
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jan 19, 2017
Now that d43fc42 (config-linux: Lift no-tweaking namespace restriction, 2017-01-11, opencontainers#649) allows us to get into this sort of situation. This sort of ownership may also apply to other resources (cgroups?), but we can handle them in follow-up commits. Also drop "Configuration" from the root header. Everything in that file is a configuration. container-namespace3 (instead of container-namespace) supports the single-page, Pandoc-generated file (see e7be40f, Cleanup the spec a bit to remove WG/git text that's not really part of the spec, 2016-11-14, opencontainers#626). Using an informative suggestion was recommended by Dao Quang Minh [1]. I've made the config JSON as small as possible while keeping it valid, but there's still an unfortunate amount of boilerplate there. There is in-flight work to let us at least drop process.args [2]. [1]: opencontainers#651 [2]: opencontainers#620 Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jan 19, 2017
Now that d43fc42 (config-linux: Lift no-tweaking namespace restriction, 2017-01-11, opencontainers#649) allows us to get into this sort of situation. This sort of ownership may also apply to other resources (cgroups?), but we can handle them in follow-up commits. Using an informative suggestion was recommended by Dao Quang Minh [1]. I've made the config JSON as small as possible while keeping it valid, but there's still an unfortunate amount of boilerplate there. There is in-flight work to let us at least drop process.args [2]. The new mount namespace in the UTS example avoids pivoting the host namespace's root. Also drop "Configuration" from the root header. Everything in that file is a configuration. container-namespace3 (instead of container-namespace) supports the single-page, Pandoc-generated file (see e7be40f, Cleanup the spec a bit to remove WG/git text that's not really part of the spec, 2016-11-14, opencontainers#626). [1]: opencontainers#651 [2]: opencontainers#620 Signed-off-by: W. Trevor King <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This restriction originally landed via #158. The hostname case landed via #214, citing the namespace restriction. The restriciton extended to runtime namespaces in #538. There was also a proposal in-flight to get config-wide consistency around the no-tweaking concept (#540).
In today's meeting, the maintainer consensus was to strike the no-tweaking restriction, 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.
Fixes #17.
Fixes #305.