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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43c9a82
Added webhook timeout variable
Nov 12, 2021
47a80cb
made timeout variable to be private
Nov 14, 2021
3e8cecc
Merge branch 'master' of https://github.com/GoogleCloudPlatform/spark…
Nov 14, 2021
8b31d3f
Fixed chart version
Nov 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/spark-operator-chart/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v2
name: spark-operator
description: A Helm chart for Spark on Kubernetes operator
version: 1.1.10
version: 1.1.11
appVersion: v1beta2-1.2.3-3.1.1
keywords:
- spark
Expand Down
1 change: 1 addition & 0 deletions charts/spark-operator-chart/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ All charts linted successfully
| webhook.enable | bool | `false` | Enable webhook server |
| webhook.namespaceSelector | string | `""` | The webhook server will only operate on namespaces with this label, specified in the form key1=value1,key2=value2. Empty string (default) will operate on all namespaces |
| webhook.port | int | `8080` | Webhook service port |
| webhook.timeout | int | `30` | Webhook timeout in seconds |

## Maintainers

Expand Down
1 change: 1 addition & 0 deletions charts/spark-operator-chart/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ spec:
- -enable-webhook=true
- -webhook-svc-namespace={{ .Release.Namespace }}
- -webhook-port={{ .Values.webhook.port }}
- -webhook-timeout={{ .Values.webhook.timeout }}
- -webhook-svc-name={{ include "spark-operator.fullname" . }}-webhook
- -webhook-config-name={{ include "spark-operator.fullname" . }}-webhook-config
- -webhook-namespace-selector={{ .Values.webhook.namespaceSelector }}
Expand Down
2 changes: 2 additions & 0 deletions charts/spark-operator-chart/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ webhook:
cleanupAnnotations:
"helm.sh/hook": pre-delete, pre-upgrade
"helm.sh/hook-delete-policy": hook-succeeded
# -- Webhook Timeout in seconds
timeout: 30

metrics:
# -- Enable prometheus metric scraping
Expand Down
3 changes: 2 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ var (
namespace = flag.String("namespace", apiv1.NamespaceAll, "The Kubernetes namespace to manage. Will manage custom resource objects of the managed CRD types for the whole cluster if unset.")
labelSelectorFilter = flag.String("label-selector-filter", "", "A comma-separated list of key=value, or key labels to filter resources during watch and list based on the specified labels.")
enableWebhook = flag.Bool("enable-webhook", false, "Whether to enable the mutating admission webhook for admitting and patching Spark pods.")
webhookTimeout = flag.Int("webhook-timeout", 30, "Webhook Timeout in seconds before the webhook returns a timeout")
enableResourceQuotaEnforcement = flag.Bool("enable-resource-quota-enforcement", false, "Whether to enable ResourceQuota enforcement for SparkApplication resources. Requires the webhook to be enabled.")
ingressURLFormat = flag.String("ingress-url-format", "", "Ingress URL format.")
enableUIService = flag.Bool("enable-ui-service", true, "Enable Spark service UI.")
Expand Down Expand Up @@ -195,7 +196,7 @@ func main() {
}
var err error
// Don't deregister webhook on exit if leader election enabled (i.e. multiple webhooks running)
hook, err = webhook.New(kubeClient, crInformerFactory, *namespace, !*enableLeaderElection, *enableResourceQuotaEnforcement, coreV1InformerFactory)
hook, err = webhook.New(kubeClient, crInformerFactory, *namespace, !*enableLeaderElection, *enableResourceQuotaEnforcement, coreV1InformerFactory, webhookTimeout)
if err != nil {
glog.Fatal(err)
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/webhook/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

}

// Configuration parsed from command-line flags
Expand Down Expand Up @@ -124,7 +125,8 @@ func New(
jobNamespace string,
deregisterOnExit bool,
enableResourceQuotaEnforcement bool,
coreV1InformerFactory informers.SharedInformerFactory) (*WebHook, error) {
coreV1InformerFactory informers.SharedInformerFactory,
webhookTimeout *int) (*WebHook, error) {

cert, err := NewCertProvider(
userConfig.serverCert,
Expand Down Expand Up @@ -153,6 +155,7 @@ func New(
failurePolicy: arv1beta1.Ignore,
coreV1InformerFactory: coreV1InformerFactory,
enableResourceQuotaEnforcement: enableResourceQuotaEnforcement,
TimeoutSeconds: func(b int32) *int32 { return &b }(int32(*webhookTimeout)),
}

if userConfig.webhookFailOnError {
Expand Down Expand Up @@ -387,6 +390,7 @@ func (wh *WebHook) selfRegistration(webhookConfigName string) error {
},
FailurePolicy: &wh.failurePolicy,
NamespaceSelector: wh.selector,
TimeoutSeconds: wh.TimeoutSeconds,
}

validatingWebhook := v1beta1.ValidatingWebhook{
Expand All @@ -398,6 +402,7 @@ func (wh *WebHook) selfRegistration(webhookConfigName string) error {
},
FailurePolicy: &wh.failurePolicy,
NamespaceSelector: wh.selector,
TimeoutSeconds: wh.TimeoutSeconds,
}

mutatingWebhooks := []v1beta1.MutatingWebhook{mutatingWebhook}
Expand Down