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

Ensure webhook backend requests are not artificially rate-limited #85810

Merged
merged 1 commit into from Dec 2, 2019

Conversation

@liggitt
Copy link
Member

liggitt commented Dec 2, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
Fixes regression in webhook performance introduced in https://github.com/kubernetes/kubernetes/pull/84692/files#diff-063e73bab84834a4187b1ad4865050adR362-R375

Special notes for your reviewer:
Backend requests to webhooks should not artifically rate-limit themselves.

Does this PR introduce a user-facing change?:

Resolved regression in admission, authentication, and authorization webhook performance in v1.17.0-rc.1

/cc @smarterclayton

@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Dec 2, 2019

/priority critical-urgent

fixes a regression introduced in 1.17

@liggitt liggitt force-pushed the liggitt:disable-webhook-ratelimit branch from 72539e3 to d620493 Dec 2, 2019
@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Dec 2, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Dec 2, 2019

/retest

1 similar comment
@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Dec 2, 2019

/retest

@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Dec 2, 2019

flake #75355

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Dec 2, 2019

To clarify from slack - the bug was that UnversionedRESTClientFor was not consistent with RESTClientFor and didn't apply default rate limits. When that PR merged it made the two consistent, which broke people who depended on UnversionedRESTClientFor not having a rate limiter. This PR fixes those callers to properly say they do not want rate limiting.

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Dec 2, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 2, 2019
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, smarterclayton

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jpbetz

This comment has been minimized.

Copy link
Contributor

jpbetz commented Dec 2, 2019

Good find.

/lgtm

@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Dec 2, 2019

/test pull-kubernetes-e2e-gce

@k8s-ci-robot k8s-ci-robot merged commit 19ee1ea into kubernetes:master Dec 2, 2019
16 checks passed
16 checks passed
cla/linuxfoundation liggitt authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-kind-ipv6 Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kind Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot modified the milestones: v1.17, v1.18 Dec 2, 2019
k8s-ci-robot added a commit that referenced this pull request Dec 3, 2019
…0-upstream-release-1.17

Automated cherry pick of #85810: Ensure webhook backend requests are not artificially
@liggitt liggitt deleted the liggitt:disable-webhook-ratelimit branch Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.