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

Adjust CI to enable cache sharing with forked PRs #2353

Merged
merged 4 commits into from
May 22, 2021

Conversation

ruffsl
Copy link
Member

@ruffsl ruffsl commented May 20, 2021

So for a recent PR #2349 after merging #2343 , I noticed that it's cache restore step was not finding the matching cache key provide by the main branch:

Prior existing cache stored by main branch CI job

Creating cache archive...
Uploading cache archive...
Stored Cache to overlay_ws-v1-arch1-linux-amd64-6_85-main-<no value>-C8DP7DcUx71NZcYchPyQBH0ElDOZYfM4Uez8CyHmtNE=-1621429706
  * /opt/overlay_ws/build
  * /opt/overlay_ws/install
  * /opt/overlay_ws/log
https://app.circleci.com/pipelines/github/ros-planning/navigation2/5359/workflows/9c999dde-f6c0-4007-b8a3-87d6e091638f/jobs/19741/parallel-runs/0/steps/0-118

Later failed cache restore from forked PR CI job

No cache is found for key: overlay_ws-v1-arch1-linux-amd64-6_85-pull/2349-2349-C8DP7DcUx71NZcYchPyQBH0ElDOZYfM4Uez8CyHmtNE=
No cache is found for key: overlay_ws-v1-arch1-linux-amd64-6_85-main-<no value>-C8DP7DcUx71NZcYchPyQBH0ElDOZYfM4Uez8CyHmtNE=
https://app.circleci.com/pipelines/github/ros-planning/navigation2/5354/workflows/97ef33a5-4b3d-4cce-bbf8-2598807fbf27/jobs/19728/parallel-runs/0/steps/0-116

This was because CircleCI does not share caches with forked repos, unless sharing of environment variables is enabled:

enabling the sharing of environment variables will enable cache sharing between the original repo and all forked builds.
https://circleci.com/docs/2.0/oss/#caching

We can simply enable sharing of environment variables in the repo's circleci settings, but we currently have two secret tokens stored in the repo's circleci environment. The codecov token and the DockerHub build trigger token. Turns out codecov token should be unnecessary for public repos when using major CI's like CircleCI or GH actions, so we could delete that variable from the current CircleCI environment.

note that this is a public repo, so a token shouldn't be necessary
codecov/codecov-circleci-orb#12
https://about.codecov.io/blog/tokenless-uploads-for-github-actions/

As for the DockerHub build trigger token, given GH actions has a slightly more nuance control for protecting repo secrets:

Note: With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.
https://docs.github.com/en/actions/reference/encrypted-secrets#using-encrypted-secrets-in-a-workflow

I've ported the cron job config from #2344 into a GH action workflow so we can move the DockerHub secret into GH actions. This also has the added benefit of allowing the use of GH action event triggers to rebuild the CI image if any of the package.xml or .repos files are changed, given they're subsequently used to pre-install depencies in the CI image.

ruffsl added 3 commits May 19, 2021 11:07
by leting codecov script pickup on circleci env
on it own to corroborate identity
@SteveMacenski
Copy link
Member

So to be clear, the github action is just for triggering dockerhub to re-build the base image? Maybe I'm missing some small detail, but if this is just on a daily cron job, doesn't dockerhub have that option itself? I see an autobuild option there.

@ruffsl
Copy link
Member Author

ruffsl commented May 20, 2021

doesn't dockerhub have that option itself? I see an autobuild option there.

Well, Dockerhub can trigger image rebuilds when autobuild is enabled, but it's rather limited. First, it can only trigger on push events to github branches. While the GH action replicates this, the action is a lot smarter or selective in that it only triggers a rebuild if certain files are changed. E.g why rebuild and break all the existing CI caches if the only thing a merge commit changed is the README file?

The second thing that Docker Hub can't do on it's own is scheduled builds, e.g. time based triggers irregardless of commit events. This is the same logic we had before in CircleCI, and checks if any ros2 package are out of date. Ideally we could use linked repos feature in Dockerhub to trigger image rebuilds when the ros:rolling parent image updates itself, but linking to official library repos has since been disabled by Dockerhub because of all churn that causes when library images do update.

Checking apt directly is also a little better and that we can detect when to rebuild the image even if the packages to update were not originally installed from the parent image.

@SteveMacenski
Copy link
Member

OK got it, thanks for the detailed explanation

@ruffsl ruffsl marked this pull request as ready for review May 22, 2021 02:47
@ruffsl ruffsl merged commit 577b401 into ros-navigation:main May 22, 2021
@ruffsl ruffsl deleted the ci-secrets branch May 22, 2021 02:50
@ruffsl ruffsl mentioned this pull request May 23, 2021
ruffsl added a commit to ruffsl/navigation2 that referenced this pull request Jul 2, 2021
* Use tokenless codecov upload
by leting codecov script pickup on circleci env
on it own to corroborate identity

* Remove dockerhub cron job from circleci

* Add dockerhub cron job from gh actions
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