change criService.runtimeHandlers slice to a map

djdongjin · djdongjin · commit 97eb1cd46f49 · 2025-04-16T20:52:56.000-04:00
This patch changes `criService.runtimeHandlers` from a slice to a map, so we don't need to for-loop slice in `createContainer`. It also refactors `introspectRuntimeHandler` a bit so it can be called given a single `config.Runtime`. This will help implement extracting runtime config to separate files (containerd#9296) Signed-off-by: Jin Dong <djdongjin95@gmail.com>
diff --git a/internal/cri/server/container_create.go b/internal/cri/server/container_create.go
@@ -283,12 +283,9 @@ func (c *criService) createContainer(r *createContainerRequest) (_ string, retEr
 		log.G(r.ctx).Debugf("Ignoring volumes defined in image %v because IgnoreImageDefinedVolumes is set", r.imageID)
 	}
 
-	var runtimeHandler *runtime.RuntimeHandler
-	for _, f := range c.runtimeHandlers {
-		if f.Name == r.sandboxRuntimeHandler {
-			runtimeHandler = f
-			break
-		}
+	runtimeHandler, ok := c.runtimeHandlers[r.sandboxRuntimeHandler]
+	if !ok {
+		return "", fmt.Errorf("failed to find runtime handler %q", r.sandboxRuntimeHandler)
 	}
 	log.G(r.ctx).Debugf("Use OCI runtime %+v for sandbox %q and container %q", ociRuntime, r.sandboxID, r.containerID)
 
diff --git a/internal/cri/server/service.go b/internal/cri/server/service.go
@@ -155,7 +155,7 @@ type criService struct {
 	// sandboxService is the sandbox related service for CRI
 	sandboxService sandboxService
 	// runtimeHandlers contains runtime handler info
-	runtimeHandlers []*runtime.RuntimeHandler
+	runtimeHandlers map[string]*runtime.RuntimeHandler
 	// runtimeFeatures container runtime features info
 	runtimeFeatures *runtime.RuntimeFeatures
 }
@@ -198,6 +198,7 @@ func NewCRIService(options *CRIServiceOptions) (CRIService, runtime.RuntimeServi
 		containerNameIndex: registrar.NewRegistrar(),
 		netPlugin:          make(map[string]cni.CNI),
 		sandboxService:     newCriSandboxService(&config, options.SandboxControllers),
+		runtimeHandlers:    make(map[string]*runtime.RuntimeHandler),
 	}
 
 	// TODO: Make discard time configurable
@@ -241,9 +242,11 @@ func NewCRIService(options *CRIServiceOptions) (CRIService, runtime.RuntimeServi
 
 	c.nri = nri.NewAPI(options.NRI, &criImplementation{c})
 
-	c.runtimeHandlers, err = c.introspectRuntimeHandlers(ctx)
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to introspect runtime handlers: %w", err)
+	intro := c.client.IntrospectionService()
+	for name, r := range c.config.Runtimes {
+		if err := c.introspectRuntimeHandler(ctx, intro, name, r); err != nil {
+			return nil, nil, fmt.Errorf("failed to introspect runtime %s: %w", name, err)
+		}
 	}
 
 	c.runtimeFeatures = &runtime.RuntimeFeatures{
@@ -365,38 +368,36 @@ func (c *criService) IsInitialized() bool {
 	return c.initialized.Load()
 }
 
-func (c *criService) introspectRuntimeHandlers(ctx context.Context) ([]*runtime.RuntimeHandler, error) {
-	var res []*runtime.RuntimeHandler
-	intro := c.client.IntrospectionService()
-	for name, r := range c.config.Runtimes {
-		h := runtime.RuntimeHandler{
-			Name: name,
-		}
-		rawFeatures, err := introspectRuntimeFeatures(ctx, intro, r)
-		if err != nil {
-			log.G(ctx).WithError(err).Debugf("failed to introspect features of runtime %q", name)
-		} else {
-			h.Features = &runtime.RuntimeHandlerFeatures{}
-			if slices.Contains(rawFeatures.MountOptions, "rro") {
-				if kernelSupportsRRO {
-					log.G(ctx).Debugf("runtime %q supports recursive read-only mounts", name)
-					h.Features.RecursiveReadOnlyMounts = true
-				} else {
-					log.G(ctx).Debugf("runtime %q supports recursive read-only mounts, but the kernel does not", name)
-				}
+func (c *criService) introspectRuntimeHandler(ctx context.Context, intro introspection.Service, name string, r config.Runtime) error {
+	h := &runtime.RuntimeHandler{
+		Name: name,
+	}
+	rawFeatures, err := introspectRuntimeFeatures(ctx, intro, r)
+	if err != nil {
+		log.G(ctx).WithError(err).Debugf("failed to introspect features of runtime %q", name)
+	} else {
+		h.Features = &runtime.RuntimeHandlerFeatures{}
+		if slices.Contains(rawFeatures.MountOptions, "rro") {
+			if kernelSupportsRRO {
+				log.G(ctx).Debugf("runtime %q supports recursive read-only mounts", name)
+				h.Features.RecursiveReadOnlyMounts = true
+			} else {
+				log.G(ctx).Debugf("runtime %q supports recursive read-only mounts, but the kernel does not", name)
 			}
-			userns := supportsCRIUserns(rawFeatures)
-			h.Features.UserNamespaces = userns
-			log.G(ctx).Debugf("runtime %q supports CRI userns: %v", name, userns)
-		}
-		res = append(res, &h)
-		if name == c.config.DefaultRuntimeName {
-			defH := h
-			defH.Name = "" // denotes default
-			res = append(res, &defH)
 		}
+		userns := supportsCRIUserns(rawFeatures)
+		h.Features.UserNamespaces = userns
+		log.G(ctx).Debugf("runtime %q supports CRI userns: %v", name, userns)
 	}
-	return res, nil
+
+	c.runtimeHandlers[name] = h
+	if name == c.config.DefaultRuntimeName {
+		defH := *h
+		defH.Name = "" // denotes default
+		c.runtimeHandlers[""] = &defH
+	}
+
+	return nil
 }
 
 func introspectRuntimeFeatures(ctx context.Context, intro introspection.Service, r config.Runtime) (*features.Features, error) {
diff --git a/internal/cri/server/status.go b/internal/cri/server/status.go
@@ -20,7 +20,9 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	goruntime "runtime"
+	"slices"
 
 	"github.com/containerd/containerd/api/services/introspection/v1"
 	"github.com/containerd/log"
@@ -57,7 +59,7 @@ func (c *criService) Status(ctx context.Context, r *runtime.StatusRequest) (*run
 			runtimeCondition,
 			networkCondition,
 		}},
-		RuntimeHandlers: c.runtimeHandlers,
+		RuntimeHandlers: slices.Collect(maps.Values(c.runtimeHandlers)),
 		Features:        c.runtimeFeatures,
 	}
 	if r.Verbose {

Original file line number	Diff line number	Diff line change
`@@ -283,12 +283,9 @@ func (c criService) createContainer(r createContainerRequest) (_ string, retEr`
`283`	`283`	`log.G(r.ctx).Debugf("Ignoring volumes defined in image %v because IgnoreImageDefinedVolumes is set", r.imageID)`
`284`	`284`	`}`
`285`	`285`
`286`		`- var runtimeHandler *runtime.RuntimeHandler`
`287`		`- for _, f := range c.runtimeHandlers {`
`288`		`- if f.Name == r.sandboxRuntimeHandler {`
`289`		`- runtimeHandler = f`
`290`		`- break`
`291`		`- }`
	`286`	`+ runtimeHandler, ok := c.runtimeHandlers[r.sandboxRuntimeHandler]`
	`287`	`+ if !ok {`
	`288`	`+ return "", fmt.Errorf("failed to find runtime handler %q", r.sandboxRuntimeHandler)`
`292`	`289`	`}`
`293`	`290`	`log.G(r.ctx).Debugf("Use OCI runtime %+v for sandbox %q and container %q", ociRuntime, r.sandboxID, r.containerID)`
`294`	`291`