Considering all nodes for the scheduler cache to allow lookups

plugin/pkg/scheduler/factory/factory.go

@@ -449,6 +449,11 @@ func getNodeConditionPredicate() cache.NodeConditionPredicate {
 				return false
 			}
 		}
+		// Ignore nodes that are marked unschedulable
+		if node.Spec.Unschedulable {
+			glog.V(4).Infof("Ignoring node %v since it is unschedulable", node.Name)
+			return false
+		}
 		return true
 	}
 }
@@ -470,9 +475,10 @@ func (factory *ConfigFactory) createAssignedNonTerminatedPodLW() *cache.ListWatc
 
 // createNodeLW returns a cache.ListWatch that gets all changes to nodes.
 func (factory *ConfigFactory) createNodeLW() *cache.ListWatch {
-	// TODO: Filter out nodes that doesn't have NodeReady condition.
-	fields := fields.Set{api.NodeUnschedulableField: "false"}.AsSelector()
-	return cache.NewListWatchFromClient(factory.Client, "nodes", api.NamespaceAll, fields)
+	// all nodes are considered to ensure that the scheduler cache has access to all nodes for lookups
+	// the NodeCondition is used to filter out the nodes that are not ready or unschedulable
+	// the filtered list is used as the super set of nodes to consider for scheduling
+	return cache.NewListWatchFromClient(factory.Client, "nodes", api.NamespaceAll, fields.ParseSelectorOrDie(""))
 }
 
 // createPersistentVolumeLW returns a cache.ListWatch that gets all changes to persistentVolumes.

plugin/pkg/scheduler/factory/factory_test.go

@@ -430,3 +430,46 @@ func TestInvalidFactoryArgs(t *testing.T) {
 	}
 
 }
+
+func TestNodeConditionPredicate(t *testing.T) {
+	nodeFunc := getNodeConditionPredicate()
+	nodeList := &api.NodeList{
+		Items: []api.Node{
+			// node1 considered
+			{ObjectMeta: api.ObjectMeta{Name: "node1"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionTrue}}}},
+			// node2 ignored - node not Ready
+			{ObjectMeta: api.ObjectMeta{Name: "node2"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionFalse}}}},
+			// node3 ignored - node out of disk
+			{ObjectMeta: api.ObjectMeta{Name: "node3"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeOutOfDisk, Status: api.ConditionTrue}}}},
+			// node4 considered
+			{ObjectMeta: api.ObjectMeta{Name: "node4"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeOutOfDisk, Status: api.ConditionFalse}}}},
+
+			// node5 ignored - node out of disk
+			{ObjectMeta: api.ObjectMeta{Name: "node5"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionTrue}, {Type: api.NodeOutOfDisk, Status: api.ConditionTrue}}}},
+			// node6 considered
+			{ObjectMeta: api.ObjectMeta{Name: "node6"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionTrue}, {Type: api.NodeOutOfDisk, Status: api.ConditionFalse}}}},
+			// node7 ignored - node out of disk, node not Ready
+			{ObjectMeta: api.ObjectMeta{Name: "node7"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionFalse}, {Type: api.NodeOutOfDisk, Status: api.ConditionTrue}}}},
+			// node8 ignored - node not Ready
+			{ObjectMeta: api.ObjectMeta{Name: "node8"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionFalse}, {Type: api.NodeOutOfDisk, Status: api.ConditionFalse}}}},
+
+			// node9 ignored - node unschedulable
+			{ObjectMeta: api.ObjectMeta{Name: "node9"}, Spec: api.NodeSpec{Unschedulable: true}},
+			// node10 considered
+			{ObjectMeta: api.ObjectMeta{Name: "node10"}, Spec: api.NodeSpec{Unschedulable: false}},
+			// node11 considered
+			{ObjectMeta: api.ObjectMeta{Name: "node11"}},
+		},
+	}
+
+	nodeNames := []string{}
+	for _, node := range nodeList.Items {
+		if nodeFunc(node) {
+			nodeNames = append(nodeNames, node.Name)
+		}
+	}
+	expectedNodes := []string{"node1", "node4", "node6", "node10", "node11"}
+	if !reflect.DeepEqual(expectedNodes, nodeNames) {
+		t.Errorf("expected: %v, got %v", expectedNodes, nodeNames)
+	}
+}

test/integration/scheduler_test.go

@@ -170,8 +170,11 @@ func DoTestUnschedulableNodes(t *testing.T, restClient *client.Client, nodeStore
 					t.Fatalf("Failed to update node with unschedulable=true: %v", err)
 				}
 				err = waitForReflection(t, s, nodeKey, func(node interface{}) bool {
-					// An unschedulable node should get deleted from the store
-					return node == nil
+					// An unschedulable node should still be present in the store
+					// Nodes that are unschedulable or that are not ready or
+					// have their disk full (Node.Spec.Conditions) are exluded
+					// based on NodeConditionPredicate, a separate check
+					return node != nil && node.(*api.Node).Spec.Unschedulable == true
 				})
 				if err != nil {
 					t.Fatalf("Failed to observe reflected update for setting unschedulable=true: %v", err)