Improvements to documentation and docker setups from my getting spun up #101

tfoote · 2021-11-30T08:33:36Z

This is an aggregate of my making progress on learning how things are working and trying to streamline things a bit.

The usage of rocker requires a new feature I just added to work around the upstream image setup: osrf/rocker#164

chapulina

The usage of rocker requires a new feature I just added

Do we need to wait for a release before merging this PR?

README.md

docker/build_and_run_docker.sh

chapulina · 2021-11-30T17:56:25Z

docker/debug_integration.sh

-  --security-opt seccomp=unconfined \
-  $DOCKER_OPTS \
-  mbari_lrauv_debug
+rocker --nvidia --x11 --user --user-override-name=developer --user-preserve-home -- mbari_lrauv_tests tmuxinator start debug_and_plot -n debug_session -p src/lrauv/docker/tests/debug_integration_mux.yml


I'll leave it up to @arjo129 to check if this covers his use case

Good News: The system launches correctly with the correct panes.

Bad News: I may be doing something wrong but when I run via rocker I get the following error:

[GUI] [Err] [Ogre2RenderEngine.cc:905] Unable to create the rendering window: OGRE EXCEPTION(3:RenderingAPIException): OpenGL 3.3 is not supported. Please update your graphics card drivers. in GL3PlusRenderSyste m::initialiseContext at /var/lib/jenkins/workspace/ogre-2.2-debbuilder/repo/RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp (line 3364) [GUI] [Err] [Ogre2RenderEngine.cc:913] Unable to create the rendering window after [11] attempts. [GUI] [Err] [Ogre2RenderEngine.cc:834] Failed to create dummy render window.

Might be some (nvidia?) driver issue or a problem with the way I setup rocker 🤔 .
Also is the --nvidia really nessecary? What about scenarios where we aren't using an nvidia card. I don't think we really need cuda acceleration.

What is your OS and graphics card and graphics card driver? It's possible if you're using a too new version

The --nvidia option is mounting the nvidia graphic card into docker to give it access to do hardware accelerated rendering. The cuda support is still in a pending PR.

This option means that you get " --gpus all " injected into your docker arguments and the nvidia runtime driver requirements are injected into the container using multi stage container builds. (instead of needing to explicitly extend from it at the top of the Dockerfile.) It's replacing

https://github.com/osrf/lrauv/pull/101/files#diff-c826ab2996e92237b4bb80b3cee5f11d231efb7ddb40884fa5bfd2bbe4dcd4b7L54

Before docker added the --gpus option for using Intel graphics cards you need to use --devices /dev/dri/card0 instead of the --runtime=nvidia But I think now having the nvidia drivers installed is just extra space.

docker/tests/Dockerfile

tfoote · 2021-12-01T16:12:33Z

I have released rocker 0.2.7 to get the new command line argument

chapulina

LGTM, I just have minor comments and I think we should wait for @arjo129 's approval too.

chapulina · 2021-12-01T20:15:35Z

README.md

@@ -24,7 +24,9 @@ Optionally, you may choose to build this repository using Docker, for
 convenience.
 Make sure you have a recent version of [Docker](https://docs.docker.com/) and
 [nvidia-docker](https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html#docker)
-installed. Next to get started simply run the following command.
+installed.
+And [rocker](https://github.com/osrf/rocker) available as debian package `python3-rocker` with the ROS packages.


Nitpick: this project doesn't use ROS, so users landing here may not have ROS packages setup. They may setup it at later stages to get colcon, but up here on the README, it may not be clear how to set it up.

I always prefer to err on the side of explicitness in this situation to avoid confusing new users who may not be familiar with ROS packages. So I'd suggest either removing the part about "debian package python3-rocker with the ROS packages" and letting the rocker entry page explain how to set it up, or repeating the step-by-step here on the README.

chapulina · 2021-12-01T20:16:54Z

README.md

+cd docker
+docker build -t mbari_lrauv -f empty_world/Dockerfile ..
+rocker --nvidia --x11 --user mbari_lrauv bash
+ign launch lrauv_world.ign


The launch file was removed in #102 , we should use plain ign gazebo commands now

chapulina · 2021-12-01T20:21:19Z

docker/tests/Dockerfile

-        wget
+        wget \
+  && rm -rf /var/lib/apt/lists/* \
+  && apt-get -qq clean


Is it worth it cleaning it here, considering we'll do an update below again?

Yes, this removes the contents of the apt cache from the layer, making the docker container much slimmer. Otherwise you just end up with outdated caches in every layer of the container.

arjo129

Thanks for these changes.

I've been unable to get rocker working on my comp correctly. Could be some GPU setup issue. I'm worrying if we really need the --nvidia flag. There are some minor knits in regards to style.

arjo129 · 2021-12-02T03:37:10Z

docker/empty_world/Dockerfile

+# setup ignition environment
+ENTRYPOINT ["/entrypoint.bash"]
+
+CMD ign launch lrauv_world.ign


Minor Knit: New line at EOF.

arjo129 · 2021-12-02T03:39:51Z

.github/workflows/integration_tests.yaml

      run: docker build -t mbari_lrauv . -f docker/tests/Dockerfile
+    - name: Run tests
+      run: docker run --rm mbari_lrauv


Minor Knit: Newline at EOF.

arjo129 · 2021-12-02T06:33:36Z

docker/debug_integration.sh

-  --security-opt seccomp=unconfined \
-  $DOCKER_OPTS \
-  mbari_lrauv_debug
+rocker --nvidia --x11 --user --user-override-name=developer --user-preserve-home -- mbari_lrauv_tests tmuxinator start debug_and_plot -n debug_session -p src/lrauv/docker/tests/debug_integration_mux.yml


Good News: The system launches correctly with the correct panes.

Bad News: I may be doing something wrong but when I run via rocker I get the following error:

[GUI] [Err] [Ogre2RenderEngine.cc:905] Unable to create the rendering window: OGRE EXCEPTION(3:RenderingAPIException): OpenGL 3.3 is not supported. Please update your graphics card drivers. in GL3PlusRenderSyste m::initialiseContext at /var/lib/jenkins/workspace/ogre-2.2-debbuilder/repo/RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp (line 3364) [GUI] [Err] [Ogre2RenderEngine.cc:913] Unable to create the rendering window after [11] attempts. [GUI] [Err] [Ogre2RenderEngine.cc:834] Failed to create dummy render window.

Might be some (nvidia?) driver issue or a problem with the way I setup rocker 🤔 .
Also is the --nvidia really nessecary? What about scenarios where we aren't using an nvidia card. I don't think we really need cuda acceleration.

tfoote · 2021-12-02T09:47:20Z

Is it worth it cleaning it here, considering we'll do an update below again?

Yes, the standard practice when building dockerfiles is to clear the cache at every layer. This has two major reasons. First is that it keeps the image size down. Every file change in the system at the end of a step is saved into the layer. So if you do 3 apt-get updates in three consecutive layers you will get three copies of the cache in each layer assuming they're slightly different because they were pulled at different times. And secondly, because docker caches previous layers, you have no knowledge of how long ago the last update was so you are going to have to do another update. And the cost of the incremental update versus a fresh update is small, and gives you a much more consistent result.

mabelzhang · 2021-12-03T10:00:46Z

Butting in with a nitpick request in my head for a while - can we rename this image so that it doesn't start with mbari? Technically, this is not mbari's image, so it might be called osrf_lrauv (name of this repo) or something other than MBARI LRAUV.

Confusion arises because we host a different, official image on MBARI DockerHub that has the MBARI binary in it: https://hub.docker.com/r/mbari/lrauv-ignition-sim When you pull it, it's called mbari/lrauv-ignition-sim. At the end of the day, I end up with a handful of images under my docker images, all called some combination of MBARI LRAUV (the official no-MBARI-source one which I push to DockerHub, the with-source one that's a dependency, and my personal development one. Not counting this one in the osrf repo).

The more we can disambiguate these names, the less confusing it will be for ourselves.

tfoote · 2021-12-06T23:38:06Z

Good idea @mabelzhang I've updated it with the osrf_lrauv to mirror the repo name for clarity.

tfoote · 2021-12-06T23:40:06Z

I worked with @arjo129 on slack and we isolated his crashing to be related to the integrated AMD card getting selected and his prime-select settings seemingly not going into the environment.

chapulina · 2022-01-05T16:59:17Z

docker/tests/Dockerfile

-
-# Clean up apt
-RUN rm -rf /var/lib/apt/lists/* \
+  ignition-fortress\


Suggested change

ignition-fortress\

ignition-garden\

chapulina · 2022-01-05T16:59:50Z

docker/tests/Dockerfile

-        wget
+        wget \
+  && rm -rf /var/lib/apt/lists/* \
+  && apt-get -qq clean

 # Add Ignition's latest packages, which may be more up-to-date than the ones from the MBARI image
 RUN /bin/sh -c 'echo "deb http://packages.osrfoundation.org/gazebo/ubuntu-stable `lsb_release -cs` main" > /etc/apt/sources.list.d/gazebo-stable.list' && \


We have to add the nightly repo now, for Garden

Signed-off-by: Tully Foote <[email protected]>

Following up feedback in review Signed-off-by: Tully Foote <[email protected]>

tfoote · 2022-01-28T12:44:03Z

I've rebased this for garden.

With the new rebuilds of the garden image with different UIDs it needs osrf/rocker#168

hidmic · 2022-02-17T19:50:15Z

I worked with @arjo129 on slack and we isolated his crashing to be related to the integrated AMD card getting selected and his prime-select settings seemingly not going into the environment.

Circling back here. I did not find anything particularly unusual in rocker. @arjo129 I'll assume that you've prime-select'ed nvidia. Could it be an issue with your LD_LIBRARY_PATH (NVIDIA/nvidia-docker#612)? Would you mind giving it a shot?

hidmic · 2022-02-18T15:49:54Z

By that I meant we try the fix proposed in NVIDIA/nvidia-docker#612 (comment).

caguero · 2022-06-28T19:05:21Z

Closing this PR for now.

chapulina reviewed Nov 30, 2021

View reviewed changes

tfoote force-pushed the tfoote/orientation_docker branch from da424d8 to 8c7c0d4 Compare December 1, 2021 16:23

chapulina approved these changes Dec 1, 2021

View reviewed changes

arjo129 requested changes Dec 2, 2021

View reviewed changes

tfoote force-pushed the tfoote/orientation_docker branch from 0c11320 to e98b8f7 Compare December 2, 2021 09:41

tfoote requested a review from arjo129 December 2, 2021 09:47

tfoote force-pushed the tfoote/orientation_docker branch from f0e7a8c to 40df96b Compare December 6, 2021 23:37

tfoote self-assigned this Dec 7, 2021

chapulina suggested changes Jan 5, 2022

View reviewed changes

caguero mentioned this pull request Jan 19, 2022

2022 M1 meta-issue #150

Closed

39 tasks

caguero added this to the 2022 M1 milestone Jan 19, 2022

caguero mentioned this pull request Nov 2, 2021

M3 meta-issue #19

Closed

40 tasks

tfoote added 12 commits January 28, 2022 02:29

clean up docker entrypoints a bit

027e012

Signed-off-by: Tully Foote <[email protected]>

enable testing in empty world builds

8e2eb8c

Signed-off-by: Tully Foote <[email protected]>

homogenize entry point

a4b5afe

Signed-off-by: Tully Foote <[email protected]>

Extend readme with my notes

ee15ffd

Signed-off-by: Tully Foote <[email protected]>

clean up image bloat

8120191

Signed-off-by: Tully Foote <[email protected]>

remove image layer

160196c

Signed-off-by: Tully Foote <[email protected]>

clean up comment

a1d367e

Signed-off-by: Tully Foote <[email protected]>

converge dockerfiles

cb11e34

Signed-off-by: Tully Foote <[email protected]>

Run the tests now that they're not run as part of the build

5ae6c73

Signed-off-by: Tully Foote <[email protected]>

add newline at EOF

8943815

Signed-off-by: Tully Foote <[email protected]>

avoid shorthand mention of how to install rocker

595ee1c

Signed-off-by: Tully Foote <[email protected]>

update image name to be osrf_lrauv from mbari_lrauv

04fe7b1

Following up feedback in review Signed-off-by: Tully Foote <[email protected]>

tfoote force-pushed the tfoote/orientation_docker branch from 40df96b to 04fe7b1 Compare January 28, 2022 12:43

caguero closed this Jun 28, 2022

tfoote mentioned this pull request Jul 14, 2022

improve experience when using intel power saving mode enabled osrf/rocker#186

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to documentation and docker setups from my getting spun up #101

Improvements to documentation and docker setups from my getting spun up #101

tfoote commented Nov 30, 2021

chapulina left a comment

chapulina Nov 30, 2021

arjo129 Dec 2, 2021

tfoote Dec 2, 2021

tfoote commented Dec 1, 2021

chapulina left a comment

chapulina Dec 1, 2021

chapulina Dec 1, 2021

chapulina Dec 1, 2021

tfoote Feb 9, 2022

arjo129 left a comment

arjo129 Dec 2, 2021

arjo129 Dec 2, 2021

arjo129 Dec 2, 2021

tfoote commented Dec 2, 2021

mabelzhang commented Dec 3, 2021 •

edited

Loading

tfoote commented Dec 6, 2021

tfoote commented Dec 6, 2021

chapulina Jan 5, 2022

chapulina Jan 5, 2022

tfoote commented Jan 28, 2022

hidmic commented Feb 17, 2022

hidmic commented Feb 18, 2022

caguero commented Jun 28, 2022

Improvements to documentation and docker setups from my getting spun up #101

Improvements to documentation and docker setups from my getting spun up #101

Conversation

tfoote commented Nov 30, 2021

chapulina left a 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

tfoote commented Dec 1, 2021

chapulina left a 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

arjo129 left a 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

tfoote commented Dec 2, 2021

mabelzhang commented Dec 3, 2021 • edited Loading

tfoote commented Dec 6, 2021

tfoote commented Dec 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tfoote commented Jan 28, 2022

hidmic commented Feb 17, 2022

hidmic commented Feb 18, 2022

caguero commented Jun 28, 2022

mabelzhang commented Dec 3, 2021 •

edited

Loading