Cherry pick #96876 in controller to 1.20: fix nodelifecyle controller not add NoExecute taint bug #98168

CKchen0726 · 2021-01-19T08:24:19Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
this PR #89059 try to fix reconcile problem, so every 5s monitorNodeHealth() run processTaintBaseEviction(), add nodes to zoneNoExecuteTainter cause these nodes' status is Unknown or False.
Ref: #96876

Which issue(s) this PR fixes:
Fix: #94183 #96183

Special notes for your reviewer:
I write a helper func to print values inside RateLimitedTimedQueue, and unit test running log as below, and log display the dirty data inside queue field.

Does this PR introduce a user-facing change?

fixing a regression in 1.19+ where a failed node may not have the NoExecute taint set correctly

…issing fix nodelifecyle controller not add NoExecute taint bug

k8s-ci-robot · 2021-01-19T08:24:28Z

Hi @CKchen0726. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

JornShen · 2021-01-19T08:56:45Z

/ok-to-test

JornShen · 2021-01-20T01:34:59Z

/cc @k82cn @Huang-Wei

JornShen · 2021-01-20T01:35:57Z

/cc @derekwaynecarr

adampl · 2021-02-05T17:09:16Z

@aveshagarwal @smarterclayton @Huang-Wei @k82cn @derekwaynecarr please approve, it's a critical issue.

Huang-Wei · 2021-02-05T22:46:40Z

/lgtm
/approve

k8s-ci-robot · 2021-02-22T23:32:37Z

@Huang-Wei: GitHub didn't allow me to assign the following users: gmarek.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/priority critical-urgent

+1 to @ehashman that #pr-reviews or personal pinging approvers can draw quick attention.

/assign @k82cn @gmarek @bowei
for approval.

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.

k82cn · 2021-02-23T05:36:27Z

/approve
/lgtm

ehashman

/priority important-soon
/triage accepted
/lgtm
/sig node

I'm going to try to get all three branches lined up.

adampl · 2021-02-24T01:40:14Z

@ehashman There's no visible option to edit issue/PR labels, so I assumed I can't do it. I'm happy to be wrong on that ;)

ehashman · 2021-02-26T18:10:27Z

@adampl you can use the bot commands! I don't have the permissions to apply labels directly either. I wrote a guide here for SIG Node: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-node/triage.md

LuckySB · 2021-03-05T08:34:25Z

@hasheddan can you add cherry-pick-approved label ?

guzmonne · 2021-03-05T13:09:12Z

At the moment this issue has both the approved and do-not-merge/cherry-pick-not-apporoved. I'll try to remove the do-not-merge tag.

/remove-do-not-merge/cherry-pick-not-approved

Also, adding cherry-pick-approved

/cherry-pick-approved

hasheddan · 2021-03-05T13:12:27Z

@guzmonne a release manager must approve cherry picks. I am reviewing this CP across branches now.

guzmonne · 2021-03-05T13:13:30Z

@hasheddan, oops. Sorry.

hasheddan

/lgtm

k8s-ci-robot · 2021-03-05T13:15:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CKchen0726, ehashman, hasheddan, Huang-Wei, k82cn

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:

~~pkg/controller/nodelifecycle/OWNERS~~ [k82cn]

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

fejta-bot · 2021-03-05T16:10:27Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

Merge pull request kubernetes#96876 from howieyuen/no-execute-taint-m…

5935da7

…issing fix nodelifecyle controller not add NoExecute taint bug

k8s-ci-robot added this to the v1.20 milestone Jan 19, 2021

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jan 19, 2021

k8s-ci-robot requested review from aveshagarwal and smarterclayton January 19, 2021 08:24

k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 19, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 19, 2021

k8s-ci-robot requested review from Huang-Wei and k82cn January 20, 2021 01:35

k8s-ci-robot requested a review from derekwaynecarr January 20, 2021 01:35

adampl mentioned this pull request Jan 21, 2021

Missing taint: node.kubernetes.io/unreachable:NoExecute when nodes enter NotReady state on new installs of 1.19+ #97100

Closed

PhanLe1010 mentioned this pull request Jan 29, 2021

[BUG] The feature Pod Deletion Policy When Node is Down doesn't work on Kubernetes >= v1.19.0 longhorn/longhorn#2062

Closed

k8s-ci-robot assigned Huang-Wei Feb 5, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 5, 2021

k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Feb 22, 2021

k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Feb 22, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 23, 2021

ehashman approved these changes Feb 23, 2021

View reviewed changes

k8s-ci-robot assigned ehashman Feb 23, 2021

ehashman added this to Done in SIG Node PR Triage Feb 23, 2021

hasheddan approved these changes Mar 5, 2021

View reviewed changes

k8s-ci-robot assigned hasheddan Mar 5, 2021

hasheddan added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Mar 5, 2021

k8s-ci-robot removed the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label Mar 5, 2021

k8s-ci-robot merged commit 6c49363 into kubernetes:release-1.20 Mar 5, 2021

bsctl mentioned this pull request Apr 30, 2021

Missing taint: node.kubernetes.io/unreachable:NoExecute when nodes enter NotReady state #101674

Closed

hconnan mentioned this pull request May 19, 2021

Pods are not moved when Node in NotReady state #55713

Closed

CKchen0726 deleted the cherrypick_96876_1.20 branch October 25, 2021 07:51

liggitt added the kind/regression Categorizes issue or PR as related to a regression from a prior release. label Apr 27, 2022

liggitt removed the kind/regression Categorizes issue or PR as related to a regression from a prior release. label Sep 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cherry pick #96876 in controller to 1.20: fix nodelifecyle controller not add NoExecute taint bug #98168

Cherry pick #96876 in controller to 1.20: fix nodelifecyle controller not add NoExecute taint bug #98168

CKchen0726 commented Jan 19, 2021 •

edited by liggitt

k8s-ci-robot commented Jan 19, 2021

JornShen commented Jan 19, 2021

JornShen commented Jan 20, 2021

JornShen commented Jan 20, 2021

adampl commented Feb 5, 2021

Huang-Wei commented Feb 5, 2021

k8s-ci-robot commented Feb 22, 2021

k82cn commented Feb 23, 2021

ehashman left a comment

adampl commented Feb 24, 2021 •

edited

ehashman commented Feb 26, 2021

LuckySB commented Mar 5, 2021

guzmonne commented Mar 5, 2021

hasheddan commented Mar 5, 2021

guzmonne commented Mar 5, 2021

hasheddan left a comment

k8s-ci-robot commented Mar 5, 2021

fejta-bot commented Mar 5, 2021

Cherry pick #96876 in controller to 1.20: fix nodelifecyle controller not add NoExecute taint bug #98168

Cherry pick #96876 in controller to 1.20: fix nodelifecyle controller not add NoExecute taint bug #98168

Conversation

CKchen0726 commented Jan 19, 2021 • edited by liggitt

k8s-ci-robot commented Jan 19, 2021

JornShen commented Jan 19, 2021

JornShen commented Jan 20, 2021

JornShen commented Jan 20, 2021

adampl commented Feb 5, 2021

Huang-Wei commented Feb 5, 2021

k8s-ci-robot commented Feb 22, 2021

k82cn commented Feb 23, 2021

ehashman left a comment

Choose a reason for hiding this comment

adampl commented Feb 24, 2021 • edited

ehashman commented Feb 26, 2021

LuckySB commented Mar 5, 2021

guzmonne commented Mar 5, 2021

hasheddan commented Mar 5, 2021

guzmonne commented Mar 5, 2021

hasheddan left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 5, 2021

fejta-bot commented Mar 5, 2021

CKchen0726 commented Jan 19, 2021 •

edited by liggitt

adampl commented Feb 24, 2021 •

edited