Add configurable timeout for digest resolution #9455
Conversation
adds the ability to configure the timeout and, now that it's configurable, makes the timeout much more aggressive (60s->2s).
There was a problem hiding this comment.
@julz: 0 warnings.
In response to this:
adds the ability to configure the digest resolution timeout in deployment.yaml and, now that it's configurable, uses the opportunity to make the timeout much more aggressive (60s->2s).
Fixes #9296.
Release Note
- Lowers the default digest resolution timeout to 2 seconds. - Allows changing the digest resolution timeout./assign @markusthoemmes @mattmoor
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.
| @@ -44,6 +44,10 @@ data: | |||
| # List of repositories for which tag to digest resolving should be skipped | |||
| registriesSkippingTagResolving: "kind.local,ko.local,dev.local" | |||
|
|
|||
| # digestResolutionTimeout is the maximum time allowed for an image's | |||
| # digests to be resolved. | |||
| digestResolutionTimeout: "2s" | |||
There was a problem hiding this comment.
Do we have any data backing this up? I.e. do we have historical data or something showing how long a digest usually takes?
There was a problem hiding this comment.
no, that's why I didn't want to lower it too much without making it configurable (hard to know with total certainty what's a safe value in all environments), but I think 2 seconds is still fairly generous -- it's only a metadata request. Very happy to bump it a bit higher though if we're concerned.
There was a problem hiding this comment.
the critical thing here is if the default is bad people can trivially bump it back up (but 60 seconds seems way way too high to me as a default, even with plenty of resolver threads)
There was a problem hiding this comment.
slack negotiation landed on 10s :). updated
|
/lgtm Not sure if @markusthoemmes had anything further? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: julz, markusthoemmes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
adds the ability to configure the digest resolution timeout in deployment.yaml and, now that it's configurable, uses the opportunity to make the timeout much more aggressive (60s->10s).
Fixes #9296.
Release Note
/assign @markusthoemmes @mattmoor