Split image config from CRI plugin

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

pkg/cri/server/images/check.go

@@ -0,0 +1,77 @@
+/*
+   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 images
+
+import (
+	"context"
+	"fmt"
+	"sync"
+
+	"github.com/containerd/containerd/v2/images"
+	"github.com/containerd/containerd/v2/platforms"
+	"github.com/containerd/log"
+)
+
+// LoadImages checks all existing images to ensure they are ready to
+// be used for CRI. It may try to recover images which are not ready
+// but will only log errors, not return any.
+func (c *CRIImageService) CheckImages(ctx context.Context) error {
+	// TODO: Move way from `client.ListImages` to directly using image store
+	cImages, err := c.client.ListImages(ctx)
+	if err != nil {
+		return fmt.Errorf("unable to list images: %w", err)
+	}
+
+	// TODO: Support all snapshotter
+	snapshotter := c.config.Snapshotter
+	var wg sync.WaitGroup
+	for _, i := range cImages {
+		wg.Add(1)
+		i := i
+		go func() {
+			defer wg.Done()
+			// TODO: Check platform/snapshot combination. Snapshot check should come first
+			ok, _, _, _, err := images.Check(ctx, i.ContentStore(), i.Target(), platforms.Default())
+			if err != nil {
+				log.G(ctx).WithError(err).Errorf("Failed to check image content readiness for %q", i.Name())
+				return
+			}
+			if !ok {
+				log.G(ctx).Warnf("The image content readiness for %q is not ok", i.Name())
+				return
+			}
+			// Checking existence of top-level snapshot for each image being recovered.
+			// TODO: This logic should be done elsewhere and owned by the image service
+			unpacked, err := i.IsUnpacked(ctx, snapshotter)
+			if err != nil {
+				log.G(ctx).WithError(err).Warnf("Failed to check whether image is unpacked for image %s", i.Name())
+				return
+			}
+			if !unpacked {
+				log.G(ctx).Warnf("The image %s is not unpacked.", i.Name())
+				// TODO(random-liu): Consider whether we should try unpack here.
+			}
+			if err := c.UpdateImage(ctx, i.Name()); err != nil {
+				log.G(ctx).WithError(err).Warnf("Failed to update reference for image %q", i.Name())
+				return
+			}
+			log.G(ctx).Debugf("Loaded image %q", i.Name())
+		}()
+	}
+	wg.Wait()
+	return nil
+}

pkg/cri/server/images/image_pull.go

@@ -194,12 +194,12 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential
 
 	// Temporarily removed for v2 upgrade
 	//pullOpts = append(pullOpts, c.encryptedImagesPullOpts()...)
-	if !c.config.ContainerdConfig.DisableSnapshotAnnotations {
+	if !c.config.DisableSnapshotAnnotations {
 		pullOpts = append(pullOpts,
 			containerd.WithImageHandlerWrapper(snpkg.AppendInfoHandlerWrapper(ref)))
 	}
 
-	if c.config.ContainerdConfig.DiscardUnpackedLayers {
+	if c.config.DiscardUnpackedLayers {
 		// Allows GC to clean layers up from the content store after unpacking
 		pullOpts = append(pullOpts,
 			containerd.WithChildLabelMap(containerdimages.ChildGCLabelsFilterLayers))
@@ -333,7 +333,8 @@ func (c *CRIImageService) createImageReference(ctx context.Context, name string,
 // 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}
-	configSandboxImage := c.config.SandboxImage
+	// TODO: Separate config here to generalize pinned image list
+	configSandboxImage := "" //c.config.SandboxImage
 	// parse sandbox image
 	sandboxNamedRef, err := distribution.ParseDockerRef(configSandboxImage)
 	if err != nil {
@@ -756,7 +757,7 @@ func (rt *pullRequestReporterRoundTripper) RoundTrip(req *http.Request) (*http.R
 // See https://github.com/containerd/containerd/issues/6657
 func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, imageRef string,
 	s *runtime.PodSandboxConfig) (string, error) {
-	snapshotter := c.config.ContainerdConfig.Snapshotter
+	snapshotter := c.config.Snapshotter
 	if s == nil || s.Annotations == nil {
 		return snapshotter, nil
 	}
@@ -766,13 +767,10 @@ func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, i
 		return snapshotter, nil
 	}
 
-	// TODO: Find other way to retrieve sandbox runtime, this must belong to the Runtime part of the CRI.
-	ociRuntime, err := c.config.GetSandboxRuntime(s, runtimeHandler)
-	if err != nil {
-		return "", fmt.Errorf("experimental: failed to get sandbox runtime for %s: %w", runtimeHandler, err)
+	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)
 	}
 
-	snapshotter = c.RuntimeSnapshotter(ctx, ociRuntime)
-	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

@@ -425,11 +425,13 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			cri, _ := newTestCRIService()
-			cri.config.ContainerdConfig.Snapshotter = defaultSnashotter
-			cri.config.ContainerdConfig.Runtimes = make(map[string]criconfig.Runtime)
-			cri.config.ContainerdConfig.Runtimes["exiting-runtime"] = criconfig.Runtime{
-				Snapshotter: runtimeSnapshotter,
-			}
+			cri.config.Snapshotter = defaultSnashotter
+			/*
+				cri.config.ContainerdConfig.Runtimes = make(map[string]criconfig.Runtime)
+				cri.config.ContainerdConfig.Runtimes["exiting-runtime"] = criconfig.Runtime{
+					Snapshotter: runtimeSnapshotter,
+				}
+			*/
 			snapshotter, err := cri.snapshotterFromPodSandboxConfig(context.Background(), "test-image", tt.podSandboxConfig)
 			assert.Equal(t, tt.expectSnapshotter, snapshotter)
 			if tt.expectErr {
@@ -531,7 +533,8 @@ func TestImageGetLabels(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			criService.config.SandboxImage = tt.configSandboxImage
+			// Change this config name
+			//criService.config.SandboxImage = tt.configSandboxImage
 			labels := criService.getLabels(context.Background(), tt.pullImageName)
 			assert.Equal(t, tt.expectedLabel, labels)
 

pkg/cri/server/images/service.go

@@ -28,7 +28,6 @@ import (
 	"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"
@@ -71,22 +70,28 @@ func init() {
 				return nil, fmt.Errorf("unable to init client for cri image service: %w", err)
 			}
 
+			snapshotterOverrides := map[string]RuntimePlatform{}
 			imageFSPaths := map[string]string{}
 			// TODO: Figure out a way to break this plugin's dependency on a shared runtime config
-			for _, ociRuntime := range c.ContainerdConfig.Runtimes {
+			for runtimeName, ociRuntime := range c.ContainerdConfig.Runtimes {
 				// Can not use `c.RuntimeSnapshotter() yet, so hard-coding here.`
 				snapshotter := ociRuntime.Snapshotter
 				if snapshotter != "" {
+					snapshotterOverrides[runtimeName] = RuntimePlatform{
+						Snapshotter: snapshotter,
+						// TODO: This must be provided by runtime
+						Platform: platforms.DefaultSpec(),
+					}
 					imageFSPaths[snapshotter] = filepath.Join(c.ContainerdRootDir, plugins.SnapshotPlugin.String()+"."+snapshotter)
 					log.L.Infof("Get image filesystem path %q for snapshotter %q", imageFSPaths[snapshotter], snapshotter)
 				}
 			}
-			snapshotter := c.ContainerdConfig.Snapshotter
+			snapshotter := c.ImageConfig.Snapshotter
 			imageFSPaths[snapshotter] = filepath.Join(c.ContainerdRootDir, plugins.SnapshotPlugin.String()+"."+snapshotter)
 			log.L.Infof("Get image filesystem path %q for snapshotter %q", imageFSPaths[snapshotter], snapshotter)
 
 			// TODO: Pull out image specific configs here!
-			service, err := NewService(c, imageFSPaths, client)
+			service, err := NewService(c.ImageConfig, imageFSPaths, snapshotterOverrides, client)
 			if err != nil {
 				return nil, fmt.Errorf("failed to create image service: %w", err)
 			}
@@ -96,6 +101,11 @@ func init() {
 	})
 }
 
+type RuntimePlatform struct {
+	Snapshotter string
+	Platform    platforms.Platform
+}
+
 type CRIImageService struct {
 	// config contains all configurations.
 	// TODO: Migrate configs from cri type once moved to its own plugin
@@ -110,12 +120,14 @@ type CRIImageService struct {
 	// - image decryption (moved to transfer service)
 	// - default runtime
 	// - stats collection interval (only used to startup snapshot sync)
-	config criconfig.Config
+	config criconfig.ImageConfig
 	// client is an instance of the containerd client
 	// TODO: Remove this in favor of using plugins directly
 	client *containerd.Client
 	// imageFSPaths contains path to image filesystem for snapshotters.
 	imageFSPaths map[string]string
+	// runtimePlatforms are the platforms configured for a runtime.
+	runtimePlatforms map[string]RuntimePlatform
 	// imageStore stores all resources associated with images.
 	imageStore *imagestore.Store
 	// snapshotStore stores information of all snapshots.
@@ -140,31 +152,29 @@ type GRPCCRIImageService struct {
 //     - Image Service (from metadata)
 //     - Content store (from metadata)
 //  3. Separate image cache and snapshot cache to first class plugins, make the snapshot cache much more efficient and intelligent
-func NewService(config criconfig.Config, imageFSPaths map[string]string, client *containerd.Client) (*CRIImageService, error) {
+func NewService(config criconfig.ImageConfig, imageFSPaths map[string]string, runtimePlatforms map[string]RuntimePlatform, client *containerd.Client) (*CRIImageService, error) {
 	svc := CRIImageService{
 		config:                      config,
 		client:                      client,
 		imageStore:                  imagestore.NewStore(client.ImageService(), client.ContentStore(), platforms.Default()),
 		imageFSPaths:                imageFSPaths,
+		runtimePlatforms:            runtimePlatforms,
 		snapshotStore:               snapshotstore.NewStore(),
 		unpackDuplicationSuppressor: kmutex.New(),
 	}
 
 	snapshotters := map[string]snapshot.Snapshotter{}
-	ctx := ctrdutil.NamespacedContext()
 
-	// Add runtime specific snapshotters
-	for _, runtime := range config.ContainerdConfig.Runtimes {
-		snapshotterName := svc.RuntimeSnapshotter(ctx, runtime)
-		if snapshotter := svc.client.SnapshotService(snapshotterName); snapshotter != nil {
-			snapshotters[snapshotterName] = snapshotter
+	for _, rp := range runtimePlatforms {
+		if snapshotter := svc.client.SnapshotService(rp.Snapshotter); snapshotter != nil {
+			snapshotters[rp.Snapshotter] = snapshotter
 		} else {
-			return nil, fmt.Errorf("failed to find snapshotter %q", snapshotterName)
+			return nil, fmt.Errorf("failed to find snapshotter %q", rp.Snapshotter)
 		}
 	}
 
 	// Add default snapshotter
-	snapshotterName := svc.config.ContainerdConfig.Snapshotter
+	snapshotterName := svc.config.Snapshotter
 	if snapshotter := svc.client.SnapshotService(snapshotterName); snapshotter != nil {
 		snapshotters[snapshotterName] = snapshotter
 	} else {
@@ -220,9 +230,10 @@ func (c *CRIImageService) LocalResolve(refOrID string) (imagestore.Image, error)
 
 // RuntimeSnapshotter overrides the default snapshotter if Snapshotter is set for this runtime.
 // See https://github.com/containerd/containerd/issues/6657
+// TODO: Pass in name and get back runtime platform
 func (c *CRIImageService) RuntimeSnapshotter(ctx context.Context, ociRuntime criconfig.Runtime) string {
 	if ociRuntime.Snapshotter == "" {
-		return c.config.ContainerdConfig.Snapshotter
+		return c.config.Snapshotter
 	}
 
 	log.G(ctx).Debugf("Set snapshotter for runtime %s to %s", ociRuntime.Type, ociRuntime.Snapshotter)

pkg/cri/server/images/service_test.go

@@ -41,7 +41,7 @@ const (
 // newTestCRIService creates a fake criService for test.
 func newTestCRIService() (*CRIImageService, *GRPCCRIImageService) {
 	service := &CRIImageService{
-		config:        testConfig,
+		config:        testConfig.ImageConfig,
 		imageFSPaths:  map[string]string{"overlayfs": testImageFSPath},
 		imageStore:    imagestore.NewStore(nil, nil, platforms.Default()),
 		snapshotStore: snapshotstore.NewStore(),
@@ -125,9 +125,7 @@ func TestRuntimeSnapshotter(t *testing.T) {
 		test := test
 		t.Run(test.desc, func(t *testing.T) {
 			cri, _ := newTestCRIService()
-			cri.config = criconfig.Config{
-				PluginConfig: criconfig.DefaultConfig(),
-			}
+			cri.config = criconfig.DefaultConfig().ImageConfig
 			assert.Equal(t, test.expectSnapshotter, cri.RuntimeSnapshotter(context.Background(), test.runtime))
 		})
 	}