fix: write runtime pidfile when provided

[sidneychang]

Ensure the PID is written to the runtime-provided --pid-file after container creation. This allows supervisors like conmon to track and reap the container process correctly, preventing early cleanup or EOF failures. This change also ensures that Podman can reliably start urunc containers.

Description

In the current code, there is a --pid-file flag, but in reality, we are not actually writing to the specified pid-file. This causes issues when starting urunc with Podman, preventing it from starting properly.

Related issues

#114

• Fixes Check integration of urunc with podman #114

How was this tested?

• Configure Podman to Use urunc
  I tested using the method described in Check integration of urunc with podman #114 and configured Podman to use urunc as the runtime. I executed the following command to modify the Podman container configuration:

sudo tee /etc/containers/containers.conf >/dev/null <<EOT
[engine]
runtime = "/usr/local/bin/urunc"
EOT

• Configure the QEMU Path for urunc
  To ensure urunc can locate the QEMU binary, the QEMU path needs to be set in the config.toml file for urunc. The configuration should look like this:

[monitors.qemu]
path = "/usr/bin/qemu-system-x86_64"

Run urunc with Podman
After configuring the runtime and QEMU path, I attempted to run the urunc container using Podman:

podman run --rm -d harbor.nbfc.io/nubificus/urunc/nginx-qemu-unikraft-initrd:latest

result:

LLM usage

Using Codex to debug the cause of the issue#114

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 ready!

Name	Link
🔨 Latest commit	8498ff4
🔍 Latest deploy log	https://app.netlify.com/projects/urunc/deploys/6985749855cc790008cd362c
😎 Deploy Preview	https://deploy-preview-439--urunc.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[cmainas]

Thank you a lot @sidneychang for this fix!

I think we can make the patch even smaller. We can simply add the pid file apth as an extra argument in unikontainers.Create https://github.com/sidneychang/urunc/blob/9622e2a938b1a911f176eee2d6a0fa00e92ae09b/cmd/urunc/create.go#L257

Inside Create if the pid file path is not specified we can simply use the default value otherwise the argument passed.

[sidneychang]

Thank you a lot @sidneychang for this fix!

I think we can make the patch even smaller. We can simply add the pid file apth as an extra argument in unikontainers.Create https://github.com/sidneychang/urunc/blob/9622e2a938b1a911f176eee2d6a0fa00e92ae09b/cmd/urunc/create.go#L257

Inside Create if the pid file path is not specified we can simply use the default value otherwise the argument passed.

I agree with this approach, and I actually considered something similar. My only hesitation was whether we still need to always write the pid file to the existing default path filepath.Join(u.State.Bundle, initPidFilename), or if, once an explicit pid file path is provided, we should skip writing to the default location entirely and only write to the provided path.

[cmainas]

Thank you a lot @sidneychang for this fix!
I think we can make the patch even smaller. We can simply add the pid file apth as an extra argument in unikontainers.Create https://github.com/sidneychang/urunc/blob/9622e2a938b1a911f176eee2d6a0fa00e92ae09b/cmd/urunc/create.go#L257
Inside Create if the pid file path is not specified we can simply use the default value otherwise the argument passed.

I agree with this approach, and I actually considered something similar. My only hesitation was whether we still need to always write the pid file to the existing default path filepath.Join(u.State.Bundle, initPidFilename), or if, once an explicit pid file path is provided, we should skip writing to the default location entirely and only write to the provided path.

I see, but we should respect the cli arguments. Also, looking a bit in the runc code for reference I see that:

• The pid file is created here https://github.com/opencontainers/runc/blob/92c80abae14c074cb26be9030a76d13e26ff2008/utils_linux.go#L296
• And the pid file path is taken from https://github.com/opencontainers/runc/blob/92c80abae14c074cb26be9030a76d13e26ff2008/utils_linux.go#L411 which is the cli argument

[sidneychang]

Thank you a lot @sidneychang for this fix!
I think we can make the patch even smaller. We can simply add the pid file apth as an extra argument in unikontainers.Create https://github.com/sidneychang/urunc/blob/9622e2a938b1a911f176eee2d6a0fa00e92ae09b/cmd/urunc/create.go#L257
Inside Create if the pid file path is not specified we can simply use the default value otherwise the argument passed.

I agree with this approach, and I actually considered something similar. My only hesitation was whether we still need to always write the pid file to the existing default path filepath.Join(u.State.Bundle, initPidFilename), or if, once an explicit pid file path is provided, we should skip writing to the default location entirely and only write to the provided path.

I see, but we should respect the cli arguments. Also, looking a bit in the runc code for reference I see that:

• The pid file is created here https://github.com/opencontainers/runc/blob/92c80abae14c074cb26be9030a76d13e26ff2008/utils_linux.go#L296
• And the pid file path is taken from https://github.com/opencontainers/runc/blob/92c80abae14c074cb26be9030a76d13e26ff2008/utils_linux.go#L411 which is the cli argument

That makes sense — we should indeed respect the CLI arguments.
I did some testing and found that the default pid file path is effectively the same as the pidfile path passed by containerd (Docker / Kubernetes). This further confirms that the original default path was primarily chosen to align with the containerd ecosystem.
Based on this, I updated the code to retain the existing default pid file path as a fallback when no pidfile is explicitly provided, and to prefer the CLI-provided pidfile path whenever it is specified. If this approach looks reasonable, I think the change should be safe to merge.

[cmainas]

Thank you @sidneychang for this fix!