ci: run the ROS jobs on ros-base images so nothing fetches rosdep's sources list - #682
Open
bburda wants to merge 3 commits into
Open
ci: run the ROS jobs on ros-base images so nothing fetches rosdep's sources list#682bburda wants to merge 3 commits into
bburda wants to merge 3 commits into
Conversation
Switch the 11 container-based CI jobs across ci.yml, quality.yml and opcua-plugin.yml from `ubuntu:<codename>` + ros-tooling/setup-ros to ros:<distro>-ros-base directly, and drop the Pre-install ROS 2 apt source step and the ros-apt-source composite action along with it. setup-ros removes /etc/ros/rosdep/sources.list.d/20-default.list and runs sudo rosdep init with no retry, which fetches that file from raw.githubusercontent.com before the step that has a retry loop around it. The ros-base images already ship rosdep initialized with sources.cache filled, the ROS apt source configured, and colcon, rosdep, git, gpg, curl and vcstool installed, so there is nothing left for setup-ros or the apt-source action to do, and the download that has caused outages before never happens. ros-base is narrower than the desktop metapackage setup-ros installs (no rviz2, demo_nodes, turtlesim, examples, rqt); nothing in the workspace depends on any of them, confirmed by building and running the full unit suite for ros2_medkit_gateway inside a real ros:jazzy-ros-base container (2886 tests, 0 failures). The coverage-merge job, which never used ROS, keeps its plain ubuntu:noble image untouched. The per-job "Install Git" steps are also removed: all three ros-base images carry git already.
The ros-base images carry no apt package index at all, unlike the ubuntu:<codename> images setup-ros used to run against. Removing the Install Git step (apt-get update; apt-get install -y git) and setup-ros (which ran its own update as part of installing ROS) left 11 step bodies across ci.yml, quality.yml and opcua-plugin.yml with an apt-get install and no update anywhere in that body: the "Install ccache" step in every container job, "Install ccache and clang-tidy" and "Install clang-tidy- cache (ctcache)" in quality.yml's clang-tidy job. Every "Install dependencies" step already opened with its own apt-get update and needed no change. Add apt-get update to each of those 11 step bodies, before the install, so each step carries its own index and does not depend on an update happening to run earlier in the job.
The coverage job's "Install dependencies" step installed gpg with a comment saying codecov/codecov-action needed it, but this job never uploads to Codecov - that happens in coverage-merge, which has its own gpg install and is untouched by this change. Nothing else in the coverage job (build, test, lcov report generation, tracefile upload) uses gpg either, so the package was dead weight justified by a reason that belonged to a different job. The install goes with the comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Every container job in
ci.yml,quality.ymlandopcua-plugin.ymlruns on a bareubuntu:<codename>image and installs ROS withros-tooling/setup-ros@v0.7. That action removes/etc/ros/rosdep/sources.list.d/20-default.listand then runssudo rosdep initwith no retry around it (src/setup-ros-ubuntu.ts:135-137in the action).rosdep initdownloads that file from raw.githubusercontent.com, which is the host that has failed jobs here before, and it happens before the step that does have a retry loop. Auses:step cannot be wrapped in a loop, so the fetch cannot be protected where it is.The 11 jobs now run on
ros:<distro>-ros-base. Those images ship ROS,rosdepalready initialised with its sources cache filled, the ROS apt source configured, and colcon, rosdep, git, gpg, curl and vcstool. With that,setup-roshas nothing left to do and the download stops happening. Theros-apt-sourcecomposite action goes with it: it existed only sosetup-roswould skip its own rate-limited lookup. The per-jobInstall Gitsteps go too, since all three images carry git.Two follow-on changes are in here because the first one requires them. The
ros-baseimages ship no apt package index, and the deleted steps were where the index came from, so each step that runsapt-get installnow runsapt-get updatefirst. And thecoveragejob's gpg install carried a comment about the Codecov action needing it; that upload happens incoverage-merge, which has its own gpg and is untouched, and nothing else in thecoveragejob uses gpg, so the install is gone.coverage-mergekeeps its plainubuntu:nobleimage. It never used ROS.Issue
Type
Testing
ros-baseis narrower than theros-<distro>-desktopmetapackage thatsetup-rosinstalled. The difference is rviz2, demo_nodes, turtlesim, examples and rqt, and nothing in the workspace depends on any of them. A grep over the workspace finds no reference to them, and a second check goes further. The three step bodies of thejazzy-testjob were read out of the YAML and run verbatim inside a realros:jazzy-ros-basecontainer, with noapt-get updateadded beyond the ones the workflow itself carries. Result: 18 packages built,ros2_medkit_gatewayunit suite 2886 tests, 0 errors, 0 failures.Mechanical checks on the branch: every workflow still parses as YAML; no
setup-ros@reference remains; noros-apt-sourcereference remains and the directory is gone; 16 image values across the 11 jobs name aros:*-ros-baseimage; exactly oneimage: ubuntu:nobleremains, incoverage-merge; and of the 24 step bodies that runapt-get install, every one also runsapt-get update.Measured on all three images, so the apt change is not guesswork:
/var/lib/apt/listsis empty andapt-get install -y --simulate ccacheexits 100 onros:humble-ros-base,ros:jazzy-ros-baseandros:lyrical-ros-base.Two things only CI can show, and reviewers should read the first run with them in mind:
LANG=C.UTF-8wheresetup-rosexporteden_US.UTF-8, and ccache includes the locale in its hash, which was confirmed by compiling the same file twice under the two locales inros:jazzy-ros-basewith ccache 4.9.1: the second compile missed. The Actions cache archive still restores, but its entries do not match, so the first run pays a full build and later runs are warm again. Cache keys are deliberately unchanged, because renaming them would save only the archive download and not the cold build.One weakness was found and deliberately left alone: ctcache's hash covers the preprocessed input, the dumped configuration and the arguments, but not the identity of the clang-tidy binary, so a changed analyser could return an old clean result. This change does not trigger it.
ubuntu:nobleandros:jazzy-ros-baseoffer the sameclang-tidycandidate (1:18.0-59~exp2) from the same apt suites, so the analyser is the same before and after.Checklist