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.

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

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

gyliu513
Copy link
Contributor

@gyliu513 gyliu513 commented Mar 12, 2017

Fixed part of #2737 , depend on #2774 , will update this PR once #2774 got merged.


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 12, 2017
@@ -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
Copy link
Contributor

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...

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks @davidopp

@@ -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`
Copy link
Contributor

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.)

Copy link
Member

Choose a reason for hiding this comment

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

s/while/and

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@aveshagarwal
Copy link
Member

FYI: @mburke5678

@caesarxuchao caesarxuchao assigned gmarek and unassigned caesarxuchao Mar 13, 2017
@caesarxuchao
Copy link
Member

@gmarek i reassigned to you since you are listed at the owner of pkg/controller/node.

@davidopp davidopp added this to the 1.6 milestone Mar 18, 2017
@davidopp davidopp changed the base branch from master to release-1.6 March 18, 2017 07:53
@gyliu513 gyliu513 changed the title WIP: Updated node controller for taints and tolerations. Updated node controller for taints and tolerations. Mar 18, 2017
@gyliu513
Copy link
Contributor Author

/cc @kubernetes/sig-docs-maintainers

@davidopp
Copy link
Member

LGTM

@chenopis chenopis merged commit adacb8d into kubernetes:release-1.6 Mar 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants