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

kubectl: discovery is throttled when there are lots of resources (CRDs) #105489

Closed
lavalamp opened this issue Oct 5, 2021 · 5 comments
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@lavalamp
Copy link
Member

lavalamp commented Oct 5, 2021

The default client side rate limit is 50qps with a burst of 100.

Kubectl does a discovery pass before doing the operation requested. Discovery involves (some combination of) reading the OpenAPI spec and the "homegrown" discovery information for the purpose of e.g. mapping kinds to resources. Sometimes these can be cached, but even if it is cached, an API call can be required to verify it hasn't changed.

This means that if there are more than 100 group versions, we will hit the client side rate limit in the process of discovery, possibly significantly slowing down the process.

AFAIK there is no way to adjust this limit when using kubectl.

A user report can be found here: #101634 (comment)

An easy way to fix this is to disable the client side limit in kubectl. Possibly caching parameters could be adjusted, too. A difficult fix would be to load the minimum amount of discovery documents necessary to perform the task at hand (but some tasks require everything).

/sig cli

@lavalamp lavalamp added the kind/bug Categorizes issue or PR as related to a bug. label Oct 5, 2021
@k8s-ci-robot k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 5, 2021
@seans3
Copy link
Contributor

seans3 commented Oct 5, 2021

/triage accepted
/assign

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 5, 2021
@justinsb
Copy link
Member

justinsb commented Oct 5, 2021

Two contributory things:

  • I think there's a bug where kubectl get clusterrolebindings.rbac.authorization.k8s.io forces a cache invalidation and full rediscovery, but ``kubectl get clusterrolebindings` does not. The gotcha is that tab completion uses the full name, and of course you're more likely to use tab completion when you have lots of CRDs, making discovery more expensive.

  • In general, we shouldn't have to do a full discovery to map GVK -> GVR. In the vast majority of cases I think we should be able to just do type discovery in the group and then find the K -> R. (@lavalamp shared something about Openshift clusters having the G in GVK not equal to the G in GVR, but I would hope we could address that with a fallback). This would be great for controllers, but I don't think it helps for kubectl where we often need shortnames from the apiserver (and in general, kubectl is going to use more of the surface than a controller which only needs GVK -> GVR mapping, so a full discovery is more justified ... ideally we wouldn't invalidate it every time the user typed <tab> though!)

@seans3
Copy link
Contributor

seans3 commented Oct 5, 2021

Moving to kubectl repo: kubernetes/kubectl#1126

/close

@k8s-ci-robot
Copy link
Contributor

@seans3: Closing this issue.

In response to this:

Moving to kubectl repo: kubernetes/kubectl#1126

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@seans3 seans3 removed their assignment Oct 5, 2021
@cannonpalms
Copy link

In general, we shouldn't have to do a full discovery to map GVK -> GVR. In the vast majority of cases I think we should be able to just do type discovery in the group and then find the K -> R. (@lavalamp shared something about Openshift clusters having the G in GVK not equal to the G in GVR, but I would hope we could address that with a fallback). This would be great for controllers, but I don't think it helps for kubectl where we often need shortnames from the apiserver (and in general, kubectl is going to use more of the surface than a controller which only needs GVK -> GVR mapping, so a full discovery is more justified ... ideally we wouldn't invalidate it every time the user typed though!)

@justinsb Is there another issue describing this work for controllers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

5 participants