Add plugins for server listeners by dmcgowan · Pull Request #12562 · containerd/containerd

dmcgowan · 2025-11-24T07:21:54Z

Add new plugin types for server listeners and metrics (which are used by the servers).

Simplifies server startup
Relies on plugin lifecycle for all long running processes
Allows extending containerd's set of listeners and their dependencies
Separates the server configuration based on the listeners

Signed-off-by: Derek McGowan <derek@mcg.dev>

vvoland · 2026-04-15T09:10:22Z

+			config.Plugins["io.containerd.server.v1.ttrpc"] = ttrpcConfig
+		}
+
+		return nil


This one will return early and skip applyPlatformFlags below, I don't think this was intended?

vvoland · 2026-04-15T09:15:51Z

-	Address       string `toml:"address"`
-	GRPCHistogram bool   `toml:"grpc_histogram"`
+	// deprecated: use server plugin io.containerd.server.v1.metrics
+	Address string `toml:"address"`


Should this one also be omitempty like the others?

prakleumas

^{beep boop}
Hello! I am @samuelkarp's friendly code review robot 🤖! Sam asked me to take a look and post my comments directly on GitHub.

Please treat my comments as suggestions and advisory, not mandates. Since I am an LLM and my output is used here directly, you should evaluate carefully and see if my suggestions make sense. Feel free to directly resolve any you disagree with.

^{This is not an automated action. This bot does not read or reply to comments.}

prakleumas · 2026-04-18T00:03:35Z

+			srv, ok := instance.(server)
+			if !ok {
+				log.G(ctx).WithField("id", id).Warn("plugin does not implement server interface, will not be started")
+			}
+			s.servers = append(s.servers, srv)


If the plugin instance does not implement the server interface, appending a nil interface here introduces the opportunity for a nil pointer dereference when Start is later invoked on s.servers. Would it be safer to protect the append operation within the type assertion's success branch?

Suggested change

srv, ok := instance.(server)

if !ok {

log.G(ctx).WithField("id", id).Warn("plugin does not implement server interface, will not be started")

}

s.servers = append(s.servers, srv)

if srv, ok := instance.(server); ok {

s.servers = append(s.servers, srv)

} else {

log.G(ctx).WithField("id", id).Warn("plugin does not implement server interface, will not be started")

}

prakleumas · 2026-04-18T00:03:35Z

 		go func() {
-			defer l.Close()
-			if err := server.ServeGRPC(l); err != nil {
-				log.G(ctx).WithError(err).WithField("address", srvconfig.GRPC.Address).Fatal("serve failure")
+			defer server.Stop()
+			if err := server.Start(ctx); err != nil {
+				log.G(ctx).WithError(err).WithField("address", defaultAddress).Fatal("serve failure")
 			}
 		}()


Because server.Start(ctx) is now non-blocking (it delegates listener execution to background goroutines via internal.Serve), this wrapper goroutine will return almost immediately. This introduces a race condition where defer server.Stop() executes instantly, tearing down the daemon before the fuzzer can meaningfully exercise it. Is there a specific reason to run this asynchronously, or would it be safer to bind the shutdown phase to the context's lifecycle?

Suggested change

go func() {

defer l.Close()

if err := server.ServeGRPC(l); err != nil {

log.G(ctx).WithError(err).WithField("address", srvconfig.GRPC.Address).Fatal("serve failure")

defer server.Stop()

if err := server.Start(ctx); err != nil {

log.G(ctx).WithError(err).WithField("address", defaultAddress).Fatal("serve failure")

}

}()

go func() {

defer server.Stop()

if err := server.Start(ctx); err != nil {

log.G(ctx).WithError(err).WithField("address", defaultAddress).Fatal("serve failure")

}

<-ctx.Done()

}()

prakleumas · 2026-04-18T00:03:37Z

+	for _, k := range key {
+		if v, ok := properties[k]; ok {
+			switch v := v.(type) {
+			case map[string]any:
+				properties = v
+				continue
+			case string:
+				return v
+			case fmt.Stringer:
+				return v.String()
+			case []byte:
+				return string(v)
+			}
+		}


It appears that readString will eagerly return a value if an intermediate key in the path resolves to a string, fmt.Stringer, or []byte. For example, if a configuration mistakenly provides a string for a parent key, this function returns that string instead of continuing the search or returning an empty string. Could we ensure that it only returns a leaf value if it has successfully reached the final key in the path?

Suggested change

for _, k := range key {

if v, ok := properties[k]; ok {

switch v := v.(type) {

case map[string]any:

properties = v

continue

case string:

return v

case fmt.Stringer:

return v.String()

case []byte:

return string(v)

}

}

for i, k := range key {

if v, ok := properties[k]; ok {

switch v := v.(type) {

case map[string]any:

properties = v

continue

case string:

if i == len(key)-1 {

return v

}

case fmt.Stringer:

if i == len(key)-1 {

return v.String()

}

case []byte:

if i == len(key)-1 {

return string(v)

}

}

}

prakleumas · 2026-04-18T00:03:37Z

 	return nil
 }

+func serviceMigrate(ctx context.Context, c *Config) error {


Can we add a new test that covers the correctness of serviceMigrate? This is implicitly covered by higher-level configuration test flows, but it'd be good to validate at least some of the basic property mapping logic (e.g. ensuring GRPC and TTRPC settings migrate accurately to the new plugins map) directly in a unit test.

prakleumas · 2026-04-18T00:03:37Z

+	// deprecated: use server plugins io.containerd.server.v1.grpc and io.containerd.server.v1.grpc-tcp
+	GRPC GRPCConfig `toml:"grpc,omitempty"`
+	// TTRPC configuration settings
+	// deprecated: use server plugin io.containerd.server.v1.ttrpc
+	TTRPC TTRPCConfig `toml:"ttrpc,omitempty"`
+	// Metrics and monitoring settings
+	// deprecated: use server plugin io.containerd.server.v1.metrics
+	Metrics MetricsConfig `toml:"metrics,omitempty"`


To establish a formal contract with standard Go static analysis tools (such as go doc and go vet), deprecation notices rely on the exact prefix Deprecated: (with a capital 'D'). It is generally safer to align with this convention to ensure IDEs and downstream consumers accurately recognize these fields as phased out during the version 3 to 4 lifecycle transition.

Suggested change

// deprecated: use server plugins io.containerd.server.v1.grpc and io.containerd.server.v1.grpc-tcp

GRPC GRPCConfig `toml:"grpc,omitempty"`

// TTRPC configuration settings

// deprecated: use server plugin io.containerd.server.v1.ttrpc

TTRPC TTRPCConfig `toml:"ttrpc,omitempty"`

// Metrics and monitoring settings

// deprecated: use server plugin io.containerd.server.v1.metrics

Metrics MetricsConfig `toml:"metrics,omitempty"`

// Deprecated: use server plugins io.containerd.server.v1.grpc and io.containerd.server.v1.grpc-tcp

GRPC GRPCConfig `toml:"grpc,omitempty"`

// TTRPC configuration settings

// Deprecated: use server plugin io.containerd.server.v1.ttrpc

TTRPC TTRPCConfig `toml:"ttrpc,omitempty"`

// Metrics and monitoring settings

// Deprecated: use server plugin io.containerd.server.v1.metrics

Metrics MetricsConfig `toml:"metrics,omitempty"`

(It is recommended to apply this capitalization to the field-level comments inside the GRPCConfig, TTRPCConfig, Debug, and MetricsConfig structs as well.)

prakleumas · 2026-04-18T00:03:37Z

+		Requires: []plugin.Type{
+			plugins.HTTPHandler,
+		},


Is there a specific reason plugins.HTTPHandler is listed as a requirement here? Unlike the debug server plugin, I don't see this plugin querying the initialization context for HTTP handlers. If it's not strictly necessary, it is generally safer to omit the requirement to prevent unintentional initialization ordering constraints.

Suggested change

Requires: []plugin.Type{

plugins.HTTPHandler,

},

prakleumas · 2026-04-18T00:03:37Z

+		grpcAddress  = readString(config.Plugins, "io.containerd.server.v1.grpc", "address")
+		ttrpcAddress = readString(config.Plugins, "io.containerd.server.v1.ttrpc", "address")


What is the expected behavior for ttrpcAddress if a user provides a v4 configuration that specifies a custom GRPC address but omits the TTRPC plugin block entirely? Previously, the TTRPC address dynamically tracked the GRPC address (grpcAddress + ".ttrpc"). Without that fallback here, readString will return an empty string, meaning plugins.PropertyTTRPCAddress will be empty for dependent plugins. Additionally, the TTRPC server plugin itself may bind to the global default rather than tracking the custom GRPC address. It might be worth re-introducing a fallback mechanism to guarantee they stay linked when unconfigured.

I think this is a reasonable change for v4 configuration. Since any migrated configuration would keep the same behavior, this would not be a breaking change, since v4 is a new configuration version. I'll make sure to document this, that the values are now independent and when ttrpc is not set, it will default to the ttrpc default, not derived from the grpc address.

Copilot

Pull request overview

Adds a new plugin-driven model for containerd server listeners (gRPC/TTRPC/debug/metrics) and introduces config version 4 to migrate legacy top-level listener settings into plugin configuration, simplifying daemon startup and making listeners extensible via the plugin lifecycle.

Changes:

Bump config version to 4 and add a v3→v4 migration that moves GRPC/TTRPC/debug/metrics settings into [plugins."io.containerd.server.v1.*"].
Introduce new plugin types (io.containerd.server.v1, io.containerd.metrics.v1) and implement listener plugins for gRPC/TCP gRPC/TTRPC/debug/metrics.
Refactor daemon startup to start listener plugins via Server.Start(), and update docs/tests/fuzz config generation accordingly.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
version/version.go	Bumps `ConfigVersion` to 4.
plugins/types.go	Adds new plugin types for server listeners and metrics.
plugins/server/ttrpc/server_other.go	Adds non-linux/windows/solaris ttrpc server constructor.
plugins/server/ttrpc/server_otel.go	Adds windows/solaris ttrpc server constructor with otel interceptor.
plugins/server/ttrpc/server_linux.go	Updates linux ttrpc server constructor (same-user handshaker + otel).
plugins/server/ttrpc/plugin.go	Registers TTRPC listener as a `ServerPlugin` and serves via shared helper.
plugins/server/metrics/plugin.go	Adds metrics HTTP listener as a `ServerPlugin`.
plugins/server/internal/serve.go	Introduces shared `Serve` helper for listener goroutines + error filtering.
plugins/server/internal/listen_windows.go	Adds helper to detect local (named pipe) addresses on Windows.
plugins/server/internal/listen_unix.go	Adds helper to detect local (absolute path) addresses on non-Windows.
plugins/server/grpc/tls_windows.go	Moves Windows TLS resource management into grpc plugin package.
plugins/server/grpc/tls_other.go	Adds non-Windows no-op TLS resource helpers.
plugins/server/grpc/plugin.go	Adds gRPC and TCP gRPC listener `ServerPlugin`s (incl. TLS + metrics hooks).
plugins/server/grpc/namespace.go	Updates package path for namespace interceptors.
plugins/server/grpc/metrics.go	Adds `MetricsPlugin` implementations for grpc prometheus + otel stats handler.
plugins/server/debug/plugin.go	Adds debug listener `ServerPlugin` wired to the existing pprof HTTP handler plugin.
integration/client/restart_monitor_test.go	Updates test config parsing to read gRPC address from new server plugin config.
docs/man/containerd-config.toml.5.md	Documents config v4, deprecates top-level listener sections, adds v4 examples.
contrib/fuzz/daemon.go	Updates fuzz daemon to configure listener via plugins and call `Server.Start()`.
cmd/containerd/server/server_other.go	Simplifies non-linux apply stub and removes legacy server constructors.
cmd/containerd/server/server_linux.go	Removes legacy in-package ttrpc constructor/TLS helpers from linux server.
cmd/containerd/server/server.go	Refactors server init to collect/start `ServerPlugin`s and adds plugin-based address lookup.
cmd/containerd/server/config/config_test.go	Updates migration expectations and adds tests for v4 service migration.
cmd/containerd/server/config/config.go	Adds v4 migration (`serviceMigrate`) and marks legacy listener fields deprecated/omitempty.
cmd/containerd/command/main_windows.go	Removes duplicated local-address helper (now in plugins/server/internal).
cmd/containerd/command/main_unix.go	Removes duplicated local-address helper (now in plugins/server/internal).
cmd/containerd/command/main.go	Starts listeners via `server.Start(ctx)` and updates `--address` flag handling to plugin config.
cmd/containerd/command/config.go	Updates default config generation to configure gRPC listener via server plugin block.
cmd/containerd/builtins/builtins.go	Adds built-in imports for new server listener plugin packages.
RELEASES.md	Updates documentation to reflect latest config version is 4.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The new server plugin type allows services which have listeners to be configured as plugins rather than defined directly in the global configuration. This provides more configuration consistency and allows containerd to be extended for new types of api handlers. Signed-off-by: Derek McGowan <derek@mcg.dev>

Migrate configuration and move server initialization Signed-off-by: Derek McGowan <derek@mcg.dev>

Signed-off-by: Derek McGowan <derek@mcg.dev>

Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>

Document the new server plugin configuration blocks for GRPC, TTRPC, debug, and metrics. Mark the legacy top-level sections as deprecated. Note that in version 4, the TTRPC plugin is configured independently from GRPC and uses its own defaults when its plugin block is omitted. Signed-off-by: Derek McGowan <derek@mcg.dev>

fuweid

LGTM

Nice refactor.

Follow-up:

It looks like it migrate before merge

there is case

# root.toml

version = 3
root = "/tmp/pr12562-repro-grpc/root"
state = "/tmp/pr12562-repro-grpc/state"
imports = ["dropin.toml"]

[grpc]
  address = "/tmp/pr12562-grpc.sock"
  uid = 0
  gid = 0

# dropin.toml
version = 3

[grpc]
  max_send_message_size = 33554432

It will show

WARN[2026-04-22T13:26:57.901574474-04:00] failed to load plugin                         error="grpc address cannot be empty: invalid argument" id=io.containerd.server.v1.grpc type=io.containerd.server.v1

fuweid · 2026-04-22T16:57:11Z

+	go func() {
+		defer l.Close()
+
+		if err := serveFunc(l); err != nil && !errors.Is(err, net.ErrClosed) && !errors.Is(err, http.ErrServerClosed) && !errors.Is(err, ttrpc.ErrServerClosed) {


maybe we need to helper function for that.

We can handle it in the follow-up

akhilerm · 2026-04-29T12:09:55Z

LGTM

Nice refactor.

Follow-up:

It looks like it migrate before merge

there is case

# root.toml

version = 3
root = "/tmp/pr12562-repro-grpc/root"
state = "/tmp/pr12562-repro-grpc/state"
imports = ["dropin.toml"]

[grpc]
  address = "/tmp/pr12562-grpc.sock"
  uid = 0
  gid = 0

# dropin.toml
version = 3

[grpc]
  max_send_message_size = 33554432

It will show

WARN[2026-04-22T13:26:57.901574474-04:00] failed to load plugin                         error="grpc address cannot be empty: invalid argument" id=io.containerd.server.v1.grpc type=io.containerd.server.v1

@fuweid This caused #13316. When any non empty value in [grpc] is set in older configs and address is not set, address goes to empty.

AkihiroSuda · 2026-05-10T04:36:51Z

Regression:

[v2.3.0-rc.0 regression] rootless: "/run/containerd/containerd.sock": no such file or directory #13362

github-project-automation Bot added this to Pull Request Review Nov 24, 2025

k8s-ci-robot added the do-not-merge/work-in-progress label Nov 24, 2025

github-project-automation Bot moved this to Needs Triage in Pull Request Review Nov 24, 2025

k8s-ci-robot added the size/XXL label Nov 24, 2025

dmcgowan force-pushed the plugin-api-handlers branch 4 times, most recently from 6515ed9 to 7f3c40f Compare November 25, 2025 23:49

dmcgowan force-pushed the plugin-api-handlers branch from 7f3c40f to ab69b6d Compare December 2, 2025 18:26

dmcgowan force-pushed the plugin-api-handlers branch from c2a5b88 to 9f021cc Compare January 7, 2026 20:04

k8s-ci-robot added the needs-rebase label Feb 19, 2026

dmcgowan force-pushed the plugin-api-handlers branch from 9f021cc to a4a8b6d Compare February 19, 2026 17:21

k8s-ci-robot removed the needs-rebase label Feb 19, 2026

dmcgowan added a commit to dmcgowan/containerd that referenced this pull request Mar 6, 2026

Merge containerd#12562: Add plugins for server listeners

275c431

Signed-off-by: Derek McGowan <derek@mcg.dev>

dmcgowan added a commit to dmcgowan/containerd that referenced this pull request Mar 7, 2026

Merge containerd#12562: Add plugins for server listeners

cfe1a53

Signed-off-by: Derek McGowan <derek@mcg.dev>

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 7, 2026

Merge containerd#12562: Add plugins for server listeners

7465432

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 7, 2026

Merge containerd#12562: Add plugins for server listeners

0e0ce69

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 7, 2026

Merge containerd#12562: Add plugins for server listeners

f87adc5

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 7, 2026

Merge containerd#12562: Add plugins for server listeners

adad151

github-actions Bot added a commit to akerouanton/containerd that referenced this pull request Mar 8, 2026

Merge containerd#12562: Add plugins for server listeners

a8ccfe7

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 9, 2026

Merge containerd#12562: Add plugins for server listeners

513459b

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 10, 2026

Merge containerd#12562: Add plugins for server listeners

342c1a3

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 10, 2026

Merge containerd#12562: Add plugins for server listeners

725d9e6

k8s-ci-robot added the needs-rebase label Mar 17, 2026

dmcgowan force-pushed the plugin-api-handlers branch from a4a8b6d to b4f06bf Compare March 18, 2026 06:51

k8s-ci-robot removed the needs-rebase label Mar 18, 2026

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 19, 2026

Merge containerd#12562: Add plugins for server listeners

544edd4

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 19, 2026

Merge containerd#12562: Add plugins for server listeners

ad52f77

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Mar 19, 2026

Merge containerd#12562: Add plugins for server listeners

b300e5b

dmcgowan force-pushed the plugin-api-handlers branch from b4f06bf to b6a2dbe Compare March 30, 2026 18:18

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Apr 10, 2026

Merge containerd#12562: Add plugins for server listeners

13d6578

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Apr 11, 2026

Merge containerd#12562: Add plugins for server listeners

e72a0fc

vvoland reviewed Apr 15, 2026

View reviewed changes

github-actions Bot added a commit to dmcgowan/containerd that referenced this pull request Apr 15, 2026

Merge containerd#12562: Add plugins for server listeners

2b3439b

prakleumas reviewed Apr 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 20, 2026 22:29

dmcgowan force-pushed the plugin-api-handlers branch from e1591b4 to 02e1eda Compare April 20, 2026 22:29

Copilot started reviewing on behalf of dmcgowan April 20, 2026 22:29 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

austinvazquez reviewed Apr 21, 2026

View reviewed changes

Comment thread cmd/containerd/server/config/config.go Outdated

dmcgowan and others added 9 commits April 21, 2026 18:02

Add server plugins for grpc and ttrpc

c15ec24

Migrate configuration and move server initialization Signed-off-by: Derek McGowan <derek@mcg.dev>

Add metrics server plugin

a2b1907

Signed-off-by: Derek McGowan <derek@mcg.dev>

Add tests for serviceMigrate config migration

841c518

Signed-off-by: Derek McGowan <derek@mcg.dev>

Update restart monitor test to not use deprecated address field

5d8c143

Signed-off-by: Derek McGowan <derek@mcg.dev>

Update fuzz to use new config location

8ea37c4

Signed-off-by: Derek McGowan <derek@mcg.dev>

Ensure Plugins is not nil during migration

a0f3c53

Signed-off-by: Derek McGowan <derek@mcg.dev>

Default to euid/egid in ttrpc & grpc server plugins

4dc3d6d

Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>

dmcgowan force-pushed the plugin-api-handlers branch from 02e1eda to 5098311 Compare April 22, 2026 03:20

austinvazquez approved these changes Apr 22, 2026

View reviewed changes

dmcgowan moved this from Needs Reviewers to Review In Progress in Pull Request Review Apr 22, 2026

dmcgowan requested a review from fuweid April 22, 2026 15:05

fuweid approved these changes Apr 22, 2026

View reviewed changes

SergeyKanzhelev mentioned this pull request Apr 23, 2026

Best effort graceful termination (of daemon) #13276

Open

AkihiroSuda mentioned this pull request May 10, 2026

[v2.3.0-rc.0 regression] rootless: "/run/containerd/containerd.sock": no such file or directory #13362

Closed

Vincinator mentioned this pull request May 13, 2026

Support containerd config file version 4 gardener/gardener-extension-runtime-gvisor#399

Merged

		grpcAddress = readString(config.Plugins, "io.containerd.server.v1.grpc", "address")
		ttrpcAddress = readString(config.Plugins, "io.containerd.server.v1.ttrpc", "address")

Conversation

dmcgowan commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prakleumas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fuweid left a comment

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!

akhilerm commented Apr 29, 2026

Uh oh!

AkihiroSuda commented May 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

dmcgowan commented Nov 24, 2025 •

edited

Loading