Add flag for changing rps limit in istioctl bug-report #40725
Conversation
|
😊 Welcome @morepork! This is either your first contribution to the Istio istio repo, or it's been You can learn more about the Istio working groups, code of conduct, and contributing guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
istio-testing
added
size/L
|
Hi @morepork. Thanks for your PR. I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
| @@ -472,29 +471,6 @@ func appendGlobalErr(err error) { | |||
| lock.Unlock() | |||
| } | |||
|
|
|||
| func BuildClientsFromConfig(kubeConfig []byte) (kube.Client, error) { | |||
There was a problem hiding this comment.
this was unused before my changes
| @@ -34,31 +34,50 @@ import ( | |||
|
|
|||
| const ( | |||
| // maxRequestsPerSecond is the max rate of requests to the API server. | |||
| maxRequestsPerSecond = 10 | |||
| // maxLogFetchConcurrency is the max number of logs to fetch simultaneously. | |||
| maxLogFetchConcurrency = 10 | |||
There was a problem hiding this comment.
I didn't see a lot of value of keeping both these constants as they're the same, and didn't want to add 2 flags for UX reasons, using the rps limit for log fetch concurrency seems like a reasonable choice
|
/ok-to-test |
istio-testing
added
ok-to-test
|
/retest |
istio-testing
added
the
needs-rebase
morepork
force-pushed
the
bug-report-rps-limit
branch
from
93278c8 to
79b2767
Compare
istio-testing
removed
the
needs-rebase
|
friendly ping |
|
@therealmitchconnors @howardjohn @ericvn @istio/wg-test-and-release-maintainers mind taking a look? |
Add a flag to change the requests per second limit when running
istioctl bug-report.For my test cluster with ~600 pods and the default request limit
of 10, it takes ~5 minutes to collect a bug report. With a 50 rps
limit it takes ~1 minute, and with 100 rps ~35s.
This has become a blocker for collecting bug reports from very
large clusters with thousands of pods.