Merge pull request #9391 from abel-von/sandbox-plugin-1117

sandbox: Move CRI Image Service and CRI Base seperate plugins

contrib/fuzz/cri_sbserver_fuzzer.go

@@ -1,46 +0,0 @@
-//go:build gofuzz
-
-/*
-   Copyright The containerd Authors.
-
-   Licensed under the Apache License, Version 2.0 (the "License");
-   you may not use this file except in compliance with the License.
-   You may obtain a copy of the License at
-
-       http://www.apache.org/licenses/LICENSE-2.0
-
-   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
-   limitations under the License.
-*/
-
-package fuzz
-
-import (
-	fuzz "github.com/AdaLogics/go-fuzz-headers"
-
-	containerd "github.com/containerd/containerd/v2/client"
-	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
-	"github.com/containerd/containerd/v2/pkg/cri/server"
-)
-
-func FuzzCRISandboxServer(data []byte) int {
-	initDaemon.Do(startDaemon)
-
-	f := fuzz.NewConsumer(data)
-
-	client, err := containerd.New(defaultAddress)
-	if err != nil {
-		return 0
-	}
-	defer client.Close()
-
-	c, err := server.NewCRIService(criconfig.Config{}, client, nil)
-	if err != nil {
-		panic(err)
-	}
-
-	return fuzzCRI(f, c)
-}

contrib/fuzz/cri_server_fuzzer.go

@@ -22,8 +22,11 @@ import (
 	fuzz "github.com/AdaLogics/go-fuzz-headers"
 
 	containerd "github.com/containerd/containerd/v2/client"
+	"github.com/containerd/containerd/v2/oci"
 	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	"github.com/containerd/containerd/v2/pkg/cri/server"
+	"github.com/containerd/containerd/v2/pkg/cri/server/base"
+	"github.com/containerd/containerd/v2/pkg/cri/server/images"
 )
 
 func FuzzCRIServer(data []byte) int {
@@ -37,7 +40,19 @@ func FuzzCRIServer(data []byte) int {
 	}
 	defer client.Close()
 
-	c, err := server.NewCRIService(criconfig.Config{}, client, nil)
+	config := criconfig.Config{}
+
+	criBase := &base.CRIBase{
+		Config:       config,
+		BaseOCISpecs: map[string]*oci.Spec{},
+	}
+
+	imageService, err := images.NewService(config, client)
+	if err != nil {
+		panic(err)
+	}
+
+	c, err := server.NewCRIService(criBase, imageService, client, nil)
 	if err != nil {
 		panic(err)
 	}

integration/image_pull_timeout_test.go

@@ -33,17 +33,21 @@ import (
 	"testing"
 	"time"
 
-	containerd "github.com/containerd/containerd/v2/client"
-	"github.com/containerd/containerd/v2/content"
-	"github.com/containerd/containerd/v2/leases"
-	"github.com/containerd/containerd/v2/namespaces"
-	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
-	criserver "github.com/containerd/containerd/v2/pkg/cri/server"
 	"github.com/containerd/log"
 	"github.com/containerd/log/logtest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
+
+	containerd "github.com/containerd/containerd/v2/client"
+	"github.com/containerd/containerd/v2/content"
+	"github.com/containerd/containerd/v2/leases"
+	"github.com/containerd/containerd/v2/namespaces"
+	"github.com/containerd/containerd/v2/oci"
+	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
+	criserver "github.com/containerd/containerd/v2/pkg/cri/server"
+	"github.com/containerd/containerd/v2/pkg/cri/server/base"
+	"github.com/containerd/containerd/v2/pkg/cri/server/images"
 )
 
 var (
@@ -500,5 +504,16 @@ func initLocalCRIPlugin(client *containerd.Client, tmpDir string, registryCfg cr
 		RootDir:           filepath.Join(criWorkDir, "root"),
 		StateDir:          filepath.Join(criWorkDir, "state"),
 	}
-	return criserver.NewCRIService(cfg, client, nil)
+
+	criBase := &base.CRIBase{
+		Config:       cfg,
+		BaseOCISpecs: map[string]*oci.Spec{},
+	}
+
+	imageService, err := images.NewService(cfg, client)
+	if err != nil {
+		panic(err)
+	}
+
+	return criserver.NewCRIService(criBase, imageService, client, nil)
 }

pkg/cri/cri.go

@@ -17,75 +17,59 @@
 package cri
 
 import (
-	"flag"
 	"fmt"
-	"path/filepath"
 
 	"github.com/containerd/log"
 	"github.com/containerd/plugin"
 	"github.com/containerd/plugin/registry"
-	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
-	"k8s.io/klog/v2"
 
 	containerd "github.com/containerd/containerd/v2/client"
-	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	"github.com/containerd/containerd/v2/pkg/cri/constants"
 	"github.com/containerd/containerd/v2/pkg/cri/nri"
 	"github.com/containerd/containerd/v2/pkg/cri/server"
+	"github.com/containerd/containerd/v2/pkg/cri/server/base"
+	"github.com/containerd/containerd/v2/pkg/cri/server/images"
 	nriservice "github.com/containerd/containerd/v2/pkg/nri"
 	"github.com/containerd/containerd/v2/platforms"
 	"github.com/containerd/containerd/v2/plugins"
-	"github.com/containerd/containerd/v2/services/warning"
 )
 
 // Register CRI service plugin
 func init() {
-	config := criconfig.DefaultConfig()
+
 	registry.Register(&plugin.Registration{
-		Type:   plugins.GRPCPlugin,
+		Type: plugins.GRPCPlugin,
-		ID:     "cri",
+		ID:   "cri",
-		Config: &config,
 		Requires: []plugin.Type{
+			plugins.CRIImagePlugin,
+			plugins.InternalPlugin,
+			plugins.SandboxControllerPlugin,
+			plugins.NRIApiPlugin,
 			plugins.EventPlugin,
 			plugins.ServicePlugin,
-			plugins.NRIApiPlugin,
+			plugins.LeasePlugin,
-			plugins.WarningPlugin,
+			plugins.SandboxStorePlugin,
-			plugins.SandboxControllerPlugin,
 		},
 		InitFn: initCRIService,
 	})
 }
 
 func initCRIService(ic *plugin.InitContext) (interface{}, error) {
-	ic.Meta.Platforms = []imagespec.Platform{platforms.DefaultSpec()}
-	ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion}
 	ctx := ic.Context
-	pluginConfig := ic.Config.(*criconfig.PluginConfig)
-	if warnings, err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
-		return nil, fmt.Errorf("invalid plugin config: %w", err)
-	} else if len(warnings) > 0 {
-		ws, err := ic.GetSingle(plugins.WarningPlugin)
-		if err != nil {
-			return nil, err
-		}
-		warn := ws.(warning.Service)
-		for _, w := range warnings {
-			warn.Emit(ctx, w)
-		}
-	}
 
-	c := criconfig.Config{
+	// Get base CRI dependencies.
-		PluginConfig:       *pluginConfig,
+	criBasePlugin, err := ic.GetByID(plugins.InternalPlugin, "cri")
-		ContainerdRootDir:  filepath.Dir(ic.Properties[plugins.PropertyRootDir]),
+	if err != nil {
-		ContainerdEndpoint: ic.Properties[plugins.PropertyGRPCAddress],
+		return nil, fmt.Errorf("unable to load CRI service base dependencies: %w", err)
-		RootDir:            ic.Properties[plugins.PropertyRootDir],
-		StateDir:           ic.Properties[plugins.PropertyStateDir],
 	}
-	log.G(ctx).Infof("Start cri plugin with config %+v", c)
+	criBase := criBasePlugin.(*base.CRIBase)
 
-	if err := setGLogLevel(); err != nil {
+	// Get image service.
-		return nil, fmt.Errorf("failed to set glog level: %w", err)
+	criImagePlugin, err := ic.GetByID(plugins.CRIImagePlugin, "cri-image-service")
+	if err != nil {
+		return nil, fmt.Errorf("unable to load CRI image service plugin dependency: %w", err)
 	}
+	imageService := criImagePlugin.(*images.CRIImageService)
 
 	log.G(ctx).Info("Connect containerd service")
 	client, err := containerd.New(
@@ -99,7 +83,7 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 		return nil, fmt.Errorf("failed to create containerd client: %w", err)
 	}
 
-	s, err := server.NewCRIService(c, client, getNRIAPI(ic))
+	s, err := server.NewCRIService(criBase, imageService, client, getNRIAPI(ic))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create CRI service: %w", err)
 	}
@@ -116,27 +100,6 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	return s, nil
 }
 
-// Set glog level.
-func setGLogLevel() error {
-	l := log.GetLevel()
-	fs := flag.NewFlagSet("klog", flag.PanicOnError)
-	klog.InitFlags(fs)
-	if err := fs.Set("logtostderr", "true"); err != nil {
-		return err
-	}
-	switch l {
-	case log.TraceLevel:
-		return fs.Set("v", "5")
-	case log.DebugLevel:
-		return fs.Set("v", "4")
-	case log.InfoLevel:
-		return fs.Set("v", "2")
-	default:
-		// glog doesn't support other filters. Defaults to v=0.
-	}
-	return nil
-}
-
 // Get the NRI plugin, and set up our NRI API for it.
 func getNRIAPI(ic *plugin.InitContext) *nri.API {
 	const (

pkg/cri/server/base/cri_base.go

@@ -0,0 +1,202 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package base
+
+import (
+	"context"
+	"encoding/json"
+	"flag"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/containerd/log"
+	"github.com/containerd/plugin"
+	"github.com/containerd/plugin/registry"
+	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
+	"k8s.io/klog/v2"
+
+	"github.com/containerd/containerd/v2/oci"
+	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
+	"github.com/containerd/containerd/v2/pkg/cri/constants"
+	"github.com/containerd/containerd/v2/platforms"
+	"github.com/containerd/containerd/v2/plugins"
+	srvconfig "github.com/containerd/containerd/v2/services/server/config"
+	"github.com/containerd/containerd/v2/services/warning"
+)
+
+// CRIBase contains common dependencies for CRI's runtime, image, and podsandbox services.
+type CRIBase struct {
+	// Config contains all configurations.
+	Config criconfig.Config
+	// BaseOCISpecs contains cached OCI specs loaded via `Runtime.BaseRuntimeSpec`
+	BaseOCISpecs map[string]*oci.Spec
+}
+
+func init() {
+	config := criconfig.DefaultConfig()
+
+	// Base plugin that other CRI services depend on.
+	registry.Register(&plugin.Registration{
+		Type:   plugins.InternalPlugin,
+		ID:     "cri",
+		Config: &config,
+		Requires: []plugin.Type{
+			plugins.WarningPlugin,
+		},
+		ConfigMigration: func(ctx context.Context, version int, plugins map[string]interface{}) error {
+			if version >= srvconfig.CurrentConfigVersion {
+				return nil
+			}
+			c, ok := plugins["io.containerd.grpc.v1.cri"]
+			if !ok {
+				return nil
+			}
+			conf := c.(map[string]interface{})
+			migrateConfig(conf)
+			plugins["io.containerd.internal.v1.cri"] = conf
+			return nil
+		},
+		InitFn: initCRIBase,
+	})
+}
+
+func initCRIBase(ic *plugin.InitContext) (interface{}, error) {
+	ic.Meta.Platforms = []imagespec.Platform{platforms.DefaultSpec()}
+	ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion}
+	ctx := ic.Context
+	pluginConfig := ic.Config.(*criconfig.PluginConfig)
+	if warnings, err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
+		return nil, fmt.Errorf("invalid plugin config: %w", err)
+	} else if len(warnings) > 0 {
+		ws, err := ic.GetSingle(plugins.WarningPlugin)
+		if err != nil {
+			return nil, err
+		}
+		warn := ws.(warning.Service)
+		for _, w := range warnings {
+			warn.Emit(ctx, w)
+		}
+	}
+
+	// For backward compatibility, we have to keep the rootDir and stateDir the same as before.
+	containerdRootDir := filepath.Dir(ic.Properties[plugins.PropertyRootDir])
+	rootDir := filepath.Join(containerdRootDir, "io.containerd.grpc.v1.cri")
+	containerdStateDir := filepath.Dir(ic.Properties[plugins.PropertyStateDir])
+	stateDir := filepath.Join(containerdStateDir, "io.containerd.grpc.v1.cri")
+	c := criconfig.Config{
+		PluginConfig:       *pluginConfig,
+		ContainerdRootDir:  containerdRootDir,
+		ContainerdEndpoint: ic.Properties[plugins.PropertyGRPCAddress],
+		RootDir:            rootDir,
+		StateDir:           stateDir,
+	}
+
+	log.G(ctx).Infof("Start cri plugin with config %+v", c)
+
+	if err := setGLogLevel(); err != nil {
+		return nil, fmt.Errorf("failed to set glog level: %w", err)
+	}
+
+	ociSpec, err := loadBaseOCISpecs(&c)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create load basic oci spec: %w", err)
+	}
+
+	return &CRIBase{
+		Config:       c,
+		BaseOCISpecs: ociSpec,
+	}, nil
+}
+
+func loadBaseOCISpecs(config *criconfig.Config) (map[string]*oci.Spec, error) {
+	specs := map[string]*oci.Spec{}
+	for _, cfg := range config.Runtimes {
+		if cfg.BaseRuntimeSpec == "" {
+			continue
+		}
+
+		// Don't load same file twice
+		if _, ok := specs[cfg.BaseRuntimeSpec]; ok {
+			continue
+		}
+
+		spec, err := loadOCISpec(cfg.BaseRuntimeSpec)
+		if err != nil {
+			return nil, fmt.Errorf("failed to load base OCI spec from file: %s: %w", cfg.BaseRuntimeSpec, err)
+		}
+
+		specs[cfg.BaseRuntimeSpec] = spec
+	}
+
+	return specs, nil
+}
+
+func loadOCISpec(filename string) (*oci.Spec, error) {
+	file, err := os.Open(filename)
+	if err != nil {
+		return nil, fmt.Errorf("failed to open base OCI spec: %s: %w", filename, err)
+	}
+	defer file.Close()
+
+	spec := oci.Spec{}
+	if err := json.NewDecoder(file).Decode(&spec); err != nil {
+		return nil, fmt.Errorf("failed to parse base OCI spec file: %w", err)
+	}
+
+	return &spec, nil
+}
+
+// Set glog level.
+func setGLogLevel() error {
+	l := log.GetLevel()
+	fs := flag.NewFlagSet("klog", flag.PanicOnError)
+	klog.InitFlags(fs)
+	if err := fs.Set("logtostderr", "true"); err != nil {
+		return err
+	}
+	switch l {
+	case log.TraceLevel:
+		return fs.Set("v", "5")
+	case log.DebugLevel:
+		return fs.Set("v", "4")
+	case log.InfoLevel:
+		return fs.Set("v", "2")
+	default:
+		// glog doesn't support other filters. Defaults to v=0.
+	}
+	return nil
+}
+
+func migrateConfig(conf map[string]interface{}) {
+	containerdConf, ok := conf["containerd"]
+	if !ok {
+		return
+	}
+	runtimesConf, ok := containerdConf.(map[string]interface{})["runtimes"]
+	if !ok {
+		return
+	}
+	for _, v := range runtimesConf.(map[string]interface{}) {
+		runtimeConf := v.(map[string]interface{})
+		if sandboxMode, ok := runtimeConf["sandbox_mode"]; ok {
+			if _, ok := runtimeConf["sandboxer"]; !ok {
+				runtimeConf["sandboxer"] = sandboxMode
+			}
+		}
+	}
+}

pkg/cri/server/base/cri_base_test.go

@@ -0,0 +1,60 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package base
+
+import (
+	"encoding/json"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/containerd/containerd/v2/oci"
+	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
+)
+
+func TestLoadBaseOCISpec(t *testing.T) {
+	spec := oci.Spec{Version: "1.0.2", Hostname: "default"}
+
+	file, err := os.CreateTemp("", "spec-test-")
+	require.NoError(t, err)
+
+	defer func() {
+		assert.NoError(t, file.Close())
+		assert.NoError(t, os.RemoveAll(file.Name()))
+	}()
+
+	err = json.NewEncoder(file).Encode(&spec)
+	assert.NoError(t, err)
+
+	config := criconfig.Config{}
+	config.Runtimes = map[string]criconfig.Runtime{
+		"runc": {BaseRuntimeSpec: file.Name()},
+	}
+
+	specs, err := loadBaseOCISpecs(&config)
+	assert.NoError(t, err)
+
+	assert.Len(t, specs, 1)
+
+	out, ok := specs[file.Name()]
+	assert.True(t, ok, "expected spec with file name %q", file.Name())
+
+	assert.Equal(t, "1.0.2", out.Version)
+	assert.Equal(t, "default", out.Hostname)
+}

pkg/cri/server/container_status_test.go

@@ -286,6 +286,8 @@ func (s *fakeImageService) LocalResolve(refOrID string) (imagestore.Image, error
 	return imagestore.Image{}, errors.New("not implemented")
 }
 
+func (s *fakeImageService) ImageFSPaths() map[string]string { return make(map[string]string) }
+
 func patchExceptedWithState(expected *runtime.ContainerStatus, state runtime.ContainerState) {
 	expected.State = state
 	switch state {

pkg/cri/server/images/service.go

@@ -19,21 +19,66 @@ package images
 import (
 	"context"
 	"fmt"
+	"path/filepath"
 	"time"
 
+	"github.com/containerd/log"
+	"github.com/containerd/plugin"
+	"github.com/containerd/plugin/registry"
+	docker "github.com/distribution/reference"
+	imagedigest "github.com/opencontainers/go-digest"
+
 	containerd "github.com/containerd/containerd/v2/client"
 	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
+	"github.com/containerd/containerd/v2/pkg/cri/constants"
+	"github.com/containerd/containerd/v2/pkg/cri/server/base"
 	imagestore "github.com/containerd/containerd/v2/pkg/cri/store/image"
 	snapshotstore "github.com/containerd/containerd/v2/pkg/cri/store/snapshot"
 	ctrdutil "github.com/containerd/containerd/v2/pkg/cri/util"
 	"github.com/containerd/containerd/v2/pkg/kmutex"
 	"github.com/containerd/containerd/v2/platforms"
+	"github.com/containerd/containerd/v2/plugins"
 	snapshot "github.com/containerd/containerd/v2/snapshots"
-	"github.com/containerd/log"
-	docker "github.com/distribution/reference"
-	imagedigest "github.com/opencontainers/go-digest"
 )
 
+func init() {
+	registry.Register(&plugin.Registration{
+		Type: plugins.CRIImagePlugin,
+		ID:   "cri-image-service",
+		Requires: []plugin.Type{
+			plugins.LeasePlugin,
+			plugins.EventPlugin,
+			plugins.SandboxStorePlugin,
+			plugins.InternalPlugin,
+			plugins.ServicePlugin,
+		},
+		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
+			// Get base CRI dependencies.
+			criPlugin, err := ic.GetByID(plugins.InternalPlugin, "cri")
+			if err != nil {
+				return nil, fmt.Errorf("unable to load CRI service base dependencies: %w", err)
+			}
+			cri := criPlugin.(*base.CRIBase)
+
+			client, err := containerd.New(
+				"",
+				containerd.WithDefaultNamespace(constants.K8sContainerdNamespace),
+				containerd.WithDefaultPlatform(platforms.Default()),
+				containerd.WithInMemoryServices(ic),
+			)
+			if err != nil {
+				return nil, fmt.Errorf("unable to init client for cri image service: %w", err)
+			}
+			service, err := NewService(cri.Config, client)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create image service: %w", err)
+			}
+
+			return service, nil
+		},
+	})
+}
+
 type CRIImageService struct {
 	// config contains all configurations.
 	config criconfig.Config
@@ -51,7 +96,25 @@ type CRIImageService struct {
 	unpackDuplicationSuppressor kmutex.KeyedLocker
 }
 
-func NewService(config criconfig.Config, imageFSPaths map[string]string, client *containerd.Client) (*CRIImageService, error) {
+func NewService(config criconfig.Config, client *containerd.Client) (*CRIImageService, error) {
+	if client.SnapshotService(config.ContainerdConfig.Snapshotter) == nil {
+		return nil, fmt.Errorf("failed to find snapshotter %q", config.ContainerdConfig.Snapshotter)
+	}
+
+	imageFSPaths := map[string]string{}
+	for _, ociRuntime := range config.ContainerdConfig.Runtimes {
+		// Can not use `c.RuntimeSnapshotter() yet, so hard-coding here.`
+		snapshotter := ociRuntime.Snapshotter
+		if snapshotter != "" {
+			imageFSPaths[snapshotter] = imageFSPath(config.ContainerdRootDir, snapshotter)
+			log.L.Infof("Get image filesystem path %q for snapshotter %q", imageFSPaths[snapshotter], snapshotter)
+		}
+	}
+
+	snapshotter := config.ContainerdConfig.Snapshotter
+	imageFSPaths[snapshotter] = imageFSPath(config.ContainerdRootDir, snapshotter)
+	log.L.Infof("Get image filesystem path %q for snapshotter %q", imageFSPaths[snapshotter], snapshotter)
+
 	svc := CRIImageService{
 		config:                      config,
 		client:                      client,
@@ -94,6 +157,12 @@ func NewService(config criconfig.Config, imageFSPaths map[string]string, client
 	return &svc, nil
 }
 
+// imageFSPath returns containerd image filesystem path.
+// Note that if containerd changes directory layout, we also needs to change this.
+func imageFSPath(rootDir, snapshotter string) string {
+	return filepath.Join(rootDir, plugins.SnapshotPlugin.String()+"."+snapshotter)
+}
+
 // LocalResolve resolves image reference locally and returns corresponding image metadata. It
 // returns errdefs.ErrNotFound if the reference doesn't exist.
 func (c *CRIImageService) LocalResolve(refOrID string) (imagestore.Image, error) {
@@ -148,3 +217,7 @@ func (c *CRIImageService) GetSnapshot(key, snapshotter string) (snapshotstore.Sn
 	}
 	return c.snapshotStore.Get(snapshotKey)
 }
+
+func (c *CRIImageService) ImageFSPaths() map[string]string {
+	return c.imageFSPaths
+}

pkg/cri/server/podsandbox/controller.go

@@ -22,6 +22,8 @@ import (
 	"time"
 
 	"github.com/containerd/log"
+	"github.com/containerd/plugin"
+	"github.com/containerd/plugin/registry"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
 	eventtypes "github.com/containerd/containerd/v2/api/events"
@@ -30,6 +32,8 @@ import (
 	"github.com/containerd/containerd/v2/oci"
 	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	"github.com/containerd/containerd/v2/pkg/cri/constants"
+	"github.com/containerd/containerd/v2/pkg/cri/server/base"
+	"github.com/containerd/containerd/v2/pkg/cri/server/images"
 	imagestore "github.com/containerd/containerd/v2/pkg/cri/store/image"
 	sandboxstore "github.com/containerd/containerd/v2/pkg/cri/store/sandbox"
 	ctrdutil "github.com/containerd/containerd/v2/pkg/cri/util"
@@ -38,8 +42,6 @@ import (
 	"github.com/containerd/containerd/v2/plugins"
 	"github.com/containerd/containerd/v2/protobuf"
 	"github.com/containerd/containerd/v2/sandbox"
-	"github.com/containerd/plugin"
-	"github.com/containerd/plugin/registry"
 )
 
 func init() {
@@ -49,20 +51,44 @@ func init() {
 		Requires: []plugin.Type{
 			plugins.EventPlugin,
 			plugins.LeasePlugin,
+			plugins.SandboxStorePlugin,
+			plugins.InternalPlugin,
+			plugins.CRIImagePlugin,
 			plugins.ServicePlugin,
 		},
 		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
-			c := Controller{}
 			client, err := containerd.New(
 				"",
 				containerd.WithDefaultNamespace(constants.K8sContainerdNamespace),
 				containerd.WithDefaultPlatform(platforms.Default()),
 				containerd.WithInMemoryServices(ic),
 			)
+			if err != nil {
+				return nil, fmt.Errorf("unable to init client for podsandbox: %w", err)
+			}
+
+			// Get base CRI dependencies.
+			criBasePlugin, err := ic.GetByID(plugins.InternalPlugin, "cri")
 			if err != nil {
 				return nil, fmt.Errorf("unable to load CRI service base dependencies: %w", err)
 			}
-			c.client = client
+			criBase := criBasePlugin.(*base.CRIBase)
+
+			// Get image service.
+			criImagePlugin, err := ic.GetByID(plugins.CRIImagePlugin, "cri-image-service")
+			if err != nil {
+				return nil, fmt.Errorf("unable to load CRI image service plugin dependency: %w", err)
+			}
+			imageService := criImagePlugin.(*images.CRIImageService)
+
+			c := Controller{
+				client:       client,
+				config:       criBase.Config,
+				os:           osinterface.RealOS{},
+				baseOCISpecs: criBase.BaseOCISpecs,
+				imageService: imageService,
+				store:        NewStore(),
+			}
 			return &c, nil
 		},
 	})
@@ -103,20 +129,11 @@ type Controller struct {
 }
 
 func (c *Controller) Init(
-	config criconfig.Config,
 	sandboxStore *sandboxstore.Store,
-	os osinterface.OS,
 	cri CRIService,
-	imageService ImageService,
-	baseOCISpecs map[string]*oci.Spec,
 ) {
 	c.cri = cri
-	c.config = config
 	c.sandboxStore = sandboxStore
-	c.os = os
-	c.baseOCISpecs = baseOCISpecs
-	c.store = NewStore()
-	c.imageService = imageService
 }
 
 var _ sandbox.Controller = (*Controller)(nil)

pkg/cri/server/service.go

@@ -18,12 +18,9 @@ package server
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
-	"os"
-	"path/filepath"
 	"sync"
 	"sync/atomic"
 
@@ -38,7 +35,7 @@ import (
 	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	"github.com/containerd/containerd/v2/pkg/cri/instrument"
 	"github.com/containerd/containerd/v2/pkg/cri/nri"
-	"github.com/containerd/containerd/v2/pkg/cri/server/images"
+	"github.com/containerd/containerd/v2/pkg/cri/server/base"
 	"github.com/containerd/containerd/v2/pkg/cri/server/podsandbox"
 	containerstore "github.com/containerd/containerd/v2/pkg/cri/store/container"
 	imagestore "github.com/containerd/containerd/v2/pkg/cri/store/image"
@@ -48,7 +45,6 @@ import (
 	ctrdutil "github.com/containerd/containerd/v2/pkg/cri/util"
 	osinterface "github.com/containerd/containerd/v2/pkg/os"
 	"github.com/containerd/containerd/v2/pkg/registrar"
-	"github.com/containerd/containerd/v2/plugins"
 	"github.com/containerd/containerd/v2/sandbox"
 )
 
@@ -83,6 +79,8 @@ type imageService interface {
 	GetSnapshot(key, snapshotter string) (snapshotstore.Snapshot, error)
 
 	LocalResolve(refOrID string) (imagestore.Image, error)
+
+	ImageFSPaths() map[string]string
 }
 
 // criService implements CRIService.
@@ -133,40 +131,17 @@ type criService struct {
 }
 
 // NewCRIService returns a new instance of CRIService
-func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.API) (CRIService, error) {
+func NewCRIService(criBase *base.CRIBase, imageService imageService, client *containerd.Client, nri *nri.API) (CRIService, error) {
 	var err error
 	labels := label.NewStore()
-
+	config := criBase.Config
-	if client.SnapshotService(config.ContainerdConfig.Snapshotter) == nil {
-		return nil, fmt.Errorf("failed to find snapshotter %q", config.ContainerdConfig.Snapshotter)
-	}
-
-	imageFSPaths := map[string]string{}
-	for _, ociRuntime := range config.ContainerdConfig.Runtimes {
-		// Can not use `c.RuntimeSnapshotter() yet, so hard-coding here.`
-		snapshotter := ociRuntime.Snapshotter
-		if snapshotter != "" {
-			imageFSPaths[snapshotter] = imageFSPath(config.ContainerdRootDir, snapshotter)
-			log.L.Infof("Get image filesystem path %q for snapshotter %q", imageFSPaths[snapshotter], snapshotter)
-		}
-	}
-
-	snapshotter := config.ContainerdConfig.Snapshotter
-	imageFSPaths[snapshotter] = imageFSPath(config.ContainerdRootDir, snapshotter)
-	log.L.Infof("Get image filesystem path %q for snapshotter %q", imageFSPaths[snapshotter], snapshotter)
-
-	// TODO: expose this as a separate containerd plugin.
-	imageService, err := images.NewService(config, imageFSPaths, client)
-	if err != nil {
-		return nil, fmt.Errorf("unable to create CRI image service: %w", err)
-	}
-
 	c := &criService{
 		imageService:       imageService,
 		config:             config,
 		client:             client,
-		imageFSPaths:       imageFSPaths,
+		imageFSPaths:       imageService.ImageFSPaths(),
 		os:                 osinterface.RealOS{},
+		baseOCISpecs:       criBase.BaseOCISpecs,
 		sandboxStore:       sandboxstore.NewStore(labels),
 		containerStore:     containerstore.NewStore(labels),
 		sandboxNameIndex:   registrar.NewRegistrar(),
@@ -207,15 +182,8 @@ func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.
 		}
 	}
 
-	// Preload base OCI specs
-	c.baseOCISpecs, err = loadBaseOCISpecs(&config)
-	if err != nil {
-		return nil, err
-	}
-
 	podSandboxController := client.SandboxController(string(criconfig.ModePodSandbox)).(*podsandbox.Controller)
-	// Initialize pod sandbox controller
+	podSandboxController.Init(c.sandboxStore, c)
-	podSandboxController.Init(config, c.sandboxStore, c.os, c, c.imageService, c.baseOCISpecs)
 
 	c.nri = nri
 
@@ -359,50 +327,3 @@ func (c *criService) register(s *grpc.Server) error {
 	runtime.RegisterImageServiceServer(s, instrumented)
 	return nil
 }
-
-// imageFSPath returns containerd image filesystem path.
-// Note that if containerd changes directory layout, we also needs to change this.
-func imageFSPath(rootDir, snapshotter string) string {
-	return filepath.Join(rootDir, plugins.SnapshotPlugin.String()+"."+snapshotter)
-}
-
-func loadOCISpec(filename string) (*oci.Spec, error) {
-	file, err := os.Open(filename)
-	if err != nil {
-		return nil, fmt.Errorf("failed to open base OCI spec: %s: %w", filename, err)
-	}
-	defer file.Close()
-
-	spec := oci.Spec{}
-	if err := json.NewDecoder(file).Decode(&spec); err != nil {
-		return nil, fmt.Errorf("failed to parse base OCI spec file: %w", err)
-	}
-
-	return &spec, nil
-}
-
-func loadBaseOCISpecs(config *criconfig.Config) (map[string]*oci.Spec, error) {
-	specs := map[string]*oci.Spec{}
-	for _, cfg := range config.Runtimes {
-		if cfg.BaseRuntimeSpec == "" {
-			continue
-		}
-
-		// Don't load same file twice
-		if _, ok := specs[cfg.BaseRuntimeSpec]; ok {
-			continue
-		}
-
-		spec, err := loadOCISpec(cfg.BaseRuntimeSpec)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load base OCI spec from file: %s: %w", cfg.BaseRuntimeSpec, err)
-		}
-
-		if spec.Process != nil && spec.Process.Capabilities != nil && len(spec.Process.Capabilities.Inheritable) > 0 {
-			log.L.WithField("base_runtime_spec", cfg.BaseRuntimeSpec).Warn("Provided base runtime spec includes inheritable capabilities, which may be unsafe. See CVE-2022-24769 for more details.")
-		}
-		specs[cfg.BaseRuntimeSpec] = spec
-	}
-
-	return specs, nil
-}

pkg/cri/server/service_test.go

@@ -17,25 +17,13 @@
 package server
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
-	"io"
-	"os"
-	"testing"
 
 	"github.com/containerd/go-cni"
-	"github.com/containerd/log"
-	"github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
 	"github.com/containerd/containerd/v2/api/types"
 	"github.com/containerd/containerd/v2/errdefs"
-	"github.com/containerd/containerd/v2/oci"
-	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	containerstore "github.com/containerd/containerd/v2/pkg/cri/store/container"
 	"github.com/containerd/containerd/v2/pkg/cri/store/label"
 	sandboxstore "github.com/containerd/containerd/v2/pkg/cri/store/sandbox"
@@ -103,88 +91,3 @@ func newTestCRIService() *criService {
 		sandboxService: &fakeSandboxService{},
 	}
 }
-
-func TestLoadBaseOCISpec(t *testing.T) {
-	spec := oci.Spec{Version: "1.0.2", Hostname: "default"}
-
-	file, err := os.CreateTemp("", "spec-test-")
-	require.NoError(t, err)
-
-	defer func() {
-		assert.NoError(t, file.Close())
-		assert.NoError(t, os.RemoveAll(file.Name()))
-	}()
-
-	err = json.NewEncoder(file).Encode(&spec)
-	assert.NoError(t, err)
-
-	config := criconfig.Config{}
-	config.Runtimes = map[string]criconfig.Runtime{
-		"runc": {BaseRuntimeSpec: file.Name()},
-	}
-
-	specs, err := loadBaseOCISpecs(&config)
-	assert.NoError(t, err)
-
-	assert.Len(t, specs, 1)
-
-	out, ok := specs[file.Name()]
-	assert.True(t, ok, "expected spec with file name %q", file.Name())
-
-	assert.Equal(t, "1.0.2", out.Version)
-	assert.Equal(t, "default", out.Hostname)
-}
-
-func Test_loadBaseOCISpecs(t *testing.T) {
-	spec := oci.Spec{
-		Version:  "1.0.2",
-		Hostname: "default",
-		Process: &specs.Process{
-			Capabilities: &specs.LinuxCapabilities{
-				Inheritable: []string{"CAP_NET_RAW"},
-			},
-		},
-	}
-	file, err := os.CreateTemp("", "spec-test-")
-	require.NoError(t, err)
-	defer func() {
-		assert.NoError(t, file.Close())
-		assert.NoError(t, os.RemoveAll(file.Name()))
-	}()
-	err = json.NewEncoder(file).Encode(&spec)
-	require.NoError(t, err)
-	config := criconfig.Config{}
-	config.Runtimes = map[string]criconfig.Runtime{
-		"runc": {BaseRuntimeSpec: file.Name()},
-	}
-	var buffer bytes.Buffer
-	logger := &logrus.Logger{
-		Out:          &buffer,
-		Formatter:    new(logrus.TextFormatter),
-		Hooks:        make(logrus.LevelHooks),
-		Level:        logrus.InfoLevel,
-		ExitFunc:     os.Exit,
-		ReportCaller: false,
-	}
-	log.L = logrus.NewEntry(logger)
-	tests := []struct {
-		name    string
-		args    *criconfig.Config
-		message string
-	}{
-		{
-			name:    "args is not nil,print warning",
-			args:    &config,
-			message: "Provided base runtime spec includes inheritable capabilities, which may be unsafe. See CVE-2022-24769 for more details.",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			loadBaseOCISpecs(tt.args)
-			readAll, _ := io.ReadAll(&buffer)
-			if tt.message != "" {
-				assert.Contains(t, string(readAll), tt.message)
-			}
-		})
-	}
-}

plugins/types.go

@@ -67,6 +67,8 @@ const (
 	ImageVerifierPlugin plugin.Type = "io.containerd.image-verifier.v1"
 	// WarningPlugin implements a warning service
 	WarningPlugin plugin.Type = "io.containerd.warning.v1"
+	// CRIImagePlugin implements a cri image service
+	CRIImagePlugin plugin.Type = "io.containerd.cri.image.v1"
 )
 
 const (