[008] Support attach socket and --stdin
#11
Conversation
jankaluza
changed the title
attach socket and --stdinattach socket and --stdin
Summary of ChangesHello @jankaluza, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances Conmon's capabilities by integrating robust Unix domain socket support for container attachment and standard input handling. It involves a substantial refactoring of the command-line interface and internal command execution logic, introducing new abstractions for runtime argument generation and session management. Additionally, a file-based logging mechanism has been added, providing more flexible output management for container processes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality for handling container attach and stdin via Unix sockets. The refactoring of the command structure from run to create is a good clarification of intent. The introduction of RuntimeSession and RuntimeProcess provides a solid foundation for managing the OCI runtime lifecycle. The code is generally well-structured, with good separation of concerns. I've identified a few issues, including a critical bug in the restore command's argument generation, potential mishandling of multiple log paths, and some areas where robustness and safety could be improved. My detailed comments and suggestions are below.
jankaluza
force-pushed
the
attach
branch
4 times, most recently
from
40340b5 to
26daba0
Compare
jankaluza
force-pushed
the
attach
branch
2 times, most recently
from
d025100 to
21dd7ff
Compare
This commit does the following: - Moves the code to run the `runtime` process from `run.rs` to `process.rs` into the `RuntimeProcess` struct. - Moves the common code shared between `create` and `exec` commands to `session.rs` into the `RuntimeSession` struct. - The `RuntimeSession` uses the `RuntimeProcess` in high-level way. - The basic `exec.rs` is implemented using `RuntimeSession`. - The exit code is now passed from the commands to the `main` and returned there. Signed-off-by: Jan Kaluza <[email protected]>
This commit does the following: - Handles the `--exec-attach` option. - Handles the _OCI_ATTACHPIPE and _OCI_STARTPIPE. It is tested by the conmon-v2 tests run as part of `make test`. Signed-off-by: Jan Kaluza <[email protected]>
jankaluza
force-pushed
the
attach
branch
2 times, most recently
from
9452a11 to
92f8aac
Compare
This commits does the following: - Adds new `UnixSocket` struct to create new Unix socket, bind it to local directory and accept new connections. - Adds new `RemoteSocket` struct representing the UnixSocket clients. - Creates `attach` UnixSocket in the `RuntimeSession`. - Handles the `attach` socket in the the `handle_stdio` - accepting new clients and handling their RemoteSockets. - Handles the `--stdin` option by creating the stdin pipe for a container and sending data to it when available. - Disables #![allow(clippy::collapsible_if)] lint globally. The Rust version in CI does not support collapsible ifs with `let =` expressions. Signed-off-by: Jan Kaluza <[email protected]>
1 participant
This commits does the following:
UnixSocketstruct to create new Unix socket, bind it tolocal directory and accept new connections.
RemoteSocketstruct representing the UnixSocket clients.attachUnixSocket in theRuntimeSession.attachsocket in the thehandle_stdio- accepting newclients and handling their RemoteSockets.
--stdinoption by creating the stdin pipe for a containerand sending data to it when available.
Signed-off-by: Jan Kaluza [email protected]