updating sysctls for 3.11 #10361
updating sysctls for 3.11 #10361
Conversation
openshift-ci-robot
added
the
size/M
admin_guide/sysctls.adoc
Outdated
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.
admin_guide/sysctls.adoc
Outdated
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
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
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.
admin_guide/sysctls.adoc
Outdated
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
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. |
8 participants
Fixes #10225.
@php-coder, @ingvagabund, PTAL