Conversation
Mirantis#350) Signed-off-by: zhangguanzhang <zhangguanzhang@qq.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The updated call-sequence expectations omit the new sandbox inspection, causing the affected tests to fail.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Backports Docker runtime-handler support so RuntimeClass handlers select and propagate per-container runtimes.
Changes:
- Validates requested handlers against Docker runtimes.
- Propagates sandbox runtimes to workload containers and status.
- Updates fakes and tests for runtime inheritance.
| File | Description |
|---|---|
core/container_create.go |
Inherits the sandbox runtime. |
core/container_test.go |
Uses real test sandboxes. |
core/docker_service.go |
Adds placeholder runtime-lock commentary. |
core/docker_service_test.go |
Synchronizes checkpoint test state. |
core/sandbox_helpers.go |
Validates configured Docker runtimes. |
core/sandbox_helpers_test.go |
Tests runtime selection and inheritance. |
core/sandbox_run.go |
Applies validated runtime handlers. |
core/sandbox_status.go |
Reports the sandbox runtime. |
libdocker/fake_client.go |
Updates Docker API types and default runtime data. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| randomError := fmt.Errorf("random error") | ||
|
|
||
| // sandBox run called "inspect_image", "pull", "create", "start", "inspect_container", | ||
| sandBoxCalls := []string{"inspect_image", "pull", "create", "start", "inspect_container"} |
Comment on lines
+67
to
+70
| sandboxInfo, err := ds.client.InspectContainer(r.GetPodSandboxId()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to get container's sandbox ID: %v", err) | ||
| } |
chenyaooo
marked this pull request as draft
October 1, 2026 21:57
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport RuntimeClass.handler support to release/0.4
Cherry-pick of #350
This feature already shipped in v0.4.0 and v0.4.1
git tag --contains 57af35d2returns:v0.4.0 v0.4.1
It is missing from
v0.4.2and later becauserelease/0.4was branched fromrelease/0.3rather than frommasterWhat it does
cri-dockerd currently rejects any pod whose RuntimeClass specifies a handler:
RuntimeHandler
nvidianot supportedWith this change, the handler from
RuntimeClassis mapped onto Docker'sper-container runtime:
docker info,and an unknown handler fails with a clear error instead of silently falling
back.
containers inherit the runtime selected for their sandbox.
PodSandboxStatusreports the runtime actually in use.dockercontinue to use Docker's defaultruntime, so existing workloads are unaffected.
Why backport to release/0.4
The NVIDIA GPU Operator requires this. From v25.10.0 it enables CDI by default
and sets
runtimeClassName: nvidiaon its operands, creating thenvidia,nvidia-cdi, andnvidia-legacyRuntimeClasses. On a node runningcri-dockerd from the 0.4 line, every one of those pods fails to start with
FailedCreatePodSandBox, so the GPU Operator cannot be used at all. With thischange the handlers resolve against the runtimes the NVIDIA container toolkit
installs in
daemon.jsonand the operands schedule normally.