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

helm,docs: Add client rate limit helm values #26711

Merged

Conversation

dylandreimerink
Copy link
Member

@dylandreimerink dylandreimerink commented Jul 7, 2023

This PR adds the helm values for the client rate limit feature. This makes it easier for users to tune the client rate limit which is necessary for L2 announcements to work properly.

Added warnings about client rate limits and sizing instructions to the L2 announcements documentation.

This doesn't fully address #26586 since we are putting an additional burden on the user, but should provide a half decent workaround for the v1.14 release which gives us time to come up with a permanent more user friendly fix.

Add helm values for K8s API server client rate limits and instructions on how to size them when using L2 announcements.

@dylandreimerink dylandreimerink added kind/enhancement This would improve or streamline existing functionality. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. feature/l2-announcement needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 7, 2023
@dylandreimerink dylandreimerink requested review from a team as code owners July 7, 2023 14:21
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 7, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.0 Jul 7, 2023
@dylandreimerink dylandreimerink added the release-note/misc This PR makes changes that have no direct user impact. label Jul 7, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 7, 2023
@dylandreimerink dylandreimerink force-pushed the feature/k8s-rate-limit-helm-values branch 5 times, most recently from fda3312 to 4ee84f5 Compare July 7, 2023 14:42
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm changes LGTM

@dylandreimerink dylandreimerink force-pushed the feature/k8s-rate-limit-helm-values branch 2 times, most recently from 17c4230 to 6c97764 Compare July 7, 2023 15:51
Copy link
Contributor

@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs all good

install/kubernetes/cilium/values.yaml.tmpl Outdated Show resolved Hide resolved
@dylandreimerink dylandreimerink force-pushed the feature/k8s-rate-limit-helm-values branch 2 times, most recently from 9664a61 to 89ecec8 Compare July 10, 2023 10:21
This commit adds the helm values for the client rate limit feature.
This makes it easier for users to tune the client rate limit which is
necessary for L2 announcements to work properly.

Added warnings about client rate limits and sizing instructions to the
L2 announcements documentation.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
@dylandreimerink dylandreimerink force-pushed the feature/k8s-rate-limit-helm-values branch from 89ecec8 to 8243c5f Compare July 10, 2023 11:45
@dylandreimerink
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 10, 2023
@julianwiedmann julianwiedmann merged commit e2f475d into cilium:main Jul 11, 2023
65 checks passed
@jibi jibi mentioned this pull request Jul 13, 2023
13 tasks
@jibi jibi added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 13, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.0 Jul 13, 2023
@aanm aanm added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jul 14, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.0 Jul 14, 2023
Comment on lines +3194 to +3205

# -- Configure the client side rate limit for the agent and operator
#
# If the amount of requests to the Kubernetes API server exceeds the configured
# rate limit, the agent and operator will start to throttle requests by delaying
# them until there is budget or the request times out.
k8sClientRateLimit:
# -- The sustained request rate in requests per second.
qps: 5
# -- The burst request rate in requests per second.
# The rate limiter will allow short bursts with a higher rate.
burst: 10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this block the same as above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/l2-announcement kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.14.0
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

8 participants