From 48116fa596fd0e926749dcdfd3ef92da00fc6dc8 Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Thu, 4 Jun 2026 04:39:15 +0000 Subject: [PATCH] cri: validate CRIU availability and version early Perform an early validation check on both container checkpoint and restore paths to fail-fast if the CRIU binary is missing or is older than the minimum required version (3.16.0). To support runtime-configured environments, the validation respects the custom PATH from the shim manager environment if configured, skipping any relative paths to avoid incorrect daemon-relative resolution. If not configured, it falls back to a standard system PATH lookup. The check result is cached using sync.Once to prevent redundant process spawning. Assisted-by: Antigravity Signed-off-by: Samuel Karp (cherry picked from commit 186397511b62120b4b99157005268339be84e796) Signed-off-by: Samuel Karp --- core/runtime/v2/shim_manager.go | 10 +++ .../cri/server/container_checkpoint_linux.go | 70 +++++++++++++--- .../server/container_checkpoint_linux_test.go | 81 +++++++++++++++++++ internal/cri/server/service.go | 9 +++ plugins/cri/cri.go | 19 +++++ 5 files changed, 177 insertions(+), 12 deletions(-) diff --git a/core/runtime/v2/shim_manager.go b/core/runtime/v2/shim_manager.go index e871a37579..20b4414db2 100644 --- a/core/runtime/v2/shim_manager.go +++ b/core/runtime/v2/shim_manager.go @@ -165,6 +165,16 @@ func (m *ShimManager) ID() string { return plugins.ShimPlugin.String() + ".manager" } +// Env returns the environment configured for the shim manager. +func (m *ShimManager) Env() []string { + if m.env == nil { + return nil + } + cp := make([]string, len(m.env)) + copy(cp, m.env) + return cp +} + // Start launches a new shim instance func (m *ShimManager) Start(ctx context.Context, id string, bundle *Bundle, opts runtime.CreateOpts) (_ ShimInstance, retErr error) { shouldInvokeShimBinary := false diff --git a/internal/cri/server/container_checkpoint_linux.go b/internal/cri/server/container_checkpoint_linux.go index 26f0d9b25f..d2c23f2345 100644 --- a/internal/cri/server/container_checkpoint_linux.go +++ b/internal/cri/server/container_checkpoint_linux.go @@ -26,11 +26,13 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" "strings" "time" crmetadata "github.com/checkpoint-restore/checkpointctl/lib" + criu "github.com/checkpoint-restore/go-criu/v7" "github.com/checkpoint-restore/go-criu/v7/utils" "github.com/containerd/containerd/api/types/runc/options" "github.com/containerd/containerd/v2/client" @@ -135,6 +137,55 @@ func assertCheckpointDirSafe(root string) error { }) } +func (c *criService) checkCriu() error { + c.checkCriuOnce.Do(func() { + c.checkCriuErr = c.doCheckCriu() + }) + return c.checkCriuErr +} + +func (c *criService) doCheckCriu() error { + path := resolveCriuPath(c.shimPath) + if path == "" { + return errors.New("criu binary not found in shim path or system PATH") + } + client := criu.MakeCriu() + client.SetCriuPath(path) + version, err := client.GetCriuVersion() + if err != nil { + return fmt.Errorf("failed to retrieve criu version: %w", err) + } + if version < utils.PodCriuVersion { + return fmt.Errorf("checkpoint/restore requires at least CRIU %d, current version is %d", utils.PodCriuVersion, version) + } + return nil +} + +func resolveCriuPath(customPath string) string { + if customPath != "" { + // This logic is Linux-specific. If CRIU is ever supported on other + // operating systems, path lookup will need to respect that OS's + // conventions. + for _, dir := range filepath.SplitList(customPath) { + if !filepath.IsAbs(dir) { + continue + } + criuPath := filepath.Join(dir, "criu") + if fi, err := os.Stat(criuPath); err == nil && fi.Mode().IsRegular() && fi.Mode()&0111 != 0 { + return criuPath + } + } + return "" + } + if criuPath, err := exec.LookPath("criu"); err == nil { + if absPath, err := filepath.Abs(criuPath); err == nil { + return absPath + } + return criuPath + } + return "" +} + // checkIfCheckpointOCIImage returns checks if the input refers to a checkpoint image. // It returns the StorageImageID of the image the input resolves to, nil otherwise. func (c *criService) checkIfCheckpointOCIImage(ctx context.Context, input string) (string, error) { @@ -189,6 +240,10 @@ func (c *criService) CRImportCheckpoint( sandbox *sandbox.Sandbox, sandboxConfig *runtime.PodSandboxConfig, ) (ctrID string, retErr error) { + if err := c.checkCriu(); err != nil { + return "", fmt.Errorf("checkpoint restore is not enabled: %w", err) + } + var mountPoint string start := time.Now() // Ensure that the image to restore the checkpoint from has been provided. @@ -536,18 +591,9 @@ func (c *criService) CRImportCheckpoint( func (c *criService) CheckpointContainer(ctx context.Context, r *runtime.CheckpointContainerRequest) (*runtime.CheckpointContainerResponse, error) { start := time.Now() - if err := utils.CheckForCriu(utils.PodCriuVersion); err != nil { - errorMessage := fmt.Sprintf( - "CRIU binary not found or too old (<%d). Failed to checkpoint container %q", - utils.PodCriuVersion, - r.GetContainerId(), - ) - log.G(ctx).WithError(err).Error(errorMessage) - return nil, fmt.Errorf( - "%s: %w", - errorMessage, - err, - ) + if err := c.checkCriu(); err != nil { + log.G(ctx).WithError(err).Errorf("Failed to checkpoint container %q", r.GetContainerId()) + return nil, fmt.Errorf("failed to checkpoint container %q: %w", r.GetContainerId(), err) } criContainerStatus, err := c.ContainerStatus(ctx, &runtime.ContainerStatusRequest{ diff --git a/internal/cri/server/container_checkpoint_linux_test.go b/internal/cri/server/container_checkpoint_linux_test.go index 65c887f81a..32cc466aef 100644 --- a/internal/cri/server/container_checkpoint_linux_test.go +++ b/internal/cri/server/container_checkpoint_linux_test.go @@ -235,3 +235,84 @@ func TestFilterAndMergeAnnotations(t *testing.T) { }) } } + +func TestResolveCriuPath(t *testing.T) { + tempDir, err := os.MkdirTemp("", "criu-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + // Create a dummy executable criu + execDir := filepath.Join(tempDir, "bin-exec") + if err := os.MkdirAll(execDir, 0755); err != nil { + t.Fatal(err) + } + execPath := filepath.Join(execDir, "criu") + if err := os.WriteFile(execPath, []byte("dummy"), 0755); err != nil { + t.Fatal(err) + } + + // Create a dummy non-executable criu + nonExecDir := filepath.Join(tempDir, "bin-nonexec") + if err := os.MkdirAll(nonExecDir, 0755); err != nil { + t.Fatal(err) + } + nonExecPath := filepath.Join(nonExecDir, "criu") + if err := os.WriteFile(nonExecPath, []byte("dummy"), 0644); err != nil { + t.Fatal(err) + } + + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + relDir, err := filepath.Rel(wd, execDir) + if err != nil { + t.Fatal(err) + } + + // Mock the system PATH to point to our executable directory + t.Setenv("PATH", execDir) + + tests := []struct { + name string + customPath string + expectedPath string + }{ + { + name: "custom PATH with executable criu", + customPath: execDir, + expectedPath: execPath, + }, + { + name: "custom PATH with non-executable criu", + customPath: nonExecDir, + expectedPath: "", + }, + { + name: "multiple directories in PATH, executable in second", + customPath: nonExecDir + string(filepath.ListSeparator) + execDir, + expectedPath: execPath, + }, + { + name: "custom PATH with relative directory is skipped", + customPath: relDir, + expectedPath: "", + }, + { + name: "empty customPath falls back to system PATH", + customPath: "", + expectedPath: execPath, // Falls back to system PATH (mocked to execDir) + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := resolveCriuPath(tt.customPath) + if path != tt.expectedPath { + t.Errorf("expected %s, got %s", tt.expectedPath, path) + } + }) + } +} diff --git a/internal/cri/server/service.go b/internal/cri/server/service.go index 87eb02e427..42dfa66902 100644 --- a/internal/cri/server/service.go +++ b/internal/cri/server/service.go @@ -161,6 +161,11 @@ type criService struct { runtimeHandlers map[string]*runtime.RuntimeHandler // runtimeFeatures container runtime features info runtimeFeatures *runtime.RuntimeFeatures + // shimPath is the custom PATH environment variable value from the shim manager + shimPath string + + checkCriuOnce sync.Once //nolint:nolintlint,unused // Ignore on non-Linux + checkCriuErr error //nolint:nolintlint,unused // Ignore on non-Linux } type CRIServiceOptions struct { @@ -179,6 +184,9 @@ type CRIServiceOptions struct { // // TODO: Replace this gradually with directly configured instances Client *containerd.Client + + // ShimPath is the custom PATH environment variable value from the shim manager + ShimPath string } // NewCRIService returns a new instance of CRIService @@ -202,6 +210,7 @@ func NewCRIService(options *CRIServiceOptions) (CRIService, runtime.RuntimeServi netPlugin: make(map[string]cni.CNI), sandboxService: newCriSandboxService(&config, options.SandboxControllers), runtimeHandlers: make(map[string]*runtime.RuntimeHandler), + shimPath: options.ShimPath, } // TODO: Make discard time configurable diff --git a/plugins/cri/cri.go b/plugins/cri/cri.go index b87be27eb4..8dd9cccffd 100644 --- a/plugins/cri/cri.go +++ b/plugins/cri/cri.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "strings" "github.com/containerd/log" "github.com/containerd/plugin" @@ -57,6 +58,7 @@ func init() { plugins.SandboxStorePlugin, plugins.TransferPlugin, plugins.WarningPlugin, + plugins.ShimPlugin, }, Config: &defaultConfig, ConfigMigration: configMigration, @@ -114,6 +116,22 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) { return nil, fmt.Errorf("failed to get streaming config: %w", err) } + var shimPath string + shimPlugin, err := ic.GetSingle(plugins.ShimPlugin) + if err != nil { + return nil, fmt.Errorf("failed to get shim plugin: %w", err) + } + if hasEnv, ok := shimPlugin.(interface{ Env() []string }); ok { + env := hasEnv.Env() + for i := len(env) - 1; i >= 0; i-- { + // iterate backwards to grab the last PATH= + if path, ok := strings.CutPrefix(env[i], "PATH="); ok { + shimPath = path + break + } + } + } + options := &server.CRIServiceOptions{ RuntimeService: criRuntimePlugin.(server.RuntimeService), ImageService: criImagePlugin.(server.ImageService), @@ -121,6 +139,7 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) { NRI: getNRIAPI(ic), Client: client, SandboxControllers: sbControllers, + ShimPath: shimPath, } is := criImagePlugin.(imageService).GRPCService()