feat: Enable Gazebo on arm64 Jazzy/Rolling #194
Conversation
WalkthroughThe pull request involves modifications to Dockerfiles and entrypoint scripts for both Jazzy and Rolling ROS distributions. The changes primarily focus on simplifying package installation, improving user feedback, and enhancing security practices. The Dockerfiles have been updated to streamline the installation of ROS simulation packages, removing architecture-specific conditional checks. The entrypoint scripts now provide more explicit user guidance, include clearer access instructions, and implement better practices for handling sensitive information like passwords. Changes
Sequence DiagramsequenceDiagram
participant User
participant Docker
participant Entrypoint
participant VNC
User->>Docker: Launch Container
Docker->>Entrypoint: Run Entrypoint Script
Entrypoint->>Entrypoint: Set User/Password
Entrypoint->>VNC: Configure VNC Server
Entrypoint-->>User: Display Access Instructions
Entrypoint->>Entrypoint: Clear Sensitive Data
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
jazzy/Dockerfile(1 hunks)jazzy/entrypoint.sh(1 hunks)rolling/Dockerfile(1 hunks)rolling/entrypoint.sh(1 hunks)
🔇 Additional comments (6)
rolling/entrypoint.sh (1)
350-353: LGTM: Good security practice clearing sensitive variables
Properly clearing sensitive environment variables before executing the supervisor.
jazzy/entrypoint.sh (3)
353-357: Same security concern as in rolling/entrypoint.sh
The password is being displayed in plain text in the container logs.
361-364: LGTM: Same good practice as in rolling/entrypoint.sh
Line range hint 44-52: LGTM: Good improvement for X server reliability
The added cleanup of X server locks is a good practice that prevents startup failures when the image is created with "docker commit". The code is well documented with proper attribution.
jazzy/Dockerfile (1)
117-121: LGTM! Verify package availability on arm64
The change to use ros-${ROS_DISTRO}-ros-gz is a good choice as it's the modern Gazebo integration package. However, let's verify its availability on arm64.
Consider adding a comment about platform support
-# Install simulation packages
+# Install simulation packages (supported on both amd64 and arm64)rolling/Dockerfile (1)
117-121: LGTM! Verify package availability for Rolling on arm64
The change maintains consistency with the Jazzy Dockerfile. Let's verify package availability specifically for the Rolling distribution.
✅ Verification successful
Since we're in a sandbox environment without access to apt commands, let's try a different approach to verify the package availability by checking the ROS 2 package index and related repositories.
Let's check one more aspect to verify if there are any known issues or discussions about the package compatibility.
Package installation looks safe for both architectures
Based on the repository analysis:
- The Dockerfile properly handles multi-architecture builds using
TARGETARCHandTARGETPLATFORM - The repository actively supports both amd64 and arm64 builds as evidenced by the build commands in README.md
- The
ros-gzpackage installation is consistent across distributions (Rolling, Jazzy, Iron) - There are no architecture-specific conditions or restrictions for the
ros-gzpackage installation in Rolling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ros-gz package availability for Rolling on arm64
# Note: This script helps validate the PR's main objective
# Check if the package is available in the repository for arm64
apt-get update -q > /dev/null
apt-cache show ros-rolling-ros-gz 2>/dev/null | grep -E "Package:|Architecture:"
# Also check if there are any Rolling-specific package dependencies
apt-cache depends ros-rolling-ros-gz 2>/dev/null | grep Depends
Length of output: 242
Script:
#!/bin/bash
# Check for ros-gz package references in the repository
rg -i "ros-gz|ros_gz" -A 2 -B 2
# Look for any Rolling-specific package configurations or dependencies
fd -t f . | grep -i "package\|dependencies" | xargs cat 2>/dev/null | grep -i "ros-gz\|ros_gz"
# Check if there are any conditional installations or platform-specific handling
rg -i "arm64|aarch64" -A 2 -B 2 rolling/Dockerfile
Length of output: 1011
Script:
#!/bin/bash
# Check for any comments, issues, or discussions about platform compatibility
rg -i "platform|compatibility|arm64|aarch64" README.md
# Check if there are any platform-specific build arguments or conditions in the Dockerfiles
rg -i "FROM|ARG.*ARCH|platform" rolling/Dockerfile -A 2 -B 2
# Look for any build or platform-related documentation
fd -t f . | grep -i "doc\|readme\|build" | xargs cat 2>/dev/null | grep -i "ros-gz\|platform\|arm64"
Length of output: 3509
| echo "Launched docker container." | ||
| echo -e 'Open \e]8;;http://127.0.0.1:6080\e\\http://127.0.0.1:6080\e]8;;\e\\ via web browser.' | ||
| echo "" | ||
| echo "NOTE 1: Default user is \"$USER\", password is \"$PASSWORD\"." | ||
| echo "NOTE 2: --security-opt seccomp=unconfined flag is required to launch Ubuntu Jammy/Noble based image on some environment." |
There was a problem hiding this comment.
Security: Avoid displaying credentials in container logs
The password is currently being displayed in plain text in the container logs. This could expose sensitive information if the logs are captured or persisted.
Consider masking the password in the output message:
-echo "NOTE 1: Default user is \"$USER\", password is \"$PASSWORD\"."
+echo "NOTE 1: Default user is \"$USER\", password is [REDACTED]"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "Launched docker container." | |
| echo -e 'Open \e]8;;http://127.0.0.1:6080\e\\http://127.0.0.1:6080\e]8;;\e\\ via web browser.' | |
| echo "" | |
| echo "NOTE 1: Default user is \"$USER\", password is \"$PASSWORD\"." | |
| echo "NOTE 2: --security-opt seccomp=unconfined flag is required to launch Ubuntu Jammy/Noble based image on some environment." | |
| echo "Launched docker container." | |
| echo -e 'Open \e]8;;http://127.0.0.1:6080\e\\http://127.0.0.1:6080\e]8;;\e\\ via web browser.' | |
| echo "" | |
| echo "NOTE 1: Default user is \"$USER\", password is [REDACTED]" | |
| echo "NOTE 2: --security-opt seccomp=unconfined flag is required to launch Ubuntu Jammy/Noble based image on some environment." |
|
@Tiryoh thank you so much for the awesome big update!! I often use this repository in my lectures and workshops, so I am very happy that Gazebo is now available regardless of the host environment. BTY, |
Thanks to @takasehideki, new Gazebo was able to use on arm64 image
Image size will increase by 440MB
Summary by CodeRabbit
Documentation
Chores