Fix docker nightly build #2643

ntfshard · 2024-10-09T15:23:52Z

🦟 Bug fix

Fixes #

Summary

Clean build by instruction in docker/README.md not working. Base image builds fine, but the next step is failing.

Ubuntu Focal (20.04) is too old to build gz-sim9 branch. It doesn't contain required GZ packages.

E: Package 'libgz-cmake4-dev' has no installation candidate
E: Unable to locate package libgz-common6-dev
E: Unable to locate package libgz-fuel-tools10-dev
E: Unable to locate package libgz-math8-eigen3-dev
E: Unable to locate package libgz-plugin3-dev
E: Unable to locate package libgz-physics8-dev
E: Unable to locate package libgz-rendering9-dev
E: Unable to locate package libgz-transport14-dev
E: Unable to locate package libgz-gui9-dev
E: Unable to locate package libgz-msgs11-dev
E: Unable to locate package libgz-sensors9-dev
E: Unable to locate package libsdformat15-dev

GCC8 is a very old compiler, I believe installation and replacing a default compiler was done for Ubuntu 18 (with default GCC7). Switched to GCC11 (default for Ubuntu 22). IMHO I'd prefer not to bind to compiler version in this case, but I made it as close as possible
Minor improvements in Readme and image
AFAIU docker/run.bash script not working well on modern systems, but it's out of scope of this question

Slightly out of scope, but couple of months ago tried same for a gz-sim8 branch and had problem with 2 missing gz libraries. If this patch will be accepted, I'll do similar changes on that branch too

Checklist

Signed all commits for DCO
Added tests
Updated documentation (as needed)
Updated migration guide (as needed)
Consider updating Python bindings (if the library has them)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Maksim Derbasov <[email protected]>

mjcarroll · 2024-10-10T13:04:00Z

Would it additionally make sense to introduce a CI job to make sure that this docker file can build on, for instance, a nightly basis?

ntfshard · 2024-10-10T14:49:53Z

Would it additionally make sense to introduce a CI job to make sure that this docker file can build on, for instance, a nightly basis?

Not a big master of GitHub actions, could try I suppose

ntfshard · 2024-10-10T15:22:27Z

Would it additionally make sense to introduce a CI job to make sure that this docker file can build on, for instance, a nightly basis?

Not a big master of GitHub actions, could try I suppose

Maybe in another PR? It not fits very well widely spread pipelines AFAIS. Actually I planned to make fix for annoying bug #2506 which affects us. (and we use older version in which I'd like to propagate fix too).
[In other words, propagate this changes on previous branches: gz-sim8, gz-sim7 and made fix for mentioned bug above on this branch and propagate it on a same branches too. It sounds reasonable?]

mjcarroll · 2024-10-10T15:39:53Z

Maybe in another PR?

Absolutely, would just be a nice addition to keep these dockerfiles fresh and not regressing.

It sounds reasonable?

Good with me.

ntfshard · 2024-10-10T16:08:02Z

Ok. The last mystery, what happened in Jenkins. Looks like something goes wrong with couple of python tests. I can't try restart build there, don't have a permissions.

iche033 · 2024-10-10T16:49:20Z

Ok. The last mystery, what happened in Jenkins. Looks like something goes wrong with couple of python tests. I can't try restart build there, don't have a permissions.

the failing python tests are likely due to osrf/homebrew-simulation#2834 which is currently affecting all gz packages. So not the fault of this PR

ntfshard · 2024-10-14T07:30:37Z

Thank you for your explanation. What is the next step for this PR?
And what is the right way to propagate this changes to other branches? Cherry-pick(commit after merge) or just implement same changes due to it still will be a merge conflict.

iche033 · 2024-10-15T18:27:13Z

yeah, once this is merged, you can cherry pick the changes for backports or use mergify, e.g. add the comment @mergifyio backport gz-sim<N>

ntfshard · 2024-10-16T02:14:01Z

Ok, will try) Thank you!
But right now while CI is red, it can't be merged.

docker/Dockerfile.base

docker/Dockerfile.nightly

docker/scripts/install_common_deps.sh

docker/README.md

Signed-off-by: Maksim Derbasov <[email protected]>

ntfshard · 2024-11-08T16:12:20Z

docker/run.bash

@@ -40,7 +31,6 @@ docker run -it \
  -v "/etc/localtime:/etc/localtime:ro" \
  -v "/dev/input:/dev/input" \
  --rm \
-  --gpus all \


after this changes this script started working fine on a Ubuntu 22.04 host

I think this is needed to get GPU acceleration when using NVIDIA GPUs. Maybe leave it as is and add a statement below to remove it if not using a GPU?

I'll roll-back. Not sure how to dispatch automatically it for a GPU/noGPU cases. In my environment (VirtualBox) it leads to error:
docker: Error response from daemon: could not select device driver "" with capabilities: [[gpu]].

docker/README.md

azeey

Thanks for the changes. I have a few more comments.

azeey · 2024-11-08T17:43:17Z

docker/run.bash

@@ -40,7 +31,6 @@ docker run -it \
  -v "/etc/localtime:/etc/localtime:ro" \
  -v "/dev/input:/dev/input" \
  --rm \
-  --gpus all \


I think this is needed to get GPU acceleration when using NVIDIA GPUs. Maybe leave it as is and add a statement below to remove it if not using a GPU?

azeey · 2024-11-08T17:44:40Z

docker/run.bash

@@ -18,17 +18,8 @@ ARGS=("$@")
 # This is necessary so Gazebo can create a context for OpenGL rendering
 # (even headless).
 XAUTH=/tmp/.docker.xauth
-if [ ! -f $XAUTH ]


Was this change necessary?

Yes, previous code not working, here is example on a system with empty /tmp:

xauth: /tmp/.docker.xauth not writable, changes will be ignored xauth: (argv):1: unable to read any entries from file "(stdin)" chmod: changing permissions of '/tmp/.docker.xauth': Operation not permitted Authorization required, but no authorization protocol specified

and previous code somehow creates /tmp/.docker.xauth as a directory

@j-rivero any thoughts on this?

It still quite simple to verify. But results can be false positive (it will be mentioned above error, but gui will work) if xhost +local was executed before (not sure, maybe it should be local:root, it's still part of ancient magic)

I just tried this locally and it didn't work for me. Running

XAUTH=/tmp/.docker.xauth xauth nlist $DISPLAY | sed -e 's/^..../ffff/' | xauth -f $XAUTH nmerge -

I get

xauth: file /tmp/.docker.xauth does not exist

I double checked and both (old and new) implementations are working fine. AFAIR old version initially didn't work. Maybe xhost +x has some permanent effect(still applied after reboot)
Tried your experiment, just run a command outside of the script and got the same error message.

TBH, it's not my area of expertise, so I'll rollback it with minor change from :0 to $DISPLAY

TBH, it's not my area of expertise, so I'll rollback it with minor change from :0 to $DISPLAY

Me neither. I usually use distrobox or rocker which handle these types of things for me. The change to $DISPLAY looks good.

azeey · 2024-11-08T17:45:03Z

docker/README.md

@@ -75,7 +75,7 @@ distribution using debians.
   image of Gazebo Garden:

    ```
-    ./build.bash gz ./Dockerfile.gz
+    ./build.bash gz-ionic ./Dockerfile.gz


Okay, but now we need to update the instruction right above to say "Gazebo Ionic"

I'll roll-back it here to garden and lines below too, due to build.bash not working properly with Ionic AFAIU. It executes Dockerfile.gz, which is based on nvidia/opengl:1.2-glvnd-devel-ubuntu20.04 image.

Signed-off-by: Maksim Derbasov <[email protected]>

ntfshard · 2024-11-21T15:57:49Z

Would it additionally make sense to introduce a CI job to make sure that this docker file can build on, for instance, a nightly basis?

Added, thanks to @skorobogatydmitry for help

ntfshard · 2024-12-12T02:09:02Z

Folks, can we wrap-up this PR? This PR fixes nightly docker build, which seems almost nobody using.

@iche033 @mjcarroll @azeey

azeey · 2024-12-12T20:22:42Z

docker/scripts/install_common_deps.sh


 sudo apt-get install --no-install-recommends -y \
  libbenchmark-dev \
-  libbenchmark1
+  libbenchmark1.8.3


I think we can get rid of libbenchmark1.8.3. It should be automatically installed with libbenchmark-dev

I guess this entire libbenchmark dependency is redundant, It's compiling fine even w/o this installation.
But as I said earlier, initial idea was to keep this PR as same as possible with previous implementation.

I'd like to remove it completely, if no objections

libbenchmark is an optional dependency. There are tests that use it, but they're disabled if libbenchmark is not installed

gz-sim/test/benchmark/CMakeLists.txt

Line 4 in c298007

if (GzBenchmark_FOUND)

So I think we should keep it.

Reverted, commend added

azeey · 2024-12-12T20:27:14Z

docker/run.bash

@@ -18,17 +18,8 @@ ARGS=("$@")
 # This is necessary so Gazebo can create a context for OpenGL rendering
 # (even headless).
 XAUTH=/tmp/.docker.xauth
-if [ ! -f $XAUTH ]


I just tried this locally and it didn't work for me. Running

XAUTH=/tmp/.docker.xauth xauth nlist $DISPLAY | sed -e 's/^..../ffff/' | xauth -f $XAUTH nmerge -

I get

xauth: file /tmp/.docker.xauth does not exist

Signed-off-by: Maksim Derbasov <[email protected]>

codecov · 2024-12-13T18:34:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (gz-sim9@f0cde05). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff             @@
##             gz-sim9    #2643   +/-   ##
==========================================
  Coverage           ?   68.81%           
==========================================
  Files              ?      341           
  Lines              ?    33084           
  Branches           ?        0           
==========================================
  Hits               ?    22767           
  Misses             ?    10317           
  Partials           ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Maksim Derbasov <[email protected]>

Fix docker build

4e308d1

Signed-off-by: Maksim Derbasov <[email protected]>

ntfshard requested a review from mjcarroll as a code owner October 9, 2024 15:23

github-actions bot added the 🏛️ ionic Gazebo Ionic label Oct 9, 2024

mjcarroll approved these changes Oct 9, 2024

View reviewed changes

Iteration

15088de

Signed-off-by: Maksim Derbasov <[email protected]>

Merge branch 'gz-sim9' into dockerbuildfix

eea8570

ntfshard requested a review from mjcarroll October 15, 2024 07:49

iche033 approved these changes Oct 15, 2024

View reviewed changes

Merge branch 'gz-sim9' into dockerbuildfix

d0fa1cd

azeey reviewed Nov 5, 2024

View reviewed changes

docker/Dockerfile.base Outdated Show resolved Hide resolved

docker/Dockerfile.nightly Outdated Show resolved Hide resolved

docker/scripts/install_common_deps.sh Outdated Show resolved Hide resolved

docker/README.md Outdated Show resolved Hide resolved

Iteration

d5bf100

Signed-off-by: Maksim Derbasov <[email protected]>

ntfshard commented Nov 8, 2024

View reviewed changes

docker/README.md Show resolved Hide resolved

azeey reviewed Nov 8, 2024

View reviewed changes

ntfshard added 4 commits November 9, 2024 03:58

Iteration

fec291b

Signed-off-by: Maksim Derbasov <[email protected]>

Merge branch 'gz-sim9' into dockerbuildfix

5bb243c

Merge branch 'gz-sim9' into dockerbuildfix

52e4445

Regular Docker build

d83556e

Signed-off-by: Maksim Derbasov <[email protected]>

ntfshard requested review from iche033 and azeey November 25, 2024 06:21

ntfshard added 3 commits November 28, 2024 19:23

Merge branch 'gz-sim9' into dockerbuildfix

1e471e6

Merge branch 'gz-sim9' into dockerbuildfix

1b1dc3c

Merge branch 'gz-sim9' into dockerbuildfix

433b6e0

azeey requested changes Dec 12, 2024

View reviewed changes

ntfshard added 2 commits December 14, 2024 02:15

Iteration

309f366

Signed-off-by: Maksim Derbasov <[email protected]>

Merge branch 'gz-sim9' into dockerbuildfix

a080733

ntfshard requested a review from azeey December 13, 2024 17:30

azeey approved these changes Dec 13, 2024

View reviewed changes

Iteration

91acee9

Signed-off-by: Maksim Derbasov <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix docker nightly build #2643

Fix docker nightly build #2643

ntfshard commented Oct 9, 2024 •

edited

Loading

mjcarroll commented Oct 10, 2024

ntfshard commented Oct 10, 2024

ntfshard commented Oct 10, 2024 •

edited

Loading

mjcarroll commented Oct 10, 2024

ntfshard commented Oct 10, 2024

iche033 commented Oct 10, 2024

ntfshard commented Oct 14, 2024

iche033 commented Oct 15, 2024 •

edited

Loading

ntfshard commented Oct 16, 2024

ntfshard Nov 8, 2024

azeey Nov 8, 2024

ntfshard Nov 8, 2024

azeey left a comment

azeey Nov 8, 2024

azeey Nov 8, 2024

ntfshard Nov 8, 2024

azeey Nov 14, 2024

ntfshard Nov 20, 2024

azeey Dec 12, 2024

ntfshard Dec 13, 2024

azeey Dec 13, 2024

azeey Nov 8, 2024

ntfshard Nov 8, 2024

ntfshard commented Nov 21, 2024

ntfshard commented Dec 12, 2024

azeey Dec 12, 2024

ntfshard Dec 13, 2024

azeey Dec 13, 2024

ntfshard Dec 14, 2024

azeey Dec 12, 2024

codecov bot commented Dec 13, 2024

Fix docker nightly build #2643

Are you sure you want to change the base?

Fix docker nightly build #2643

Conversation

ntfshard commented Oct 9, 2024 • edited Loading

🦟 Bug fix

Summary

Checklist

mjcarroll commented Oct 10, 2024

ntfshard commented Oct 10, 2024

ntfshard commented Oct 10, 2024 • edited Loading

mjcarroll commented Oct 10, 2024

ntfshard commented Oct 10, 2024

iche033 commented Oct 10, 2024

ntfshard commented Oct 14, 2024

iche033 commented Oct 15, 2024 • edited Loading

ntfshard commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azeey 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

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

ntfshard commented Nov 21, 2024

ntfshard commented Dec 12, 2024

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

codecov bot commented Dec 13, 2024

Codecov Report

ntfshard commented Oct 9, 2024 •

edited

Loading

ntfshard commented Oct 10, 2024 •

edited

Loading

iche033 commented Oct 15, 2024 •

edited

Loading