Merge pull request #2576 from losipiuk/lo/ignore-nominated-node-name-…

…if-node-missing

Ignore NominatedNodeName on Pod if node is gone

cluster-autoscaler/core/static_autoscaler.go

@@ -322,7 +322,7 @@ func (a *StaticAutoscaler) RunOnce(currentTime time.Time) errors.AutoscalerError
 	// todo: move split and append below to separate PodListProcessor
 	// Some unschedulable pods can be waiting for lower priority pods preemption so they have nominated node to run.
 	// Such pods don't require scale up but should be considered during scale down.
-	unschedulablePods, unschedulableWaitingForLowerPriorityPreemption := core_utils.FilterOutExpendableAndSplit(unschedulablePods, a.ExpendablePodsPriorityCutoff)
+	unschedulablePods, unschedulableWaitingForLowerPriorityPreemption := core_utils.FilterOutExpendableAndSplit(unschedulablePods, allNodes, a.ExpendablePodsPriorityCutoff)
 
 	// we tread pods with nominated node-name as scheduled for sake of scale-up considerations
 	scheduledPods = append(scheduledPods, unschedulableWaitingForLowerPriorityPreemption...)

cluster-autoscaler/core/utils/expendable.go

@@ -24,15 +24,26 @@ import (
 // FilterOutExpendableAndSplit filters out expendable pods and splits into:
 //   - waiting for lower priority pods preemption
 //   - other pods.
-func FilterOutExpendableAndSplit(unschedulableCandidates []*apiv1.Pod, expendablePodsPriorityCutoff int) ([]*apiv1.Pod, []*apiv1.Pod) {
+func FilterOutExpendableAndSplit(unschedulableCandidates []*apiv1.Pod, nodes []*apiv1.Node, expendablePodsPriorityCutoff int) ([]*apiv1.Pod, []*apiv1.Pod) {
 	var unschedulableNonExpendable []*apiv1.Pod
 	var waitingForLowerPriorityPreemption []*apiv1.Pod
+
+	nodeNames := make(map[string]bool)
+	for _, node := range nodes {
+		nodeNames[node.Name] = true
+	}
+
 	for _, pod := range unschedulableCandidates {
 		if pod.Spec.Priority != nil && int(*pod.Spec.Priority) < expendablePodsPriorityCutoff {
 			klog.V(4).Infof("Pod %s has priority below %d (%d) and will scheduled when enough resources is free. Ignoring in scale up.", pod.Name, expendablePodsPriorityCutoff, *pod.Spec.Priority)
 		} else if nominatedNodeName := pod.Status.NominatedNodeName; nominatedNodeName != "" {
-			waitingForLowerPriorityPreemption = append(waitingForLowerPriorityPreemption, pod)
-			klog.V(4).Infof("Pod %s will be scheduled after low priority pods are preempted on %s. Ignoring in scale up.", pod.Name, nominatedNodeName)
+			if nodeNames[nominatedNodeName] {
+				klog.V(4).Infof("Pod %s will be scheduled after low priority pods are preempted on %s. Ignoring in scale up.", pod.Name, nominatedNodeName)
+				waitingForLowerPriorityPreemption = append(waitingForLowerPriorityPreemption, pod)
+			} else {
+				klog.V(4).Infof("Pod %s has nominatedNodeName set to %s but node is gone", pod.Name, nominatedNodeName)
+				unschedulableNonExpendable = append(unschedulableNonExpendable, pod)
+			}
 		} else {
 			unschedulableNonExpendable = append(unschedulableNonExpendable, pod)
 		}

cluster-autoscaler/core/utils/expendable_test.go

@@ -33,27 +33,38 @@ func TestFilterOutExpendableAndSplit(t *testing.T) {
 	p1.Spec.Priority = &priority1
 	p2 := BuildTestPod("p2", 1000, 200000)
 	p2.Spec.Priority = &priority100
+	n1 := BuildTestNode("node1", 10, 10)
+	n2 := BuildTestNode("node2", 10, 10)
 
 	podWaitingForPreemption1 := BuildTestPod("w1", 1000, 200000)
 	podWaitingForPreemption1.Spec.Priority = &priority1
 	podWaitingForPreemption1.Status.NominatedNodeName = "node1"
 	podWaitingForPreemption2 := BuildTestPod("w2", 1000, 200000)
 	podWaitingForPreemption2.Spec.Priority = &priority100
-	podWaitingForPreemption2.Status.NominatedNodeName = "node1"
+	podWaitingForPreemption2.Status.NominatedNodeName = "node2"
 
-	res1, res2 := FilterOutExpendableAndSplit([]*apiv1.Pod{p1, p2, podWaitingForPreemption1, podWaitingForPreemption2}, 0)
+	res1, res2 := FilterOutExpendableAndSplit([]*apiv1.Pod{p1, p2, podWaitingForPreemption1, podWaitingForPreemption2}, []*apiv1.Node{n1, n2}, 0)
 	assert.Equal(t, 2, len(res1))
 	assert.Equal(t, p1, res1[0])
 	assert.Equal(t, p2, res1[1])
 	assert.Equal(t, 2, len(res2))
 	assert.Equal(t, podWaitingForPreemption1, res2[0])
 	assert.Equal(t, podWaitingForPreemption2, res2[1])
 
-	res1, res2 = FilterOutExpendableAndSplit([]*apiv1.Pod{p1, p2, podWaitingForPreemption1, podWaitingForPreemption2}, 10)
+	res1, res2 = FilterOutExpendableAndSplit([]*apiv1.Pod{p1, p2, podWaitingForPreemption1, podWaitingForPreemption2}, []*apiv1.Node{n1, n2}, 10)
 	assert.Equal(t, 1, len(res1))
 	assert.Equal(t, p2, res1[0])
 	assert.Equal(t, 1, len(res2))
 	assert.Equal(t, podWaitingForPreemption2, res2[0])
+
+	// if node2 is missing podWaitingForPreemption2 should be treated as standard pod not one waiting for preemption
+	res1, res2 = FilterOutExpendableAndSplit([]*apiv1.Pod{p1, p2, podWaitingForPreemption1, podWaitingForPreemption2}, []*apiv1.Node{n1}, 0)
+	assert.Equal(t, 3, len(res1))
+	assert.Equal(t, p1, res1[0])
+	assert.Equal(t, p2, res1[1])
+	assert.Equal(t, podWaitingForPreemption2, res1[2])
+	assert.Equal(t, 1, len(res2))
+	assert.Equal(t, podWaitingForPreemption1, res2[0])
 }
 
 func TestFilterOutExpendablePods(t *testing.T) {
@@ -75,4 +86,5 @@ func TestFilterOutExpendablePods(t *testing.T) {
 	assert.Equal(t, p1, res[0])
 	assert.Equal(t, p2, res[1])
 	assert.Equal(t, podWaitingForPreemption2, res[2])
+
 }