updating sysctls for 3.11 #10361
updating sysctls for 3.11 #10361
Conversation
openshift-ci-robot
added
the
size/M
| - name: net.ipv4.route.min_pmtu | ||
| value: "552" | ||
| - name: kernel.msgmax | ||
| value: "65536" |
There was a problem hiding this comment.
Why the sysctl values have been changed? I'm not against such a change but I'd like to ensure that there is no mistakes.
There was a problem hiding this comment.
@php-coder, that's what the K8 doc used:
securityContext:
sysctls:
- name: kernel.shm_rmid_forced
value: "0"
- name: net.ipv4.route.min_pmtu
value: "552"
- name: kernel.msgmax
value: "65536"
There was a problem hiding this comment.
Ok; perhaps our example with kernel.msgmax=1 2 3 was wrong.
There was a problem hiding this comment.
| For {product-title} 3.3.1, the following sysctls are supported (whitelisted) in | ||
| the safe set: | ||
| Currently, {product-title} supports, or whitelists, the following sysctls | ||
| in the safe set: | ||
|
|
||
| - *_kernel.shm_rmid_forced_* | ||
| - *_net.ipv4.ip_local_port_range_* |
There was a problem hiding this comment.
net.ipv4.tcp_syncookies is in the safe set as well
There was a problem hiding this comment.
@ingvagabund, how long has net.ipv4.tcp_syncookies been in the safe set? (I can open up a separate PR to fix older versions, if applicable.)
There was a problem hiding this comment.
Since k8s 1.4: kubernetes/kubernetes#27180
There was a problem hiding this comment.
There was a problem hiding this comment.
Here's a PR to make that change in older versions: #11232
kalexand-rh
added
the
peer-review-needed
|
@stuartchuan, will you PTAL? @openshift/team-documentation PTAL 🌠 |
admin_guide/sysctls.adoc
Outdated
| With the warning above in mind, the cluster administrator can allow certain | ||
| unsafe sysctls for very special situations, e.g., high-performance or real-time | ||
| With the previous warning in mind, the cluster administrator can allow certain | ||
| unsafe sysctls for very special situations such as high-performance or real-time |
There was a problem hiding this comment.
@kalexand-rh Should this be more explicit to point to the previous warning?
Enabling unsafe sysctls
The use of unsafe sysctls is at-your-own-risk. See the xref:linktext[previous warning].
The cluster administrator can allow...
There was a problem hiding this comment.
You're right that it could be clearer, but I don't want to add an xref that I'll have to remove next month. I might move that paragraph into the previous section - if these were modules, that detail would belong in the concept instead of the task.
There was a problem hiding this comment.
@kalexand-rh I would think you want the warning in both the concept and the task. But, that is a debate for later...
admin_guide/sysctls.adoc
Outdated
|
|
||
| Sysctls are set on pods using annotations. They apply to all containers in the | ||
| same pod. | ||
| Sysctls are set on pods using the pod's securityContext. The securityContext |
There was a problem hiding this comment.
Other references to securityContext take markup, securityContext or *securityContext*
There was a problem hiding this comment.
I think it's a parameter, so securityContext is correct.
| @@ -135,31 +136,49 @@ ifdef::openshift-origin[] | |||
| endif::[] | |||
|
|
|||
| [[setting-sysctls-for-a-pod]] | |||
| == Setting Sysctls for a Pod | |||
| == Setting sysctls for a pod | |||
There was a problem hiding this comment.
@kalexand-rh This heading appears to be formatted incorrectly on the View page here. But, I think it is OK; the formatting looks off in all points in the repo history since the ifdefs were added right before the heading.
There was a problem hiding this comment.
Interesting. It looks ok in a local build, too. Thanks for checking it.
admin_guide/sysctls.adoc
Outdated
| ... | ||
| ---- | ||
|
|
||
| [NOTE] | ||
| ==== | ||
| A pod with the unsafe sysctls specified above will fail to launch on any node | ||
| that has not enabled those two unsafe sysctls explicitly. As with node-level | ||
| that has not explicitly enabled those two unsafe sysctls. As with node-level |
There was a problem hiding this comment.
s/that has not explicitly enabled//that the admin has not explicitly enabled/which were not explicitly enabled/ ??
Currently, it sounds like the node enables the sysctls itself.
|
@kalexand-rh A few comments. Otherwise LGTM |
openshift-bot
added
the
needs-rebase
openshift-ci-robot
added
size/L
openshift-bot
removed
the
needs-rebase
|
The preview will be availble shortly at:
|
kalexand-rh
added
peer-review-done
|
@mdshuai PTAL |
|
Checked and LGTM |
|
Thank you @wjiangjay! |
|
/cherrypick enterprise-3.11 |
|
@kalexand-rh: new pull request created: #11384 In response to this:
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. |
Fixes #10225.
@php-coder, @ingvagabund, PTAL