pid limiting documentation #23929

SergeyKanzhelev · 2020-09-16T20:52:31Z

Quickly combined info about pid limiting. It is a follow up from kubernetes/kubernetes#94140 (comment)

It also requires this PR #23883 as it adds the description of the feature flag that this PR refer to.

Related #23906 (issue can be closed once this branch is merged back to master)

/sig node
/assign @sftim
/assign @derekwaynecarr

k8sio-netlify-preview-bot · 2020-09-16T20:52:37Z

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 7b7ed6b

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5f63963c9331a6000839477b

sftim · 2020-09-16T22:20:21Z

node-to-pod PID isolation

What does this mean? I think it's about imposing a resource limit on Pods, where the resource is the number of host system PIDs that the Pod can use.

PIDs are always isolated between Pods. I don't think it's possible for two Pods in the same namespace hierarchy (ie, both running on one Node with a Linux namespaces-and-cgroups container runtime) to have the same host system PID, that just doesn't make sense.

sftim

Hi @SergeyKanzhelev

Thanks for this PR - here are some suggestions on the wording.

content/en/docs/concepts/policy/pid-limiting.md

sftim · 2020-09-16T22:40:50Z

content/en/docs/concepts/policy/pid-limiting.md

+- derekwaynecarr
+title: PID Limiting
+content_type: concept
+weight: 10


I recommend setting a higher value, as there are other documents in the policy section that should come first.

I should have read contributing.md. Just copied from other file. Let me take a look

this is the doc I found: https://v1-16.docs.kubernetes.io/docs/contribute/style/content-organization/#page-order I updated weight for more articles. Was it the right decision? Or just assign 40 to the new page?

content/en/docs/concepts/configuration/manage-resources-containers.md

content/en/docs/concepts/policy/pid-limiting.md

SergeyKanzhelev · 2020-09-16T22:50:02Z

node-to-pod PID isolation

What does this mean? I think it's about imposing a resource limit on Pods, where the resource is the number of host system PIDs that the Pod can use.

PIDs are always isolated between Pods. I don't think it's possible for two Pods in the same namespace hierarchy (ie, both running on one Node with a Linux namespaces-and-cgroups container runtime) to have the same host system PID, that just doesn't make sense.

This is isolation form attack perspective. I agree it worth re-phrasing. Looking...

SergeyKanzhelev · 2020-09-16T23:35:58Z

@sftim thank you very much for review and rewrites!

content/en/docs/concepts/policy/pid-limiting.md

SergeyKanzhelev · 2020-09-17T05:54:09Z

@tengqm thank you for pointing all uncertainties in a language. PTAL - I rephrased

tengqm

LGTM other than some nits.

content/en/docs/concepts/policy/pid-limiting.md

tengqm · 2020-09-17T08:12:17Z

Preview: https://deploy-preview-23929--kubernetes-io-vnext-staging.netlify.app/docs/concepts/policy/pid-limiting/

annajung · 2020-09-17T12:29:13Z

@kubernetes/sig-node-pr-reviews Hello 👋, 1.20 docs lead here. Can we get a tech review?

sftim

A bit more feedback. IMO it'd be fine to merge without addressing that, we can pick those nits up later.

content/en/docs/concepts/policy/pid-limiting.md

kbhawkey · 2020-09-17T16:55:17Z

content/en/docs/concepts/policy/pid-limiting.md

+
+{{< feature-state for_k8s_version="v1.20" state="stable" >}}
+
+Kubernetes allow you to limit the number of process IDs (PIDs) that a {{< glossary_tooltip term_id="Pod" text="Pod" >}} can use.


What do think about adding a heading for the content starting at line 13?
The first heading I see is Node PID limits.

I just followed the patter from other files. I think the title will become an H1

Looks nice when rendered:

kbhawkey · 2020-09-17T16:57:35Z

content/en/docs/concepts/policy/pid-limiting.md

+
+{{< note >}}
+On certain Linux installations, the operating system sets the PIDs limit to a low default,
+such as `32768`. Consider raising the value of `/proc/sys/kernel/pid_max`.


This is referring to something like:
In order to use the pids controller, set the maximum number of tasks in pids.max (this is not available in the root cgroup for obvious reasons). The number of processes currently in the cgroup is given by pids.current. ?

It's this tip:
https://www.cyberciti.biz/tips/howto-linux-increase-pid-limits.html

Nothing to do with Kubernetes per se, but relevant if like most people you're using Linux nodes.

is it something you want to add to the note? Or replace with?

kbhawkey · 2020-09-17T16:59:47Z

content/en/docs/concepts/policy/pid-limiting.md

+You can configure a kubelet to limit the number of PIDs a given pod can consume.
+For example, if your node's host OS is set to use a maximum of `262144` PIDs and
+expect to host less than `250` pods, one can give each pod a budget of `1000`
+PIDs to prevent using up that node's overall number of available PIDs. If the


What do you think about this edit?
For example, if your node's host OS is set to use a maximum of 262144 PIDs and
you expect to host less than 250 pods, give each pod a budget of 1000
PIDs to prevent using up that node's overall number of available PIDs.

i think that is fine.

its an operator choice to choose to over-commit or under-commit pids.

the main goal with pid limiting per pod is to stop a simple fork-bomb as we don't require pods to requests pids as a resource in favor of letting an admin set a per pod max.

SergeyKanzhelev · 2020-09-17T17:44:21Z

Should there be a note that on windows these flags has no effect? I'm not sure what's the best practice here

derekwaynecarr

thanks for doing this follow-up @SergeyKanzhelev !

the doc looks great.

/lgtm

derekwaynecarr · 2020-09-17T18:56:10Z

content/en/docs/concepts/policy/pid-limiting.md

+You can configure a kubelet to limit the number of PIDs a given pod can consume.
+For example, if your node's host OS is set to use a maximum of `262144` PIDs and
+expect to host less than `250` pods, one can give each pod a budget of `1000`
+PIDs to prevent using up that node's overall number of available PIDs. If the


i think that is fine.

its an operator choice to choose to over-commit or under-commit pids.

the main goal with pid limiting per pod is to stop a simple fork-bomb as we don't require pods to requests pids as a resource in favor of letting an admin set a per pod max.

derekwaynecarr · 2020-09-17T18:59:58Z

@SergeyKanzhelev its hard to keep up with the delta among linux and windows nodes, but we could put a piece of text that says pid limiting only applies to kubelets running on linux hosts.

tengqm · 2020-09-18T00:52:20Z

LGTM
Defer approval to @sftim to check if all the feedbacks addressed.

sftim · 2020-09-21T08:05:43Z

Relevant to #23906

SergeyKanzhelev · 2020-09-21T16:48:48Z

Relevant to #23906

I think it is fixes the issue, correct? I can add to description for auto-closing the issue on PR merge

sftim · 2020-09-21T23:09:16Z

I think it is fixes the issue, correct? I can add to description for auto-closing the issue on PR merge

This PR targets the v1.20 release. I would hold off on closing #23906 given that the master branch (and live documentation) will not include documentation for this (existing) feature until the v1.20 release. Once this merges, though, we can drop the priority of #23906.

sftim · 2020-09-30T23:50:43Z

@annajung happy for this to land in dev-1.20?

annajung · 2020-10-01T03:50:46Z

Yes, that sounds good to me
/milestone 1.20

k8s-ci-robot · 2020-10-01T03:50:47Z

@annajung: You must be a member of the kubernetes/website-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Website milestone maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

Yes, that sounds good to me
/milestone 1.20

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.

annajung · 2020-10-01T18:38:50Z

/milestone 1.20

annajung · 2020-10-02T23:10:28Z

/assign @annajung

sftim · 2020-10-07T22:10:12Z

Hi @SergeyKanzhelev

https://5f63963c9331a6000839477b--kubernetes-io-vnext-staging.netlify.app/docs/reference/command-line-tools-reference/feature-gates/ also needs updating, to mark the related feature gates as graduated.

If you change that, I think this will be good to go.
/lgtm cancel
(tech review stands)

zhouya0 · 2020-10-09T02:49:31Z

content/en/docs/concepts/policy/pid-limiting.md

+
+{{< note >}}
+Before Kubernetes version 1.20, PID resource limiting for Pods required enabling
+the [feature gate](/docs/reference/command-line-tools-reference/feature-gates/)


I guess it is opened by default since 1.14

SupportPodPidsLimit true Beta 1.14

irvifa · 2020-10-09T08:14:41Z

Looks good to me, thanks!
/lgtm
/approve

k8s-ci-robot · 2020-10-09T08:14:58Z

LGTM label has been added.

Git tree hash: 2fa56c7877086b8d0a56541e0b8ce078f9bab5ac

k8s-ci-robot · 2020-10-09T08:14:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irvifa

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:

~~content/en/OWNERS~~ [irvifa]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot assigned derekwaynecarr and sftim Sep 16, 2020

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 16, 2020

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 16, 2020

SergeyKanzhelev mentioned this pull request Sep 16, 2020

Promote PidLimits to GA kubernetes/kubernetes#94140

Merged

k8s-ci-robot requested review from kbhawkey and rajeshdeshpande02 September 16, 2020 20:53

k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Sep 16, 2020

sftim reviewed Sep 16, 2020

View reviewed changes

SergeyKanzhelev force-pushed the pidlimiting branch from 5a336e4 to 95631e3 Compare September 16, 2020 23:07

tengqm reviewed Sep 17, 2020

View reviewed changes

content/en/docs/concepts/policy/pid-limiting.md Outdated Show resolved Hide resolved

tengqm reviewed Sep 17, 2020

View reviewed changes

content/en/docs/concepts/policy/pid-limiting.md Outdated Show resolved Hide resolved

SergeyKanzhelev force-pushed the pidlimiting branch from 95631e3 to 94af350 Compare September 17, 2020 05:53

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 17, 2020

tengqm reviewed Sep 17, 2020

View reviewed changes

content/en/docs/concepts/policy/pid-limiting.md Outdated Show resolved Hide resolved

content/en/docs/concepts/policy/pid-limiting.md Outdated Show resolved Hide resolved

SergeyKanzhelev force-pushed the pidlimiting branch from 94af350 to 13699bc Compare September 17, 2020 06:27

sftim reviewed Sep 17, 2020

View reviewed changes

kbhawkey reviewed Sep 17, 2020

View reviewed changes

derekwaynecarr approved these changes Sep 17, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 17, 2020

kikisdeliveryservice mentioned this pull request Sep 23, 2020

Pid Limiting kubernetes/enhancements#757

Closed

k8s-ci-robot added this to the 1.20 milestone Oct 1, 2020

k8s-ci-robot assigned annajung Oct 2, 2020

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2020

zhouya0 reviewed Oct 9, 2020

View reviewed changes

k8s-ci-robot assigned irvifa Oct 9, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2020

k8s-ci-robot merged commit 245f8b6 into kubernetes:dev-1.20 Oct 9, 2020

mrueg mentioned this pull request Oct 29, 2020

Document the SupportPodPidsLimit feature #7386

Closed

2 tasks

sftim mentioned this pull request Nov 19, 2020

Document PID limiting #23906

Closed

SergeyKanzhelev deleted the pidlimiting branch November 21, 2020 00:10

s-kawamura-w664 mentioned this pull request Mar 9, 2023

[ja] Update page weights under 5 sections of concepts #39869

Merged


		{{< feature-state for_k8s_version="v1.20" state="stable" >}}

		Kubernetes allow you to limit the number of process IDs (PIDs) that a {{< glossary_tooltip term_id="Pod" text="Pod" >}} can use.

pid limiting documentation #23929

pid limiting documentation #23929

Conversation

SergeyKanzhelev commented Sep 16, 2020 • edited Loading

k8sio-netlify-preview-bot commented Sep 16, 2020 • edited Loading

sftim commented Sep 16, 2020

sftim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKanzhelev commented Sep 16, 2020

SergeyKanzhelev commented Sep 16, 2020

SergeyKanzhelev commented Sep 17, 2020

tengqm left a comment

Choose a reason for hiding this comment

tengqm commented Sep 17, 2020

annajung commented Sep 17, 2020

sftim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sftim Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKanzhelev commented Sep 17, 2020

derekwaynecarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derekwaynecarr commented Sep 17, 2020

tengqm commented Sep 18, 2020

sftim commented Sep 21, 2020

SergeyKanzhelev commented Sep 21, 2020

sftim commented Sep 21, 2020

sftim commented Sep 30, 2020

annajung commented Oct 1, 2020

k8s-ci-robot commented Oct 1, 2020

annajung commented Oct 1, 2020

annajung commented Oct 2, 2020

sftim commented Oct 7, 2020

Choose a reason for hiding this comment

irvifa commented Oct 9, 2020

k8s-ci-robot commented Oct 9, 2020

k8s-ci-robot commented Oct 9, 2020

SergeyKanzhelev commented Sep 16, 2020 •

edited

Loading

k8sio-netlify-preview-bot commented Sep 16, 2020 •

edited

Loading

sftim Sep 17, 2020 •

edited

Loading