Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Augment Dockerfile for DockerHub #36

Merged
merged 14 commits into from
Mar 22, 2023
Merged

Conversation

mabelzhang
Copy link
Collaborator

There were a few preferences I want to iterate on this PR about.

Currently this PR adds a new Dockerfile in a new directory. I didn't replace the original one, in case we are using it for development. @quarkytale how heavy do you depend on the rocker features? The biggest one I can see is the --home arg, which mounts your host directory. That can be done with pure rocker, but I didn't want to disrupt your workflow. Let me know if you think we can replace the rocker Dockerfile, and I can edit that instead of adding a new one.

Summary of changes:

  • New run.bash without dependency on rocker
    Since the plan is to release the image on DockerHub, when people discover an image on DockerHub and docker pull it, they should be able to just docker run it, without having to install a third-party tool like rocker. We can’t expect people to look for a README, install rocker on their host, and then use that to run the Docker image. That adds a lot of overhead. While rocker is a handy tool for developers familiar with it, for the general public, raw docker run is a more familiar standard tool and more powerful. Everything that rocker does can be already done by docker.
  • Consolidate apt update occurrences in Dockerfile, because each apt-get voids the previous cache and creates a bigger resulting image. For a smaller image, reduce apt updates, and each one should be followed by an apt clean
  • Add non-root user in Dockerfile (used to be shortcut by rocker). Put source code in /home/developer as opposed to /tmp.

Notes on NVIDIA:

  • run.bash
    There are two run scripts, one passes in NVIDIA, the other doesn’t. People without NVIDIA cards can fall back to the latter, but it will be slower (I thought it wasn’t too much slower, but I was only testing simple worlds). Both work for me right now. I have an NVIDIA card.
    I need someone else to test to make sure both work for them. Without NVIDIA, .gz/rendering/ogre2.log should show that it's using llvmpipe for software rendering. With NVIDIA, that file should show it's using NVIDIA.

  • Base image
    For Gazebo to work in Dockerfiles, we usually default to basing on an NVIDIA image, such as FROM nvidia/opengl:1.0-glvnd-devel-ubuntu20.04 (there isn’t an official one for 22.04, but I have a custom working version).
    Since Gazebo Garden, it’s no longer required to have an NVIDIA card for rendering in Docker. If there’s no NVIDIA card, Gazebo will use software rendering based on Mesa Llvmpipe. The caveat is that some people have found that they need to specify some environment variables for it to work. On my laptop, it just worked without specifying anything extra, and I didn’t notice any slowdown.
    For now, I’m leaving the base image the way it is (no NVIDIA), in the hopes that it is accessible to people without NVIDIA cards. If that doesn't work for others (e.g. cannot launch GUI even with environment variables, which we can add to the image), then I can change to basing on an NVIDIA image.

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang mabelzhang requested a review from quarkytale March 9, 2023 20:23
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

PR Preview Action v1.3.0
Preview removed because the pull request was closed.
2023-03-22 05:56 UTC

@quarkytale
Copy link
Contributor

Yes definitely you can edit the original Dockerfile, it's not a very heavy dependence on rocker. I would just suggest to keep the bash scripts for users not familiar with docker won't have to deal with the raw docker tags. Removing NVIDIA seems fine too, any case not covered by this configuration would be revealed once we get a user base.

@mabelzhang mabelzhang linked an issue Mar 9, 2023 that may be closed by this pull request
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang
Copy link
Collaborator Author

mabelzhang commented Mar 10, 2023

It is now editing the existing Dockerfile. The rocker run.bash has been renamed run_rocker.bash.

I've removed all references to NVIDIA. Let's see if this works for you. If not, I can add NVIDIA.
I tested this on a computer with NVIDIA, even though NVIDIA is not passed in.
If you have a computer without an NVIDIA card, that's especially good to test.

There are two quick scripts, setup.bash and run_simulation.bash, once you enter the Docker container. The latter should fire up the GUI.

docker/buoy/Dockerfile Show resolved Hide resolved
docker/run_rocker.bash Outdated Show resolved Hide resolved
docker/build.bash Outdated Show resolved Hide resolved
docker/run.bash Outdated Show resolved Hide resolved
docker/run.bash Show resolved Hide resolved
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang
Copy link
Collaborator Author

7ce8715 is a major change

After thinking about it, I think the no-NVIDIA setup is too slow for regular use. It’s probably safer to offer an NVIDIA option, rather than waiting till things don’t work for people, to come back to add it.

I added an NVIDIA option --no-nvidia in build.bash, so the user can choose whether to build with an NVIDIA base image.

Same for run.bash.

While updating the README, I realized that having NVIDIA / no-NVIDIA options alongside the rocker instructions is too confusing, because to use rocker, which injects NVIDIA, they should build without NVIDIA. Then that creates 3 options, raw docker with NVIDIA, raw docker without NVIDIA, and rocker which should use the build option without NVIDIA. I think that’s too confusing.

With the current bash scripts, there isn’t really a reason to keep the rocker instructions around, because the home folder, NVIDIA, and non-root user were the main things rocker provided. These are already provided by the bash scripts.

Other changes

  • apt-key had a deprecation warning. That is now replaced by the latest installation instructions on ROS and Gazebo websites.

  • One improvement that I can make in a future PR is to use named layers, so that if not using NVIDIA, can base on the ros:humble-ros-base as before, and save image build time. Right now, all the lines need to be run, whether using NVIDIA or not, so the image takes longer to build than before.

@mabelzhang mabelzhang requested a review from quarkytale March 20, 2023 05:10
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@quarkytale
Copy link
Contributor

--no-nvidia works as expected, yet to test without the tag

Copy link
Contributor

@quarkytale quarkytale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me with both the options, good to go after last touches.

README.md Outdated
Otherwise
```
./run.bash buoy --no-nvidia
```

1. To have another window running the same docker container, run this command in a new terminal:

```
./join.bash buoy_latest_runtime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
./join.bash buoy_latest_runtime
./join.bash <container ID>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also adding a note on how to find the container ID would be helpful too

Copy link
Collaborator Author

@mabelzhang mabelzhang Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't realize the join.bash is a different one from the one I usually use. I've updated the file. It automatically finds the container ID. You should be able to join using ./join.bash buoy. I've updated the README.
baadcf3

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if that works for you, before I merge

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes works for me!

docker/build.bash Outdated Show resolved Hide resolved
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@quarkytale
Copy link
Contributor

Merging this in, hoping there weren't any touches left

@quarkytale quarkytale merged commit c354206 into main Mar 22, 2023
@quarkytale quarkytale deleted the mabelzhang/add_nonrocker_option branch March 22, 2023 05:56
@mabelzhang mabelzhang mentioned this pull request Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Host Docker image
2 participants