-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Add ability to set cgroup parent for all containers #18379
Conversation
@@ -76,6 +77,7 @@ func (config *Config) InstallFlags(cmd *flag.FlagSet, usageFn func(string) strin | |||
cmd.BoolVar(&config.Bridge.EnableUserlandProxy, []string{"-userland-proxy"}, true, usageFn("Use userland proxy for loopback traffic")) | |||
cmd.BoolVar(&config.EnableCors, []string{"#api-enable-cors", "#-api-enable-cors"}, false, usageFn("Enable CORS headers in the remote API, this is deprecated by --api-cors-header")) | |||
cmd.StringVar(&config.CorsHeaders, []string{"-api-cors-header"}, "", usageFn("Set CORS headers in the remote API")) | |||
cmd.StringVar(&config.CgroupParent, []string{"-cgroup-parent"}, "docker", usageFn("Set parent cgroup for all containers")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not default to /docker
to be accurate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay
Just one nit. Otherwise LGTM. Do you want to include a note in the docs on how this flag can be used alongside systemd? |
@vishh Actually people can't use it with systemd until opencontainers/runc#336 :/ |
Why is that the case? This is a daemon level flag. On systemd machines, if On Wed, Dec 2, 2015 at 2:31 PM, Alexander Morozov [email protected]
|
@@ -19,6 +19,7 @@ weight = -1 | |||
--api-cors-header="" Set CORS headers in the remote API | |||
-b, --bridge="" Attach containers to a network bridge | |||
--bip="" Specify network bridge IP | |||
--cgroup-parent=docker Set parent cgroup for all containers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be "/docker"?
Maybe we should mention the path is relative to cgroup mount point in docs? |
@vishh I think @LK4D4 means people can't use it with systemd cgroup until opencontainers/runc#336 |
@hqhq I have patch for |
@LK4D4 Great, so we are simulating the work systemd api does in fs cgroup. Have you checked systemd source code to make sure we did all the necessary work? |
@LK4D4 Do you mean create dirs and join it? I think that'll be enough for |
@hqhq yeah, I meant that. |
There are now two cases with systemd:
|
Hmm, #18395 seems to be a simpler fix, this PR seems not necessary now? |
@hqhq It's actually new feature, not fix. |
@LK4D4 Right, we can still have this feature, if people want all containers in specific directory and don't want to use |
@LK4D4 needs rebase |
@tiborvass updated |
@LK4D4 sure, I'll have a look; might be tomorrow morning (my timezone 😄) |
Otherwise cgroup will be created under daemon cgroup (e.g. for `docker` if | ||
daemon is in `daemoncgroup` cgroup: `/sys/fs/cgroup/memory/daemoncgroup/docker`). | ||
By default this option is `/docker`. It can be changed per-container with | ||
`--cgroup-parent`argument for `create` or `run`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had this draft, not really satisfied with it, but feel free to use if you think it reads better 😄
# Default cgroup parent
The `--cgroup-parent` option allows you to set the default cgroup parent
to use for containers. If this option is not set, it defaults to `/docker`.
If the cgroup has a leading forward slash (`/`), the cgroup is created
under the root cgroup, otherwise the cgroup is created under the daemon
cgroup.
Assuming the daemon is running in cgroup `daemoncgroup`,
`--cgroup-parent=/foobar` creates a cgroup in
`/sys/fs/cgroup/memory/foobar`, wheras using `--cgroup-parent=foobar`
creates the cgroup in `/sys/fs/cgroup/memory/daemoncgroup/foobar`
This setting can also be set per container, using the `--cgroup-parent`
option on `docker create` and `docker run`, and takes precedence over
the `--cgroup-parent` option on the daemon.
Fix #18022 Signed-off-by: Alexander Morozov <[email protected]>
@thaJeztah I changed docs. However, it's still in design-review :) |
design/code LGTM |
LGTM moving to docs review. |
docs LGTM We need updates to the completion scripts after this has merged, so I'll ping @albers and @sdurrheimer here (thanks guys!) ping @moxiegirl for review |
@thaJeztah OK. |
@thaJeztah Fine, I will do it soon. |
thanks @albers and @sdurrheimer!! |
ping @moxiegirl |
@@ -643,4 +644,20 @@ set like this: | |||
/usr/local/bin/docker daemon -D -g /var/lib/docker -H unix:// > /var/lib/docker-machine/docker.log 2>&1 | |||
|
|||
|
|||
# Default cgroup parent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LK4D4 Is this value only valid when the native.cgroupdriver
is cgroupsfs
? Or does it work if that value is also systemd
? I'd change the text depending on your answer.
Regarding the material, it took me a few tries to untangle it. I'm sorry I couldn't test it entirely, but I think this might be better way to phrase the section. Please review.
--->
Default cgroup parent
The cgroups (control group) feature is used to limit a container's access to the
host's resources. The --cgroup-parent
option sets the default cgroup parent
for all containers. You can also set this option per container when you call
docker create
or docker run
commands. The subcommand value overrides the
daemon's --cgroup-parent
option setting.
The Docker host stores cgroups in relation to the /sys/fs/cgroup/memory
root
directory. Where the cgroup is created in relation to this root directory,
depends on whether and how you specify the --cgroup-parent
option value. If
you don't specify the --cgroup-parent
option, the daemon uses
/sys/fs/cgroup/memory/docker
as the parent.
If you set this option, the option value can start with a leading forward slash (/
)
or without one. The presence or absence of a slash determines where the cgroup
is located in the /sys/fs/cgroup/memory
directory. If you specify a slash in
the value, for example --cgroup-parent=/foobar
, Docker creates a
/sys/fs/cgroup/memory/foobar
folder.
If you don't include the slash, Docker creates a
/sys/fs/cgroup/memory/daemoncgroup
folder and places your group within it. For
example, if you specify --cgroup-parent=foobar
the system creates the cgroup
in the /sys/fs/cgroup/memory/daemoncgroup/foobar
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work for systemd driver if the argument is a systemd slice.
Sent from my iPhone
On Dec 19, 2015, at 8:21 PM, moxiegirl [email protected] wrote:
In docs/reference/commandline/daemon.md:
@@ -643,4 +644,20 @@ set like this:
/usr/local/bin/docker daemon -D -g /var/lib/docker -H unix:// > /var/lib/docker-machine/docker.log 2>&1+# Default cgroup parent
@LK4D4 Is this value only valid when the native.cgroupdriver is cgroupsfs? Or does it work if that value is also systemd?—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrunalp can you provide me with an example of specifying the option in this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like "containers.slice"
Sent from my iPhone
On Dec 19, 2015, at 9:27 PM, moxiegirl [email protected] wrote:
In docs/reference/commandline/daemon.md:
@@ -643,4 +644,20 @@ set like this:
/usr/local/bin/docker daemon -D -g /var/lib/docker -H unix:// > /var/lib/docker-machine/docker.log 2>&1+# Default cgroup parent
@mrunalp can you provide me with an example of specifying the option in this way?—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@moxiegirl: What you have is a good start. Couple of high level comments:
- This option is available both for "docker daemon" and "docker run". We can add help at the daemon level and reference it from the run subcommand (or vice versa).
- Given that the behaviour is different depending on native.cgroupdriver choice, it's better to split this section by the same (native.cgroupdriver)
Proceeding to review what you have for cgroupfs:
- Avoid having references to the cgroup memory subsystem alone. For ex, /sys/fs/cgroup/memory. All cgroup subsystems listed in /proc/subsystems will have the newly created cgroup.
- Reference to daemoncgroup sounds incorrect. Can you point the source of this info?
For cgroup-parent option on systemd cgroups, you can use the following content:
When cgroups are managed by systemd (using --exec-opt native.cgroupdriver=systemd), containers are created by default under system.slice. Custom systemd slices can be used to group containers and control resource limits on them. Custom slices can be created by adding slice units in /etc/systemd/system. For example, create a limits.slice (ref. http://www.freedesktop.org/software/systemd/man/systemd.resource-control.html)
Run containers in limits.slice using --cgroup-parent:
docker run -it --cgroup-parent=limits.slice busybox sh
docker run -it --cgroup-parent=limits.slice ubuntu bash
Using systemd-cgls, observe that both containers are created under limits.slice. Note that limits.slice is in the same hierarchical level as system.slice.
├─system.slice
│ ├─thermald.service
│ │ └─654 /usr/sbin/thermald --no-daemon --dbus-enable
│ ├─dbus.service
│ │ ├─ 728
...
├─limits.slice
│ ├─docker-d8119d36eea49874ddfa8e8f83c3aa72730b3e57ae6cb43bfd4d299154a24a5c.scope
│ │ └─2330 bash
│ └─docker-d4f053a9bbefb30831ead236ed9716522f3eb2cc50d3443f30ea41f4da766939.scope
│ └─2284 sh
For each container, a docker-containerID.scope is created under /sys/fs/cgroup/subsystem/<cgroup-parent
@anusha-ragunathan Great feedback, as I suspected there was more. My preference is to actually improve the documentation around cgroups and pull all the detail into there --- rather than duplicate it among so many commands. The commands can ref the detail page. I'll come chat to you about that. |
@moxiegirl do you want to update the docs as part of this PR, or do you want to merge this PR, and address the docs in a follow-up? Just checking to see if we can move this PR forward @LK4D4 unfortunately, this does need a rebase now 😢 |
@thaJeztah I'll carry the docs on this. We need a redo of the cgroups section in general. |
@moxiegirl thanks! @LK4D4 can you rebase this so that we can merge? |
Because @LK4D4 is on PTO, I asked @anusha-ragunathan to carry the rebase so that we can merge this ❤️ |
@anusha-ragunathan I think this can be closed, because it's carried in #19062? |
Yes @thaJeztah |
Fix #18022
ping @vishh