Add docker file that builds against crystal release for use in testing the crystal-devel branch · Pull Request #562 · ros-navigation/navigation2

ghost · 2019-02-12T19:27:10Z

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	related to #421
Primary OS tested on	Ubuntu
Robotic platform tested on	NA

Description of contribution in a few bullet points

This new docker file builds against the Cyrstal release instead of ROS2 master, and relies entirely on rosdep to bring in dependencies. It is intended to be used to test the crystal-devel branch, and maybe make a crystal dockerhub image.

ghost · 2019-02-12T19:31:32Z

Oh. I should note that is currently doesn't work on either master or crystal-devel. It doesn't work on master because master has the new NodeOptions api change. It doesn't work on crystal-devel because the that branch doesn't have the COLCON_IGNORE file in nav2_system_tests.

My plan is to merge everything up to the API breaking change from yesterday over to crystal-devel and then test with this dockerfile.

Move ARG insertion after rosdep to preserve more caching layers

Seems necessary when sourcing a colcon workspace that was first initialized using the debian ros2 install, as opposed to parent workspace installed from the ros2 nightly fat archives.

ruffsl

I added some changes to the ARGs and entypoint

Dockerfile.crystal

Also * fixing a syntax error on if statement * moving rm nav2_system_test/COLCON_IGNORE before rosdep call

ruffsl · 2019-02-15T20:46:34Z

Dockerfile.crystal

 WORKDIR $NAV2_WS/src
 COPY ./ navigation2/

+# delete nav2_system_tests/COLCON_IGNORE before running rosdep, otherwise it


Is this true? I've never known rosdep to care what a COLCON_IGNORE file is or what it means. I think it just recurses the --from-paths looking for package.xml files. Our CI Dockerfile is setup similarly.

Moving this arg and logic to before the rosdep install would needlessly break the build caching layers when switching system tests on and off.

Searching the rosdep github repo for any mentioning of COLCON_IGNORE returns nothing:
https://github.com/ros-infrastructure/rosdep/search?q=COLCON_IGNORE&unscoped_q=COLCON_IGNORE

It's true that the build breaks if the rm is done after the rosdep call. It breaks because it can't find the gazebo_ros_pkgs which are the additional dependency that the nav2_system_test package brings in.

I'll admit that I'm assuming the underlying root cause.

I created a minimal docker to experiment with.

FROM ros:crystal # install ROS2 dependencies RUN apt-get update && apt-get install -q -y \ build-essential \ cmake \ git \ python3-colcon-common-extensions \ python3-vcstool \ wget \ && rm -rf /var/lib/apt/lists/* # copy ros package repo ENV NAV2_WS /opt/nav2_ws RUN mkdir -p $NAV2_WS/src WORKDIR $NAV2_WS/src COPY ./ navigation2/

With the COLCON_IGNORE file in place, I get the following output from rosdep

> rosdep install -s -q -y --from-paths src --ignore-src #[apt] Installation commands: apt-get install -y -qq libboost-all-dev apt-get install -y -qq ros-crystal-launch-testing apt-get install -y -qq ros-crystal-behaviortree-cpp apt-get install -y -qq libbullet-dev apt-get install -y -qq libsdl1.2-dev apt-get install -y -qq libsdl-image1.2-dev apt-get install -y -qq ros-crystal-yaml-cpp-vendor apt-get install -y -qq ros-crystal-map-msgs apt-get install -y -qq ros-crystal-tf2-sensor-msgs apt-get install -y -qq libpcl-dev apt-get install -y -qq ros-crystal-laser-geometry apt-get install -y -qq ros-crystal-angles

If I delete the COLCON_IGNORE file

> rosdep install -s -q -y --from-paths src --ignore-src #[apt] Installation commands: apt-get install -y -qq ros-crystal-launch-testing apt-get install -y -qq ros-crystal-gazebo-ros-pkgs apt-get install -y -qq ros-crystal-angles apt-get install -y -qq ros-crystal-behaviortree-cpp apt-get install -y -qq libboost-all-dev apt-get install -y -qq ros-crystal-map-msgs apt-get install -y -qq ros-crystal-tf2-sensor-msgs apt-get install -y -qq libpcl-dev apt-get install -y -qq ros-crystal-laser-geometry apt-get install -y -qq libbullet-dev apt-get install -y -qq libsdl1.2-dev apt-get install -y -qq libsdl-image1.2-dev apt-get install -y -qq ros-crystal-yaml-cpp-vendor

You'll see ros-crystal-gazebo-ros-pkgs is missing from the first list, so rosdep results in different action depending on whether the file is there or not. How it does that without ever actually referencing the file, I don't know.

Ok, it looks like rosdep is using catkin's find_packages to loop search for packages, and thus the transitive ignore.

https://github.com/ros-infrastructure/rosdep/blob/e5d6772686874165cf1ef96d0e6b64ff63076af1/src/rosdep2/catkin_packages.py#L7

ghost · 2019-02-15T21:38:47Z

I believe the circle CI build failure is because the branch is off an old version of master. I'll rebase and verify the build after the review is done to avoid messing up the history.

ruffsl · 2019-02-16T00:45:11Z

I've been thinking and realized rather than creating a new dockerfile for each release, we could template a dockerfile for building against any given release, e.g by using ARG in the Dockerfile's FROM:

ARG FROM_IMAGE=ros
FROM $FROM_IMAGE
...

https://www.jeffgeerling.com/blog/2017/use-arg-dockerfile-dynamic-image-specification

ROS_DISTRO=crystal
docker build \
    --tag rosplanning/navigation2:${ROS_DISTRO}-devel \
    --build-arg ros:${ROS_DISTRO}

We then add custom build phase hooks to override build command in the automated Docker Hub repo so that it uses the correct base image when build a docker image tag for each added devel branch.

/hooks/build

#!/bin/bash
ROS_DISTRO=${SOURCE_BRANCH%"-devel"}
docker build \
    --tag rosplanning/navigation2:${SOURCE_BRANCH} \
    --build-arg ros:${ROS_DISTRO}

https://docs.docker.com/docker-hub/builds/advanced/#override-build-test-or-push-commands

I'll work up a separate PR for this.

ros-navigation#562 (comment)

ruffsl · 2019-02-16T04:30:16Z

I've asked the circleci folks on a issue of perhaps a missing feature:
https://discuss.circleci.com/t/conditioning-docker-image-via-base-branch-name-from-pr/28426

orduno

Looks good.

ghost · 2019-02-19T18:54:36Z

@ruffsl Lacking direct support from CircleCI, it looks like we can get the information from the Github API if we can get at least the PR number from CircleCI. ie. For this PR, this command line seems to do the trick
curl -s https://api.github.com/repos/ros-planning/navigation2/pulls/562 | python3 -c "import json; import sys; pr=json.load(sys.stdin); print(pr['base']['label'])"

…ts in the MPPI controller (ros-navigation#562) * Update Migration guide Signed-off-by: Ayush Singh <64409716+Ayush1285@users.noreply.github.com> * Update configuration guide for MPPI Signed-off-by: Ayush Singh <64409716+Ayush1285@users.noreply.github.com> --------- Signed-off-by: Ayush Singh <64409716+Ayush1285@users.noreply.github.com>

Add docker file that builds against crystal release.

a093c2c

ghost requested review from mhpanah, orduno and ruffsl February 12, 2019 19:27

Carl Delsey and others added 4 commits February 12, 2019 13:28

Need to remove the system_test colcon ignore before running rosdep

2298401

Indent lists 💄

a1365bf

Add build ARG to disable system tests

8e0c372

Move ARG insertion after rosdep to preserve more caching layers

Update entrypoint to primary source workspace

e4fe896

Seems necessary when sourcing a colcon workspace that was first initialized using the debian ros2 install, as opposed to parent workspace installed from the ros2 nightly fat archives.

ruffsl approved these changes Feb 14, 2019

View reviewed changes

Dockerfile.crystal Outdated Show resolved Hide resolved

Change base docker to ros:crystal

545bd40

Also * fixing a syntax error on if statement * moving rm nav2_system_test/COLCON_IGNORE before rosdep call

ruffsl reviewed Feb 15, 2019

View reviewed changes

ruffsl added a commit to ruffsl/navigation2 that referenced this pull request Feb 16, 2019

Ensure rosdep install system test depenacies

e713cc9

ros-navigation#562 (comment)

orduno approved these changes Feb 19, 2019

View reviewed changes

ghost merged commit 4a17d4a into ros-navigation:master Feb 19, 2019

Forsyth-Creations pushed a commit to Forsyth-Creations/navigation2 that referenced this pull request Feb 19, 2025

[CI] Fixup the repos file for humble and rolling. (ros-navigation#562)

1508d45

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docker file that builds against crystal release for use in testing the crystal-devel branch#562

Add docker file that builds against crystal release for use in testing the crystal-devel branch#562
6 commits merged intomasterfrom
unknown repository

ghost commented Feb 12, 2019

Uh oh!

ghost commented Feb 12, 2019

Uh oh!

ruffsl left a comment

Uh oh!

Uh oh!

ruffsl Feb 15, 2019

Uh oh!

ghost Feb 15, 2019

Uh oh!

ghost Feb 15, 2019

Uh oh!

ruffsl Feb 16, 2019

Uh oh!

ghost commented Feb 15, 2019

Uh oh!

ruffsl commented Feb 16, 2019 •

edited

Loading

Uh oh!

ruffsl commented Feb 16, 2019

Uh oh!

orduno left a comment

Uh oh!

ghost commented Feb 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ghost commented Feb 12, 2019

Basic Info

Description of contribution in a few bullet points

Uh oh!

ghost commented Feb 12, 2019

Uh oh!

ruffsl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ruffsl Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

ghost Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

ghost Feb 15, 2019

Choose a reason for hiding this comment

Uh oh!

ruffsl Feb 16, 2019

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 15, 2019

Uh oh!

ruffsl commented Feb 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

/hooks/build

Uh oh!

ruffsl commented Feb 16, 2019

Uh oh!

orduno left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ruffsl commented Feb 16, 2019 •

edited

Loading