[RFC] libcontainerd/supervisor: use containerd's defaults by thaJeztah · Pull Request #43950 · moby/moby

thaJeztah · 2022-08-11T17:14:52Z

relates to libcontainerd/supervisor: add support for custom containerd.toml config #43946
relates to libcontainerd/supervisor: refactoring and cleanup in preparation of custom containerd.toml #43945 (libcontainerd/supervisor: refactoring and cleanup in preparation of custom containerd.toml #43945 (comment))

When starting a managed containerd instance, we currently configure the instance
to use alternative runtime and storage locations, and alternative paths for the
socket (for Windows, we used the same named pipes).

The reason for this was that we didn't want to conflict with an already running
containerd service when manually starting dockerd. As we currently only use
containerd as runtime (but not for image management), this works well, as all
(most) information in containerd is considered ephemeral, and we re-create
containers when starting the daemon.

However, with work being in progress on using containerd also for image management,
containerd's storage can no longer be considered ephemeral, and using the
alternative configuration will limit the ability to (temporarily) start
dockerd manually for debugging, i.e.;

systemctr stop containerd docker
dockerd --debug

When using containerd for image storage, the above will now start a containerd
instance with a different storage location as the one that was running as
(systemd) service, and does not contain the images and snapshots that are
expected.

This patch changes the managed containerd configuration to use most of containerd's
default settings, with the exception of the CRI plugin being disabled by default
on the managed instance (which can be enabled using the --cri-containerd flag).

The daemon will still detect if an instance of containerd is already running, so
that in situations where the docker service was stopped, but the containerd
service is still running, the already running containerd service to be used
(assuming it's using the default location).

To be discussed

This patch currently does not contain any migration code, so no content is migrated
from the "old" location to the new; as most (all?) data we add to containerd is
ephemeral (except for manifests we store?) we may not have to do any migration,
but we should look if anything is needed.

- Description for the changelog

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

thaJeztah · 2022-08-11T17:16:17Z

This might explode in CI, as we're spinning up many daemons there 🤔 😂 , so possibly some option is needed to make storage and socket configurable.

thaJeztah · 2022-08-11T20:07:52Z

We discussed this in the maintainers meeting, and there's a lot of different angles on this change; this change will have both "pros" and "const", but so does the old configuration. Some of this scenarios can be a big concern, so we need to write down possible scenarios and pros/cons for each of them.

thaJeztah · 2022-08-12T10:02:03Z

libcontainerd/supervisor/remote_daemon.go

-			Root:    filepath.Join(rootDir, "daemon"),
-			State:   filepath.Join(stateDir, "daemon"),


I just noticed that we may have an issue here; it looks like libcontainerd client has these paths effectively hard-coded to be inside /var/run/docker/containerd/; at least, I see no code that checks what containerd's directory is for those.

moby/daemon/daemon_unix.go

Line 1738 in 6e21389

filepath.Join(daemon.configStore.ExecRoot, "containerd"),

moby/daemon/daemon_windows.go

Line 651 in 49f021e

filepath.Join(daemon.configStore.ExecRoot, "containerd"),

moby/libcontainerd/remote/client_linux.go

Line 61 in 7b9275c

func WithBundle(bundleDir string, ociSpec *specs.Spec) containerd.NewContainerOpts {

moby/libcontainerd/remote/client.go

Lines 904 to 906 in 5de7704

func (c *client) bundleDir(id string) string {

return filepath.Join(c.stateDir, id)

}

For plugins (linux) it uses /run/docker/plugins;

moby/daemon/daemon_linux.go

Lines 23 to 25 in a71463b

func getPluginExecRoot(root string) string {

return "/run/docker/plugins"

}

For windows, something under the daemon's "root" directory (which is confusing, as "root" (data) is used as "exec" (runtime) path;

moby/daemon/daemon_windows.go

Lines 58 to 60 in a71463b

func getPluginExecRoot(root string) string {

return filepath.Join(root, "plugins")

}

Oh, I guess part of this is addressed in rumpl#40, but we probably still need to look for the "not-containerd as snapshotter" situation with a custom configuration? 🤔

ctr -n moby c ls CONTAINER IMAGE RUNTIME 88029cbb7fce14c22801513c7fd551fe1a1918719a33dd5f3fbfff3100e3c120 - io.containerd.runc.v2 ls /var/run/docker/containerd 88029cbb7fce14c22801513c7fd551fe1a1918719a33dd5f3fbfff3100e3c120

And creating a container through ctr;

ctr -n moby image pull docker.io/lilbrary/nginx:alpine ctr -n moby container create docker.io/library/nginx:alpine mycontainer ctr -n moby task start --detach mycontainer ctr -n moby container ls CONTAINER IMAGE RUNTIME 88029cbb7fce14c22801513c7fd551fe1a1918719a33dd5f3fbfff3100e3c120 - io.containerd.runc.v2 mycontainer docker.io/library/nginx:alpine io.containerd.runc.v2

So, the container does not end up in /var/run/docker/containerd

But the task does;

ls /var/run/containerd/io.containerd.runtime.v2.task/moby/ 88029cbb7fce14c22801513c7fd551fe1a1918719a33dd5f3fbfff3100e3c120 mycontainer

tianon · 2022-08-31T18:26:08Z

IMO this is good -- this won't affect most Linux users who should have "proper" systemd dependencies, and for Docker-in-Docker users this is going to be a transition no matter how it works (this just makes that transition more straightforward by having containerd use the expected locations, config, etc no matter how it's started).

When starting a managed containerd instance, we currently configure the instance to use alternative runtime and storage locations, and alternative paths for the socket (for Windows, we used the same named pipes). The reason for this was that we didn't want to conflict with an already running containerd service when manually starting dockerd. As we currently only use containerd as runtime (but not for image management), this works well, as all (most) information in containerd is considered ephemeral, and we re-create containers when starting the daemon. However, with work being in progress on using containerd also for image management, containerd's storage can no longer be considered ephemeral, and using the alternative configuration will limit the ability to (temporarily) start dockerd manually for debugging, i.e.; ```bash systemctr stop containerd docker dockerd --debug ``` When using containerd for image storage, the above will now start a containerd instance with a different storage location as the one that was running as (systemd) service, and does not contain the images and snapshots that are expected. This patch changes the managed containerd configuration to use most of containerd's default settings, with the exception of the CRI plugin being disabled by default on the managed instance (which can be enabled using the `--cri-containerd` flag). The daemon will still detect if an instance of containerd is already running, so that in situations where the `docker` service was stopped, but the `containerd` service is still running, the already running `containerd` service to be used (assuming it's using the default location). To be discussed ------------------------- This patch currently does not contain any migration code, so no content is migrated from the "old" location to the new; as most (all?) data we add to containerd is ephemeral (except for manifests we store?) we may not have to do any migration, but we should look if anything is needed. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah added area/runtime Runtime status/1-design-review impact/changelog docs/revisit impact/documentation containerd-integration Issues and PRs related to containerd integration labels Aug 11, 2022

thaJeztah added this to the v-next milestone Aug 11, 2022

thaJeztah changed the title ~~libcontainerd/supervisor: use containerd's defaults~~ [RFC] libcontainerd/supervisor: use containerd's defaults Aug 11, 2022

thaJeztah marked this pull request as draft August 11, 2022 20:06

thaJeztah commented Aug 12, 2022

View reviewed changes

thaJeztah mentioned this pull request Feb 1, 2023

Dockerfile: Make it easier to use ctr in shell container #44896

Merged

thaJeztah force-pushed the containerd_defaults branch 3 times, most recently from 57bc789 to 056f405 Compare March 9, 2023 17:34

rumpl modified the milestones: 24.0.0, v-future Apr 24, 2023

thaJeztah mentioned this pull request Jun 13, 2023

Update dockerd to support JSON logging format #45737

Merged

thaJeztah force-pushed the containerd_defaults branch from 056f405 to 3ad1e71 Compare June 13, 2023 22:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] libcontainerd/supervisor: use containerd's defaults#43950

[RFC] libcontainerd/supervisor: use containerd's defaults#43950
thaJeztah wants to merge 1 commit intomoby:masterfrom
thaJeztah:containerd_defaults

thaJeztah commented Aug 11, 2022

Uh oh!

thaJeztah commented Aug 11, 2022

Uh oh!

thaJeztah commented Aug 11, 2022

Uh oh!

thaJeztah Aug 12, 2022

Uh oh!

thaJeztah Aug 12, 2022 •

edited

Loading

Uh oh!

tianon commented Aug 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Root: filepath.Join(rootDir, "daemon"),
		State: filepath.Join(stateDir, "daemon"),

	func (c *client) bundleDir(id string) string {
	return filepath.Join(c.stateDir, id)
	}

	func getPluginExecRoot(root string) string {
	return "/run/docker/plugins"
	}

	func getPluginExecRoot(root string) string {
	return filepath.Join(root, "plugins")
	}

Conversation

thaJeztah commented Aug 11, 2022

To be discussed

Uh oh!

thaJeztah commented Aug 11, 2022

Uh oh!

thaJeztah commented Aug 11, 2022

Uh oh!

thaJeztah Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

thaJeztah Aug 12, 2022 • 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!

tianon commented Aug 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thaJeztah Aug 12, 2022 •

edited

Loading