Docker cleanup #1946

filiperinaldi · 2019-01-31T17:18:29Z

Status

PRODUCTION / DEVELOPMENT

Description

Unify Dockerfiles to both Arm and x86. There is a lot of duplication for the generic part (creating user, installing ROS)
Create multiple images with different levels of support (allows to disable Cuda)
Install dependencies on the base image using rosdep so only the necessary is installed
Added DockerHub hooks
When using the base images, mount the user's Autoware source code as a volume

See autowarefoundation/autoware_ai#530

Related PRs

None.

Todos

Documentation
Add run.sh
Fix Cuda support
Simplify ROS package dependencies

Tests

AArch64 (Host: Ubuntu 18.04)
x86_64 (Host: Ubuntu 16.04)
x86_64 + Cuda (Host: Ubuntu 16.04)

Steps to Test or Reproduce

cd docker/generic
# Building:
./build.sh --tag-prefix local

# Running with cuda
./run.sh --tag-prefix local

# Running without cuda
./run.sh --tag-prefix local --cuda off

sgermanserrano

@filiperinaldi Thanks for the PR!
@esteve @gbiggs @amc-nu I have been involved in the development of this PR, hence I'd like for at least one of you to have a look at it before merging. The changes on this PR are also needed for #2042

amc-nu

@filiperinaldi @sgermanserrano Thanks for all the effort put on this PR. Looks great.

I have some questions/observations:

Is there any recommended nvidia-docker version?
I would suggest adding --rm to the docker build in the scripts.
After pulling the branch and running ./run.sh I get a message telling me the image autoware/autoware:latest-kinetic-cuda is not available. Do you plan to upload it to docker hub before or after merging this PR?
I'm getting an error when building the CUDA version. liblibdpm_ttic.so: undefined reference to 'cuMemFree_v2' (Plus other undefined references when linking that same file). I think this PR might be missing Fix/colcon build #2000 . Can you confirm? If that's the case, it will be fixed after merging, so please ignore this point.
CPU version builds flawlessly. However, seems it still doesn't contain [fix] Install commands for all the packages #1861 Fix install directives #1990 it is also pointing to devel space. I wasn't able to run anything inside the container.

As a note @sgermanserrano @kfunaoka we'll need to update the Wiki. Maybe we need an issue and add it to the release todo list.

amc-nu · 2019-02-27T16:13:29Z

docker/generic/Dockerfile

+RUN su -c "bash -c 'source /opt/ros/$ROS_DISTRO/setup.bash; \
+                  cd /home/$USERNAME/Autoware/ros; \
+                  ./colcon_release'" $USERNAME
+RUN echo "source /home/$USERNAME/Autoware/ros/devel/setup.bash" >> \


here points to devel

esteve · 2019-02-27T16:18:36Z

docker/generic/Dockerfile.cuda

+    rm -rf /var/lib/apt/lists/*
+COPY --from=nvidia/opengl:1.0-glvnd-runtime-ubuntu16.04 \
+  /usr/local/lib/x86_64-linux-gnu \
+  /usr/lib/x86_64-linux-gnu


Why the change to /usr/lib? /usr/local/lib makes more sense IMHO

@esteve that change was made to address a duplication problem: filiperinaldi#10

esteve · 2019-02-27T16:19:53Z

docker/generic/Dockerfile.cuda

+  /usr/local/share/glvnd/egl_vendor.d/10_nvidia.json
+RUN echo '/usr/local/lib/x86_64-linux-gnu' >> /etc/ld.so.conf.d/glvnd.conf && \
+    ldconfig && \
+    echo '/usr/$LIB/libGL.so.1' >> /etc/ld.so.preload && \


What's the value of $LIB? It doesn't seem to be set anywhere.

Unify docker support with a new (and more generic) set of docker files. Two main images are created: - Base image containing an Ubuntu installation with all dependencies - Build image containing a pre-built Autoware When Cuda support is enabled, there is a further Base+Cuda support image. These Dockerfiles can be used for both AArch64 and x86_64 images. Co-authored-by: Filipe Rinaldi <filipe.rinaldi@arm.com>

esteve · 2019-03-01T10:46:50Z

docker/generic/Dockerfile.base

+COPY ./ /tmp/Autoware
+RUN apt-get update && \
+        apt-get install -y ros-$ROS_DISTRO-desktop-full && \
+        rosdep install -y --from-paths /tmp/Autoware/ros/src --ignore-src && \


I'd advise against using rosdep here as it prevents Docker from using the cache effectively. There's also no trace of what packages are being installed, and more importantly, given that ROS1 does not guarantee ABI compatibility, any ROS package installed afterwards may cause a segfault.

The previous Dockerfile had a list of ROS packages being installed. They were not comprehensive and were very likely to be out of sync as the dependencies change from version to version.

The rationale for doing it like this was that for a given version of Autoware, its corresponding Docker base image will have the dependencies pre-installed. If you are using this based to develop new features, then you will have to manually run rosdep again to complement the new dependencies required.

The effect on Docker's cache could be minimised with a list, but the cache would still be discarded eventually when the list gets updated. It is a trade-off between getting the dependencies automatically sorted out (instead of someone manually finding them out and updating the Dockerfile) vs increasing the usage of cache.
What do you think would be a better balance? Perhaps splitting ros-desktop-full and a few other basic ROS packages out into their own RUN command to at least re-use that layer? Move the rosdep step to be part of the build steps?

I'm not sure I understand the compatibility issue. Aren't these all deb packages which are meant to be compatible on the same distro?

What do you think would be a better balance? Perhaps splitting ros-desktop-full and a few other basic ROS packages out into their own RUN command to at least re-use that layer? Move the rosdep step to be part of the build steps?

I think having a list of known packages and then running rosdep install is a good compromise. OSRF has the process entirely automated in their images, perhaps in the future we could just have a script that runs rosdep install and if there are any new packages, add them to Dockerfile and commit the changes.

I'm not sure I understand the compatibility issue. Aren't these all deb packages which are meant to be compatible on the same distro?

Unfortunately not. If I have packages installed from a previous sync and I install new ones, they are most certainly to crash because they are not ABI-compatible. ROS releases must be upgraded/installed all at once, you can't mix packages from different syncs, even if they are from the same distribution.

Just a correction, it's not certain as I stated, but there's a chance that the packages are incompatible.

filiperinaldi · 2019-03-01T11:08:47Z

@amc-nu See below reply to the different points:
@filiperinaldi @sgermanserrano Thanks for all the effort put on this PR. Looks great.
I have some questions/observations:
1. Is there any recommended nvidia-docker version?
There is a need to use nvidia-docker v2

I'll add the version to the README.md.

Make it explicit in the documentation that we recommend NVIDIA Docker v2.

filiperinaldi · 2019-03-06T15:04:39Z

@amc-nu See below reply to the different points:
@filiperinaldi @sgermanserrano Thanks for all the effort put on this PR. Looks great.
I have some questions/observations:
1. Is there any recommended nvidia-docker version?
There is a need to use nvidia-docker v2
I'll add the version to the README.md.

Done.

MouriNaruto · 2019-03-10T23:43:41Z

docker/generic/Dockerfile.cuda

+
+ENV LIBRARY_PATH /usr/local/cuda/lib64/stubs
+
+# Support for Nvidia docker v2


Good job! This is where I want to add.

MouriNaruto · 2019-03-10T23:48:21Z

Good Job. You have done something I want to.

I want to add OpenGL support to the dockerfile. But I found you have done it.

Thank you very much.

Mouri

I think the changes look overall, but unfortunately I won't be able to review it before 1.12, so I'm dismising my review

* Docker: Add truly generic docker support Unify docker support with a new (and more generic) set of docker files. Two main images are created: - Base image containing an Ubuntu installation with all dependencies - Build image containing a pre-built Autoware When Cuda support is enabled, there is a further Base+Cuda support image. These Dockerfiles can be used for both AArch64 and x86_64 images. Co-authored-by: Filipe Rinaldi <filipe.rinaldi@arm.com> * ros/.gitignore: Ignore log and coverage folders * Autoware built using colcon instead of catkin * Install ROS dependencies via file * Docker: Recommend NVIDIA Docker version 2 Make it explicit in the documentation that we recommend NVIDIA Docker v2.

filiperinaldi force-pushed the topic/docker_cleanup branch 6 times, most recently from 60c6976 to 856c638 Compare February 4, 2019 17:13

filiperinaldi changed the title ~~[DO NOT MERGE YET] Docker cleanup~~ Docker cleanup Feb 4, 2019

filiperinaldi force-pushed the topic/docker_cleanup branch 4 times, most recently from 7debdcb to 61dd64b Compare February 11, 2019 14:02

This was referenced Mar 14, 2023

[Discussion] Docker images tags autowarefoundation/autoware_ai#551

Closed

[DO NOT MERGE YET] Feature/crossbuild dockerhub #1980

Closed

crossbuild dockerhub #1982

Closed

filiperinaldi mentioned this pull request Mar 14, 2023

Release 1.11 autowarefoundation/autoware_ai#476

Closed

17 tasks

sgermanserrano mentioned this pull request Feb 19, 2019

Fix/colcon build #2000

Merged

maximosipov mentioned this pull request Feb 19, 2019

[MERGE AFTER #1982] Integrate ament into CI and docker builds #2007

Closed

1 task

filiperinaldi force-pushed the topic/docker_cleanup branch 2 times, most recently from c06974e to 6e79278 Compare February 20, 2019 11:11

amc-nu mentioned this pull request Feb 23, 2019

Error in launching Rviz from Autoware Docker autowarefoundation/autoware_ai#359

Closed

sgermanserrano requested review from esteve, sgermanserrano, amc-nu and gbiggs February 27, 2019 08:13

sgermanserrano approved these changes Feb 27, 2019

View reviewed changes

amc-nu reviewed Feb 27, 2019

View reviewed changes

esteve reviewed Feb 27, 2019

View reviewed changes

This was referenced Feb 28, 2019

Fix/docker cleanup filiperinaldi/Autoware#12

Closed

Fix/docker cleanup filiperinaldi/Autoware#13

Merged

Servando German and others added 3 commits March 1, 2019 09:35

Autoware built using colcon instead of catkin

bb6db7b

ros/.gitignore: Ignore log and coverage folders

0b014cf

sgermanserrano mentioned this pull request Mar 1, 2019

melodic support #2032

Merged

esteve self-requested a review March 1, 2019 10:37

filiperinaldi force-pushed the topic/docker_cleanup branch from 5d265ba to 0b014cf Compare March 1, 2019 10:44

esteve previously requested changes Mar 1, 2019

View reviewed changes

gbiggs added the In review label Mar 4, 2019

sgermanserrano and others added 2 commits March 6, 2019 15:02

Install ROS dependencies via file

0a60288

Docker: Recommend NVIDIA Docker version 2

c007199

Make it explicit in the documentation that we recommend NVIDIA Docker v2.

filiperinaldi force-pushed the topic/docker_cleanup branch from 405250d to c007199 Compare March 6, 2019 15:03

MouriNaruto reviewed Mar 10, 2019

View reviewed changes

MouriNaruto mentioned this pull request Mar 10, 2019

nvidia docker v2 support autowarefoundation/autoware_ai#93

Closed

sgermanserrano requested a review from esteve March 11, 2019 09:15

amc-nu mentioned this pull request Mar 12, 2019

[Bug] ndt_gpu not built using catkin_make_release/colcon_release on ros kinetic gpu desktop autowarefoundation/autoware_ai#599

Closed

gbiggs approved these changes Mar 13, 2019

View reviewed changes

sgermanserrano merged commit b1a9c0b into autowarefoundation:develop Mar 13, 2019

sgermanserrano mentioned this pull request Mar 13, 2019

Autoware Dockerfiles clean up autowarefoundation/autoware_ai#530

Closed

amc-nu mentioned this pull request Mar 18, 2019

[Fix] Remove importing darknet in Autoware Docker #1548

Closed

JWhitleyWork mentioned this pull request May 8, 2019

Bugfix/fix nvidia docker v2 (redux) #1416

Closed

2 tasks

mitsudome-r added the version:autoware-ai Autoware.AI label Jun 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docker cleanup #1946

Docker cleanup #1946

filiperinaldi commented Jan 31, 2019 •

edited

Loading

sgermanserrano left a comment

amc-nu left a comment •

edited

Loading

amc-nu Feb 27, 2019

esteve Feb 27, 2019

sgermanserrano Feb 28, 2019

esteve Feb 27, 2019

esteve Mar 1, 2019

filiperinaldi Mar 1, 2019 •

edited

Loading

esteve Mar 1, 2019

esteve Mar 1, 2019

filiperinaldi Mar 7, 2019

filiperinaldi commented Mar 1, 2019

filiperinaldi commented Mar 6, 2019

MouriNaruto Mar 10, 2019

MouriNaruto commented Mar 10, 2019


		ENV LIBRARY_PATH /usr/local/cuda/lib64/stubs

		# Support for Nvidia docker v2

Docker cleanup #1946

Docker cleanup #1946

Conversation

filiperinaldi commented Jan 31, 2019 • edited Loading

Status

Description

Related PRs

Todos

Steps to Test or Reproduce

sgermanserrano left a comment

Choose a reason for hiding this comment

amc-nu left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filiperinaldi Mar 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filiperinaldi commented Mar 1, 2019

filiperinaldi commented Mar 6, 2019

Choose a reason for hiding this comment

MouriNaruto commented Mar 10, 2019

filiperinaldi commented Jan 31, 2019 •

edited

Loading

amc-nu left a comment •

edited

Loading

filiperinaldi Mar 1, 2019 •

edited

Loading