Conversation
dany74q
changed the title
Signed-off-by: Danny Hershko Shemesh <danny@wiz.io>
839285e to
c92b0ac
Compare
There was a problem hiding this comment.
Would it be possible to track processes detached from the process group of the container init too?
Maybe by walking through procfs, and comparing /proc/PID/ns/mnt
There was a problem hiding this comment.
🟡 Changes recommended
PID reuse can kill unrelated processes, failure paths can still hang indefinitely, and the regression test does not exercise rootless operation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds process-group cleanup for hung containers using NoProcessSandbox.
Changes:
- Records run/exec process IDs and kills surviving process groups.
- Adds cancellation coverage for run and exec paths.
File summaries
| File | Description |
|---|---|
executor/runcexecutor/executor.go |
Implements process-group termination. |
executor/runcexecutor/executor_linux.go |
Enables PID files in no-sandbox mode. |
worker/runc/runc_test.go |
Adds cancellation regression coverage. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
|
|
||
| bklog.G(ctx).Debugf("sending sigkill to process group %d in container %s", pid, k.id) | ||
| err = syscall.Kill(-pid, syscall.SIGKILL) |
| if err := p.killer.KillProcessGroup(context.WithoutCancel(ctx)); err != nil { | ||
| bklog.G(ctx).Errorf("failed to kill process group in container %s: %+v", p.killer.id, err) | ||
| } |
| t.Parallel() | ||
| checkRequirement(t) | ||
|
|
||
| workerOpt := newWorkerOpt(t, oci.NoProcessSandbox) |
|
Hi, is this still being worked on? It would be very useful to me. Happy to help if needed. |
Under --oci-worker-no-process-sandbox,
runc killonly SIGKILLs the in-container init: the container shares the host pid namespace, so subprocesses survive it, and rootless runc cannot enumerate them without cgroups (opencontainers/runc#4398). Survivors keep the container's IO open,runc runnever returns, and the solve never completes. See #4483 and #2855.The fix passes a pidfile to
runc rununder NoProcessSandbox and, when the process has not ended within the existing grace after Kill, SIGKILLs its process group.