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

webhook timeout variable #1387

Merged
merged 4 commits into from
Nov 14, 2021
Merged

webhook timeout variable #1387

merged 4 commits into from
Nov 14, 2021

Conversation

sairamankumar2
Copy link
Contributor

Webhook timeout by default is set to 30s (as we use AdmissionregistrationV1beta1) and sometimes that causes the request to fail when the number of pods in the cluster is huge. We fix the webhook error by recreating the webhook manually with a lower timeout value (This does get modified when the operator pod restarts).

We found kubernetes/kubernetes#71508, which talks about how the default client timeout is 30s, so if the webhooks have a timeout equal or greater, the client can give up before the apiserver has, which means the failurePolicy: ignore has no effect.

This PR provides a way to set a default value for webhook timeout which allow the users to configure the timeout time based on their usage.

@sairamankumar2
Copy link
Contributor Author

Also, can we set the default timeout value to 10s as it is the default timeout value in webhook starting from AdmissionregistrationV1?

@@ -86,6 +86,7 @@ type WebHook struct {
enableResourceQuotaEnforcement bool
resourceQuotaEnforcer resourceusage.ResourceQuotaEnforcer
coreV1InformerFactory informers.SharedInformerFactory
TimeoutSeconds *int32
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: timeoutSeconds since it doesn't need to be exposed outside the package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed the variable to be timeoutSeconds and also bumped the chart version to have the intermediate fix

@liyinan926 liyinan926 merged commit 6939cbd into kubeflow:master Nov 14, 2021
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
* Added webhook timeout variable

* made timeout variable to be private

* Fixed chart version

Co-authored-by: Sairaman Kumar <sairamank@in.ibm.com>
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