delete a node from its cache if it gets node not found error #56622

wackxu · 2017-11-30T08:49:49Z

What this PR does / why we need it:

delete a node from its cache if it gets node not found error

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #56261

Special notes for your reviewer:

Release note:

NONE

wackxu · 2017-11-30T09:00:10Z

/assign @bsalamat

dims · 2017-11-30T12:26:26Z

/ok-to-test

bsalamat · 2017-11-30T23:33:35Z

plugin/pkg/scheduler/factory/factory.go

@@ -1109,6 +1109,11 @@ func (factory *configFactory) MakeDefaultErrorFunc(backoff *util.PodBackoff, pod
 		} else {
 			if _, ok := err.(*core.FitError); ok {
 				glog.V(4).Infof("Unable to schedule %v %v: no fit: %v; waiting", pod.Namespace, pod.Name, err)
+			} else if errors.IsNotFound(err) {
+				if errStatus, ok := err.(errors.APIStatus); ok && errStatus.Status().Details.Kind == "node" {
+					node := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: errStatus.Status().Details.Name}}


Can we try again to get the node and if we still see the "not found" error, then remove the node?

@bsalamat Yeah, that is good, I will fix it.

@bsalamat Done, PTAL

bsalamat · 2017-12-01T21:57:03Z

plugin/pkg/scheduler/factory/factory.go

+					_, err := factory.client.CoreV1().Nodes().Get(errStatus.Status().Details.Name, metav1.GetOptions{})
+					if err != nil && errors.IsNotFound(err) {
+						node := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: errStatus.Status().Details.Name}}
+						factory.schedulerCache.RemoveNode(&node)


Thanks, looks better. After removing the node, we also need to invalidate eCache predicates for the node. In order to do so, please add the following lines:

if factory.enableEquivalenceClassCache { factory.equivalencePodCache.InvalidateAllCachedPredicateItemOfNode(node.GetName()) }

Later on, we should refactor our code base so that the function that deletes a node from scheduler cache, also does the invalidation of the eCache.

nit: it would be easier to read if you set
nodeName := errStatus.Status().Details.Name
and use nodeName.

bsalamat · 2017-12-01T22:06:32Z

@davidopp Could you please add 1.9 milestone to this PR?

wackxu · 2017-12-02T01:42:31Z

@bsalamat Done, PTAL

bsalamat · 2017-12-02T02:09:30Z

I wish we could write a test for this with reasonable amount of effort, but it needs a lot of effort. So, this is fine.
/lgtm

k8s-github-robot · 2017-12-02T02:10:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsalamat, wackxu

Associated issue: 56261

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

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

~~plugin/pkg/scheduler/OWNERS~~ [bsalamat]

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 · 2017-12-12T17:49:53Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-12-12T18:44:32Z

[MILESTONENOTIFIER] Milestone Pull Request Needs Attention

@bsalamat @davidopp @timothysc @wackxu @kubernetes/sig-scheduling-misc

Action required: During code freeze, pull requests in the milestone should be in progress.
If this pull request is not being actively worked on, please remove it from the milestone.
If it is being worked on, please add the status/in-progress label so it can be tracked with other in-flight pull requests.

Action Required: This pull request has not been updated since Dec 2. Please provide an update.

Note: This pull request is marked as priority/critical-urgent, and must be updated every 1 day during code freeze.

Example update:

ACK.  In progress
ETA: DD/MM/YYYY
Risks: Complicated fix required

Pull Request Labels

sig/scheduling: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/bug: Fixes a bug discovered during the current release.

Help

k8s-github-robot · 2017-12-12T19:22:12Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

ravisantoshgudimetla · 2018-01-09T06:06:53Z

Can this be backported to 1.8?

resouer · 2018-01-10T02:14:44Z

I think so. @wackxu Would you like to try a cherry pick PR? Just do this under Kubernetes repo:

hack/cherry_pick_pull.sh upstream/release-1.8 56622

wackxu · 2018-01-10T02:24:02Z

Thanks for your help @resouer . See #58038 @ravisantoshgudimetla

…-upstream-release-1.8 Automatic merge from submit-queue. Automated cherry pick of #56622 Cherry pick of #56622 on release-1.8. #56622: delete a node from its cache if it gets node not found error

k8s-github-robot assigned davidopp and timothysc Nov 30, 2017

k8s-ci-robot assigned bsalamat Nov 30, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 30, 2017

bsalamat reviewed Nov 30, 2017

View reviewed changes

wackxu force-pushed the nodemiss branch from 464b714 to b1748f9 Compare December 1, 2017 11:54

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 1, 2017

bsalamat reviewed Dec 1, 2017

View reviewed changes

delete a node from its cache if it gets node not found error

aac60b6

wackxu force-pushed the nodemiss branch from b1748f9 to aac60b6 Compare December 2, 2017 01:35

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 2, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 2, 2017

bsalamat mentioned this pull request Dec 12, 2017

Statefulsets from 1.7 become unschedulable after an upgrade to 1.8 #56930

Closed

davidopp added this to the v1.9 milestone Dec 12, 2017

k8s-github-robot added the milestone/incomplete-labels label Dec 12, 2017

timothysc added kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. status/approved-for-milestone and removed milestone/incomplete-labels labels Dec 12, 2017

k8s-github-robot added the milestone/incomplete-labels label Dec 12, 2017

jberkus mentioned this pull request Dec 12, 2017

[1.9] Issue Burndown kubernetes/sig-release#38

Closed

enisoc added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Dec 12, 2017

k8s-github-robot added milestone/needs-attention and removed milestone/incomplete-labels labels Dec 12, 2017

k8s-github-robot merged commit 7335c41 into kubernetes:master Dec 12, 2017

timothysc mentioned this pull request Dec 14, 2017

Scheduler should delete a node from its cache if it gets "node not found" error #56261

Closed

wackxu mentioned this pull request Jan 10, 2018

Automated cherry pick of #56622 #58038

Merged

bsalamat mentioned this pull request Jan 12, 2018

kube-scheduler: Failed to get all terms #51331

Closed

wackxu deleted the nodemiss branch January 13, 2018 09:34

bsalamat mentioned this pull request Feb 14, 2018

kube-scheduler gets stuck if there's a pod in a namespace that doesn't exist assigned to a node that doesn't exist #43806

Closed

Huang-Wei mentioned this pull request Aug 16, 2018

Node shutdown & deployment with replicas #64720

Closed

ahg-g mentioned this pull request Sep 8, 2020

Missing a single update message will cause the scheduler never able to schedule pods to the right nodes #94437

Closed

alculquicondor mentioned this pull request Nov 27, 2023

Remove unnecessary error catch in scheduling failure #121981

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

delete a node from its cache if it gets node not found error #56622

delete a node from its cache if it gets node not found error #56622

wackxu commented Nov 30, 2017 •

edited by timothysc

wackxu commented Nov 30, 2017

dims commented Nov 30, 2017

bsalamat Nov 30, 2017 •

edited

wackxu Dec 1, 2017

wackxu Dec 1, 2017

bsalamat Dec 1, 2017

bsalamat Dec 1, 2017

bsalamat commented Dec 1, 2017

wackxu commented Dec 2, 2017

bsalamat commented Dec 2, 2017

k8s-github-robot commented Dec 2, 2017

k8s-github-robot commented Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

ravisantoshgudimetla commented Jan 9, 2018

resouer commented Jan 10, 2018

wackxu commented Jan 10, 2018

delete a node from its cache if it gets node not found error #56622

delete a node from its cache if it gets node not found error #56622

Conversation

wackxu commented Nov 30, 2017 • edited by timothysc

wackxu commented Nov 30, 2017

dims commented Nov 30, 2017

bsalamat Nov 30, 2017 • edited

Choose a reason for hiding this comment

wackxu Dec 1, 2017

Choose a reason for hiding this comment

wackxu Dec 1, 2017

Choose a reason for hiding this comment

bsalamat Dec 1, 2017

Choose a reason for hiding this comment

bsalamat Dec 1, 2017

Choose a reason for hiding this comment

bsalamat commented Dec 1, 2017

wackxu commented Dec 2, 2017

bsalamat commented Dec 2, 2017

k8s-github-robot commented Dec 2, 2017

k8s-github-robot commented Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

ravisantoshgudimetla commented Jan 9, 2018

resouer commented Jan 10, 2018

wackxu commented Jan 10, 2018

wackxu commented Nov 30, 2017 •

edited by timothysc

bsalamat Nov 30, 2017 •

edited