Refactor actions to rebuild CI image from CI

[ruffsl]

With the recent announcement of Docker Hub deprecating free access to autobuild, we can migrate our builds to GitHub.

• https://www.docker.com/blog/changes-to-docker-hub-autobuilds/
• https://news.ycombinator.com/item?id=27434009

This PR refactors the github action we used to trigger rebuilds of the CI image using Docker Hub's autobuild service, and instead builds the CI image directly from the github action and then pushes it to the public registry. This has the added benefit of allowing us to build our CI image using buildkit, giving access to more advanced build options and better caching strategies.

Used actions:

• https://github.com/marketplace/actions/docker-setup-buildx
• https://github.com/marketplace/actions/docker-login
• https://github.com/marketplace/actions/build-and-push-docker-images

This also tags built images with an additional timestamp, allowing more reproducible builds and version tracking. This same tagging approach will be applied to the upstream testing image, allowing us to use dependabot to track when our CI's parent image updates and automatically open PRs to bump the version tag we build FROM.

osrf/docker_images#546

Example run:

https://github.com/ruffsl/navigation2/actions/runs/934144502

[ruffsl]

Given how cumbersome Docker Hub's token system is, specifically having to resort to:

• Having to create a unique machine user account on Docker Hub to avoid using one's own personal account
• Having to create a unique group in the Docker Hub org to curtaile repo permissions for machine user account
• Having to create a unique token to push to the registry that can't be managed at the org level, only by the machine account

docker/hub-feedback#1419

I'm thinking of ditching Docker Hub for hosting the Nav2 CI images, and using GitHub's Container Registry instead:

https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry

This not only reduces the number of 3rd party integrations we have to juggle, but also consolidates the admin control for managing the CI, given the registry login uses the same secrets.GITHUB_TOKEN that is already previsioned by GitHub actions:

https://docs.github.com/en/actions/reference/authentication-in-a-workflow#about-the-github_token-secret

This can help simplify redeployment for contributors who would like to run CI on their own forks, or use our CI as a template.

[ruffsl]

@SteveMacenski , a0f4bc8 will necessitate the organization owner to enable the feature for organization members:

https://docs.github.com/en/packages/working-with-a-github-packages-registry/enabling-improved-container-support-with-the-container-registry

[SteveMacenski]

Set to a specific date?

[ruffsl]

Fixing the tag (to the date + utc time as proposed in osrf/docker_images#546 ) will allow dependabot to notify us that the parent image has updated. When merging dependabot's PRs, we can determistcily break the build cache to ensure our CI image is built using the latest image layers from the upstream parent image registry repo.

[SteveMacenski]

We'd have to update this every time there's a testing sync update? That's alot of overhead to maintain long-term.

[ruffsl]

The osrf/ros2:testing image updates with the same cadence as the library/ubuntu:20.04 image, so about once or twice every two months. So the overhead is essentially one of us clicking the merge button once or twice every two months. Doesn't seem like a lot to maintain for a rigorous and reproducible build environment.

For example, if someone pushes a breaking change to one of our dependencies in the testing apt repo, we could just roll back and pin to the last working CI image tag until a fix is synced, unblocking our CI jobs in the meantime.

Or, let's say we're triaging an issue and we want to determine when the issue first surfaced by building against previous versions of our package dependencies. Because the apt testing repo only host's the most recent sync, we could still go back to previous CI image tags to access and build and test against these prior dependency versions.

For me personally though, when I'm debugging an issue with the CI, I'd like to be able to locally build and test against the exact same docker image that the CI used for a particular workflow run. One can use the image SHA digest for this, but timestamp tags are a little nicer to work with, as they are self chronological and still don't shift underneath you.

[SteveMacenski]

What about people that want to fork nav2 and work on it? They wouldn't get any of these updates that are otherwise compiled intrinsically. What about updates to existing nav2 distribution branches like Foxy and Galactic? Are we also going to specifically tag images for those as well?

So the overhead is essentially one of us clicking the merge button once or twice every two months. Doesn't seem like a lot to maintain for a rigorous and reproducible build environment.

I agree and I get the point that its nice to explicitly know when things changed for debugging. I think that would be nice as well.

However this statement:

For example, if someone pushes a breaking change to one of our dependencies in the testing apt repo, we could just roll back and pin to the last working CI image tag until a fix is synced, unblocking our CI jobs in the meantime.

is concerning to me, because testing changes are bound for the distribution release. Hopefully its patched, but we have to be aware of these bugs and somewhat roll with the punches because if its not patched then it will be given out to users in the next distribution sync! It saves our CI at the expense of actual user issues in the field using the debians.

I agree its nice for us to know when things change, but honestly if we think there's going to be these many issues, I don't think we should be using testing at all. Maybe we should just use rolling. I know that was an issue due to TF/rclcpp issues in the aftermath of Galactic, but I don't see why we don't revert to that now so that folks can apt install the dependencies for Nav2 for development into main. Was there some reason we went away from the released rolling distribution other than the issues we were having with TF/rclcpp at the time? I don't recall.

I'm concerned at this level of specificity for 1) end users that are not developers 2) forkers (so most developers that aren't us) 3) existing released distribution policies

[ruffsl]

What about people that want to fork nav2 and work on it? They wouldn't get any of these updates that are otherwise compiled intrinsically.

You may be confusing two things.
This PR does pin the parent image tag for our CI image, but it does not pin the tag for the CI image (though one could modify our CI's executors config to do that). I.e pinning the parent image tag vs pinning the CI image tag:

• https://github.com/ruffsl/navigation2/blob/757e1853f1c7ba1cfab62493c6d1f11b9591b86c/Dockerfile#L14

• https://github.com/ruffsl/navigation2/blob/757e1853f1c7ba1cfab62493c6d1f11b9591b86c/.github/workflows/update_ci_image.yaml#L102-L104

Forks who don't modify the CI's executors config would still pull from the same latest and greatest navigation2:main tag:

https://github.com/ruffsl/navigation2/blob/757e1853f1c7ba1cfab62493c6d1f11b9591b86c/.circleci/config.yml#L436-L439

What about updates to existing nav2 distribution branches like Foxy and Galactic? Are we also going to specifically tag images for those as well?

No, we aren't currently building CI images for previous release branches. I had hope to, but CircleCI doesn't yet have a workflow config syntax to make that simple to implement.

https://discuss.circleci.com/t/conditioning-docker-image-via-base-branch-name-from-pr/28426

Perhaps GitHub actions may facilitate this better after a few version iterations of their workflow config syntax. For production use, I always encourage users to build FROM the official library/ros:latest images and apt install nav2 like normal.

Maybe we should just use rolling. ... Was there some reason we went away from the released rolling distribution other than the issues we were having with TF/rclcpp at the time? I don't recall.

Well, we moved the parent image for CI from ros:rolling to osrf/ros2:testing at the behest of the core ROS2 team from an offline email chain about the aftermath of Galactic. But this touches back to something you've just mentioned here:

but we have to be aware of these bugs and somewhat roll with the punches because if its not patched then it will be given out to users in the next distribution sync!

Having our CI run against rolling testing sync allows us to catch breakages early on before the next distribution sync.

It saves our CI at the expense of actual user issues in the field using the debians.

However, this won't be what the current PR configures. I only hypothesized changing the CI's executors config to pull from a pinned CI image tag as a last resort, incase someone wanted to continue running tests for a WIP feature PR unrelated to proper patch fix PR. It's not something I think would be commonly used, just thinking out loud for posterity.

[SteveMacenski]

This PR does pin the parent image tag for our CI image, but it does not pin the tag for the CI image (though one could modify our CI's executors config to do that). I.e pinning the parent image tag vs pinning the CI image tag:

If its FROM osrf/ros2:testing-20210605003201 AS from_image and I fork it and never update the docker image, it will forever be FROM osrf/ros2:testing-20210605003201 AS from_image every time I want to build a docker image using it (unless I'm a vigilant enough user to notice it, which I wouldn't count on, on average). A forked user will never get the updated the base image unless they keep really extra special care to?

I'm wasn't mentioning about forked users's CI experience versus main repo CI experiences, But that does bring up the good question of what happens if a user on a forked branch submits a PR, does it use our version that might be up to date or their version that could be months old? I don't see any ARGs in the dockerfile that would let our CI override to the main branch's current default.

---

Well, we moved the parent image for CI from ros:rolling to osrf/ros2:testing at the behest of the core ROS2 team from an offline email chain about the aftermath of Galactic.

Admittedly I'm juggling a number of balls so my memory on any given email chain is a little fuzzy, but I thought that the main reason we asked about that was because they didn't want to do regular and predictable syncs to Rolling -- which was more of an extremely helpful suggestion than a critical need.

The core of my concern is that we're making alot more work for ourselves. I can say pretty confidently I'm not going to have any more free time in the future than I've had historically to manage CI related issues so if anything here breaks, I'm probably not going to be able to fix it. So something relatively simple and straight forward is something I prefer. Tagging specific images seems like it might create a bunch of problems and I'm not exactly sure that the benefits of being technically correct outweigh the additional maintenance effort / complexity. I pride myself on making Nav very approachable to many people and the more esoteric we make the CI or local developer installs, it steers away from my intended mission of making it easy to contribute and use Nav2 for experts and newbies alike.

The "nightly" builds I could understand better and I think make some intuitive sense to people. The rolling-testing is 1 somewhat hard to explain and new concept to many users (rolling) buried in another (shadow-fixed / testing) that primarily only core maintainers are even aware of, let alone deeply familiar with.

So I'd just like us to take a step back and think about the trade offs before going too far down this rabbit hole. I agree with you 100% that this is workable and is probably the most technically correct option to make sure we track everything finely. If we were a company and I had a dev ops team dedicated to these kinds of things, I would agree this is the right course of action, we should be in all situations maintaining known versions of all dependencies. But if we run CI using this system, we'll need to tell community developers to do the same so that the environments are the same and they can find issues to resolve that are displayed in CI. As such, the complexity we can make our CI versioning system has to be balanced with what we can reasonably expect community developers/users to put up with to help out. As far as I know, we have 3 major sets of options:

• Nightly: Pro latest and greatest, Con no debians for easy developer install / use, can be problematic if things break upstream of us that a release cycle would at least shield us from
• Rolling: Pro debians can be easily installed for developers to develop and debians if researchers want to use this on their robots as if it were a released distribution to use the latest and greatest of Nav2's features/bug fixes, Con not updated as often as we'd like so we might not see any of the API/ABI breaking changes right up until a new distribution release if other maintainers aren't regularly releasing to it. The months around distribution releases are problematic and stressful.
• Rolling/testing: Pro complete control over dates released so we can roll back if something breaks and we get any new changes to dependencies 2-3 weeks before in Rolling proper to resolve any issues, get debians as well, Con have to explain 2 somewhat complex concepts to new users to be able to understand what they're doing just to build the codebase. Can't install rolling next to Rolling-testing.

We've definitely had our fair share of issues with all 3, especially around releases or major API changes. My feeling is that getting access to rolling/testing changes the 2-3 weeks earlier won't really matter in the long run. If we used Rolling as our base, we'd lag rolling then by a little bit since we'd only see issues after synced, but I think that's OK, nav2 isn't officially released to rolling. And that might be a spot where we could actually leverage the community for help (e.g. if Nav2 suddenly stops working in Rolling and they use Rolling, PRs might come in without us needing to directly do anything). Nightly is still a decent enough option, but development on it after rclcpp API changes becomes challenging since users can't use the last released distribution anymore and need to build master from source, which has been a mixed experience from the community (most have no issues, the ones that do have a hell of a time).

So IMO, barring any other technical assessments I've forgotten or misunderstood, I put (nightly, rolling) > rolling-testing where I could go either way on nightly vs rolling. I think the biggest question is what we want to prioritize: developer/user experience or being able to quickly identify and resolve problems as they arise. I supported the rolling-testing direction as a solution to a problem (that was supposed to be a solution to another problem). I think we've hit a few levels of indirection from the initial problem we wanted to solve.

@wjwwood Your input here would also be valuable if you disagree with anything I mention

[ruffsl]

A forked user will never get the updated the base image unless they keep really extra special care to?

Fair enough, I've fixed the Dockerfile to prevent this. Developers may continue using the default unpinned development stage, while our CI may continue using the pinned integration. See 0c221cd

A reason I'd like to keep with the dependabot approach is that there is no other easy way of determining if a parent image layer is outdated in the CI image unless we rebuild the image. I'd like to insure the CI image never gets too stale without rebuilding the entire thing every day. We've looked for alternatives, but they aren't nearly as elegant or minimal as just enabling dependabot:

osrf/docker_images#362

But that does bring up the good question of what happens if a user on a forked branch submits a PR, does it use our version that might be up to date or their version that could be months old?

The docker image tags selected in the CI executor config are fully qualified, and will thus pull our version of the CI images to test the PR. Even if they opened a PR internal to their own repo, it would still use our version of the CI images unless they manually modified the CI config to something else:

executors:
  debug_exec:
    docker:
      - image: ghcr.io/ros-planning/navigation2:main
# https://github.com/ruffsl/navigation2/blob/757e1853f1c7ba1cfab62493c6d1f11b9591b86c/.circleci/config.yml#L439

---

So I'd just like us to take a step back and think about the trade offs before going too far down this rabbit hole. I agree with you 100% that this is workable and is probably the most technically correct option to make sure we track everything finely.

I'll get back to you on this with a longer write up with my thoughts tomorrow, but I agree keeping with ros:rolling would have been my ideal choice. I'm looking into ways we can ensure even the official ros library images can be better kept up-to-date over at https://github.com/osrf/docker_images , such as ensuring syncs trigger rebuilds for the library images, and a version tagged so we can leverage dependant without needing to use the osrf/ros2:testing images.

[SteveMacenski]

I don't have that level of admin on ros-planning, @tfoote or @wjwwood might be able to assist us? https://docs.github.com/en/packages/working-with-a-github-packages-registry/enabling-improved-container-support-with-the-container-registry

[wjwwood]

What specifically can I help with? Is this something that I can give you permission to enable/disable at your will?

[wjwwood]

Sorry, I misread the link, so you just want that on for this org? I'd still like to just give you the power to enable/disable that yourself, if anyone knows how to do that.

[wjwwood]

Do the container images need to be public? Currently only private is selected by default.

[ruffsl]

Do the container images need to be public? Currently only private is selected by default.

Public is preferred. That way external contributors can pull the CI image as well in case they need to debug their own builds, or run CI from thier own forks.

[wjwwood]

Ok, public is enabled too.

[SteveMacenski]

Thanks @wjwwood

[ruffsl]

@SteveMacenski , I've reverted this PR to use ros:rolling. To verify CI for this PR, could you pull the CI image from my fork and push it to this repo? I don't appear to have the org/repo permissions to push myself to the registry.

docker pull ghcr.io/ruffsl/navigation2:main
docker tag ghcr.io/ruffsl/navigation2:main ghcr.io/ros-planning/navigation2:main
docker push ghcr.io/ros-planning/navigation2:main

You'll need to docker login to the github registry to do push:

https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry#authenticating-to-the-container-registry

I'll then push a commit reverting fbddd72 to re-trigger CI and test the repo setup.

[SteveMacenski]

Done main: digest: sha256:904d116995270bacefbde1eace690784d57ced7ab6719d93df45c26f7df3c3e9 size: 4703

[ruffsl]

@SteveMacenski , I'm not sure where you must have pushed it, but it doesn't seem to be showing up for me or for CircleCI:

• https://github.com/orgs/ros-planning/packages?repo_name=navigation2
• https://circleci.com/gh/ros-planning/navigation2/20597?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

For reference, here what the list of container images looks like under the github package listing from my fork:

• https://github.com/ruffsl/navigation2/pkgs/container/navigation2

[SteveMacenski]

I'm not sure, honestly I just read the login form to login and then ran the commands you listed.

Error response from daemon: Head https://ghcr.io/v2/ros-planning/navigation2/manifests/main: unauthorized

Unauthorized wouldn't typically be the error I'd expect if it doesn't exist?

I see it here:

https://github.com/orgs/ros-planning/packages/container/package/navigation2

Not quite at the link you sent me, but that looks like the Nav2 analog

[ruffsl]

That last link doesn't seem to be publicly accessable, e.g. open that link from incognito mode. I'm guessing that the container registry still isn't configured as public?

[SteveMacenski]

Ah ok, I changed to public

[ruffsl]

@SteveMacenski , the CI build is passing, so this should be ready to merge.

[SteveMacenski]

Should we do something about our existing dockerhub? Update links in the readme to the new docker image registry? Update the badge for the pull count?

[SteveMacenski]

How would a user find these images? I don't really see a way through the github interface on the nav2 landing page, I think we'd have to add a link (And probably do a discourse blast that we're moving entirely from dockerhub)

[ruffsl]

Should we do something about our existing dockerhub? Update links in the readme to the new docker image registry?

We should remove existing tags from our repo on the dockerhub registry to avoid any pulling of stale images. We can also modify the repo description to denote it's deprecation and redirect traffic to the new github image registry.

Update the badge for the pull count?

Not sure there is a badge for that yet with github container registry, though the pull count does is displayed from container package listing on the ros-planning org.

https://shields.io/

How would a user find these images? I don't really see a way through the github interface on the nav2 landing page, I think we'd have to add a link (And probably do a discourse blast that we're moving entirely from dockerhub)

Yeah, I kind of wish that the tags for container registry where showcased under the Packages sidebar. This seems to be the case for my personal fork, but not for organization repos yet. Perhaps that may change after the github beta.

The image tag only includes an environment to build nav2 packages from source and not prebuilt nav2 binaries, so I'm not sure who else would be pulling them other than our CI and fellow maintainers. But we could do the discourse blast if you'd prefer.

[SteveMacenski]

OK, please update the readme with links to the registry prominently placed and a note somewhere below about moving from dockerhub. We can remove tags form dockerhub / ockerhub badge

[ruffsl]

Hey it showed up now: