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.

Sign up for GitHub

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

Jump to bottom

Warn about CPU limits in teleport-cluster Helm chart #36251

Merged
merged 2 commits into from Jan 4, 2024
Merged

Warn about CPU limits in teleport-cluster Helm chart #36251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eda9681
Warn about CPU limits
hugoShaka Jan 3, 2024
a2f3b6b
fixup! Warn about CPU limits
hugoShaka Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/pages/reference/helm-reference/teleport-cluster.mdx
View file
Open in desktop
Expand Up @@ -1822,6 +1822,15 @@ A `postStart` lifecycle handler to be configured on the main Teleport container.
Resource requests/limits which should be configured for Teleport containers. These resource limits will also be
applied to `initContainers`.

<Admonition type="danger">
Setting CPU limits is an anti-pattern and is harmful in most cases. Unless you enabled
[the Static CPU management policy](https://kubernetes.io/docs/tasks/administer-cluster/cpu-management-policies/#static-policy),
a multithreaded workload with CPU limits will very likely not behave the way you expect when approaching its CPU limit.

Teleport will become unstable once throttling starts. We recommend not to set CPU limits.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a paragraph about the implications of such actions?
Since people don't seem to know how it works, it's probably good to give them an idea that CPU limits control the CPU time of the process and not the actual CPU cores reserved. This leads to huge latencies because Teleport will quickly consume its quota and will not be scheduled on any cores for long periods of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a link to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

From prev experience, no one will read it.

See [the GitHub PR](https://github.com/gravitational/teleport/pull/36251) for technical details.
</Admonition>

`values.yaml` example:

```yaml
Expand Down
10 changes: 10 additions & 0 deletions examples/chart/teleport-cluster/values.yaml
View file
Open in desktop
Expand Up @@ -658,10 +658,20 @@ postStart:

# Resources to request for the teleport container
# https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
#
# DANGER: Setting CPU limits is an anti-pattern and harmful in most cases.
# Unless you enabled [the Static CPU management policy](https://kubernetes.io/docs/tasks/administer-cluster/cpu-management-policies/#static-policy),
# a multithreaded workload with CPU limits will very likely not behave the way
# you expect when approaching its CPU limit.
#
# Teleport will become unstable once throttling starts. We recommend not to set CPU limits.
# See [the GitHub PR](https://github.com/gravitational/teleport/pull/36251) for technical details.
resources: {}
# requests:
# cpu: "1"
# memory: "2Gi"
# limits:
# memory: "2Gi"

# Security context to add to the container
securityContext: {}
Expand Down