From 083e68a6a8e43e1c1727e3804e6cc100f8ae4aa3 Mon Sep 17 00:00:00 2001 From: Luke Lombardi <33990301+luke-lombardi@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:46:15 -0400 Subject: [PATCH 1/3] Preserve guest OOM evidence and init status in final metrics --- pkg/sentry/kernel/kernel.go | 8 +++++- pkg/sentry/kernel/task_run.go | 4 +++ runsc/boot/loader.go | 24 ++++++++++++++++ runsc/cmd/boot.go | 1 + runsc/container/container_test.go | 48 +++++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 8340b896215..a1743834abc 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -223,7 +223,8 @@ type Kernel struct { // guestOOMKills counts successful guest-budget OOM kills in this runtime. // Reset on restore so earlier kills are not reported as new failures. - guestOOMKills atomicbitops.Uint64 `state:"nosave"` + guestOOMKills atomicbitops.Uint64 `state:"nosave"` + guestInitOOMKills atomicbitops.Uint64 `state:"nosave"` // runningTasksCond is signaled when runningTasks is incremented from 0 to 1. // @@ -1998,6 +1999,11 @@ func (k *Kernel) GuestOOMKills() uint64 { return k.guestOOMKills.Load() } +// GuestInitOOMKills counts guest OOM kills of the sandbox's root init process. +func (k *Kernel) GuestInitOOMKills() uint64 { + return k.guestInitOOMKills.Load() +} + // SupervisorContext returns a Context with maximum privileges in k. It should // only be used by goroutines outside the control of the emulated kernel // defined by e. diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index 13779e3633f..89025046899 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -458,6 +458,7 @@ func (t *Task) killMemoryOOMVictim() bool { if victim == nil { return false } + rootInit := t.k.GlobalInit() signals := victim.tg.signalLock() if victim.killedLocked() || victim.tg.exiting { signals.mu.Unlock() @@ -467,6 +468,9 @@ func (t *Task) killMemoryOOMVictim() bool { err := victim.sendSignalLocked(SignalInfoPriv(linux.SIGKILL), true /* group */) if err == nil { t.k.guestOOMKills.Add(1) + if victim.tg == rootInit { + t.k.guestInitOOMKills.Add(1) + } } signals.mu.Unlock() if err != nil { diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index f4fe2858424..7a014f233a3 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -1724,6 +1724,30 @@ func (l *Loader) WaitForStartSignal() { <-l.ctrl.manager.startChan } +// RegisterExitMetrics preserves exit status and OOM evidence in final metrics, +// even when init exits before a stats observer or wait RPC can see it. +func (l *Loader) RegisterExitMetrics() { + register := func(name string, read func(*kernel.Kernel) uint64) { + metric.MustRegisterCustomUint64Metric(name, metric.Uint64Metadata{ + Cumulative: true, + Description: "Guest-budget OOM kills since startup or restore.", + }, func(...*metric.FieldValue) uint64 { + l.mu.Lock() + defer l.mu.Unlock() + return read(l.k) + }) + } + register("/memory/guest_oom_kills", (*kernel.Kernel).GuestOOMKills) + register("/memory/guest_init_oom_kills", (*kernel.Kernel).GuestInitOOMKills) + metric.MustRegisterCustomUint64Metric("/task/init_exit_status", metric.Uint64Metadata{ + Description: "Root init wait status, valid after init exits.", + }, func(...*metric.FieldValue) uint64 { + l.mu.Lock() + defer l.mu.Unlock() + return uint64(l.k.GlobalInit().ExitStatus()) + }) +} + // WaitExit waits for the root container to exit, and returns its exit status. func (l *Loader) WaitExit() linux.WaitStatus { // Wait for container. diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index c50a2d5f3f8..95361eb1c9e 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -641,6 +641,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma // This needs to happen after the kernel is initialized (such that all metrics are registered) // but before the start-sync file is notified, as the parent process needs to query for // registered metrics prior to sending the start signal. + l.RegisterExitMetrics() metric.Initialize() var finalMetricsFile *os.File if b.finalMetricsFD != -1 { diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index a19ddd692de..fcef8534fd1 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -128,6 +128,54 @@ for thread in threads: } } +// Final metrics must distinguish an OOM of init from a prior child OOM. +func TestGuestOOMFinalMetrics(t *testing.T) { + const allocate = "a=bytearray(128<<20);exec('for i in range(0,len(a),4096): a[i]=1')" + for _, test := range []struct { + name string + args []string + total, init string + status unix.WaitStatus + }{ + {"init OOM", []string{"/usr/bin/python3", "-c", allocate}, "1", "1", 9}, + {"child OOM then SIGKILL", []string{"/bin/sh", "-c", "/usr/bin/python3 -c \"" + allocate + "\"; kill -9 $$"}, "1", "0", 9}, + {"plain SIGKILL", []string{"/bin/sh", "-c", "kill -9 $$"}, "0", "0", 9}, + {"success", []string{"/bin/sh", "-c", "exit 0"}, "0", "0", 0}, + {"error", []string{"/bin/sh", "-c", "exit 42"}, "0", "0", 42 << 8}, + } { + t.Run(test.name, func(t *testing.T) { + spec, conf := sleepSpecConf(t) + spec.Process.Args = test.args + spec.Annotations[specutils.AnnotationTotalMemory] = strconv.Itoa(64 << 20) + hostLimit := int64(2 << 30) + spec.Linux = &specs.Linux{Resources: &specs.LinuxResources{Memory: &specs.LinuxMemory{Limit: &hostLimit}}} + conf.FinalMetricsLog = filepath.Join(t.TempDir(), "metrics") + _, bundle, clean, err := testutil.SetupContainer(spec, conf) + if err != nil { + t.Fatal(err) + } + defer clean() + status, err := Run(conf, Args{ID: testutil.RandomContainerID(), Spec: spec, BundleDir: bundle, Attached: true}) + if err != nil || status != test.status { + t.Fatalf("status=%v want=%v, err=%v", status, test.status, err) + } + data, err := os.ReadFile(conf.FinalMetricsLog) + if err != nil { + t.Fatal(err) + } + counts := map[string]string{"runsc_memory_guest_oom_kills": "0", "runsc_memory_guest_init_oom_kills": "0"} + for _, line := range strings.Split(string(data), "\n") { + if fields := strings.Fields(line); len(fields) >= 2 { + counts[fields[0]] = fields[1] + } + } + if counts["runsc_task_init_exit_status"] != strconv.FormatUint(uint64(test.status), 10) || counts["runsc_memory_guest_oom_kills"] != test.total || counts["runsc_memory_guest_init_oom_kills"] != test.init { + t.Fatalf("want total=%s init=%s, final metrics:\n%s", test.total, test.init, data) + } + }) + } +} + func TestMain(m *testing.M) { config.RegisterFlags(flag.CommandLine) log.SetLevel(log.Debug) From 03fffdcf44d63fa3619d69f41b2065b0b9ff25e3 Mon Sep 17 00:00:00 2001 From: Luke Lombardi <33990301+luke-lombardi@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:20:46 -0400 Subject: [PATCH 2/3] Keep OOM accounting within the task lock hierarchy --- pkg/sentry/kernel/task_run.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index 89025046899..b7040897808 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -458,7 +458,6 @@ func (t *Task) killMemoryOOMVictim() bool { if victim == nil { return false } - rootInit := t.k.GlobalInit() signals := victim.tg.signalLock() if victim.killedLocked() || victim.tg.exiting { signals.mu.Unlock() @@ -468,7 +467,8 @@ func (t *Task) killMemoryOOMVictim() bool { err := victim.sendSignalLocked(SignalInfoPriv(linux.SIGKILL), true /* group */) if err == nil { t.k.guestOOMKills.Add(1) - if victim.tg == rootInit { + // globalInit is immutable once tasks start; do not take extMu here. + if victim.tg == t.k.globalInit { t.k.guestInitOOMKills.Add(1) } } From 3bd9d1184827d4c51f365c4e28aff81871c7422b Mon Sep 17 00:00:00 2001 From: Luke Lombardi <33990301+luke-lombardi@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:40:30 -0400 Subject: [PATCH 3/3] Handle exit metric snapshots before init exists --- runsc/boot/loader.go | 14 +++++++++----- runsc/container/container_test.go | 4 ++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 7a014f233a3..1b572044705 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -1727,24 +1727,28 @@ func (l *Loader) WaitForStartSignal() { // RegisterExitMetrics preserves exit status and OOM evidence in final metrics, // even when init exits before a stats observer or wait RPC can see it. func (l *Loader) RegisterExitMetrics() { - register := func(name string, read func(*kernel.Kernel) uint64) { + register := func(name, description string, read func(*kernel.Kernel) uint64) { metric.MustRegisterCustomUint64Metric(name, metric.Uint64Metadata{ Cumulative: true, - Description: "Guest-budget OOM kills since startup or restore.", + Description: description, }, func(...*metric.FieldValue) uint64 { l.mu.Lock() defer l.mu.Unlock() return read(l.k) }) } - register("/memory/guest_oom_kills", (*kernel.Kernel).GuestOOMKills) - register("/memory/guest_init_oom_kills", (*kernel.Kernel).GuestInitOOMKills) + register("/memory/guest_oom_kills", "Guest-budget OOM kills since startup or restore.", (*kernel.Kernel).GuestOOMKills) + register("/memory/guest_init_oom_kills", "Root init guest-budget OOM kills since startup or restore.", (*kernel.Kernel).GuestInitOOMKills) metric.MustRegisterCustomUint64Metric("/task/init_exit_status", metric.Uint64Metadata{ Description: "Root init wait status, valid after init exits.", }, func(...*metric.FieldValue) uint64 { l.mu.Lock() defer l.mu.Unlock() - return uint64(l.k.GlobalInit().ExitStatus()) + init := l.k.GlobalInit() + if init == nil { + return 0 + } + return uint64(init.ExitStatus()) }) } diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index fcef8534fd1..b2c3d10a9b3 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -77,6 +77,10 @@ func TestGuestOOMKillEventsPreserveSandbox(t *testing.T) { t.Fatal(err) } defer c.Destroy() + // A metrics scrape before start must tolerate init not existing yet. + if _, err := c.Sandbox.ExportMetrics(control.MetricsExportOpts{}); err != nil { + t.Fatalf("metrics before start: %v", err) + } if err := c.Start(conf); err != nil { t.Fatal(err) }