-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat: Support disabling Kubernetes client side rate limiting #336
feat: Support disabling Kubernetes client side rate limiting #336
Conversation
b5be08f
to
fd13001
Compare
main.go
Outdated
@@ -75,7 +78,16 @@ func main() { | |||
|
|||
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) | |||
|
|||
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ | |||
config = ctrl.GetConfigOrDie() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the test failed at this line of code: https://github.com/cert-manager/aws-privateca-issuer/actions/runs/10475862575/job/29013543290
3088af5
to
820c12e
Compare
Hi, we're currently in the process of testing this PR to make sure everything works properly! We're currently not seeing any improvements by enabling this so looking into why that may be. |
820c12e
to
0d070a4
Compare
0d070a4
to
ec91cdd
Compare
Signed-off-by: Alex Richman <[email protected]>
ec91cdd
to
2f22d86
Compare
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: 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 |
This should theoretically resolve #334.
Note that I haven't had the chance to test this at all - I believe it works (and I based a good bit of my work on the changes in cert-manager/istio-csr#352 which does the same thing for
istio-csr
), but I haven't actually proven that out end-to-end by adding unit and integration tests because I don't really have a deep understanding of the test suite here.This PR includes:
disableClientSideRateLimiting
config.QPS
andconfig.Burst
for the Kubernetes REST client