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

Speed up build image process for PRs #17623

Merged
merged 2 commits into from
Oct 19, 2021
Merged

Speed up build image process for PRs #17623

merged 2 commits into from
Oct 19, 2021

Conversation

aanm
Copy link
Member

@aanm aanm commented Oct 17, 2021

Most of the PRs use the same code base. Thus, we can store Golang's
build cache on GitHub and re-use it for PR builds to speed up the
building process of the PRs code.

Since Docker buildx share the mount of type 'cache' across image builds,
the GitHub workflow to build images will populate its cache using a
'dummy' Dockerfile located in 'images/cache/Dockerfile'. This cache will
always be updated every time a build is executed in the master branch
and all PRs will reuse this cache to build their images.

The times that take each build process are:

Current build process     - master branch - ~ 11m3s
Build with cache hit      - master branch - ~ 9m26s (-14.63%)
Build with cache miss [1] - master branch - ~ 12m (+8.59%)

Current build process     - Pull Request  - ~ 11m1s
Cache hit PR [2]          - Pull Request  - ~ 4m17 (-61.11%)

[1] A cache miss will only happen if the cache is cleaned up / deleted
will only be deliberate.

[2] This was tested on a source code where there was a new line added
in the cilium-agent source code. Adding a new line meant that most of
the build cache was reused. The best case scenario are PRs that don't
change a single line of the Golang's source code, where the build
process will take less than 4m17s. The worst case scenario will be a PR
that change all of the source code for which the build time will be no
worse than 11m1s.

The reason why it takes more time on a cache hit on the master branch
than on a PR cache hit is due to the fact that the master branch will
spend the time storing the new cache where the PR will not store any
cache

Fixes #14928

@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Oct 17, 2021
@aanm aanm requested review from a team as code owners October 17, 2021 19:34
Most of the PRs use the same code base. Thus, we can store Golang's
build cache on GitHub and re-use it for PR builds to speed up the
building process of the PRs code.

Since Docker buildx share the mount of type 'cache' across image builds,
the GitHub workflow to build images will populate its cache using a
'dummy' Dockerfile located in 'images/cache/Dockerfile'. This cache will
always be updated every time a build is executed in the master branch
and all PRs will reuse this cache to build their images.

The times that take each build process are:

Current build process     - master branch - ~ 11m3s
Build with cache hit      - master branch - ~ 9m26s (-14.63%)
Build with cache miss [1] - master branch - ~ 12m (+8.59%)

Current build process     - Pull Request  - ~ 11m1s
Cache hit PR [2]          - Pull Request  - ~ 4m17 (-61.11%)

[1] A cache miss will only happen if the cache is cleaned up / deleted
will only be deliberate.

[2] This was tested on a source code where there was a new line added
in the cilium-agent source code. Adding a new line meant that most of
the build cache was reused. The best case scenario are PRs that don't
change a single line of the Golang's source code, where the build
process will take less than 4m17s. The worst case scenario will be a PR
that change all of the source code for which the build time will be no
worse than 11m1s.

The reason why it takes more time on a cache hit on the master branch
than on a PR cache hit is due to the fact that the master branch will
spend the time storing the new cache where the PR will not store any
cache.

Signed-off-by: André Martins <andre@cilium.io>
To avoid Golang's build cache to grow infinitely, we will clean it up
every week.

However, as soon we clean up its cache we should rebuild it by
triggering a new image build run to avoid any cache misses for the
PRs that try to use this cache to speed up the building process.

Signed-off-by: André Martins <andre@cilium.io>
Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

AFAIK there is no need for the Image CI Cache Cleaner workflow due to GitHub Actions default behaviour: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy

Is there any reason we need the manual cleaning on top of it? 🤔

@aanm
Copy link
Member Author

aanm commented Oct 18, 2021

AFAIK there is no need for the Image CI Cache Cleaner workflow due to GitHub Actions default behaviour: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy

Is there any reason we need the manual cleaning on top of it? 🤔

That is correct but the golang cache will grow forever, that's what needs to be cleaned up once in a while. The master branch will use the GH cache as well to create the new GH cache. The old GH cache will be GCed but the new GH cache will contain the old golang cache and that needs to be cleaned up.

@aanm aanm requested a review from nbusseneau October 18, 2021 16:51
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

🚀

@nbusseneau
Copy link
Member

nbusseneau commented Oct 19, 2021

That is correct but the golang cache will grow forever, that's what needs to be cleaned up once in a while. The master branch will use the GH cache as well to create the new GH cache. The old GH cache will be GCed but the new GH cache will contain the old golang cache and that needs to be cleaned up.

Aaah I see. I'm not familiar with Golang's build cache, unfortunate it does not optimize/clean by itself on subsequent iterations.

@aanm aanm merged commit b195361 into master Oct 19, 2021
@aanm aanm deleted the pr/add-gh-cache-build branch October 19, 2021 13:07
@jrajahalme
Copy link
Member

@aanm It looks like all pull requests from forks must be rebased as the workflow from master now expects images/cache to exist: https://github.com/cilium/cilium/runs/4009068247?check_suite_focus=true

@aanm
Copy link
Member Author

aanm commented Nov 3, 2021

@jrajahalme yes that is correct. Not only from forks but any PR needs to be rebased with master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub action CI/CD follow ups
5 participants