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.

Sign up for GitHub

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

Jump to bottom

NODE_TAINTS in gce startup scripts #47632

Merged
merged 1 commit into from
Jun 21, 2017
Merged

NODE_TAINTS in gce startup scripts #47632

merged 1 commit into from
Jun 21, 2017

Conversation

mwielgus
Copy link
Contributor

Currently there is now way to pass a list of taints that should be added on node registration (at least not in gce or other saltbased deployment). This PR adds necessary plumbing to pass the taints from user or instance group template to kubelet startup flags.

Taints support in gce/salt startup scripts.

The PR was manually tested.

NODE_TAINTS: 'dedicated=ml:NoSchedule'

in kube-env results in

spec:
[...]
  taints:
  - effect: NoSchedule
    key: dedicated
    timeAdded: null
    value: ml

cc: @davidopp @gmarek @dchen1107 @MaciekPytel

@mwielgus mwielgus added area/kubelet release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Jun 16, 2017
@mwielgus mwielgus added this to the v1.7 milestone Jun 16, 2017
@mwielgus mwielgus requested review from gmarek and davidopp June 16, 2017 05:36
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 16, 2017
@k8s-github-robot k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 16, 2017
@dchen1107
Copy link
Member

Is this required for 1.7 release or not? There is no issue associating with this pr, and no review.

@mwielgus
Copy link
Contributor Author

mwielgus commented Jun 19, 2017 •
edited
Loading

This small fix is needed to have a reasonable support for taints in node pools in GKE. It has an issue in our internal bug tracking system ... since November.

@gmarek did an initial review today. I need to run one more check and we should be good to go.

@gmarek
Copy link
Contributor

gmarek commented Jun 20, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2017
@gmarek
Copy link
Contributor

gmarek commented Jun 20, 2017

/approve no-issue

@mikedanese
Copy link
Member

Do you need to do this with GCI as well?

@mwielgus
Copy link
Contributor Author

@mikedanese I modified both for consistency.

@mikedanese
Copy link
Member

/approve

@mikedanese
Copy link
Member

/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gmarek, mikedanese, mwielgus

Associated issue requirement bypassed by: mikedanese

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2017
@mwielgus
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-etcd3

@mwielgus
Copy link
Contributor Author

/retest

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 45268, 47573, 47632, 47818)

k8s-github-robot pushed a commit that referenced this pull request Jun 21, 2017
Merge pull request #47632 from mwielgus/node-taints-scripts
26c431a 
Automatic merge from submit-queue (batch tested with PRs 45268, 47573, 47632, 47818)

NODE_TAINTS in gce startup scripts

Currently there is now way to pass a list of taints that should be added on node registration (at least not in gce or other saltbased deployment). This PR adds necessary plumbing to pass the taints from user or instance group template to kubelet startup flags. 

```release-note
Taints support in gce/salt startup scripts. 
```

The PR was manually tested. 
```
NODE_TAINTS: 'dedicated=ml:NoSchedule'
```
in kube-env results in 
```
spec:
[...]
  taints:
  - effect: NoSchedule
    key: dedicated
    timeAdded: null
    value: ml
```

cc: @davidopp @gmarek @dchen1107 @MaciekPytel
@k8s-github-robot k8s-github-robot merged commit 26c431a into kubernetes:master Jun 21, 2017
@MaciekPytel MaciekPytel mentioned this pull request Jul 5, 2017
Closed
@MaciekPytel MaciekPytel mentioned this pull request Jul 11, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmarek gmarek Awaiting requested review from gmarek

@davidopp davidopp Awaiting requested review from davidopp

Assignees

@gmarek gmarek

@davidopp davidopp

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
7 participants
@mwielgus @dchen1107 @gmarek @mikedanese @k8s-github-robot @davidopp @k8s-ci-robot