update node info no matter what node info changed #859
Conversation
k8s-ci-robot
added
cncf-cla: yes
| return !reflect.DeepEqual(oldNode.Status.Allocatable, newNode.Status.Allocatable) || | ||
| !reflect.DeepEqual(oldNode.Spec.Taints, newNode.Spec.Taints) || | ||
| !reflect.DeepEqual(oldNode.Labels, newNode.Labels) || | ||
| !reflect.DeepEqual(oldNode.Spec.Unschedulable, newNode.Spec.Unschedulable) |
There was a problem hiding this comment.
Is it OK to add !reflect.DeepEqual(oldNode.Status.Conditions, newNode.Status.Conditions) here? There are a lot of NodeUpdate, and we do not need update it every time.
There was a problem hiding this comment.
Check condition is better :)
There was a problem hiding this comment.
In default scheduler, no matter what node info changed, it always update the cache. There are not only node condition that we cared about when scheduling but also some other info, such as when we add ImageLocalityPriority, we need care about the change of node status Images info. I do not think it will have any benefits if we add more and more DeepEqual than we update the cache directly when node update.
There was a problem hiding this comment.
hm... let's get this merged firstly; if any performance issue, let's enhance it in another PR.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hex108, wackxu 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 |
k8s-ci-robot
added
the
approved
we should update node info no matter what node info changed. for now, if node Conditions changed, we do not update the cache and pod may schedule on node that was not ready.
/assign @k82cn @hex108