Skip to content

Commit c5fdfd4

Browse files
committed
fix(shim): support non-CRI callers (BuildKit, ctr) in containerd shim
Fix two issues that prevent gVisor containers from working when created through containerd's direct API (non-CRI), such as BuildKit's containerd worker or `ctr run`: 1. Sandbox detection: SpecContainerType returns ContainerTypeUnspecified for non-CRI callers that don't set the container type annotation. Previously this resulted in p.Sandbox=false, which skipped passing IO file descriptors to runsc create. Without IO, container processes receive SIGPIPE on stdout/stderr writes and exit with code 128. Fix: treat ContainerTypeUnspecified as sandbox, since non-CRI containers are always root/sandbox containers. 2. Wait error handling: For short-lived containers, the sandbox may exit before `runsc wait` retrieves the exit status, producing "sandbox no longer running and its exit status is unavailable". This error was treated as fatal (setting internalErrorCode=128) even when the container exited successfully (status=0). Fix: extract adjustWaitStatus() and when status is 0, log the error as a warning but preserve the exit status. Includes unit tests for both fixes: - TestSandboxDetection: 7 cases covering all container type variants - TestAdjustWaitStatus: 7 cases covering all status/error combinations Fixes #12198 Tested: BuildKit containerd worker with gVisor on GCE e2-standard-4, containerd v1.7.29, runsc release-20260330.0. Six consecutive RUN steps (echo, ls, cat, apk add curl) all pass with exit code 0.
1 parent 0f29b0e commit c5fdfd4

6 files changed

Lines changed: 216 additions & 7 deletions

File tree

pkg/shim/v1/proc/BUILD

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("//tools:defs.bzl", "go_library")
1+
load("//tools:defs.bzl", "go_library", "go_test")
22

33
package(
44
default_applicable_licenses = ["//:license"],
@@ -40,3 +40,12 @@ go_library(
4040
"@org_golang_x_sys//unix:go_default_library",
4141
],
4242
)
43+
44+
go_test(
45+
name = "proc_test",
46+
srcs = ["init_test.go"],
47+
library = ":proc",
48+
deps = [
49+
"//pkg/shim/v1/runsccmd",
50+
],
51+
)

pkg/shim/v1/proc/init.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,7 @@ func (p *Init) start(ctx context.Context, restoreConf *extension.RestoreConfig)
252252
}
253253
go func() {
254254
status, err := p.runtime.Wait(context.Background(), p.id)
255-
if err != nil {
256-
log.G(ctx).WithError(err).Errorf("Failed to wait for container %q", p.id)
257-
p.killAllLocked(ctx)
258-
status = internalErrorCode
259-
}
255+
status = adjustWaitStatus(ctx, p, status, err)
260256
ExitCh <- Exit{
261257
Timestamp: time.Now(),
262258
ID: p.id,
@@ -485,6 +481,23 @@ func (p *Init) convertStatus(status string) string {
485481
return status
486482
}
487483

484+
// adjustWaitStatus handles the exit status from runtime.Wait. When status is
485+
// 0 but an error is returned (e.g., "sandbox no longer running" for short-lived
486+
// containers), the error is benign and the status is preserved. For non-zero
487+
// status with an error, the container is killed and internalErrorCode is used.
488+
func adjustWaitStatus(ctx context.Context, p *Init, status int, err error) int {
489+
if err == nil {
490+
return status
491+
}
492+
if status == 0 {
493+
log.G(ctx).WithError(err).Warnf("Container %q wait error with exit status 0 (ignoring)", p.id)
494+
return 0
495+
}
496+
log.G(ctx).WithError(err).Errorf("Failed to wait for container %q", p.id)
497+
p.killAllLocked(ctx)
498+
return internalErrorCode
499+
}
500+
488501
func withConditionalIO(c stdio.Stdio) runc.IOOpt {
489502
return func(o *runc.IOOption) {
490503
o.OpenStdin = c.Stdin != ""

pkg/shim/v1/proc/init_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright 2026 The gVisor Authors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package proc
16+
17+
import (
18+
"context"
19+
"fmt"
20+
"testing"
21+
22+
"gvisor.dev/gvisor/pkg/shim/v1/runsccmd"
23+
)
24+
25+
func TestAdjustWaitStatus(t *testing.T) {
26+
ctx := context.Background()
27+
28+
for _, tc := range []struct {
29+
name string
30+
status int
31+
err error
32+
wantStatus int
33+
}{
34+
{
35+
name: "success with no error",
36+
status: 0,
37+
err: nil,
38+
wantStatus: 0,
39+
},
40+
{
41+
name: "non-zero exit with no error",
42+
status: 1,
43+
err: nil,
44+
wantStatus: 1,
45+
},
46+
{
47+
name: "signal exit with no error",
48+
status: 137,
49+
err: nil,
50+
wantStatus: 137,
51+
},
52+
{
53+
name: "success with benign error (short-lived container)",
54+
status: 0,
55+
err: fmt.Errorf("sandbox no longer running and its exit status is unavailable"),
56+
wantStatus: 0,
57+
},
58+
{
59+
name: "success with any error preserves zero status",
60+
status: 0,
61+
err: fmt.Errorf("some unexpected error"),
62+
wantStatus: 0,
63+
},
64+
{
65+
name: "non-zero exit with error becomes internalErrorCode",
66+
status: 1,
67+
err: fmt.Errorf("wait failed"),
68+
wantStatus: internalErrorCode,
69+
},
70+
{
71+
name: "signal exit with error becomes internalErrorCode",
72+
status: 137,
73+
err: fmt.Errorf("wait failed"),
74+
wantStatus: internalErrorCode,
75+
},
76+
{
77+
name: "negative status with error becomes internalErrorCode",
78+
status: -1,
79+
err: fmt.Errorf("wait failed unexpectedly"),
80+
wantStatus: internalErrorCode,
81+
},
82+
{
83+
name: "exit 255 with no error passes through",
84+
status: 255,
85+
err: nil,
86+
wantStatus: 255,
87+
},
88+
} {
89+
t.Run(tc.name, func(t *testing.T) {
90+
// Create a minimal Init with a dummy runtime to prevent nil
91+
// dereference in killAllLocked for the error path.
92+
p := &Init{
93+
id: "test-container",
94+
runtime: &runsccmd.Runsc{Command: "false"},
95+
}
96+
got := adjustWaitStatus(ctx, p, tc.status, tc.err)
97+
if got != tc.wantStatus {
98+
t.Errorf("adjustWaitStatus(status=%d, err=%v) = %d, want %d", tc.status, tc.err, got, tc.wantStatus)
99+
}
100+
})
101+
}
102+
}

pkg/shim/v1/runsc/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ go_test(
6363
library = ":runsc",
6464
deps = [
6565
"//pkg/shim/v1/utils",
66+
"//runsc/specutils",
6667
"@com_github_opencontainers_runtime_spec//specs-go:go_default_library",
6768
],
6869
)

pkg/shim/v1/runsc/service.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,11 @@ func newInit(workDir, namespace string, platform stdio.Platform, r *proc.CreateC
656656
p.WorkDir = workDir
657657
p.IoUID = int(options.IoUID)
658658
p.IoGID = int(options.IoGID)
659-
p.Sandbox = specutils.SpecContainerType(spec) == specutils.ContainerTypeSandbox
659+
// Non-CRI callers (BuildKit, ctr) don't set the container type
660+
// annotation, so SpecContainerType returns ContainerTypeUnspecified.
661+
// These are always root containers that need IO passed to runsc create.
662+
ct := specutils.SpecContainerType(spec)
663+
p.Sandbox = ct == specutils.ContainerTypeSandbox || ct == specutils.ContainerTypeUnspecified
660664
p.UserLog = utils.UserLogPath(spec)
661665
p.Monitor = reaper.Default
662666
return p, nil

pkg/shim/v1/runsc/service_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919

2020
specs "github.com/opencontainers/runtime-spec/specs-go"
2121
"gvisor.dev/gvisor/pkg/shim/v1/utils"
22+
"gvisor.dev/gvisor/runsc/specutils"
2223
)
2324

2425
func TestCgroupPath(t *testing.T) {
@@ -90,6 +91,85 @@ func TestCgroupPath(t *testing.T) {
9091
}
9192
}
9293

94+
func TestSandboxDetection(t *testing.T) {
95+
for _, tc := range []struct {
96+
name string
97+
annotations map[string]string
98+
wantSandbox bool
99+
}{
100+
{
101+
name: "no-annotation (non-CRI caller like BuildKit or ctr)",
102+
annotations: nil,
103+
wantSandbox: true,
104+
},
105+
{
106+
name: "empty-annotations",
107+
annotations: map[string]string{},
108+
wantSandbox: true,
109+
},
110+
{
111+
name: "containerd-sandbox",
112+
annotations: map[string]string{
113+
specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeSandbox,
114+
},
115+
wantSandbox: true,
116+
},
117+
{
118+
name: "containerd-container",
119+
annotations: map[string]string{
120+
specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeContainer,
121+
},
122+
wantSandbox: false,
123+
},
124+
{
125+
name: "crio-sandbox",
126+
annotations: map[string]string{
127+
specutils.CRIOContainerTypeAnnotation: specutils.CRIOContainerTypeSandbox,
128+
},
129+
wantSandbox: true,
130+
},
131+
{
132+
name: "crio-container",
133+
annotations: map[string]string{
134+
specutils.CRIOContainerTypeAnnotation: specutils.CRIOContainerTypeContainer,
135+
},
136+
wantSandbox: false,
137+
},
138+
{
139+
name: "unknown-annotation-value",
140+
annotations: map[string]string{
141+
specutils.ContainerdContainerTypeAnnotation: "unknown-value",
142+
},
143+
wantSandbox: false,
144+
},
145+
{
146+
name: "both-annotations-container-wins",
147+
annotations: map[string]string{
148+
specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeContainer,
149+
specutils.CRIOContainerTypeAnnotation: specutils.CRIOContainerTypeSandbox,
150+
},
151+
// containerd annotation is checked first
152+
wantSandbox: false,
153+
},
154+
{
155+
name: "unrelated-annotations-treated-as-unspecified",
156+
annotations: map[string]string{
157+
"some.other.annotation": "value",
158+
},
159+
wantSandbox: true,
160+
},
161+
} {
162+
t.Run(tc.name, func(t *testing.T) {
163+
spec := &specs.Spec{Annotations: tc.annotations}
164+
ct := specutils.SpecContainerType(spec)
165+
got := ct == specutils.ContainerTypeSandbox || ct == specutils.ContainerTypeUnspecified
166+
if got != tc.wantSandbox {
167+
t.Errorf("sandbox detection for %v: got %v, want %v (containerType=%v)", tc.annotations, got, tc.wantSandbox, ct)
168+
}
169+
})
170+
}
171+
}
172+
93173
// Test cases that cgroup path should not be updated.
94174
func TestCgroupNoUpdate(t *testing.T) {
95175
for _, tc := range []struct {

0 commit comments

Comments
 (0)