Increase the default webhook timeout to its maximum value of 30 seconds

[wallrj]

@maelvls I'm sure that I saw you suggest this change on Slack or in a GitHub issue, but I can't find the reference now.
Was it you and is this what you meant?

Users sometimes report that the connection between the K8S API server and the cert-manager webhook server times out.

But the error message is often only "context deadline exceeded", which doesn't help the user know what phase of the HTTPS connection timed out.

It could be during DNS resolution, TCP connection, TLS negotiation, HTTP channel negotiation, or slow HTTP response from the webhook server.

So this change increases the context timeout to its maximum value so that the underlying timeout error message has more chance of being returned to the end user.

Increase the default webhook timeout to its maximum value of 30 seconds, so that the underlying timeout error message has more chance of being returned to the end user.

Testing

$ make helm-chart
$ helm template _bin/cert-manager-v1.13.0-beta.0-118-ga0e5afc0f45cab.tgz | grep -B 40 'timeoutSeconds: 30'
            readOnlyRootFilesystem: true
          env:
          - name: POD_NAMESPACE
            valueFrom:
              fieldRef:
                fieldPath: metadata.namespace
      nodeSelector:
        kubernetes.io/os: linux
---
# Source: cert-manager/templates/webhook-mutating-webhook.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
  name: release-name-cert-manager-webhook
  labels:
    app: webhook
    app.kubernetes.io/name: webhook
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/component: "webhook"
    app.kubernetes.io/version: "v1.13.0-beta.0-118-ga0e5afc0f45cab"
    app.kubernetes.io/managed-by: Helm
    helm.sh/chart: cert-manager-v1.13.0-beta.0-118-ga0e5afc0f45cab
  annotations:
    cert-manager.io/inject-ca-from-secret: "default/release-name-cert-manager-webhook-ca"
webhooks:
  - name: webhook.cert-manager.io
    rules:
      - apiGroups:
          - "cert-manager.io"
        apiVersions:
          - "v1"
        operations:
          - CREATE
        resources:
          - "certificaterequests"
    admissionReviewVersions: ["v1"]
    # This webhook only accepts v1 cert-manager resources.
    # Equivalent matchPolicy ensures that non-v1 resource requests are sent to
    # this webhook (after the resources have been converted to v1).
    matchPolicy: Equivalent
    timeoutSeconds: 30
--
---
# Source: cert-manager/templates/webhook-validating-webhook.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  name: release-name-cert-manager-webhook
  labels:
    app: webhook
    app.kubernetes.io/name: webhook
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/component: "webhook"
    app.kubernetes.io/version: "v1.13.0-beta.0-118-ga0e5afc0f45cab"
    app.kubernetes.io/managed-by: Helm
    helm.sh/chart: cert-manager-v1.13.0-beta.0-118-ga0e5afc0f45cab
  annotations:
    cert-manager.io/inject-ca-from-secret: "default/release-name-cert-manager-webhook-ca"
webhooks:
  - name: webhook.cert-manager.io
    namespaceSelector:
      matchExpressions:
      - key: "cert-manager.io/disable-validation"
        operator: "NotIn"
        values:
        - "true"
    rules:
      - apiGroups:
          - "cert-manager.io"
          - "acme.cert-manager.io"
        apiVersions:
          - "v1"
        operations:
          - CREATE
          - UPDATE
        resources:
          - "*/*"
    admissionReviewVersions: ["v1"]
    # This webhook only accepts v1 cert-manager resources.
    # Equivalent matchPolicy ensures that non-v1 resource requests are sent to
    # this webhook (after the resources have been converted to v1).
    matchPolicy: Equivalent
    timeoutSeconds: 30

[inteon]

@wallrj There is a reason the timeout was set to 10s (might not be valid): 5aa8726

[wallrj]

@wallrj There is a reason the timeout was set to 10s (might not be valid): 5aa8726

Here's the reason from that commit:

Because webhooks add to API request latency, they should evaluate as
quickly as possible. timeoutSeconds allows configuring how long the
API server should wait for a webhook to respond before treating the call
as a failure.

The default values from Kubernetes are, however, too large. For
admissionregistration.k8s.io/v1 the default value is 10 seconds while
for admissionregistration.k8s.io/v1beta1 is 30 seconds!

This default value of 10 seconds should be more than enough for standard
installs.

I don't think that makes sense. Lowering the timeout doesn't lower the latency.
It may reduce the time before failure, but the webhook is configured to fail shut,
so it's not as if the lower timeout will fix any problems.

What do you think?

[maelvls]

Was it you and is this what you meant?

Yes, that's exactly what I meant. I forgot to make the change in the website, thank you for doing so! I had kind of given up on finding out why 30 seconds would not affect production installations of cert-manager. Thanks to you, I am now convinced that it won't affect anyone since this "latency" thing was just some fear and doubt due to a misunderstanding of why HTTP client timeouts exist.

There is a reason the timeout was set to 10s (might not be valid): 5aa8726

This timeout is meant to prevent an exhaustion of file descriptors available ports due to lots of TCP connections stacking up on the kube-apiserver's side. That's the only purpose of HTTP client timeouts: prevent the possibility of lots of open connections clogging the apiserver in a DDOS style.

I agree with @wallrj: I don't see how lowering the client timeout from 10 seconds to 30 seconds can lower the webhook's response latency; it doesn't really make sense to think that even a 10 seconds timeout will help in achieving that. At best, lowering from 30 seconds to 10 seconds will lower the chance to have one of the webhooks (e.g., kyverno or cert-manager-webhook) to make the kube-apiserver fail due to a shortage of available ports.

[maelvls]

/lgtm

[maelvls]

/approve

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls

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:

• OWNERS [maelvls]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wallrj]

For anyone encountering the "context deadline exceeded" error from the webhook, please do the following:

• Increase the ValidatingWebhookConfiguration and MutatingWebhookConfiguration timeout to its maximum value of 30 seconds, so that the underlying cause of the error is revealed. In cert-manager 1.14 this will become the default value. See Increase the default webhook timeout to its maximum value of 30 seconds #6488
• Read https://cert-manager.io/docs/troubleshooting/webhook/#error-context-deadline-exceeded
• Read https://cert-manager.io/docs/installation/best-practice/#network-requirements-and-network-policy

[wallrj]

/kind bug