feat(monitors): expose a configurable control socket for each monitor - #850
Open
Anamika1608 wants to merge 1 commit into
Open
feat(monitors): expose a configurable control socket for each monitor#850Anamika1608 wants to merge 1 commit into
Anamika1608 wants to merge 1 commit into
Conversation
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Expose each monitor's control socket in the normal boot flow, so the runtime can keep talking to the VMM after the guest starts. Every monitor boots exactly as before; the only change is that its control socket stays open and reachable: - Firecracker launches with --api-sock instead of --no-api, keeping --config-file so the guest still boots from the config file. - QEMU exposes a QMP Unix socket in server mode, configured not to wait for a client before booting, alongside the disabled human monitor. - Cloud Hypervisor exposes its REST API socket (--api-socket). The socket location is configurable through a new socket_path option under a monitor's configuration, wired through MonitorConfig, ExecArgs and the state.json annotation passthrough, with a per-container default of /tmp/<id>.sock behind a DefaultSocketDir constant and a shared resolveSocketPath helper. After changeRoot, urunc creates the socket path's directory inside the monitor rootfs, so any custom path works; it fails only if the location is invalid, such as a file already existing at one of the path's components. Extend the QEMU BuildExecCmd tests to cover the new argument and document the socket_path option. Signed-off-by: Anamika Aggarwal <anamikaagg18@gmail.com>
Anamika1608
force-pushed
the
config-socket-all-monitors
branch
from
July 28, 2026 12:02
48afaad to
3e02d31
Compare
4 tasks
cmainas
requested changes
Aug 3, 2026
cmainas
left a comment
cmainas
left a comment
Contributor
There was a problem hiding this comment.
Hello @Anamika1608 ,
thank you for the PR. I have added some comments in the code. Also, some generic comments:
- There is still the case where an admin configures the urunc to run the monitors without any socket. This might be for security reasons or because they do not require it. Therefore, if the socket path is not in the configuration, then there should be no socket (no default value).
- We need to cleanup the socket path, because in case of a container restart (e.g. pod) the monitor might fail to use the same path.
Comment on lines
+653
to
+665
| // Ensure the monitor's control socket directory exists inside the monitor | ||
| // rootfs, so the monitor can bind its socket there. changeRoot has already | ||
| // made this process' root the monitor rootfs, so the socket path is | ||
| // created relative to it. The default (/tmp) already exists; a custom | ||
| // socket_path may point at a directory that does not, and MkdirAll fails | ||
| // if that location is invalid (e.g. a file already exists there). | ||
| if hypervisors.UsesControlSocket(hypervisors.VmmType(vmmType)) { | ||
| sockDir := filepath.Dir(hypervisors.ResolveSocketPath(vmmArgs)) | ||
| if err = os.MkdirAll(sockDir, 0o755); err != nil { | ||
| return fmt.Errorf("failed to create control socket directory %q: %w", sockDir, err) | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
This should be done after setupUser. We might not be able to access the socket or directory if the monitor is not a root user.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Expose each monitor's control socket in the normal (config-file / CLI) boot flow, so the runtime can talk to the VMM after the guest starts (graceful shutdown now, snapshots later). Each monitor still boots exactly as before — this only keeps its control socket open and reachable:
--api-sockwhile keeping--config-file, so the guest still boots from the config file and the socket stays open (drops only--no-api).-qmp unix:<path>,server,nowaitalongside the disabled human monitor.--api-socket path=<path>.The socket path is configurable via a
socket_pathmonitor field, shared across all monitors through a commonResolveSocketPathhelper, defaulting to a per-container/tmp/<id>.sock. AfterchangeRoot, urunc creates the socket path's directory inside the monitor rootfs so a custom path works; it fails only if the location is invalid (a file already exists on the path).Related issues
How was this tested?
go build ./...,go test ./pkg/... ./internal/...,gofmt,make lint, and cspell pass; the QEMUBuildExecCmdtests cover the new-qmpargument (Ubuntu 24.04 aarch64 VM, KVM).socket_path; QEMU (chttp-qemu-linux-aarch64) boots, the guest serves HTTP 200, launches with-qmp unix:/tmp/qemu.sock,server,nowait, and the QMP socket answersquery-statuswithrunning.LLM usage
claude code (opus 4.8) for the understanding of codebase, approach decisions and reviews
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).