Fix/594 signal handling

[jiwahn]

Description

• Update the kill command to actually use the argument and forward signals to init.
• Currently urunc kill command always send SIGKILL, resulting non-terminating signals(ex, SIGWINCH) could hard kill the containers.
• Hide the ignored --all option to keep compatibility.
• Not sure yet whether SIGTERM should follow the same path as SIGKILL.

Related issues

• Fixes Container terminated after receiving SIGWINCH #594

How was this tested?

1. sudo docker run --runtime io.containerd.urunc.v2 harbor.nbfc.io/nubificus/urunc/nginx-firecracker-unikraft-initrd:latest
2. sudo docker kill --signal <signo> <container_id>
3. See if the container still alives

LLM usage

GPT-5.5

Checklist

• I have read the contribution guide.
• The linter passes locally (make lint).
• The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
• If LLMs were used: I have read the llm policy.

[netlify]

✅ Deploy Preview for urunc canceled.

Name	Link
🔨 Latest commit	7d28306
🔍 Latest deploy log	https://app.netlify.com/projects/urunc/deploys/69fd7c3c97ca520008c640a4

[jiwahn]

@sksingh2005, Could you review this branch and add some tests if possible? After that, we can mark it as ready for review. Appreciate it!

[sksingh2005]

@jiwahn sure I will look into it

[sksingh2005]

@jiwahn Overall it looks nice just some follow ups:

1. parseSignal numeric path when strconv.Atoi succeeds, please reject non-positive values (<= 0) so inputs like "0" don’t slip through.

2. Tests :

   • parseSignal: empty/default, known names, numeric derived from platform constant (avoid hardcoding 28), unknown name → error, invalid numeric → error.
   • Unikontainer.Signal: invalid PID → error; signal 0 to Getpid() as a cheap sanity check that forwarding works without delivering a real signal.

[jiwahn]

Thanks, handling non-positive values sounds good to me.

Could you add the numeric signal guard as a GitHub suggested change on the relevant line? I can apply it directly from the review.

[sksingh2005]

Thanks, handling non-positive values sounds good to me.

Could you add the numeric signal guard as a GitHub suggested change on the relevant line? I can apply it directly from the review.

@jiwahn its mentioned i guess ?

[jiwahn]

Yes it is

[abhaygoudannavar]

Hey @jiwahn @sksingh2005 i was just going through this PR and I tried to send signal "0" to check if container is alive:

sudo docker kill --signal 0 my_container

the Expected was Docker reports container is alive or container not found but what happend is urunc silently does unix.Kill(pid, 0), returns success, Docker thinks the kill succeeded and marks the container as killed,but the container is still running.Happy to help if needed.

[jiwahn]

@abhaygoudannavar I was not able to reproduce the behavior on this branch.
And just because the docker kill command succeeds, Docker does not necessarily mark the container as “killed.”
Could you leave a code review comment on the specific part of the code where you think this issue is coming from? That would help me understand and investigate it more clearly.

╰─$ sudo docker ps -a
[sudo] password for jiwoo:
CONTAINER ID   IMAGE                                                                     COMMAND                  CREATED         STATUS         PORTS     NAMES
7ea0896cb461   harbor.nbfc.io/nubificus/urunc/nginx-firecracker-unikraft-initrd:latest   "-c /nginx/conf/ngin…"   6 seconds ago   Up 5 seconds             peaceful_varahamihira

╰─$ sudo docker kill --signal 0 7ea0896cb461
Error response from daemon: cannot kill container: 7ea0896cb461: invalid signal: 0

╰─$ sudo docker ps -a                                                      1 ↵
CONTAINER ID   IMAGE                                                                     COMMAND                  CREATED          STATUS          PORTS     NAMES
7ea0896cb461   harbor.nbfc.io/nubificus/urunc/nginx-firecracker-unikraft-initrd:latest   "-c /nginx/conf/ngin…"   35 seconds ago   Up 34 seconds             peaceful_varahamihira

[cmainas]

Thank you @jiwahn ,

for the fix. I have added some comments, but overall the PR looks good and it is working.

[jiwahn]

@cmainas, changes made! PTAL

[abhaygoudannavar]

Hey @jiwahn, you are right,
I dug a bit deeper and realized the Docker daemon explicitly intercepts and rejects --signal 0 before it even reaches the runtime. Furthermore, checking the OCI standard and runc's implementation, parsing 0 is actually valid and desired behavior so that lower-level tools can use unix.Kill(pid, 0) for liveness probes. That was my mistake.

[cmainas]

Hello @jiwahn ,

you will need to add the word sigstr in https://github.com/urunc-dev/urunc/blob/main/.github/linters/urunc-dict.txt for the spell checker to pass.

[jiwahn]

I updated the dict. Should I squash these commits?

[cmainas]

Yes and please rebase over main, so we can merge this one.