Add s6-overlay and Talos system manager support for physical_ai_server#85
Add s6-overlay and Talos system manager support for physical_ai_server#85
Conversation
…ust numpy installation
…all requirements from a file
Signed-off-by: Hyungyu Kim <kimhg@robotis.com>
…rfile and update PYTHONPATH
Signed-off-by: Hyungyu Kim <kimhg@robotis.com>
… into feature-system
…d adjust PYTHONPATH
…agent path to talos package. Adjust permissions and clean up Dockerfile for better organization of services.
…hysical AI server, simplifying the setup by removing the build context and Dockerfile specification.
…rvice startup diagnostics
Summary of ChangesHello @GyuH13, 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 the operational stability and manageability of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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 s6-overlay for process management within the physical_ai_server container, significantly improving service supervision and reliability. A critical security vulnerability has been identified: the use of chmod 777 on the agent's socket directory (/var/run/agent), which is shared with the host, creates a high risk of local privilege escalation. Additionally, the review highlighted a critical issue with service auto-start configuration contradicting comments, a high-severity issue with using a floating git branch impacting build reproducibility, and several medium-severity issues related to insecure file permissions and script correctness. Addressing these points, especially the critical security vulnerability, will solidify the new service management infrastructure.
There was a problem hiding this comment.
Pull request overview
Adds s6-overlay process supervision to the physical_ai_server container, including an s6-agent service for Talos system manager integration and an s6-managed ROS2 longrun with a logging pipeline, plus version/changelog bumps across related packages.
Changes:
- Install and configure s6-overlay in
physical_ai_server(amd64/arm64) and add s6-rc service definitions fors6-agent+physical_ai_server+ logging pipeline. - Update docker-compose to run the prebuilt
robotis/physical-ai-server:latestimage and mount the agent socket directory. - Bump package versions to
0.8.1and update CHANGELOGs across multiple ROS2 and UI packages.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| rosbag_recorder/package.xml | Bump package version to 0.8.1 |
| rosbag_recorder/CHANGELOG.rst | Add 0.8.1 changelog entry |
| physical_ai_tools/package.xml | Bump package version to 0.8.1 |
| physical_ai_tools/CHANGELOG.rst | Add 0.8.1 changelog entry noting s6 additions |
| physical_ai_server/setup.py | Bump Python package version to 0.8.1 |
| physical_ai_server/package.xml | Bump package version to 0.8.1 |
| physical_ai_server/Dockerfile.arm64 | Install s6-overlay, Talos agent deps, and copy s6-rc service definitions |
| physical_ai_server/Dockerfile.amd64 | Install s6-overlay, Talos agent deps, and copy s6-rc service definitions |
| physical_ai_server/CHANGELOG.rst | Add 0.8.1 changelog entry noting s6 additions |
| physical_ai_manager/package.json | Bump UI version to 0.8.1 |
| physical_ai_manager/package-lock.json | Bump lockfile package version entries to 0.8.1 |
| physical_ai_manager/CHANGELOG.rst | Add 0.8.1 changelog entry |
| physical_ai_interfaces/package.xml | Bump package version to 0.8.1 |
| physical_ai_interfaces/CHANGELOG.rst | Add 0.8.1 changelog entry |
| physical_ai_bt/setup.py | Bump Python package version to 0.8.1 |
| physical_ai_bt/package.xml | Bump package version to 0.8.1 |
| physical_ai_bt/CHANGELOG.rst | Add 0.8.1 changelog entry |
| docker/s6-services/physical_ai_server/type | Define physical_ai_server as a longrun service |
| docker/s6-services/physical_ai_server/run | Start ROS2 launch via shared run script |
| docker/s6-services/physical_ai_server/producer-for | Wire service output into the log consumer |
| docker/s6-services/physical_ai_server/finish | Stop service via shared finish script (PGID-based termination) |
| docker/s6-services/physical_ai_server/dependencies | Service dependency file (currently empty) |
| docker/s6-services/physical_ai_server-log/type | Define log consumer as a longrun service |
| docker/s6-services/physical_ai_server-log/run | Run logutil-service to consume/rotate logs |
| docker/s6-services/physical_ai_server-log/pipeline-name | Name the producer/consumer pipeline |
| docker/s6-services/physical_ai_server-log/consumer-for | Bind log consumer to physical_ai_server producer |
| docker/s6-services/physical_ai_server-log-prepare/up | Oneshot log directory preparation script |
| docker/s6-services/physical_ai_server-log-prepare/type | Define log prepare as a oneshot service |
| docker/s6-services/common/ros2_service_run.sh | Shared ROS2 service runner (env setup, PGID tracking, exec) |
| docker/s6-services/common/ros2_service_finish.sh | Shared ROS2 finish script (SIGTERM+timeout+SIGKILL by PGID) |
| docker/s6-agent/type | Define s6-agent as a longrun service |
| docker/s6-agent/run | Start Talos FastAPI agent via uvicorn on a Unix domain socket |
| docker/s6-agent/finish | Cleanup agent UDS file on stop |
| docker/s6-agent/dependencies | Service dependency file (currently empty) |
| docker/docker-compose.yml | Use prebuilt server image and mount /var/run/agent socket directory |
Files not reviewed (1)
- physical_ai_manager/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
physical_ai_server/Dockerfile.arm64:14
- In this Dockerfile the venv PATH/PYTHONPATH are only appended to
/root/.bashrc(and exported within this singleRUNlayer). SubsequentRUN pip install ...commands won’t automatically use/opt/venv/bin/pipunlessENV PATH=/opt/venv/bin:$PATHis set (or the venv pip is invoked explicitly), which can lead to installing packages into the system interpreter instead of the venv.
RUN echo 'export PATH=/opt/venv/bin:$PATH' >> /root/.bashrc && \
echo 'export PYTHONPATH=/opt/venv/lib/python3.12/site-packages:$PYTHONPATH' >> /root/.bashrc && \
export PATH=/opt/venv/bin:$PATH && \
export PYTHONPATH=/opt/venv/lib/python3.12/site-packages:$PYTHONPATH;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 participants
Description
Summary
This PR adds s6-overlay as the process supervisor inside the physical_ai_server container and integrates the Talos system manager via an s6-agent service. The physical_ai_server ROS2 stack runs as a supervised longrun with a logging pipeline.
Changes
s6-overlay and services
talos.agent.s6_agent) on a Unix domain socket at/var/run/agent/s6_agent.sock. Started at container boot via the user bundle.physical_ai_server_bringup.launch.py) managed by s6 with shared run/finish scripts.physical_ai_server-log-prepare(oneshot): Creates/var/log/physical_ai_serverand sets permissions.physical_ai_server-log(consumer): Consumes the main service’s output for logging.ros2_service_run.shandros2_service_finish.shunderdocker/s6-services/common/provide reusable run/finish logic (PGID tracking, graceful SIGTERM with timeout, then SIGKILL).Docker
docker/s6-agentanddocker/s6-servicesinto/etc/s6-overlay/s6-rc.d/(withcommonremoved from s6-rc.d); copy common scripts to/usr/local/lib/s6-services; set execute bits for run/finish/up scripts; create log directory and user bundle entries for s6-agent and physical_ai_server-pipeline.robotis/physical-ai-server:latest) without a build context or s6-related volume mounts; only required volumes (e.g./var/run/robotis/agent_sockets/physical_ai_server:/var/run/agent) are kept.Other
physical_ai_server,physical_ai_bt,physical_ai_interfaces,physical_ai_manager,physical_ai_tools, androsbag_recorder.physical_ai_manager: dependency/version updates inpackage.jsonandpackage-lock.json.ros2_service_run.sh: optionalRMW_IMPLEMENTATIONlogging for startup diagnostics.