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

Minor Dockerfile optimizations #288

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

amarburg
Copy link
Collaborator

@amarburg amarburg commented Sep 4, 2024

Changes Made

Cleaned up and focused version of #226.

An opinionated cleanup of the dockerfile. Includes:

In the "ci" image:

  • Move the apt-get above the "copy"
  • Only copy */package.xml (requires the recent "copy --parent" syntax in the Dockerfile) and blue.repos into the ci image, rather than the full contents of this repo. I believe this prevents changes to e.g. files in .docker/ from breaking cache.
  • vcs import < blue.repos so rosdep dependencies for those packages are also installed in the CI image.

In the "robot" image:

  • Move the apt-get for gstreamer above the "copy" so changes to this repo don't break that cache
  • Minor cleanup to installation of geographic libs

This doesn't include some of the more opinionated hadolint changes (e.g. gosu, etc). This PR is focused on moving relatively static steps (apt-get) above cache-breaking changes (pulling files from the local filesystem)

Associated Issues

None

Testing

Completed "joystick teleoperation" demo with built images in rolling and jazzy

@amarburg amarburg changed the title Move apt-gets up in Dockerfile Minor Dockerfile optimizations Sep 4, 2024
@amarburg amarburg marked this pull request as draft September 4, 2024 23:24
@amarburg
Copy link
Collaborator Author

amarburg commented Sep 4, 2024

Hm, odd that ROS-Industrial CI is now breaking. Debugging ...

Hm, so ROS-industrial CI is looking for mavros, mavros-extras etc. Which makes sense. The question is how did it ever work?

@amarburg
Copy link
Collaborator Author

amarburg commented Sep 6, 2024

As a note, as expected, industrial CI does work if we force it to run on "humble" (where mavros is available through rosdep), but fails for "rolling" (and presumably "jazzy")

- IMAGE: rolling-ci
ROS_DISTRO: rolling
- IMAGE: humble-ci
ROS_DISTRO: humble
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As noted, industrial CI tries to install mavros viaapt, which is currently not available for rolling (#248).

CI does work if we force it to use Humble, for which there's a binary image.

Note this is just the ros_industrial CI stage, not building the Docker images (where Mavros is built from source for rolling/jazzy.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for looking into that. Because we know it's an issue with MAVROS, It's not a big deal if the CI fails in this PR. We can just update the CI stage in #241 to use the new mavros stage.

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.

2 participants