Skip to content

Conversation

draveness
Copy link
Contributor

@draveness draveness commented Sep 13, 2019

/kind feature
/sig scheduling
/priority important-soon
/assign @k82cn

What this PR does / why we need it:

Graduate TaintNodesByCondition to GA

Which issue(s) this PR fixes:

Fixes #82635

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Graduate TaintNodesByCondition to GA in 1.17. (feature gate will be removed in 1.18) action required

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 13, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/apiserver area/kubelet area/test sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 15, 2019
@draveness draveness force-pushed the feature/graduate-taint-nodes-by-condition-to-ga branch 2 times, most recently from 32e67c2 to e82e01f Compare September 16, 2019 15:31
@draveness
Copy link
Contributor Author

The node e2e integration tests are failing since we are removing the TaintNodeByCondition feature gates from the codebase.

https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/82703/pull-kubernetes-node-e2e/1173534864813592576/

I scanned through the PR history and found out TaintNodeByCondition was disabled on purpose in node e2e test. #73097 (comment)

cc/ @bsalamat @liggitt @yujuhong, do you have any idea on how to fix this?

@liggitt
Copy link
Member

liggitt commented Sep 16, 2019

If the node e2e doesn't want to run the node controller (which handles resolving this taint if I remember correctly), the node e2e will need to mimic that behavior in its setup.

In general, running partial environments in e2e tests is not recommended, because of the duplication/drift inherent in running a system different than a "real" cluster.

@roycaihw
Copy link
Member

/remove-sig api-machinery
/sig cloud-provider

@k8s-ci-robot k8s-ci-robot added sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Sep 16, 2019
@draveness
Copy link
Contributor Author

/assign @deads2k @wojtek-t

@deads2k
Copy link
Contributor

deads2k commented Oct 17, 2019

/approve

controller, admission, and kubeapiserver changes all look fine.

@ahg-g
Copy link
Member

ahg-g commented Oct 17, 2019

/approve

@draveness
Copy link
Contributor Author

/assign @tallclair
for kubelet approval

@wojtek-t
Copy link
Member

/approve

/hold
Please rebase and hold cancel.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 18, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, deads2k, draveness, wojtek-t

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 18, 2019
@draveness draveness force-pushed the feature/graduate-taint-nodes-by-condition-to-ga branch from 89b423b to f729058 Compare October 18, 2019 09:45
@ahg-g
Copy link
Member

ahg-g commented Oct 18, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 18, 2019
@draveness
Copy link
Contributor Author

draveness commented Oct 18, 2019 via email

@draveness draveness force-pushed the feature/graduate-taint-nodes-by-condition-to-ga branch from f729058 to 823183a Compare October 18, 2019 17:41
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 18, 2019
@draveness draveness force-pushed the feature/graduate-taint-nodes-by-condition-to-ga branch from 823183a to 1163a1d Compare October 19, 2019 01:17
@draveness
Copy link
Contributor Author

/retest

@ahg-g
Copy link
Member

ahg-g commented Oct 19, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 19, 2019
@ahg-g
Copy link
Member

ahg-g commented Oct 19, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2019
@k8s-ci-robot k8s-ci-robot merged commit aab740f into kubernetes:master Oct 19, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Oct 19, 2019
@draveness draveness deleted the feature/graduate-taint-nodes-by-condition-to-ga branch October 19, 2019 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Graduate TaintNodesByCondition to GA
10 participants