diff --git a/pkg/shim/v1/proc/BUILD b/pkg/shim/v1/proc/BUILD index e3bf358468..9123a70be1 100644 --- a/pkg/shim/v1/proc/BUILD +++ b/pkg/shim/v1/proc/BUILD @@ -1,4 +1,4 @@ -load("//tools:defs.bzl", "go_library") +load("//tools:defs.bzl", "go_library", "go_test") package( default_applicable_licenses = ["//:license"], @@ -40,3 +40,12 @@ go_library( "@org_golang_x_sys//unix:go_default_library", ], ) + +go_test( + name = "proc_test", + srcs = ["init_test.go"], + library = ":proc", + deps = [ + "//pkg/shim/v1/runsccmd", + ], +) diff --git a/pkg/shim/v1/proc/init.go b/pkg/shim/v1/proc/init.go index 16413d7072..4c77819c0a 100644 --- a/pkg/shim/v1/proc/init.go +++ b/pkg/shim/v1/proc/init.go @@ -252,11 +252,7 @@ func (p *Init) start(ctx context.Context, restoreConf *extension.RestoreConfig) } go func() { status, err := p.runtime.Wait(context.Background(), p.id) - if err != nil { - log.G(ctx).WithError(err).Errorf("Failed to wait for container %q", p.id) - p.killAllLocked(ctx) - status = internalErrorCode - } + status = adjustWaitStatus(ctx, p, status, err) ExitCh <- Exit{ Timestamp: time.Now(), ID: p.id, @@ -485,6 +481,23 @@ func (p *Init) convertStatus(status string) string { return status } +// adjustWaitStatus handles the exit status from runtime.Wait. When status is +// 0 but an error is returned (e.g., "sandbox no longer running" for short-lived +// containers), the error is benign and the status is preserved. For non-zero +// status with an error, the container is killed and internalErrorCode is used. +func adjustWaitStatus(ctx context.Context, p *Init, status int, err error) int { + if err == nil { + return status + } + if status == 0 { + log.G(ctx).WithError(err).Warnf("Container %q wait error with exit status 0 (ignoring)", p.id) + return 0 + } + log.G(ctx).WithError(err).Errorf("Failed to wait for container %q", p.id) + p.killAllLocked(ctx) + return internalErrorCode +} + func withConditionalIO(c stdio.Stdio) runc.IOOpt { return func(o *runc.IOOption) { o.OpenStdin = c.Stdin != "" diff --git a/pkg/shim/v1/proc/init_test.go b/pkg/shim/v1/proc/init_test.go new file mode 100644 index 0000000000..d03006933c --- /dev/null +++ b/pkg/shim/v1/proc/init_test.go @@ -0,0 +1,102 @@ +// Copyright 2026 The gVisor 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 +// +// https://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 proc + +import ( + "context" + "fmt" + "testing" + + "gvisor.dev/gvisor/pkg/shim/v1/runsccmd" +) + +func TestAdjustWaitStatus(t *testing.T) { + ctx := context.Background() + + for _, tc := range []struct { + name string + status int + err error + wantStatus int + }{ + { + name: "success with no error", + status: 0, + err: nil, + wantStatus: 0, + }, + { + name: "non-zero exit with no error", + status: 1, + err: nil, + wantStatus: 1, + }, + { + name: "signal exit with no error", + status: 137, + err: nil, + wantStatus: 137, + }, + { + name: "success with benign error (short-lived container)", + status: 0, + err: fmt.Errorf("sandbox no longer running and its exit status is unavailable"), + wantStatus: 0, + }, + { + name: "success with any error preserves zero status", + status: 0, + err: fmt.Errorf("some unexpected error"), + wantStatus: 0, + }, + { + name: "non-zero exit with error becomes internalErrorCode", + status: 1, + err: fmt.Errorf("wait failed"), + wantStatus: internalErrorCode, + }, + { + name: "signal exit with error becomes internalErrorCode", + status: 137, + err: fmt.Errorf("wait failed"), + wantStatus: internalErrorCode, + }, + { + name: "negative status with error becomes internalErrorCode", + status: -1, + err: fmt.Errorf("wait failed unexpectedly"), + wantStatus: internalErrorCode, + }, + { + name: "exit 255 with no error passes through", + status: 255, + err: nil, + wantStatus: 255, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Create a minimal Init with a dummy runtime to prevent nil + // dereference in killAllLocked for the error path. + p := &Init{ + id: "test-container", + runtime: &runsccmd.Runsc{Command: "false"}, + } + got := adjustWaitStatus(ctx, p, tc.status, tc.err) + if got != tc.wantStatus { + t.Errorf("adjustWaitStatus(status=%d, err=%v) = %d, want %d", tc.status, tc.err, got, tc.wantStatus) + } + }) + } +} diff --git a/pkg/shim/v1/runsc/BUILD b/pkg/shim/v1/runsc/BUILD index b854a22f36..487ed0eade 100644 --- a/pkg/shim/v1/runsc/BUILD +++ b/pkg/shim/v1/runsc/BUILD @@ -63,6 +63,7 @@ go_test( library = ":runsc", deps = [ "//pkg/shim/v1/utils", + "//runsc/specutils", "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", ], ) diff --git a/pkg/shim/v1/runsc/service.go b/pkg/shim/v1/runsc/service.go index cb0b44ca9d..c52aaee2bd 100644 --- a/pkg/shim/v1/runsc/service.go +++ b/pkg/shim/v1/runsc/service.go @@ -656,7 +656,11 @@ func newInit(workDir, namespace string, platform stdio.Platform, r *proc.CreateC p.WorkDir = workDir p.IoUID = int(options.IoUID) p.IoGID = int(options.IoGID) - p.Sandbox = specutils.SpecContainerType(spec) == specutils.ContainerTypeSandbox + // Non-CRI callers (BuildKit, ctr) don't set the container type + // annotation, so SpecContainerType returns ContainerTypeUnspecified. + // These are always root containers that need IO passed to runsc create. + ct := specutils.SpecContainerType(spec) + p.Sandbox = ct == specutils.ContainerTypeSandbox || ct == specutils.ContainerTypeUnspecified p.UserLog = utils.UserLogPath(spec) p.Monitor = reaper.Default return p, nil diff --git a/pkg/shim/v1/runsc/service_test.go b/pkg/shim/v1/runsc/service_test.go index 30543f37ab..b720c6b511 100644 --- a/pkg/shim/v1/runsc/service_test.go +++ b/pkg/shim/v1/runsc/service_test.go @@ -19,6 +19,7 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" "gvisor.dev/gvisor/pkg/shim/v1/utils" + "gvisor.dev/gvisor/runsc/specutils" ) func TestCgroupPath(t *testing.T) { @@ -90,6 +91,85 @@ func TestCgroupPath(t *testing.T) { } } +func TestSandboxDetection(t *testing.T) { + for _, tc := range []struct { + name string + annotations map[string]string + wantSandbox bool + }{ + { + name: "no-annotation (non-CRI caller like BuildKit or ctr)", + annotations: nil, + wantSandbox: true, + }, + { + name: "empty-annotations", + annotations: map[string]string{}, + wantSandbox: true, + }, + { + name: "containerd-sandbox", + annotations: map[string]string{ + specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeSandbox, + }, + wantSandbox: true, + }, + { + name: "containerd-container", + annotations: map[string]string{ + specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeContainer, + }, + wantSandbox: false, + }, + { + name: "crio-sandbox", + annotations: map[string]string{ + specutils.CRIOContainerTypeAnnotation: specutils.CRIOContainerTypeSandbox, + }, + wantSandbox: true, + }, + { + name: "crio-container", + annotations: map[string]string{ + specutils.CRIOContainerTypeAnnotation: specutils.CRIOContainerTypeContainer, + }, + wantSandbox: false, + }, + { + name: "unknown-annotation-value", + annotations: map[string]string{ + specutils.ContainerdContainerTypeAnnotation: "unknown-value", + }, + wantSandbox: false, + }, + { + name: "both-annotations-container-wins", + annotations: map[string]string{ + specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeContainer, + specutils.CRIOContainerTypeAnnotation: specutils.CRIOContainerTypeSandbox, + }, + // containerd annotation is checked first + wantSandbox: false, + }, + { + name: "unrelated-annotations-treated-as-unspecified", + annotations: map[string]string{ + "some.other.annotation": "value", + }, + wantSandbox: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + spec := &specs.Spec{Annotations: tc.annotations} + ct := specutils.SpecContainerType(spec) + got := ct == specutils.ContainerTypeSandbox || ct == specutils.ContainerTypeUnspecified + if got != tc.wantSandbox { + t.Errorf("sandbox detection for %v: got %v, want %v (containerType=%v)", tc.annotations, got, tc.wantSandbox, ct) + } + }) + } +} + // Test cases that cgroup path should not be updated. func TestCgroupNoUpdate(t *testing.T) { for _, tc := range []struct {