-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Updated node controller for taints and tolerations. #2777
Conversation
50e2a9d
to
91a205e
Compare
docs/admin/node.md
Outdated
@@ -165,6 +165,11 @@ completely unhealthy (i.e. there are no healthy nodes in the cluster). In such | |||
case, the node controller assumes that there's some problem with master | |||
connectivity and stops all evictions until some connectivity is restored. | |||
|
|||
In Kubernetes 1.6, we introduced the logic for the node controller to handle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kubernetes 1.6 introduces logic...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But here https://github.com/kubernetes/kubernetes.github.io/blob/master/docs/admin/node.md#node-controller we did have similar sentence as
In Kubernetes 1.4, we updated the logic of the node controller
Shall we keep consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rewrite this as:
Starting in Kubernetes 1.6, the NodeController is also responsible for evicting pods that are running
on nodes with `NoExecute` taints, when the pods do not tolerate the taints. Additionally, as an alpha
feature that is disabled by default, the NodeController is responsible for adding taints
corresponding to node problems like node unreachable or not ready. See
[this documentation](/docs/user-guide/node-selection/index.md) for details about `NoExecute`
taints and the alpha feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks @davidopp
docs/admin/node.md
Outdated
@@ -165,6 +165,11 @@ completely unhealthy (i.e. there are no healthy nodes in the cluster). In such | |||
case, the node controller assumes that there's some problem with master | |||
connectivity and stops all evictions until some connectivity is restored. | |||
|
|||
In Kubernetes 1.6, we introduced the logic for the node controller to handle | |||
`taints` and `tolerance`. Basically, `taints` go on nodes while `tolerations` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider rephrasing this sentence.
e.g. (Taints are applied to nodes and tolerations are applied to pods.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/while/and
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "basically" acceptable language for upstream? You could probably remove it without affecting the meaning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I think basically should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
FYI: @mburke5678 |
@gmarek i reassigned to you since you are listed at the owner of pkg/controller/node. |
91a205e
to
c49836a
Compare
c49836a
to
ce87af7
Compare
/cc @kubernetes/sig-docs-maintainers |
LGTM |
Fixed part of #2737 , depend on #2774 , will update this PR once #2774 got merged.
This change is