Update CRI runtime platform and pinned image configuration

Updates the CRI image service to own image related configuration and
separate it from the runtime configuration.

Signed-off-by: Derek McGowan <[email protected]>

Author: dmcgowan

integration/image_pull_timeout_test.go

@@ -86,7 +86,7 @@ func testCRIImagePullTimeoutBySlowCommitWriter(t *testing.T) {
 	delayDuration := 2 * defaultImagePullProgressTimeout
 	cli := buildLocalContainerdClient(t, tmpDir, tweakContentInitFnWithDelayer(delayDuration))
 
-	criService, err := initLocalCRIPlugin(cli, tmpDir, criconfig.Registry{})
+	criService, err := initLocalCRIImageService(cli, tmpDir, criconfig.Registry{})
 	assert.NoError(t, err)
 
 	ctx := namespaces.WithNamespace(logtest.WithT(context.Background(), t), k8sNamespace)
@@ -109,7 +109,7 @@ func testCRIImagePullTimeoutByHoldingContentOpenWriter(t *testing.T) {
 
 	cli := buildLocalContainerdClient(t, tmpDir, nil)
 
-	criService, err := initLocalCRIPlugin(cli, tmpDir, criconfig.Registry{})
+	criService, err := initLocalCRIImageService(cli, tmpDir, criconfig.Registry{})
 	assert.NoError(t, err)
 
 	ctx := namespaces.WithNamespace(logtest.WithT(context.Background(), t), k8sNamespace)
@@ -287,7 +287,7 @@ func testCRIImagePullTimeoutByNoDataTransferred(t *testing.T) {
 			},
 		},
 	} {
-		criService, err := initLocalCRIPlugin(cli, tmpDir, registryCfg)
+		criService, err := initLocalCRIImageService(cli, tmpDir, registryCfg)
 		assert.NoError(t, err)
 
 		dctx, _, err := cli.WithLease(ctx)
@@ -468,27 +468,27 @@ func (l *ioCopyLimiter) limitedCopy(ctx context.Context, dst io.Writer, src io.R
 	return nil
 }
 
-// initLocalCRIPlugin uses containerd.Client to init CRI plugin.
+// initLocalCRIImageService uses containerd.Client to init CRI plugin.
 //
 // NOTE: We don't need to start the CRI plugin here because we just need the
 // ImageService API.
-func initLocalCRIPlugin(client *containerd.Client, tmpDir string, registryCfg criconfig.Registry) (criserver.ImageService, error) {
+func initLocalCRIImageService(client *containerd.Client, tmpDir string, registryCfg criconfig.Registry) (criserver.ImageService, error) {
 	containerdRootDir := filepath.Join(tmpDir, "root")
-	criWorkDir := filepath.Join(tmpDir, "cri-plugin")
-
-	cfg := criconfig.Config{
-		PluginConfig: criconfig.PluginConfig{
-			ImageConfig: criconfig.ImageConfig{
-				Snapshotter:              containerd.DefaultSnapshotter,
-				Registry:                 registryCfg,
-				ImagePullProgressTimeout: defaultImagePullProgressTimeout.String(),
-				StatsCollectPeriod:       10,
-			},
-		},
-		ContainerdRootDir: containerdRootDir,
-		RootDir:           filepath.Join(criWorkDir, "root"),
-		StateDir:          filepath.Join(criWorkDir, "state"),
+
+	cfg := criconfig.ImageConfig{
+		Snapshotter:              containerd.DefaultSnapshotter,
+		Registry:                 registryCfg,
+		ImagePullProgressTimeout: defaultImagePullProgressTimeout.String(),
+		StatsCollectPeriod:       10,
 	}
 
-	return images.NewService(cfg.ImageConfig, map[string]string{}, map[string]images.RuntimePlatform{}, client)
+	return images.NewService(cfg, &images.CRIImageServiceOptions{
+		ImageFSPaths: map[string]string{
+			containerd.DefaultSnapshotter: containerdRootDir,
+		},
+		RuntimePlatforms: map[string]images.ImagePlatform{},
+		Content:          client.ContentStore(),
+		Images:           client.ImageService(),
+		Client:           client,
+	})
 }

pkg/cri/config/config.go

@@ -237,6 +237,15 @@ type ImageDecryption struct {
 	KeyModel string `toml:"key_model" json:"keyModel"`
 }
 
+// ImagePlatform represents the platform to use for an image including the
+// snapshotter to use. If snapshotter is not provided, the platform default
+// can be assumed. When platform is not provided, the default platform can
+// be assumed
+type ImagePlatform struct {
+	Platform    string
+	Snapshotter string
+}
+
 type ImageConfig struct {
 	// Snapshotter is the snapshotter used by containerd.
 	Snapshotter string `toml:"snapshotter" json:"snapshotter"`
@@ -251,6 +260,20 @@ type ImageConfig struct {
 	// layers to the snapshotter.
 	DiscardUnpackedLayers bool `toml:"discard_unpacked_layers" json:"discardUnpackedLayers"`
 
+	// PinnedImages are images which the CRI plugin uses and should not be
+	// removed by the CRI client.
+	// Image names should be full names including domain and tag
+	// Examples:
+	//   docker.io/library/ubuntu:latest
+	//   images.k8s.io/core/pause:1.55
+	PinnedImages []string
+
+	// RuntimePlatforms is map between the runtime and the image platform to
+	// use for that runtime. When resolving an image for a runtime, this
+	// mapping will be used to select the image for the platform and the
+	// snapshotter for unpacking.
+	RuntimePlatforms map[string]ImagePlatform
+
 	// Registry contains config related to the registry
 	Registry Registry `toml:"registry" json:"registry"`
 

pkg/cri/server/images/image_pull.go

@@ -38,6 +38,7 @@ import (
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
+	eventstypes "github.com/containerd/containerd/v2/api/events"
 	containerd "github.com/containerd/containerd/v2/client"
 	"github.com/containerd/containerd/v2/diff"
 	"github.com/containerd/containerd/v2/errdefs"
@@ -319,32 +320,44 @@ func (c *CRIImageService) createImageReference(ctx context.Context, name string,
 	// TODO(random-liu): Figure out which is the more performant sequence create then update or
 	// update then create.
 	// TODO: Call CRIImageService directly
-	oldImg, err := c.client.ImageService().Create(ctx, img)
-	if err == nil || !errdefs.IsAlreadyExists(err) {
+	oldImg, err := c.images.Create(ctx, img)
+	if err == nil {
+		if c.publisher != nil {
+			if err := c.publisher.Publish(ctx, "/images/create", &eventstypes.ImageCreate{
+				Name:   img.Name,
+				Labels: img.Labels,
+			}); err != nil {
+				return err
+			}
+		}
+		return nil
+	} else if !errdefs.IsAlreadyExists(err) {
 		return err
 	}
 	if oldImg.Target.Digest == img.Target.Digest && oldImg.Labels[crilabels.ImageLabelKey] == labels[crilabels.ImageLabelKey] {
 		return nil
 	}
-	_, err = c.client.ImageService().Update(ctx, img, "target", "labels."+crilabels.ImageLabelKey)
+	_, err = c.images.Update(ctx, img, "target", "labels."+crilabels.ImageLabelKey)
+	if err == nil && c.publisher != nil {
+		if c.publisher != nil {
+			if err := c.publisher.Publish(ctx, "/images/update", &eventstypes.ImageUpdate{
+				Name:   img.Name,
+				Labels: img.Labels,
+			}); err != nil {
+				return err
+			}
+		}
+	}
 	return err
 }
 
 // getLabels get image labels to be added on CRI image
 func (c *CRIImageService) getLabels(ctx context.Context, name string) map[string]string {
 	labels := map[string]string{crilabels.ImageLabelKey: crilabels.ImageLabelValue}
-	// TODO: Separate config here to generalize pinned image list
-	configSandboxImage := "" //c.config.SandboxImage
-	// parse sandbox image
-	sandboxNamedRef, err := distribution.ParseDockerRef(configSandboxImage)
-	if err != nil {
-		log.G(ctx).Errorf("failed to parse sandbox image from config %s", sandboxNamedRef)
-		return nil
-	}
-	sandboxRef := sandboxNamedRef.String()
-	// Adding pinned image label to sandbox image
-	if sandboxRef == name {
-		labels[crilabels.PinnedImageLabelKey] = crilabels.PinnedImageLabelValue
+	for _, pinned := range c.config.PinnedImages {
+		if pinned == name {
+			labels[crilabels.PinnedImageLabelKey] = crilabels.PinnedImageLabelValue
+		}
 	}
 	return labels
 }
@@ -767,9 +780,12 @@ func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, i
 		return snapshotter, nil
 	}
 
-	if p, ok := c.runtimePlatforms[runtimeHandler]; ok && p.Snapshotter != snapshotter {
-		snapshotter = p.Snapshotter
-		log.G(ctx).Infof("experimental: PullImage %q for runtime %s, using snapshotter %s", imageRef, runtimeHandler, snapshotter)
+	// TODO: Ensure error is returned if runtime not found?
+	if c.runtimePlatforms != nil {
+		if p, ok := c.runtimePlatforms[runtimeHandler]; ok && p.Snapshotter != snapshotter {
+			snapshotter = p.Snapshotter
+			log.G(ctx).Infof("experimental: PullImage %q for runtime %s, using snapshotter %s", imageRef, runtimeHandler, snapshotter)
+		}
 	}
 
 	return snapshotter, nil

pkg/cri/server/images/image_pull_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/containerd/containerd/v2/pkg/cri/annotations"
 	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	"github.com/containerd/containerd/v2/pkg/cri/labels"
+	"github.com/containerd/containerd/v2/platforms"
 )
 
 func TestParseAuth(t *testing.T) {
@@ -402,14 +403,13 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) {
 			expectSnapshotter: defaultSnashotter,
 		},
 		{
-			desc: "should return error for runtime not found",
+			desc: "should return default snapshotter for runtime not found",
 			podSandboxConfig: &runtime.PodSandboxConfig{
 				Annotations: map[string]string{
 					annotations.RuntimeHandler: "runtime-not-exists",
 				},
 			},
-			expectErr:         true,
-			expectSnapshotter: "",
+			expectSnapshotter: defaultSnashotter,
 		},
 		{
 			desc: "should return snapshotter provided in podSandboxConfig.Annotations",
@@ -426,12 +426,10 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) {
 		t.Run(tt.desc, func(t *testing.T) {
 			cri, _ := newTestCRIService()
 			cri.config.Snapshotter = defaultSnashotter
-			/*
-				cri.config.ContainerdConfig.Runtimes = make(map[string]criconfig.Runtime)
-				cri.config.ContainerdConfig.Runtimes["exiting-runtime"] = criconfig.Runtime{
-					Snapshotter: runtimeSnapshotter,
-				}
-			*/
+			cri.runtimePlatforms["exiting-runtime"] = ImagePlatform{
+				Platform:    platforms.DefaultSpec(),
+				Snapshotter: runtimeSnapshotter,
+			}
 			snapshotter, err := cri.snapshotterFromPodSandboxConfig(context.Background(), "test-image", tt.podSandboxConfig)
 			assert.Equal(t, tt.expectSnapshotter, snapshotter)
 			if tt.expectErr {
@@ -492,49 +490,51 @@ func TestImageGetLabels(t *testing.T) {
 	criService, _ := newTestCRIService()
 
 	tests := []struct {
-		name               string
-		expectedLabel      map[string]string
-		configSandboxImage string
-		pullImageName      string
+		name          string
+		expectedLabel map[string]string
+		pinnedImages  []string
+		pullImageName string
 	}{
 		{
-			name:               "pinned image labels should get added on sandbox image",
-			expectedLabel:      map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
-			configSandboxImage: "k8s.gcr.io/pause:3.9",
-			pullImageName:      "k8s.gcr.io/pause:3.9",
+			name:          "pinned image labels should get added on sandbox image",
+			expectedLabel: map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
+			pinnedImages:  []string{"k8s.gcr.io/pause:3.9"},
+			pullImageName: "k8s.gcr.io/pause:3.9",
 		},
 		{
-			name:               "pinned image labels should get added on sandbox image without tag",
-			expectedLabel:      map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
-			configSandboxImage: "k8s.gcr.io/pause",
-			pullImageName:      "k8s.gcr.io/pause:latest",
+			name:          "pinned image labels should get added on sandbox image without tag",
+			expectedLabel: map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
+			pinnedImages:  []string{"k8s.gcr.io/pause", "k8s.gcr.io/pause:latest"},
+			pullImageName: "k8s.gcr.io/pause:latest",
 		},
 		{
-			name:               "pinned image labels should get added on sandbox image specified with tag and digest both",
-			expectedLabel:      map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
-			configSandboxImage: "k8s.gcr.io/pause:3.9@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
-			pullImageName:      "k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
+			name:          "pinned image labels should get added on sandbox image specified with tag and digest both",
+			expectedLabel: map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
+			pinnedImages: []string{
+				"k8s.gcr.io/pause:3.9@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
+				"k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
+			},
+			pullImageName: "k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
 		},
 
 		{
-			name:               "pinned image labels should get added on sandbox image specified with digest",
-			expectedLabel:      map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
-			configSandboxImage: "k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
-			pullImageName:      "k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
+			name:          "pinned image labels should get added on sandbox image specified with digest",
+			expectedLabel: map[string]string{labels.ImageLabelKey: labels.ImageLabelValue, labels.PinnedImageLabelKey: labels.PinnedImageLabelValue},
+			pinnedImages:  []string{"k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2"},
+			pullImageName: "k8s.gcr.io/pause@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2",
 		},
 
 		{
-			name:               "pinned image labels should not get added on other image",
-			expectedLabel:      map[string]string{labels.ImageLabelKey: labels.ImageLabelValue},
-			configSandboxImage: "k8s.gcr.io/pause:3.9",
-			pullImageName:      "k8s.gcr.io/random:latest",
+			name:          "pinned image labels should not get added on other image",
+			expectedLabel: map[string]string{labels.ImageLabelKey: labels.ImageLabelValue},
+			pinnedImages:  []string{"k8s.gcr.io/pause:3.9"},
+			pullImageName: "k8s.gcr.io/random:latest",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Change this config name
-			//criService.config.SandboxImage = tt.configSandboxImage
+			criService.config.PinnedImages = tt.pinnedImages
 			labels := criService.getLabels(context.Background(), tt.pullImageName)
 			assert.Equal(t, tt.expectedLabel, labels)
 

pkg/cri/server/images/image_remove.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 
+	eventstypes "github.com/containerd/containerd/v2/api/events"
 	"github.com/containerd/containerd/v2/errdefs"
 	"github.com/containerd/containerd/v2/images"
 	"github.com/containerd/containerd/v2/tracing"
@@ -56,12 +57,20 @@ func (c *GRPCCRIImageService) RemoveImage(ctx context.Context, r *runtime.Remove
 			// someone else before this point.
 			opts = []images.DeleteOpt{images.SynchronousDelete()}
 		}
-		err = c.client.ImageService().Delete(ctx, ref, opts...)
+		err = c.images.Delete(ctx, ref, opts...)
 		if err == nil || errdefs.IsNotFound(err) {
 			// Update image store to reflect the newest state in containerd.
 			if err := c.imageStore.Update(ctx, ref); err != nil {
 				return nil, fmt.Errorf("failed to update image reference %q for %q: %w", ref, image.ID, err)
 			}
+
+			if c.publisher != nil {
+				if err := c.publisher.Publish(ctx, "/images/delete", &eventstypes.ImageDelete{
+					Name: ref,
+				}); err != nil {
+					return nil, err
+				}
+			}
 			continue
 		}
 		return nil, fmt.Errorf("failed to delete image reference %q for %q: %w", ref, image.ID, err)