pipeline
pipeline copied to clipboard
tektoncd
Add shared cache for resolvers
Changes
Resolvers are awesome but can be noisy. In high traffic environments we have seen rate limiting both from registries when using the bundle resolver and from git forges when using the git resolver. This PR adds caching to the resolvers (except the cluster resolver). It is only enabled by default when 'safe' - meaning that the object was fetched by some hash / digest. Otherwise, it can be turned on with a resolver parameter.
[!NOTE]
This PR is a work in progress posted for initial thoughts and feedback. It works for some cases already (git resolver) - others, maybe not.
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
- [x] Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
- [x] Has Tests included if any functionality added or changed
- [x] pre-commit Passed
- [x] Follows the commit message standard
- [x] Meets the Tekton contributor standards (including functionality, content, code)
- [x] Has a kind label. You can add one by adding a comment on this PR that contains
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
- [x] Release notes contains the string "action required" if the change requires additional action from users switching to the new release
Release Notes
Remote resolver results for bundle and git resolvers which use immutable references will be cached by default for 5 minutes. User can opt-in to caching non-immutable references or disable caching entirely. The default caching mode (auto) should be safe and entirely transparent for existing users.
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: brianwcook / name: Brian Cook (2778bf988b5bad691902ef7c16ccf6f80fc48066, f6d00d5acf75ed260281421d35199fef9dc2caac, 55c16eee89d1c57abbf10bd1f0573372573756b9, 0b9dd7ad1f001ee6851cb0c3e1f804fb1fcb615e, caa66327b4f6c1e8323e8270822953958dd19ce3, 15356118d414b9d77c20d6ffb203747a3756635a, d73b4c0d66c91331a65bdde31ec47197de767c57, 5a845d2e61a8bb3ab33a35260d4ca5d7a37b2159, e5bd29887e0d15e8673b934263045547e3f9775d, 74c6f9ac8163de4655d28a846e4385881d2701d4, 0850727b5eeffa5c5e8cf381df38862fc5cc8e75, b5827c9c461a1feac9f85346a4538793b7c4b0ee, a6bd0558e30f10896ae3f5dc5bb976d4bac6d40d, eae911941d5e406e8fa68c60ec7c9234c39d6b94, bdb7c454694ce885dfc099995e092c8a02e1d2d6, 4a362d0202cb006ceaefecbd653845d21d230cf9, fed67d7ef42295b1badca8d8d18daa5c02250d10, a3078a6364fbb4bd4c3ecb04d491859563983ef8, f5d402bd599005d22e1750022f8e91a495b7b4be)
- :white_check_mark: login: waveywaves / name: Vibhav Bobade (88e5b814efb6924d6f2ed7bd5e4c9e41865a9539, e325e6f806f886e95224bdddbfbb77e0d9fd699e, e6e935b3ffbf156535625968084deda5b5f0f145, 4259a120aa02fbc4f2b4d749d0f34f9a161c796f, 5404c195439f92f006d12353d84817af63239742, c5023e5c7283ea70803fd65325c102fe0471e17d, f3f10f9bf3ef021065cfaf6d40b55b81d7a662b9, 05adc2c3c288e27a15ba07deded14e7b98e01e31, f316e076257d2c366ea8bf53a7a32c4a9cc7d554)
![linux-foundation-easycla[bot] avatar](https://avatars.githubusercontent.com/in/17893?v=4 "Published by a pub.dev verified publisher"){kind=small}
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 86.4% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 62.1% | -19.7 |
| pkg/remoteresolution/resolver/http/resolver.go | 88.9% | 78.1% | -10.8 |
| pkg/remoteresolution/resolver/hub/resolver.go | 50.0% | 57.1% | 7.1 |
| pkg/resolution/resolver/framework/cache.go | Do not exist | 29.4% |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 86.4% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 62.1% | -19.7 |
| pkg/remoteresolution/resolver/http/resolver.go | 88.9% | 78.1% | -10.8 |
| pkg/remoteresolution/resolver/hub/resolver.go | 50.0% | 57.1% | 7.1 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 88.2% | 0.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 86.4% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 82.9% | 1.1 |
| pkg/remoteresolution/resolver/http/resolver.go | 88.9% | 78.1% | -10.8 |
| pkg/remoteresolution/resolver/hub/resolver.go | 50.0% | 57.1% | 7.1 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 86.4% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 82.9% | 1.1 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 65.0% | -17.4 |
| pkg/remoteresolution/resolver/http/resolver.go | 88.9% | 78.1% | -10.8 |
| pkg/remoteresolution/resolver/hub/resolver.go | 50.0% | 57.1% | 7.1 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 58.3% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 58.3% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 58.3% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 58.3% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 77.1% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
@brianwcook: No presubmit jobs available for tektoncd/pipeline@main
In response to this:
/test all
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 83.7% | 1.9 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
i see in the failed test:
arendelle-bqh7s 28m Warning Failed pod/task-run-steps-termination-reasons-termin-oequuhxg-pod Failed to pull image "busybox@sha256:895ab622e92e18d6b461d671081757af7dbaa3b00e3e28e12505af7817f73649": failed to pull and unpack image "docker.io/library/busybox@sha256:895ab622e92e18d6b461d671081757af7dbaa3b00e3e28e12505af7817f73649": failed to copy: httpReadSeeker: failed open: unexpected status code https://registry-1.docker.io/v2/library/busybox/manifests/sha256:895ab622e92e18d6b461d671081757af7dbaa3b00e3e28e12505af7817f73649: 429 Too Many Requests - Server message: toomanyrequests: You have reached your unauthenticated pull rate limit.
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/annotated_resource.go | Do not exist | 100.0% | |
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 70.5% | -11.4 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/annotated_resource.go | Do not exist | 100.0% | |
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 70.5% | -11.4 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/annotated_resource.go | Do not exist | 100.0% | |
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 70.5% | -11.4 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
Thank you @twoGiants for the thorough review!
I've addressed the following points from your feedback:
- Guard patterns - Restructured both resolvers with early returns
- Cache as resolver property - Added cache field and updated initialization
- Simplified cache usage - Enhanced cache interface to handle key generation internally
- Function properties for testing - Added injectable functions for easier unit testing
- Inlined useCache variables - Removed unnecessary local variables
- Type safety - Enhanced cache to be more specific to ResolvedResource
- Code structure - Renamed existing cache to secretCache and improved error handling
- Cache interface - Added methods that encapsulate key generation
In addition I added cache support for the cluster resolver and added more tests, including integration / e2e tests.
I've addressed the following points from your feedback: ...
Thank you for addressing @brianwcook! :+1:
It looks like you forgot to push the latest changes.
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/remoteresolution/cache/annotated_resource.go | Do not exist | 100.0% | |
| pkg/remoteresolution/cache/cache.go | Do not exist | 85.0% | |
| pkg/remoteresolution/resolver/bundle/resolver.go | 81.8% | 70.5% | -11.4 |
| pkg/remoteresolution/resolver/cluster/resolver.go | 81.8% | 61.4% | -20.4 |
| pkg/remoteresolution/resolver/git/resolver.go | 82.4% | 83.9% | 1.5 |
| pkg/resolution/resolver/bundle/bundle.go | 71.1% | 71.4% | 0.4 |
| pkg/resolution/resolver/bundle/params.go | 87.9% | 86.1% | -1.8 |
| pkg/resolution/resolver/cluster/resolver.go | 80.4% | 79.9% | -0.6 |
| pkg/resolution/resolver/git/resolver.go | 84.9% | 84.3% | -0.6 |
Actually, I was too hasty in declaring victory on the clusterResolver caching. The current implementation is broken. In order to properly support cluster tasks I think we have the following options:
- use mutating webhook to add checksum to cluster task objects and use that as the cache key. It won't help inline task specs unless we add more webhooks.
- accept that cluster tasks are mutable and be defensive by requiring explicit cache: true. (auto-mode is always false)
- don't offer caching for clusterResolver. am i missing anything?
If you like #2, I think we can add that in this PR. If you like #1 better, I would prefer to remove the clusterResolver code from this PR and offer a followup PR to add that, as I am being affected by the lack of git and bundle resolver and would like to see that get merged.