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
Make webhook retry backoff parameters configurable #95705
Conversation
88d8718
to
7f1fcd3
Compare
7f1fcd3
to
d9fbf6c
Compare
/retest |
1f3d9eb
to
af47846
Compare
/retest |
2 similar comments
/retest |
/retest |
af47846
to
c220782
Compare
/retest |
/assign @deads2k |
/retest |
1 similar comment
/retest |
75fbcf4
to
dea1437
Compare
/retest |
/lgtm |
/approve |
Currently webhook retry backoff parameters are hard coded, we want to have the ability to configure the backoff parameters for webhook retry logic.
dea1437
to
2f812c3
Compare
/lgtm |
// DefaultAuthWebhookRetryBackoff is the default backoff parameters for | ||
// both authentication and authorization webhook used by the apiserver. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a webhook returns a 5xx response that has Retry-After
set, the client end (Kubernetes) does not pay attention to that.
Would it be OK to briefly mention in a comment that this implementation does not adjust behavior based on Retry-After
?
(to be clear - it's OK not to check that header; maybe a future PR can start checking that response header)
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, dims, sjenning, sttts, tkashem 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 |
kubernetes/kubernetes#95705 added the requirement for specifying the `WebhookRetryBackoff` field in authorizerfactory.DelegatingAuthorizerConfig{} struct used for metrics authorization. This commit explicitly sets the field to the previous implicitly used defaults, available in "k8s.io/apiserver/pkg/server/options". This commit is purely for compatability with kubernetes apiserver 1.20 and does not change behavior.
kubernetes/kubernetes#95705 added the requirement for specifying the `WebhookRetryBackoff` field in authorizerfactory.DelegatingAuthenticatorConfig{} struct used for metrics authorization. This commit explicitly sets the field to the previous implicitly used defaults, available in "k8s.io/apiserver/pkg/server/options". This commit is purely for compatability with kubernetes apiserver 1.20 and does not change behavior.
pkg/cmd/infra/router/template.go: kubernetes/kubernetes#95705 added the requirement for specifying the `WebhookRetryBackoff` field in authentication/authorizer DelegatingAuthenticatorConfig{} struct used for metrics authentication. This commit explicitly sets the field to the previous implicitly used defaults, available in "k8s.io/apiserver/pkg/server/options". This commit is purely for compatability with kubernetes apiserver 1.20 and does not change behavior.
pkg/cmd/infra/router/template.go: kubernetes/kubernetes#95705 added the requirement for specifying the `WebhookRetryBackoff` field in authentication/authorizer DelegatingAuthenticatorConfig{} struct used for metrics authentication. This commit explicitly sets the field to the previous implicitly used defaults, available in "k8s.io/apiserver/pkg/server/options". This commit is purely for compatability with kubernetes apiserver 1.20 and does not change behavior.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Currently webhook retry backoff parameters are hard coded, we want to have the ability to configure the backoff parameters for webhook retry logic.
The objective of this PR is to make the backoff parameters configurable only. So the default that is being used today should still be in effect.
Does this PR introduce a user-facing change?: