cgroup2: implement `docker info` by AkihiroSuda · Pull Request #40662 · moby/moby

AkihiroSuda · 2020-03-10T12:46:55Z

- What I did

Implemented docker info for cgroup v2.

The REST API now includes .CgroupVersion field.

- How I did it
Updated pkg/sysinfo to parse cgroup v2 info.

ref: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html

- How to verify it
Run curl --unix-socket /var/run/docker.sock http://localhost/v1.41/info.

Rootful:

json.CgroupDriver = "cgroupfs";
json.CgroupVersion = "2";
...
json.Warnings[0] = "WARNING: No kernel memory limit support";
json.Warnings[1] = "WARNING: No kernel memory TCP limit support";
json.Warnings[2] = "WARNING: No oom kill disable support";
json.Warnings[3] = "WARNING: Support for cgroup v2 is experimental";

Rootless (No systemd):

json.CgroupDriver = "none";
json.CgroupVersion = "2";
...
json.Warnings[0] = "WARNING: No cgroup support";

Rootless (With --exec-opt native.cgroupdriver=systemd, Delegate=pids memory cpu):

json.CgroupDriver = "systemd";
json.CgroupVersion = "2";
...
json.Warnings[0] = "WARNING: No kernel memory limit support";
json.Warnings[1] = "WARNING: No kernel memory TCP limit support";
json.Warnings[2] = "WARNING: No oom kill disable support";
json.Warnings[3] = "WARNING: No cpuset support";
json.Warnings[4] = "WARNING: Support for cgroup v2 is experimental";

- Description for the changelog

cgroup2: implement docker info

- A picture of a cute animal (not mandatory but encouraged)
🐧

api/server/router/system/system_routes.go

api/swagger.yaml

api/server/router/system/system_routes.go

thaJeztah · 2020-03-11T21:00:14Z

daemon/daemon_unsupported.go

should this just return nil or panic(unsupported) ?

or not implement this at all? 🤔

I think that is up to pkg/sysinfo implementation.

thaJeztah · 2020-03-12T09:43:31Z

daemon/daemon_unix.go

Is there any situation where we want sysinfo.New() to not use this path if ROOTLESSKIT_PARENT_EUID is set?

Just wondering if we should make sysinfo.New() just do the right thing. I see it already uses a findCgroupMountpoints() function

Also; looks like we may want to sanitise rootlesskitParentEUID before using

A very quick attempt;
suggestions.patch.txt

Details

diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 46a62b5d2f..8b59189d57 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -45,7 +45,6 @@ import ( "github.com/docker/docker/pkg/pidfile" "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/pkg/signal" - "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/pkg/system" "github.com/docker/docker/plugin" "github.com/docker/docker/rootless" @@ -459,11 +458,7 @@ func warnOnDeprecatedConfigOptions(config *config.Config) { } func initRouter(opts routerOptions) { - decoder := runconfig.ContainerDecoder{ - GetSysInfo: func() *sysinfo.SysInfo { - return opts.daemon.RawSysInfo(true) - }, - } + decoder := runconfig.ContainerDecoder{} routers := []router.Router{ // we need to add the checkpoint router before the container router or the DELETE gets masked diff --git a/daemon/daemon.go b/daemon/daemon.go index 35d83d6148..7df8d7f957 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -20,10 +20,6 @@ import ( "sync" "time" - "github.com/docker/docker/pkg/fileutils" - "google.golang.org/grpc" - "google.golang.org/grpc/backoff" - "github.com/containerd/containerd" "github.com/containerd/containerd/defaults" "github.com/containerd/containerd/pkg/dialer" @@ -38,27 +34,23 @@ import ( "github.com/docker/docker/daemon/discovery" "github.com/docker/docker/daemon/events" "github.com/docker/docker/daemon/exec" + _ "github.com/docker/docker/daemon/graphdriver/register" // register graph drivers "github.com/docker/docker/daemon/images" "github.com/docker/docker/daemon/logger" "github.com/docker/docker/daemon/network" - "github.com/docker/docker/errdefs" - "github.com/moby/buildkit/util/resolver" - "github.com/moby/buildkit/util/tracing" - rsystem "github.com/opencontainers/runc/libcontainer/system" - "github.com/sirupsen/logrus" - - // register graph drivers - _ "github.com/docker/docker/daemon/graphdriver/register" "github.com/docker/docker/daemon/stats" dmetadata "github.com/docker/docker/distribution/metadata" "github.com/docker/docker/dockerversion" + "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/docker/docker/libcontainerd" libcontainerdtypes "github.com/docker/docker/libcontainerd/types" + "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/locker" "github.com/docker/docker/pkg/plugingetter" + "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/pkg/system" "github.com/docker/docker/pkg/truncindex" "github.com/docker/docker/plugin" @@ -70,8 +62,14 @@ import ( "github.com/docker/libnetwork" "github.com/docker/libnetwork/cluster" nwconfig "github.com/docker/libnetwork/config" + "github.com/moby/buildkit/util/resolver" + "github.com/moby/buildkit/util/tracing" + rsystem "github.com/opencontainers/runc/libcontainer/system" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "golang.org/x/sync/semaphore" + "google.golang.org/grpc" + "google.golang.org/grpc/backoff" ) // ContainersNamespace is the name of the namespace used for users containers @@ -1044,7 +1042,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, err } - sysInfo := d.RawSysInfo(false) + sysInfo := sysinfo.New(false) // Check if Devices cgroup is mounted, it is hard requirement for container security, // on Linux. if runtime.GOOS == "linux" && !sysInfo.CgroupDevicesEnabled && !rsystem.RunningInUserNS() { diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 04a9beda4d..5763298dbc 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -643,7 +643,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes. if hostConfig == nil { return nil, nil } - sysInfo := daemon.RawSysInfo(true) + sysInfo := sysinfo.New(true) w, err := verifyPlatformContainerResources(&hostConfig.Resources, sysInfo, update) @@ -1631,11 +1631,11 @@ func (daemon *Daemon) initCgroupsPath(path string) error { } path = filepath.Join(mnt, root, path) - sysinfo := daemon.RawSysInfo(true) - if err := maybeCreateCPURealTimeFile(sysinfo.CPURealtimePeriod, daemon.configStore.CPURealtimePeriod, "cpu.rt_period_us", path); err != nil { + sysInfo := sysinfo.New(true) + if err := maybeCreateCPURealTimeFile(sysInfo.CPURealtimePeriod, daemon.configStore.CPURealtimePeriod, "cpu.rt_period_us", path); err != nil { return err } - return maybeCreateCPURealTimeFile(sysinfo.CPURealtimeRuntime, daemon.configStore.CPURealtimeRuntime, "cpu.rt_runtime_us", path) + return maybeCreateCPURealTimeFile(sysInfo.CPURealtimeRuntime, daemon.configStore.CPURealtimeRuntime, "cpu.rt_runtime_us", path) } func maybeCreateCPURealTimeFile(sysinfoPresent bool, configValue int64, file string, path string) error { @@ -1665,16 +1665,3 @@ func (daemon *Daemon) setupSeccompProfile() error { func (daemon *Daemon) useShimV2() bool { return cgroups.IsCgroup2UnifiedMode() } - -// RawSysInfo returns *sysinfo.SysInfo . -func (daemon *Daemon) RawSysInfo(quiet bool) *sysinfo.SysInfo { - var opts []sysinfo.Opt - if daemon.getCgroupDriver() == cgroupSystemdDriver { - rootlesskitParentEUID := os.Getenv("ROOTLESSKIT_PARENT_EUID") - if rootlesskitParentEUID != "" { - groupPath := fmt.Sprintf("/user.slice/user-%s.slice", rootlesskitParentEUID) - opts = append(opts, sysinfo.WithCgroup2GroupPath(groupPath)) - } - } - return sysinfo.New(quiet, opts...) -} diff --git a/daemon/daemon_unsupported.go b/daemon/daemon_unsupported.go index 4c2476edcf..e6bcbdaab1 100644 --- a/daemon/daemon_unsupported.go +++ b/daemon/daemon_unsupported.go @@ -4,15 +4,9 @@ package daemon // import "github.com/docker/docker/daemon" import ( "github.com/docker/docker/daemon/config" - "github.com/docker/docker/pkg/sysinfo" ) const platformSupported = false func setupResolvConf(config *config.Config) { } - -// RawSysInfo returns *sysinfo.SysInfo . -func (daemon *Daemon) RawSysInfo(quiet bool) *sysinfo.SysInfo { - return sysinfo.New(quiet) -} diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 7fffbfb609..021b7b8f0a 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -657,8 +657,3 @@ func setupResolvConf(config *config.Config) { func (daemon *Daemon) useShimV2() bool { return true } - -// RawSysInfo returns *sysinfo.SysInfo . -func (daemon *Daemon) RawSysInfo(quiet bool) *sysinfo.SysInfo { - return sysinfo.New(quiet) -} diff --git a/daemon/info.go b/daemon/info.go index 91eb826f8a..cb1d39b04b 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -28,7 +28,7 @@ import ( func (daemon *Daemon) SystemInfo() (*types.Info, error) { defer metrics.StartTimer(hostInfoFunctions.WithValues("system_info"))() - sysInfo := daemon.RawSysInfo(true) + sysInfo := sysinfo.New(true) cRunning, cPaused, cStopped := stateCtr.get() v := &types.Info{ diff --git a/pkg/sysinfo/sysinfo_linux.go b/pkg/sysinfo/sysinfo_linux.go index c619559e12..de15166329 100644 --- a/pkg/sysinfo/sysinfo_linux.go +++ b/pkg/sysinfo/sysinfo_linux.go @@ -106,7 +106,11 @@ func newV2(quiet bool, opts *opts) *SysInfo { } g := opts.cg2GroupPath if g == "" { - g = "/" + if euid := os.Getenv("ROOTLESSKIT_PARENT_EUID"); euid != "" { + g = fmt.Sprintf("/user.slice/user-%s.slice", euid) + } else { + g = "/" + } } m, err := cgroupsV2.LoadManager("/sys/fs/cgroup", g) if err != nil {

g = fmt.Sprintf("/user.slice/user-%s.slice", euid) is expected only when running in systemd mode

"systemd mode" == "cgroupv2" or something else? (my patch above handles it in newV2(), or is that incorrect?)

"systemd mode" == "--exec-opt native.cgroupdriver=systemd" (&& v2, in this context)

daemon/info_unix.go

pkg/sysinfo/sysinfo_linux.go

AkihiroSuda · 2020-03-25T01:35:54Z

@thaJeztah PTAL

AkihiroSuda · 2020-03-30T21:05:03Z

@kolyshkin PTAL?

AkihiroSuda · 2020-04-02T09:04:02Z

updated

AkihiroSuda · 2020-04-06T20:57:24Z

@thaJeztah @kolyshkin PTAL?

kolyshkin

SGTM

AkihiroSuda · 2020-04-15T05:40:15Z

rebased

api/swagger.yaml

api/types/types.go

cpuguy83 · 2020-04-16T18:35:15Z

daemon/daemon_unix.go

I'm not sure I understand the need for this.
If we are adding options to the wrapped function call, why not add the operations in RawSysInfo as an option that callers can add when neccessary?

Because sysinfo.New() should be decoupled from the daemon, and should not call daemon.getCgroupDriver() and os.Getenv("ROOTLESSKIT_PARENT_EUID")

Could we expose the sysinfo option fields?

Could you be more specific?

ref: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

cpuguy83

LGTM

I'm not sure what to do with this new info function right now, but it's not changing any public interfaces, so I think it's fine.

AkihiroSuda · 2020-04-24T20:48:57Z

@thaJeztah Ready to merge?

thaJeztah

LGTM

I'm not sure what to do with this new info function right now, but it's not changing any public interfaces, so I think it's fine.

We discussed this in the maintainers meeting, and (hopefully) we could find a slightly cleaner/simpler approach, but as @cpuguy83 mentioned; it's not an "public" interface, some should still be able to make such changes in future

AkihiroSuda mentioned this pull request Mar 10, 2020

cgroup2 check list #40360

Closed

8 tasks

AkihiroSuda added impact/api impact/changelog labels Mar 10, 2020

AkihiroSuda added this to the 20.03.0 milestone Mar 10, 2020

AkihiroSuda force-pushed the cgroup2-dockerinfo branch from 105b8c2 to a649a6d Compare March 12, 2020 01:50

tao12345666333 reviewed Mar 12, 2020

View reviewed changes

api/server/router/system/system_routes.go Outdated Show resolved Hide resolved

AkihiroSuda force-pushed the cgroup2-dockerinfo branch 2 times, most recently from e6feebc to 58b3385 Compare March 12, 2020 09:24

thaJeztah requested changes Mar 12, 2020

View reviewed changes

thaJeztah added the status/2-code-review label Mar 12, 2020

AkihiroSuda force-pushed the cgroup2-dockerinfo branch 3 times, most recently from 0505663 to e219912 Compare March 14, 2020 10:14

AkihiroSuda force-pushed the cgroup2-dockerinfo branch from e219912 to 51fb1b9 Compare April 2, 2020 09:03

kolyshkin approved these changes Apr 6, 2020

View reviewed changes

AkihiroSuda force-pushed the cgroup2-dockerinfo branch from 51fb1b9 to daf5d13 Compare April 15, 2020 05:40

thaJeztah reviewed Apr 16, 2020

View reviewed changes

api/swagger.yaml Outdated Show resolved Hide resolved

cpuguy83 reviewed Apr 16, 2020

View reviewed changes

api/types/types.go Outdated Show resolved Hide resolved

cpuguy83 reviewed Apr 16, 2020

View reviewed changes

AkihiroSuda force-pushed the cgroup2-dockerinfo branch 2 times, most recently from a795d44 to f7dbbe6 Compare April 16, 2020 22:12

cgroup2: implement docker info

f350b53

ref: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda force-pushed the cgroup2-dockerinfo branch from f7dbbe6 to f350b53 Compare April 16, 2020 22:20

cpuguy83 approved these changes Apr 23, 2020

View reviewed changes

thaJeztah approved these changes Apr 29, 2020

View reviewed changes

thaJeztah merged commit c3b3aed into moby:master Apr 29, 2020

AkihiroSuda mentioned this pull request Apr 29, 2020

info: print Cgroup Version docker/cli#2479

Merged

AkihiroSuda added the area/cgroup2 cgroup v2 label Jan 23, 2021

xruins mentioned this pull request May 20, 2022

Missing some fields to support Stats API on Docker host uses cgroup2 fsouza/go-dockerclient#919

Closed

Conversation

AkihiroSuda commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AkihiroSuda commented Mar 25, 2020

Uh oh!

AkihiroSuda commented Mar 30, 2020

Uh oh!

AkihiroSuda commented Apr 2, 2020

Uh oh!

AkihiroSuda commented Apr 6, 2020

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Apr 15, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Apr 24, 2020

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AkihiroSuda commented Mar 10, 2020 •

edited

Loading

AkihiroSuda Apr 16, 2020 •

edited

Loading